3 ms·
That's an unnecessary rabbit-hole to go down. The root cause was likely Github returning 200 when they should've returned a 5xx error. A better solution in th
by anonsivalley652 7y ago
That's an unnecessary rabbit-hole to go down.
The root cause was likely Github returning 200 when they should've returned a 5xx error.
A better solution in this case is to verify the push worked by looking at the remote for a ref in the push before returning success. I would never just depend on git2 to libgit2 callbacks succeeding as "proof" that it worked, because there's too much to go wrong.
Also, the crates.io patch should be upstreamed into git2 by wrapping the block. And, the ultimate fix is to address libgit2 to make sure it's handling errors correctly. It's not a very good fix, but it's better than what exists now.
Furthermore, it seems like a pure Rust git would be better than problematic wrapper layers stacked one on of top of the other.
- klysm 7y agoI encourage you to look at the patch (it’s short). The callbacks are not between git2 and libgit2 so I think you are misunderstanding the nature of the problem. I agree though that a pure rust implementation of libgit2 would be ideal.
- steveklabnik 7y agoSome fun history: https://github.com/libgit2/libgit2/pull/3540 https://github.com/libgit2/libgit2/pull/3540
- klysm 7y agoI was pleasantly surprised at how civil and thoughtful that conversation was! I was also expecting the “flame-retardant pants” to be necessary after reading the title. I didn’t realize how complicated the backend tooling is to achieve the same portability as C - it seems so much simpler as a consumer of the toolchain. IIRC the bootstrapping problem is still something distros are fighting with today, but hopefully one day we can have rustc bundled with all the other standard build tools.