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

Negociation broke fetching from the bcachefs git repository #888

Closed
1 task done
g2p opened this issue Jun 8, 2023 · 3 comments
Closed
1 task done

Negociation broke fetching from the bcachefs git repository #888

g2p opened this issue Jun 8, 2023 · 3 comments

Comments

@g2p
Copy link

g2p commented Jun 8, 2023

Duplicates

  • I have searched the existing issues

Current behavior 馃槸

This is another followup to negotiation changes.

I am testing gix fetch on a lot of cloned repos, and noticed one of them breaking in 0.26.0, after bisecting this happens at the point #861 was merged (ae845de Merge branch 'integrate-gix-negotiate').

Bisection is slightly imprecise because some of the commits in the PR didn't compile individually and had to be skipped.

git bisect log
git bisect start
# status: waiting for both good and bad commits
# bad: [4e081f2141dcb9919597c53dfd5706cc9439d541] update changelog with information for the `gix` CLI.
git bisect bad 4e081f2141dcb9919597c53dfd5706cc9439d541
# status: waiting for good commit(s), bad commit known
# good: [234aee07b3693f5cf08ed375b9257b1327e0f7d1] Release gitoxide-core v0.27.0, gitoxide v0.25.0
git bisect good 234aee07b3693f5cf08ed375b9257b1327e0f7d1
# good: [97b3f7e2eaddea20c98f2f7ab6a0d2e2117b0793] Merge branch 'consecutive-negotiation'
git bisect good 97b3f7e2eaddea20c98f2f7ab6a0d2e2117b0793
# good: [0269eedc08c21589b5381d9b7d7fcc7004160bf8] Include missing changelog file in some crates
git bisect good 0269eedc08c21589b5381d9b7d7fcc7004160bf8
# skip: [ae1bc41817bec3b83fe65104e7e3efe4bd798a78] Add test to validate alternates in the context of fetching
git bisect skip ae1bc41817bec3b83fe65104e7e3efe4bd798a78
# bad: [7983f6fd4ef242d494962fe35b8b8f91a6135f32] adapt to changes in `gix`
git bisect bad 7983f6fd4ef242d494962fe35b8b8f91a6135f32
# good: [11ad8a890a6233befb5d2b6b41caadbcb296c3f5] feat!: Add version of Graph that handles fully-parsed commits
git bisect good 11ad8a890a6233befb5d2b6b41caadbcb296c3f5
# good: [877aa2921d8491d301945f86d5a69382e40fb081] feat: Add `fetch::Arguments::is_stateless()` to aid proper use of arguments.
git bisect good 877aa2921d8491d301945f86d5a69382e40fb081
# skip: [6a3c02131c9ebca911be5f751e5a6c67fbdbf609] fix!: make V1 stateless negotations work.
git bisect skip 6a3c02131c9ebca911be5f751e5a6c67fbdbf609
# skip: [af0ef2f36736e3805f769d8cd59c9fa7eb6a22a0] feat: use `gix-negotiate` in fetch machinery.
git bisect skip af0ef2f36736e3805f769d8cd59c9fa7eb6a22a0
# good: [c5dc7b4c43f07c04cdfb218de03e6725ff3fdb64] fix: `include-tag` is now properly handled in V1 fetch arguments
git bisect good c5dc7b4c43f07c04cdfb218de03e6725ff3fdb64
# only skipped commits left to test
# possible first bad commit: [7983f6fd4ef242d494962fe35b8b8f91a6135f32] adapt to changes in `gix`
# possible first bad commit: [ae1bc41817bec3b83fe65104e7e3efe4bd798a78] Add test to validate alternates in the context of fetching
# possible first bad commit: [af0ef2f36736e3805f769d8cd59c9fa7eb6a22a0] feat: use `gix-negotiate` in fetch machinery.
# possible first bad commit: [6a3c02131c9ebca911be5f751e5a6c67fbdbf609] fix!: make V1 stateless negotations work.
git checkout ae845dea6cee6523c88a23d7a14293589cf8092f
cargo build --no-default-features --features=max-pure
cd ~/src/evilpiepirate.org/git/bcachefs
~/src/github.com/Byron/gitoxide/target/debug/gix fetch

Error: Could not decode server reply

Caused by:
    0: Failed to read from line reader
    1: failed to fill whole buffer

Expected behavior 馃

gix fetch fetches

Steps to reproduce 馃暪

https://evilpiepirate.org/git/bcachefs.git is cloned inside ~/src/evilpiepirate.org/git/bcachefs

# From a gitoxide checkout
git checkout ae845dea6cee6523c88a23d7a14293589cf8092f
cargo build --no-default-features --features=max-pure
cd ~/src/evilpiepirate.org/git/bcachefs
~/src/github.com/Byron/gitoxide/target/debug/gix fetch

Error: Could not decode server reply

Caused by:
    0: Failed to read from line reader
    1: failed to fill whole buffer
@g2p
Copy link
Author

g2p commented Jun 8, 2023

I can reproduce it with smaller repositories as well, like https://github.com/tokio-rs/console.git

@Byron
Copy link
Owner

Byron commented Jun 8, 2023

This is a strange finding as both evilpiepirate.org as well as github.com use version 2 of the protocol by default. There is no question about interaction there, so no chance for deadlocks or other issues.

I cloned both the bcachefs and the console repo and had no issues.

Fetching the bcachefs repo didn't yet yield any result as nothing changed, but I will try again later to see if anything can be reproduced.

Something of note here is that I am testing with the release max build, not the debug max-pure one. Max-pure uses a sync reqwest based backend, and I don't even test that on CI as it shows exactly that flaky-ness that we seem to be seeing here. Sometimes it just fails to read the expected bytes.

It's a bit sad that failed to fill whole buffer is all we see from the underlying std::io::Error, as the exact error code might be helpful. However, I think that this is related to getting an unexpected EOF that simply shouldn't be happening.

Can you build with max to see if it is indeed related to the HTTP backend?

@Byron
Copy link
Owner

Byron commented Aug 3, 2023

A lot has changed since then and without feedback there can't be progress here, I assume it's working, and am thus closing the issue.
If anything changes, please let me know in the comments though.

@Byron Byron closed this as completed Aug 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants