diff --git a/src/sources/git/source.rs b/src/sources/git/source.rs index f7c9787b97a..52751030de9 100644 --- a/src/sources/git/source.rs +++ b/src/sources/git/source.rs @@ -4,12 +4,14 @@ use crate::sources::IndexSummary; use crate::sources::RecursivePathSource; use crate::sources::git::utils::GitDatabase; use crate::sources::git::utils::GitRemote; +use crate::sources::git::utils::GitShortID; use crate::sources::git::utils::rev_to_oid; use crate::sources::source::MaybePackage; use crate::sources::source::QueryKind; use crate::sources::source::Source; use crate::util::GlobalContext; use crate::util::cache_lock::CacheLockMode; +use crate::util::data_structures::HashMap; use crate::util::errors::CargoResult; use crate::util::hex::short_hash; use crate::util::interning::InternedString; @@ -76,19 +78,16 @@ pub struct GitSource<'gctx> { locked_rev: RefCell, /// The unique identifier of this source. source_id: RefCell, - /// The underlying path source to discover packages inside the Git repository. + /// The underlying path sources to discover packages inside the Git repository. + /// This map is keyed by the revision of their checkout. /// - /// This gets set to `Some` after the git repo has been checked out - /// (automatically handled via [`GitSource::update`]). - path_source: RefCell>>, - /// A short string that uniquely identifies the version of the checkout. + /// The short id is typically a 7-character string of the OID hash, + /// automatically increasing in size if it is ambiguous. /// - /// This is typically a 7-character string of the OID hash, automatically - /// increasing in size if it is ambiguous. - /// - /// This is set to `Some` after the git repo has been checked out - /// (automatically handled via [`GitSource::update`]). - short_id: RefCell>, + /// An new entry would be added after a revision has been checked out + /// (automatically handled via [`GitSource::update`]), + /// and [`GitSource::checked_out_rev`] would select the entry in use. + path_sources: RefCell, GitShortID)>>, /// The identifier of this source for Cargo's Git cache directory. /// See [`ident`] for more. ident: InternedString, @@ -140,8 +139,7 @@ impl<'gctx> GitSource<'gctx> { remote, locked_rev: RefCell::new(locked_rev), source_id: RefCell::new(source_id), - path_source: RefCell::new(None), - short_id: RefCell::new(None), + path_sources: RefCell::new(HashMap::default()), ident: ident.into(), gctx, quiet: false, @@ -159,40 +157,71 @@ impl<'gctx> GitSource<'gctx> { /// repository as well as walk the filesystem if package information /// haven't yet updated. pub fn read_packages(&mut self) -> CargoResult> { - if self.path_source.borrow().is_none() { + if self.checked_out_rev().is_none() { self.invalidate_cache(); self.update()?; } - self.path_source + let rev = self.checked_out_rev().unwrap(); + self.path_sources .borrow_mut() - .as_mut() - .unwrap() + .get_mut(&rev) + .expect("rev is already fetched") + .0 .read_packages() } - fn mark_used(&self) -> CargoResult<()> { + fn mark_used(&self, rev: git2::Oid) -> CargoResult<()> { + let short_name = self + .path_sources + .borrow() + .get(&rev) + .expect("update before download") + .1 + .as_str() + .into(); self.gctx .deferred_global_last_use()? .mark_git_checkout_used(global_cache_tracker::GitCheckout { encoded_git_name: self.ident, - short_name: self.short_id.borrow().expect("update before download"), + short_name, size: None, }); Ok(()) } + /// The revision this source is checked out at, if any. + /// + /// Returns `None` if nothing has yet fetched. + fn checked_out_rev(&self) -> Option { + match &*self.locked_rev.borrow() { + Revision::Locked(rev) if self.path_sources.borrow().contains_key(rev) => Some(*rev), + _ => None, + } + } + /// Fetch and return a [`GitDatabase`] with the resolved revision /// for this source, /// /// This won't fetch anything if the required revision is /// already available locally. pub(crate) fn fetch_db(&self, is_submodule: bool) -> CargoResult<(GitDatabase, git2::Oid)> { + let rev = self.locked_rev.borrow().clone(); + self.fetch_db_with_rev(is_submodule, &rev) + } + + /// Like [`GitSource::fetch_db`], but resolving the given revision + /// rather than [`GitSource::locked_rev`]. + fn fetch_db_with_rev( + &self, + is_submodule: bool, + rev: &Revision, + ) -> CargoResult<(GitDatabase, git2::Oid)> { let db_path = self.gctx.git_db_path().join(&self.ident); let db_path = db_path.into_path_unlocked(); let db = self.remote.db_at(&db_path).ok(); - let (db, actual_rev) = match (&*self.locked_rev.borrow(), db) { + let (db, actual_rev) = match (rev, db) { // If we have a locked revision, and we have a preexisting database // which has that revision, then no update needs to happen. (Revision::Locked(oid), Some(db)) if db.contains(*oid) => (db, *oid), @@ -249,8 +278,8 @@ impl<'gctx> GitSource<'gctx> { } fn update(&self) -> CargoResult<()> { - if self.path_source.borrow().is_some() { - self.mark_used()?; + if let Some(rev) = self.checked_out_rev() { + self.mark_used(rev)?; return Ok(()); } @@ -274,12 +303,38 @@ impl<'gctx> GitSource<'gctx> { let (db, actual_rev) = self.fetch_db(false)?; + let source_id = self + .source_id + .borrow() + .with_git_precise(Some(actual_rev.to_string())); + self.checkout(&db, actual_rev, source_id)?; + self.locked_rev.replace(Revision::Locked(actual_rev)); + + self.mark_used(actual_rev)?; + Ok(()) + } + + /// Ensures a path source exists and is fetched for `rev`. + fn ensure_checkout(&self, rev: git2::Oid, source_id: SourceId) -> CargoResult<()> { + if self.path_sources.borrow().contains_key(&rev) { + return Ok(()); + } + + let git_fs = self.gctx.git_path(); + self.gctx + .assert_package_cache_locked(CacheLockMode::DownloadExclusive, &git_fs); + let (db, _) = self.fetch_db_with_rev(false, &Revision::Locked(rev))?; + self.checkout(&db, rev, source_id) + } + + /// Checks `rev` out of the Git database + fn checkout(&self, db: &GitDatabase, rev: git2::Oid, source_id: SourceId) -> CargoResult<()> { // Don’t use the full hash, in order to contribute less to reaching the // path length limit on Windows. See // . - let short_id = db.to_short_id(actual_rev)?; + let short_id = db.to_short_id(rev)?; - // Check out `actual_rev` from the database to a scoped location on the + // Check out `rev` from the database to a scoped location on the // filesystem. This will use hard links and such to ideally make the // checkout operation here pretty fast. let checkout_path = self @@ -288,20 +343,13 @@ impl<'gctx> GitSource<'gctx> { .join(&self.ident) .join(short_id.as_str()); let checkout_path = checkout_path.into_path_unlocked(); - db.copy_to(actual_rev, &checkout_path, self.gctx, self.quiet)?; + db.copy_to(rev, &checkout_path, self.gctx, self.quiet)?; - let source_id = self - .source_id - .borrow() - .with_git_precise(Some(actual_rev.to_string())); let path_source = RecursivePathSource::new(&checkout_path, source_id, self.gctx); - - self.path_source.replace(Some(path_source)); - self.short_id.replace(Some(short_id.as_str().into())); - self.locked_rev.replace(Revision::Locked(actual_rev)); - self.path_source.borrow().as_ref().unwrap().load()?; - - self.mark_used()?; + path_source.load()?; + self.path_sources + .borrow_mut() + .insert(rev, (path_source, short_id)); Ok(()) } } @@ -393,11 +441,12 @@ impl<'gctx> Source for GitSource<'gctx> { kind: QueryKind, f: &mut dyn FnMut(IndexSummary), ) -> CargoResult<()> { - if self.path_source.borrow().is_none() { + if self.checked_out_rev().is_none() { self.update()?; } - let src = self.path_source.borrow(); - let src = src.as_ref().unwrap(); + let rev = self.checked_out_rev().unwrap(); + let sources = self.path_sources.borrow(); + let (src, _) = sources.get(&rev).expect("rev is already fetched"); src.query(dep, kind, f).await } @@ -418,11 +467,25 @@ impl<'gctx> Source for GitSource<'gctx> { "getting packages for package ID `{}` from `{:?}`", id, self.remote ); - self.mark_used()?; - self.path_source - .borrow_mut() - .as_mut() - .expect("BUG: `update()` must be called before `get()`") + // In a multi-rev lockfile, + // a package may be served from a revision other than `locked_rev`, + // so here we check SourceId precise field + // See rust-lang/cargo#13641 + let rev = match id.source_id().precise_git_fragment().and_then(rev_to_oid) { + Some(requested) => { + self.ensure_checkout(requested, id.source_id())?; + requested + } + None => self + .checked_out_rev() + .expect("BUG: `update()` must be called before `get()`"), + }; + self.mark_used(rev)?; + self.path_sources + .borrow() + .get(&rev) + .expect("rev is already fetched") + .0 .download(id) .await } @@ -431,7 +494,20 @@ impl<'gctx> Source for GitSource<'gctx> { panic!("no download should have started") } - fn fingerprint(&self, _pkg: &Package) -> CargoResult { + fn fingerprint(&self, pkg: &Package) -> CargoResult { + // In a multi-rev lockfile, + // a package may be served from a revision other than `locked_rev`, + // so here we check SourceId precise field + // See rust-lang/cargo#13641 + if let Some(rev) = pkg + .package_id() + .source_id() + .precise_git_fragment() + .and_then(rev_to_oid) + { + return Ok(rev.to_string()); + } + match &*self.locked_rev.borrow() { Revision::Locked(oid) => Ok(oid.to_string()), _ => unreachable!("locked_rev must be resolved when computing fingerprint"), diff --git a/tests/testsuite/git.rs b/tests/testsuite/git.rs index 184715c6368..dbe2f17ec82 100644 --- a/tests/testsuite/git.rs +++ b/tests/testsuite/git.rs @@ -4818,3 +4818,387 @@ Caused by: "#]]) .run(); } + +/// A lockfile can pin packages from one git URL at different revisions. +/// Each package is expected be downloaded from the revision it is pinned to. +/// +/// See rust-lang/cargo#13641. +#[cargo_test] +fn lockfile_with_multiple_revisions_bump_pkg_version() { + // #1: the upstream workspace at rev1 + let (upstream, upstream_repo) = git::new_repo("upstream", |p| { + p.file( + "Cargo.toml", + r#" + [workspace] + members = ["a"] + "#, + ) + .file("a/Cargo.toml", &basic_manifest("a", "0.1.0")) + .file("a/src/lib.rs", "") + }); + let rev1 = upstream_repo.head().unwrap().target().unwrap(); + + // #2: `foo` locks a@rev1 + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}" }} + "#, + upstream.url() + ), + ) + .file("src/lib.rs", "") + .build(); + p.cargo("fetch").run(); + assert!(p.read_file("Cargo.lock").contains(&rev1.to_string())); + + // #3: upstream add a new member `b` and moves to rev2 + upstream.change_file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ); + upstream.change_file("a/Cargo.toml", &basic_manifest("a", "0.2.0")); + upstream.change_file("b/Cargo.toml", &basic_manifest("b", "0.1.0")); + upstream.change_file("b/src/lib.rs", ""); + git::add(&upstream_repo); + let rev2 = git::commit(&upstream_repo); + + // #4: `foo` gains `b` through the new `m2` + let m2 = git::new("m2", |p| { + p.file( + "Cargo.toml", + &format!( + r#" + [package] + name = "m2" + version = "0.1.0" + edition = "2015" + + [dependencies] + b = {{ git = "{}" }} + "#, + upstream.url() + ), + ) + .file("src/lib.rs", "") + }); + p.change_file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}" }} + m2 = {{ git = "{}" }} + "#, + upstream.url(), + m2.url() + ), + ); + + // #5: `a@rev1` should remains locked + // and `m2` is at `rev2` + p.cargo("fetch") + .with_stderr_data(str![[r#" +[UPDATING] git repository `[ROOTURL]/m2` +[UPDATING] git repository `[ROOTURL]/upstream` +[LOCKING] 2 packages to latest compatible versions +[ADDING] b v0.1.0 ([ROOTURL]/upstream#[..]) +[ADDING] m2 v0.1.0 ([ROOTURL]/m2#[..]) + +"#]]) + .run(); + + // #6: multi-rev lockfile was written out + let lock = p.read_file("Cargo.lock"); + assert!(lock.contains(&rev1.to_string())); + assert!(lock.contains(&rev2.to_string())); + + // #7: a fresh checkout should succeed + paths::cargo_home().join("git").rm_rf(); + p.cargo("fetch").run(); +} + +/// Like [`lockfile_with_multiple_revisions_bump_pkg_version`] +/// but instead of bump package version, the code content changes. +/// +/// See rust-lang/cargo#13641 +#[cargo_test] +fn lockfile_with_multiple_revisions_change_code_content() { + // #1: the upstream workspace at rev1 + let (upstream, upstream_repo) = git::new_repo("upstream", |p| { + p.file( + "Cargo.toml", + r#" + [workspace] + members = ["a"] + "#, + ) + .file("a/Cargo.toml", &basic_manifest("a", "0.1.0")) + .file( + "a/src/lib.rs", + r#"pub fn which() -> &'static str { "rev1" }"#, + ) + }); + let rev1 = upstream_repo.head().unwrap().target().unwrap(); + + // #2: `foo` locks a@rev1 and prints "rev1" + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}" }} + "#, + upstream.url() + ), + ) + .file( + "src/main.rs", + r#" + extern crate a; + fn main() { + println!("{}", a::which()); + } + "#, + ) + .build(); + p.cargo("run") + .with_stdout_data(str![[r#" +rev1 + +"#]]) + .run(); + assert!(p.read_file("Cargo.lock").contains(&rev1.to_string())); + + // #3: upstream add a new member `b` and moves to rev2 + upstream.change_file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ); + upstream.change_file( + "a/src/lib.rs", + r#"pub fn which() -> &'static str { "rev2" }"#, + ); + upstream.change_file("b/Cargo.toml", &basic_manifest("b", "0.1.0")); + upstream.change_file("b/src/lib.rs", ""); + git::add(&upstream_repo); + let rev2 = git::commit(&upstream_repo); + + // #4: `foo` gains `b` through the new `m2` + let m2 = git::new("m2", |p| { + p.file( + "Cargo.toml", + &format!( + r#" + [package] + name = "m2" + version = "0.1.0" + edition = "2015" + + [dependencies] + b = {{ git = "{}" }} + "#, + upstream.url() + ), + ) + .file("src/lib.rs", "") + }); + p.change_file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}" }} + m2 = {{ git = "{}" }} + "#, + upstream.url(), + m2.url() + ), + ); + + // #5: `a@rev1` should remain locked, no recompile, and print "rev1" + // `m2` should be at `rev2` + p.cargo("run") + .with_stderr_data( + str![[r#" +[UPDATING] git repository `[ROOTURL]/m2` +[UPDATING] git repository `[ROOTURL]/upstream` +[LOCKING] 2 packages to latest compatible versions +[ADDING] b v0.1.0 ([ROOTURL]/upstream#[..]) +[ADDING] m2 v0.1.0 ([ROOTURL]/m2#[..]) +[COMPILING] b v0.1.0 ([ROOTURL]/upstream#[..]) +[COMPILING] m2 v0.1.0 ([ROOTURL]/m2#[..]) +[COMPILING] foo v0.1.0 ([ROOT]/foo) +[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s +[RUNNING] `target/debug/foo[EXE]` + +"#]] + .unordered(), + ) + .with_stdout_data(str![[r#" +rev1 + +"#]]) + .run(); + let lock = p.read_file("Cargo.lock"); + assert!(lock.contains(&rev1.to_string())); + assert!(lock.contains(&rev2.to_string())); +} + +/// Like [`lockfile_with_multiple_revisions_change_code_content`] +/// but instead of changing code content, +/// specify on the same branch ref that moves forward +/// +/// See rust-lang/cargo#14230 +#[cargo_test] +fn lockfile_with_multiple_revisions_of_same_git_branch() { + // #1: the upstream workspace at rev1 + let (upstream, upstream_repo) = git::new_repo("upstream", |p| { + p.file( + "Cargo.toml", + r#" + [workspace] + members = ["a"] + "#, + ) + .file("a/Cargo.toml", &basic_manifest("a", "0.1.0")) + .file( + "a/src/lib.rs", + r#"pub fn which() -> &'static str { "rev1" }"#, + ) + }); + let rev1 = upstream_repo.head().unwrap().target().unwrap(); + + // #2: `foo` locks a@rev1 and prints "rev1" + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}", branch = "master" }} + "#, + upstream.url() + ), + ) + .file( + "src/main.rs", + r#" + extern crate a; + fn main() { + println!("{}", a::which()); + } + "#, + ) + .build(); + p.cargo("run") + .with_stdout_data(str![[r#" +rev1 + +"#]]) + .run(); + let lock = p.read_file("Cargo.lock"); + assert!(lock.contains(&format!("?branch=master#{rev1}"))); + + // #3: upstream add a new member `b` and moves to rev2 + upstream.change_file( + "Cargo.toml", + r#" + [workspace] + members = ["a", "b"] + "#, + ); + upstream.change_file( + "a/src/lib.rs", + r#"pub fn which() -> &'static str { "rev2" }"#, + ); + upstream.change_file("b/Cargo.toml", &basic_manifest("b", "0.1.0")); + upstream.change_file("b/src/lib.rs", ""); + git::add(&upstream_repo); + let rev2 = git::commit(&upstream_repo); + + // #4: `foo` gains `b` through the new `m2` + let m2 = git::new("m2", |p| { + p.file( + "Cargo.toml", + &format!( + r#" + [package] + name = "m2" + version = "0.1.0" + edition = "2015" + + [dependencies] + b = {{ git = "{}", branch = "master" }} + "#, + upstream.url() + ), + ) + .file("src/lib.rs", "") + }); + p.change_file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2015" + + [dependencies] + a = {{ git = "{}", branch = "master" }} + m2 = {{ git = "{}" }} + "#, + upstream.url(), + m2.url() + ), + ); + + // #5: `a@rev1` should remain locked, no recompile, and print "rev1" + // `m2` should be at `rev2` + p.cargo("run") + .with_stdout_data(str![[r#" +rev1 + +"#]]) + .run(); + let lock = p.read_file("Cargo.lock"); + assert!(lock.contains(&format!("?branch=master#{rev1}"))); + assert!(lock.contains(&format!("?branch=master#{rev2}"))); +}