git-protocol: git protocol v2 machinery #724
Conversation
|
@Byron would you mind taking a look? It seems to work basically, but then hangs forever reading the packfile. By replacing |
|
@Byron nevermind -- somewhat surprisingly, I need to spawn |
|
@Byron nevermind the nevermind -- it's pretty bad, though, as the client can keep |
Sorry for the hassle! I will take a closer look tomorrow. It could be that this setup is different enough from the tests (which are against the daemon and against github) to cause something to be a little off. Failing to flush a |
|
It does work as designed, except that the Sending a flush pkt would also cause the server to exit, but is not satisfying in our environment. So, until we have a replacement for |
There was a problem hiding this comment.
For now I took a look at all code except for the tests which are merely postponed to another session.
All looks good to me and it's nice to see that git_protocol::client::fetch() does indeed compose.
I can't say anything about what to do with git-upload-pack, you seem to know what works and what doesn't, but if you think that gitoxide isn't speaking git correctly confusing the server in the process then it's definitely something to reproduce and fix.
There was a problem hiding this comment.
Looking at the integration test was very helpful as I came to believe that the server hanging is a shortcoming of gitoxide that should be investigated and hopefully fixed. I don't fully understand what the expectations in the server are though as I think you mentioned it should not shut down when sending the flush packet, nor do I fully understand the interplay between stateless RPC and the daemon sending capabilities (instead of the upload-pack program).
kim
force-pushed
the
kim/git2
branch
5 times, most recently
from
6da938e to
9263108
Compare
|
This is now ready, I think. |
There was a problem hiding this comment.
Probably it's best to use libgit2 pack writing for now as the gitoxide implementation of pack writing exists in isolation, that is isn't yet aware of any additional requirements that might apply to writing packs while dealing with locks, multi-pack indices and the likes.
Some more research should be done to be sure there is no unwanted side effects.
On the other hand, all it does it to write tmp files and rename the finished files to move into their desired spot. This should be find if git is indeed backwards compatible with itself.
I also left a comment about using gitoxide for setting up the test fixture, maybe that could be done in a separate PR contribute by me if there is interest.
Besides that: YAY 🎉, gitoxide lands in Radicle-Link :)
| } | ||
|
|
||
| #[test] | ||
| fn thin_pack_gitoxide() { |
There was a problem hiding this comment.
I am definitely loving these tests and am glad it does indeed work as expected.
Currently thin pack support is tested on a thin pack of gitoxide's own making only.
|
|
||
| let tree = { | ||
| let empty = repo.treebuilder(None).unwrap(); | ||
| let oid = empty.write().unwrap(); |
There was a problem hiding this comment.
Actually all of work done here except for Repository::init_bare(…) and .set_namespace() can already be done in gitoxide. Maybe once all is supported I could contribute a PR which oxidizes the test setup.
Introduces a new crate with the lower-level machinery for handling replication based on git protocol v2. Signed-off-by: Kim Altintop <[email protected]>
Introduces a new crate with the lower-level machinery for handling
replication based on git protocol v2.
Signed-off-by: Kim Altintop [email protected]