From 0f3611cfc90c6af62cd674785fd20b5070982734 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Thu, 19 Mar 2026 14:15:01 -0500 Subject: [PATCH 1/2] show broken behavior --- tests/testsuite/git.rs | 43 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 4f2e3feeb7a..dffe3ab5d6e 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -249,10 +249,20 @@ fn cargo_compile_git_dep_pull_request() { // Make a reference in GitHub's pull request ref naming convention. let repo = git2::Repository::open(&git_project.root()).unwrap(); - let oid = repo.refname_to_id("HEAD").unwrap(); - let force = false; let log_message = "open pull request"; - repo.reference("refs/pull/330/head", oid, force, log_message) + let parent = repo.head().unwrap().peel_to_commit().unwrap(); + let author = repo.signature().unwrap(); + let oid = repo + .commit( + None, + &author, + &author, + log_message, + &parent.tree().unwrap(), + &[&parent], + ) + .unwrap(); + repo.reference("refs/pull/330/head", oid, false, log_message) .unwrap(); let project = project @@ -290,6 +300,33 @@ fn cargo_compile_git_dep_pull_request() { .run(); assert!(project.bin("foo").is_file()); + + // Delete the local cache, but keep the Cargo.lock to prevent regression + // of + // This ensures we fetch the refs/pull/330/head spec explicitly, even + // if we have a locked commit. + paths::cargo_home().join("git").rm_rf(); + + project + .cargo("check") + .env("CARGO_NET_GIT_FETCH_WITH_CLI", "true") + .with_stderr_data(str![[r#" +[UPDATING] git repository `[ROOTURL]/dep1` +... +[ERROR] failed to get `dep1` as a dependency of package `foo v0.0.0 ([ROOT]/foo)` + +Caused by: + failed to load source for dependency `dep1` + +Caused by: + unable to update [ROOTURL]/dep1?rev=refs%2Fpull%2F330%2Fhead#[..] + +Caused by: + revspec '[..]' not found; class=Reference (4); code=NotFound (-3) + +"#]]) + .with_status(101) + .run(); } #[cargo_test] From a24052f0dfde720dd4ae19d1839bdbe2fe862584 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Thu, 19 Mar 2026 14:33:56 -0500 Subject: [PATCH 2/2] fix: fetching non-standard git refspecs on non-github repos --- src/cargo/sources/git/source.rs | 4 +++- src/cargo/sources/git/utils.rs | 16 +++++++++++++--- src/cargo/sources/registry/remote.rs | 1 + tests/testsuite/git.rs | 14 +++----------- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index d63e6dca283..fce8d6ac724 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -236,7 +236,9 @@ impl<'gctx> GitSource<'gctx> { trace!("updating git source `{:?}`", self.remote); let locked_rev = locked_rev.clone().into(); - self.remote.checkout(&db_path, db, &locked_rev, self.gctx)? + let manifest_reference = self.source_id.git_reference().unwrap(); + self.remote + .checkout(&db_path, db, manifest_reference, &locked_rev, self.gctx)? } }; Ok((db, actual_rev)) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 12d52e3aa95..99cf75cdf33 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -109,6 +109,7 @@ impl GitRemote { &self, into: &Path, db: Option, + manifest_reference: &GitReference, reference: &GitReference, gctx: &GlobalContext, ) -> CargoResult<(GitDatabase, git2::Oid)> { @@ -116,6 +117,7 @@ impl GitRemote { fetch( &mut db.repo, self.url(), + manifest_reference, reference, gctx, RemoteKind::GitDependency, @@ -138,6 +140,7 @@ impl GitRemote { fetch( &mut repo, self.url(), + manifest_reference, reference, gctx, RemoteKind::GitDependency, @@ -993,7 +996,8 @@ pub fn with_fetch_options( pub fn fetch( repo: &mut git2::Repository, remote_url: &str, - reference: &GitReference, + manifest_reference: &GitReference, + locked_reference: &GitReference, gctx: &GlobalContext, remote_kind: RemoteKind, ) -> CargoResult<()> { @@ -1009,7 +1013,7 @@ pub fn fetch( // Flag to keep track if the rev is a full commit hash let mut fast_path_rev: bool = false; - let oid_to_fetch = match github_fast_path(repo, remote_url, reference, gctx) { + let oid_to_fetch = match github_fast_path(repo, remote_url, locked_reference, gctx) { Ok(FastPathRev::UpToDate) => return Ok(()), Ok(FastPathRev::NeedsFetch(rev)) => Some(rev), Ok(FastPathRev::Indeterminate) => None, @@ -1030,7 +1034,7 @@ pub fn fetch( // The `+` symbol on the refspec means to allow a forced (fast-forward) // update which is needed if there is ever a force push that requires a // fast-forward. - match reference { + match locked_reference { // For branches and tags we can fetch simply one reference and copy it // locally, no need to fetch other branches/tags. GitReference::Branch(b) => { @@ -1061,6 +1065,12 @@ pub fn fetch( // The reason we write to `refs/remotes/origin/HEAD` is that it's of special significance // when during `GitReference::resolve()`, but otherwise it shouldn't matter. refspecs.push(format!("+{0}:refs/remotes/origin/HEAD", rev)); + } else if let GitReference::Rev(rev) = manifest_reference + && rev.starts_with("refs/") + { + // If the lockfile has a commit. we can't directly fetch it (unless we're talking + // to GitHub), so we fetch the ref associated with it from the manifest. + refspecs.push(format!("+{0}:{0}", rev)); } else { // We don't know what the rev will point to. To handle this // situation we fetch all branches and tags, and then we pray diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index 045059f378d..b99fa60822f 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -387,6 +387,7 @@ impl<'gctx> RegistryData for RemoteRegistry<'gctx> { repo, url.as_str(), &self.index_git_ref, + &self.index_git_ref, self.gctx, RemoteKind::Registry, ) diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index dffe3ab5d6e..a6b3d68977b 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -313,19 +313,11 @@ fn cargo_compile_git_dep_pull_request() { .with_stderr_data(str![[r#" [UPDATING] git repository `[ROOTURL]/dep1` ... -[ERROR] failed to get `dep1` as a dependency of package `foo v0.0.0 ([ROOT]/foo)` - -Caused by: - failed to load source for dependency `dep1` - -Caused by: - unable to update [ROOTURL]/dep1?rev=refs%2Fpull%2F330%2Fhead#[..] - -Caused by: - revspec '[..]' not found; class=Reference (4); code=NotFound (-3) +[CHECKING] dep1 v0.5.0 ([ROOTURL]/dep1?rev=refs%2Fpull%2F330%2Fhead#[..]) +[CHECKING] foo v0.0.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s "#]]) - .with_status(101) .run(); }