fix(git): download each locked rev from its own checkout

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<Revision>,
     /// The unique identifier of this source.
     source_id: RefCell<SourceId>,
-    /// 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<Option<RecursivePathSource<'gctx>>>,
-    /// 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<Option<InternedString>>,
+    /// 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<HashMap<git2::Oid, (RecursivePathSource<'gctx>, 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<Vec<Package>> {
-        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<git2::Oid> {
+        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
         // <https://github.com/servo/servo/pull/14397>.
-        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<String> {
+    fn fingerprint(&self, pkg: &Package) -> CargoResult<String> {
+        // 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"),