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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a deadlocking test with master libgit2 #7179

Merged
merged 1 commit into from
Jul 25, 2019

Conversation

alexcrichton
Copy link
Member

This commit fixes a test in Cargo to work around a seeming regression in
behavior in libgit2 around HTTP authentication. The expected flow for
HTTP authentication with git is that git sends an HTTP request and
receives an "unauthorized" response. It then sends another request with
authorization information and that's what we're testing is received in
the our test.

Previously libgit2 would issue a new HTTP connection if the previous one
was closed, but it looks like changes in libgit2 now require that the
same HTTP connection is used for the initial request and the subsequent
request with authorization information. This broke our test since it's
not using an HTTP compliant server at all and is just some handwritten
TCP reads/writes. The fix here is to basically stay with handwritten TCP
reads/writes but tweak how it happens so it's all on the same HTTP/TCP
connection to match what libgit2 is expecting.

Some extra assertions have also been added to try to prevent deadlocks
from happening in the future and instead make the test fail fast if this
situation comes up again.

@rust-highfive
Copy link

r? @ehuss

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2019
This commit fixes a test in Cargo to work around a seeming regression in
behavior in libgit2 around HTTP authentication. The expected flow for
HTTP authentication with git is that git sends an HTTP request and
receives an "unauthorized" response. It then sends another request with
authorization information and that's what we're testing is received in
the our test.

Previously libgit2 would issue a new HTTP connection if the previous one
was closed, but it looks like changes in libgit2 now require that the
same HTTP connection is used for the initial request and the subsequent
request with authorization information. This broke our test since it's
not using an HTTP compliant server at all and is just some handwritten
TCP reads/writes. The fix here is to basically stay with handwritten TCP
reads/writes but tweak how it happens so it's all on the same HTTP/TCP
connection to match what libgit2 is expecting.

Some extra assertions have also been added to try to prevent deadlocks
from happening in the future and instead make the test fail fast if this
situation comes up again.
@ehuss
Copy link
Contributor

ehuss commented Jul 25, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Jul 25, 2019

📌 Commit bd7fe89 has been approved by ehuss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2019
@bors
Copy link
Contributor

bors commented Jul 25, 2019

⌛ Testing commit bd7fe89 with merge b8029a8...

bors added a commit that referenced this pull request Jul 25, 2019
Fix a deadlocking test with master libgit2

This commit fixes a test in Cargo to work around a seeming regression in
behavior in libgit2 around HTTP authentication. The expected flow for
HTTP authentication with git is that git sends an HTTP request and
receives an "unauthorized" response. It then sends another request with
authorization information and that's what we're testing is received in
the our test.

Previously libgit2 would issue a new HTTP connection if the previous one
was closed, but it looks like changes in libgit2 now require that the
same HTTP connection is used for the initial request and the subsequent
request with authorization information. This broke our test since it's
not using an HTTP compliant server at all and is just some handwritten
TCP reads/writes. The fix here is to basically stay with handwritten TCP
reads/writes but tweak how it happens so it's all on the same HTTP/TCP
connection to match what libgit2 is expecting.

Some extra assertions have also been added to try to prevent deadlocks
from happening in the future and instead make the test fail fast if this
situation comes up again.
@bors
Copy link
Contributor

bors commented Jul 25, 2019

☀️ Test successful - checks-azure
Approved by: ehuss
Pushing b8029a8 to master...

@bors bors merged commit bd7fe89 into rust-lang:master Jul 25, 2019
@alexcrichton alexcrichton deleted the fix-tests branch July 25, 2019 18:22
bors added a commit to rust-lang/rust that referenced this pull request Aug 1, 2019
Update cargo, rls

## cargo

12 commits in d0f828419d6ce6be21a90866964f58eb2c07cd56..26092da337b948719549cd5ed3d1051fd847afd7
2019-07-23 21:58:59 +0000 to 2019-07-31 23:24:32 +0000
- tests: Enable features to fix unstabilized `#[bench]` (rust-lang/cargo#7198)
- Fix excluding target dirs from backups on OSX (rust-lang/cargo#7192)
- Handle symlinks to directories (rust-lang/cargo#6817)
- Enable pipelined compilation by default (rust-lang/cargo#7143)
- Refactor resolve `Method` (rust-lang/cargo#7186)
- Update `cargo_compile` module doc. (rust-lang/cargo#7187)
- Clean up TargetInfo (rust-lang/cargo#7185)
- Fix some issues with absolute paths in dep-info files. (rust-lang/cargo#7137)
- Update the `url` crate to 2.0 (rust-lang/cargo#7175)
- Tighten requirements for git2 crates (rust-lang/cargo#7176)
- Fix a deadlocking test with master libgit2 (rust-lang/cargo#7179)
- Fix detection of cyclic dependencies through `[patch]` (rust-lang/cargo#7174)

## rls

1 commits in 70347b5d4dfe78eeb9e6f6db85f773c8d43cd22b..93d9538c6000fcf6c8da763ef4ce7a8d407b7d24
2019-07-30 12:56:38 +0200 to 2019-07-31 21:42:49 +0200
- Update cargo (rust-lang/rls#1529)
@ehuss ehuss added this to the 1.38.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants