Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assertion failed when fetching from gitweb: "Naive always finishes after the first round" #812

Closed
1 task done
g2p opened this issue Apr 12, 2023 · 6 comments 路 Fixed by #850
Closed
1 task done
Labels
acknowledged an issue is accepted as shortcoming to be fixed

Comments

@g2p
Copy link

g2p commented Apr 12, 2023

Duplicates

  • I have searched the existing issues

Current behavior 馃槸

I'm using the gix command to fetch from many repositories, which is a welcome speed boost, but some of them consistently fail. They seem to be served by gitweb.

Expected behavior 馃

gix fetch fetches

Steps to reproduce 馃暪

  1. Clone from a gitweb served repository (ideally high-traffic to help with the next step; https://erol.kernel.org/?s=idle has some) using Git (2.39.2 works).
  2. Wait for upstream commits to show up
  3. Try to fetch commits using gix (released version or current git main).

I get this assert:

RUST_BACKTRACE=1 ~/src/github.com/Byron/gitoxide/target/debug/gix -c http.sslVersion=tlsv1.2 -c http.sslCAInfo=/etc/ssl/certs/ca-certificates.crt --no-verbose fetch 
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `2`,
 right: `1`: Naive always finishes after the first round, it claims.', gix/src/remote/connection/fetch/negotiate.rs:43:13
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:64:14
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/panicking.rs:212:5
   4: gix::remote::connection::fetch::negotiate::one_round
             at /home/g2p/src/github.com/Byron/gitoxide/gix/src/remote/connection/fetch/negotiate.rs:43:13
   5: gix::remote::connection::fetch::receive_pack::<impl gix::remote::connection::fetch::Prepare<T,P>>::receive
             at /home/g2p/src/github.com/Byron/gitoxide/gix/src/remote/connection/fetch/receive_pack.rs:114:33
   6: gitoxide_core::repository::fetch::function::fetch
             at /home/g2p/src/github.com/Byron/gitoxide/gitoxide-core/src/repository/fetch.rs:51:48
   7: gix::plumbing::main::main::{{closure}}
             at /home/g2p/src/github.com/Byron/gitoxide/src/plumbing/main.rs:175:21
   8: gix::shared::pretty::prepare_and_run
             at /home/g2p/src/github.com/Byron/gitoxide/src/shared.rs:116:31
   9: gix::plumbing::main::main
             at /home/g2p/src/github.com/Byron/gitoxide/src/plumbing/main.rs:168:13
  10: gix::main
             at /home/g2p/src/github.com/Byron/gitoxide/src/gix.rs:17:5
  11: core::ops::function::FnOnce::call_once
             at /rustc/9eb3afe9ebe9c7d2b84b71002d44f4a0edac95e0/library/core/src/ops/function.rs:250:5
@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label Apr 12, 2023
@Byron
Copy link
Owner

Byron commented Apr 12, 2023

That's interesting! I have heard of this happening before, but now it seems the assumption of single-round negotiation is broken if the remote head changes while the negotiation is in progress? Or maybe it's really just any fetch that fails if the server acts a little differently than GitHub does.

On another note, I love that the example fetch invocation uses additional http options which seemingly work as well. After all, I have never tested these in particular.

Context

gitoxide currently uses a 'naive' approach to pack-file negotiation which trades simplicity for correctness. With most servers this isn't a problem, especially if it's a real git-upload-pack on the other end, but some servers seem to do something special.

Thus it's very helpful to know that gitweb is used in the example, as it might help improve 'naive' to deal with that or help with the development of the actual 'consecutive' one.

@g2p
Copy link
Author

g2p commented Apr 12, 2023

Re the options, I have http.sslVersion set to tls1.3 in ~/.gitconfig, which I need to override for hosts that only support 1.2. Gitoxide handles it well.

Passing http.sslCAInfo is required for some reason, maybe there should be a cert store working out of the box? Though I don't mind the workaround. I'm on Linux and gitoxide is installed with default features.

@Byron
Copy link
Owner

Byron commented Apr 12, 2023

That http.sslCAInfo is interesting - does git need the same workaround?

In theory gitoxide should act similarly to git, and maybe that flag isn't yet faithfully implemented.

@g2p
Copy link
Author

g2p commented Apr 12, 2023

No, Git doesn't need http.sslCAInfo to be set. Should I report a separate issue?

@Byron
Copy link
Owner

Byron commented Apr 12, 2023

Yes, a separate issue with additional context would be great, in particular, gix error messages if the CAInfo isn't provided. Thank you!

@Byron Byron linked a pull request May 16, 2023 that will close this issue
3 tasks
@g2p
Copy link
Author

g2p commented May 26, 2023

Maybe this will be fixed when #861 integrates support for consecutive negotiation into gix/src/remote/connection/fetch/negotiate.rs, at the moment the assert still triggers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants