Skip to content

Commit bcd8e6d

Browse files
committed
Auto merge of #13563 - ehuss:github-fast-path-token, r=<try>
Use GITHUB_TOKEN in github_fast_path if available. This changes the GitHub fast-path to include an Authorization header if the GITHUB_TOKEN environment variable is set. This is intended to help with API rate limits, which is not too hard to hit in CI. If it hits the rate limit, then the fetch falls back to fetching all branches which is significantly slower for some repositories. This is also a partial solution for #13555, where the user has a `rev="<commit-hash>"` where that commit hash points to a PR. The default refspec of `["+refs/heads/*:refs/remotes/origin/*", "+HEAD:refs/remotes/origin/HEAD"]` will not fetch commits from PRs (unless they are reachable from some branch). There is some risk that if the user has a GITHUB_TOKEN that is invalid or expired, then this will cause `github_fast_path` to fail where it previously wouldn't. I think that is probably unlikely to happen, though. This also adds redirect support. Apparently GitHub now issues a redirect for `repos/{org}/{repo}/commits/{commit}` to `/repositories/{id}/commits/{commit}`. The test `https::github_works` exercises this code path, and will use the token on CI.
2 parents 7065f0e + fe3cb3d commit bcd8e6d

File tree

2 files changed

+72
-17
lines changed

2 files changed

+72
-17
lines changed

src/cargo/sources/git/utils.rs

+11
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,7 @@ fn github_fast_path(
14131413
// the branch has moved.
14141414
if let Some(local_object) = local_object {
14151415
if is_short_hash_of(rev, local_object) {
1416+
debug!("github fast path already has {local_object}");
14161417
return Ok(FastPathRev::UpToDate);
14171418
}
14181419
}
@@ -1452,12 +1453,19 @@ fn github_fast_path(
14521453
handle.get(true)?;
14531454
handle.url(&url)?;
14541455
handle.useragent("cargo")?;
1456+
handle.follow_location(true)?; // follow redirects
14551457
handle.http_headers({
14561458
let mut headers = List::new();
14571459
headers.append("Accept: application/vnd.github.3.sha")?;
14581460
if let Some(local_object) = local_object {
14591461
headers.append(&format!("If-None-Match: \"{}\"", local_object))?;
14601462
}
1463+
if let Ok(token) = gctx.get_env("GITHUB_TOKEN") {
1464+
if !token.is_empty() {
1465+
// Avoid API rate limits if possible.
1466+
headers.append(&format!("Authorization: Bearer {token}"))?;
1467+
}
1468+
}
14611469
headers
14621470
})?;
14631471

@@ -1472,14 +1480,17 @@ fn github_fast_path(
14721480

14731481
let response_code = handle.response_code()?;
14741482
if response_code == 304 {
1483+
debug!("github fast path up-to-date");
14751484
Ok(FastPathRev::UpToDate)
14761485
} else if response_code == 200 {
14771486
let oid_to_fetch = str::from_utf8(&response_body)?.parse::<Oid>()?;
1487+
debug!("github fast path fetch {oid_to_fetch}");
14781488
Ok(FastPathRev::NeedsFetch(oid_to_fetch))
14791489
} else {
14801490
// Usually response_code == 404 if the repository does not exist, and
14811491
// response_code == 422 if exists but GitHub is unable to resolve the
14821492
// requested rev.
1493+
debug!("github fast path bad response code {response_code}");
14831494
Ok(FastPathRev::Indeterminate)
14841495
}
14851496
}

tests/testsuite/https.rs

+61-17
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
//! or CARGO_PUBLIC_NETWORK_TESTS.
55
66
use cargo_test_support::containers::Container;
7+
use cargo_test_support::paths::{self, CargoPathExt};
78
use cargo_test_support::project;
89

910
#[cargo_test(container_test)]
@@ -134,22 +135,65 @@ fn self_signed_with_cacert() {
134135
#[cargo_test(public_network_test)]
135136
fn github_works() {
136137
// Check that an https connection to github.com works.
137-
let p = project()
138-
.file(
139-
"Cargo.toml",
140-
r#"
141-
[package]
142-
name = "foo"
143-
version = "0.1.0"
144-
edition = "2015"
138+
// This tries all the different types of git references, and verifies the fast-path behavior.
139+
for (manifest_ref, oid, refspecs, up_to_date) in [
140+
(
141+
r#", tag = "1.3.2""#,
142+
"ed185cfb1c447c1b4bd6ac021c9ec3bb02c9e2f2",
143+
r#""+refs/tags/1.3.2:refs/remotes/origin/tags/1.3.2""#,
144+
"github fast path up-to-date",
145+
),
146+
(
147+
r#", rev = "6c67922300d5abae779ca147bac00f6ff9c87f8a""#,
148+
"6c67922300d5abae779ca147bac00f6ff9c87f8a",
149+
r#""+6c67922300d5abae779ca147bac00f6ff9c87f8a:refs/commit/6c67922300d5abae779ca147bac00f6ff9c87f8a""#,
150+
"github fast path already has 6c67922300d5abae779ca147bac00f6ff9c87f8a",
151+
),
152+
(
153+
r#", branch = "main""#,
154+
"[..]",
155+
r#""+refs/heads/main:refs/remotes/origin/main""#,
156+
"github fast path up-to-date",
157+
),
158+
(
159+
"",
160+
"[..]",
161+
r#""+HEAD:refs/remotes/origin/HEAD""#,
162+
"github fast path up-to-date",
163+
),
164+
] {
165+
eprintln!("test {manifest_ref}");
166+
let p = project()
167+
.file(
168+
"Cargo.toml",
169+
&format!(
170+
r#"
171+
[package]
172+
name = "foo"
173+
version = "0.1.0"
174+
edition = "2015"
145175
146-
[dependencies]
147-
bitflags = { git = "https://github.com/rust-lang/bitflags.git", tag="1.3.2" }
148-
"#,
149-
)
150-
.file("src/lib.rs", "")
151-
.build();
152-
p.cargo("fetch")
153-
.with_stderr("[UPDATING] git repository `https://github.com/rust-lang/bitflags.git`")
154-
.run();
176+
[dependencies]
177+
bitflags = {{ git = "https://github.com/rust-lang/bitflags.git"{manifest_ref}}}
178+
"#
179+
),
180+
)
181+
.file("src/lib.rs", "")
182+
.build();
183+
p.cargo("fetch")
184+
.env("CARGO_LOG", "cargo::sources::git::utils=debug")
185+
.with_stderr_contains("[UPDATING] git repository `https://github.com/rust-lang/bitflags.git`")
186+
.with_stderr_contains("[..]attempting GitHub fast path[..]")
187+
.with_stderr_contains(&format!("[..]github fast path fetch {oid}"))
188+
.with_stderr_contains(&format!("[..]initiating fetch of [{refspecs}] from https://github.com/rust-lang/bitflags.git"))
189+
.run();
190+
// Remove the lock file, and test the up-to-date code path.
191+
p.root().join("Cargo.lock").rm_rf();
192+
p.cargo("fetch")
193+
.env("CARGO_LOG", "cargo::sources::git::utils=debug")
194+
.with_stderr_contains(&format!("[..]{up_to_date}"))
195+
.run();
196+
197+
paths::home().join(".cargo/git").rm_rf();
198+
}
155199
}

0 commit comments

Comments
 (0)