From 19e974a365400eefc88399b7026ab1c554a90e7a Mon Sep 17 00:00:00 2001
From: Sebastian Thiel <sebastian.thiel@icloud.com>
Date: Wed, 1 Dec 2021 17:37:12 +0800
Subject: [PATCH] fix: linked::Store now assures unique IDs across compound
 stores (#260)

---
 git-odb/src/store/compound/init.rs      |  6 ++++--
 git-odb/src/store/linked/init.rs        | 10 ++++++++--
 git-odb/tests/odb/store/compound/mod.rs |  2 +-
 git-pack/src/data/mod.rs                |  5 ++---
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/git-odb/src/store/compound/init.rs b/git-odb/src/store/compound/init.rs
index 1d8b102e00a..cf9f0991cf6 100644
--- a/git-odb/src/store/compound/init.rs
+++ b/git-odb/src/store/compound/init.rs
@@ -25,7 +25,9 @@ impl compound::Store {
     ///
     /// Only loose and packed objects will be considered. See the [linked Db][crate::store::linked::Store] for a database with
     /// support for _git alternates_, i.e. linking to other repositories.
-    pub fn at(objects_directory: impl Into<PathBuf>) -> Result<compound::Store, Error> {
+    ///
+    /// `pack_id_offset` is used to allow multiple compound databases to be used for lookups without their pack-ids clashing.
+    pub fn at(objects_directory: impl Into<PathBuf>, pack_id_offset: u32) -> Result<compound::Store, Error> {
         let loose_objects = objects_directory.into();
         if !loose_objects.is_dir() {
             return Err(Error::Inaccessible(loose_objects));
@@ -44,7 +46,7 @@ impl compound::Store {
                                 (
                                     {
                                         // don't rely on crc32 for producing non-clashing ids. It's the kind of bug we don't want
-                                        b.pack.id = idx as u32;
+                                        b.pack.id = idx as u32 + pack_id_offset;
                                         b
                                     },
                                     mod_time,
diff --git a/git-odb/src/store/linked/init.rs b/git-odb/src/store/linked/init.rs
index b39ff01a447..bb2ca1abdcf 100644
--- a/git-odb/src/store/linked/init.rs
+++ b/git-odb/src/store/linked/init.rs
@@ -20,9 +20,15 @@ impl linked::Store {
     ///
     /// _git alternate_ files will be traversed to build a chain of [`compound::Store`] instances.
     pub fn at(objects_directory: impl Into<PathBuf>) -> Result<Self, Error> {
-        let mut dbs = vec![compound::Store::at(objects_directory.into())?];
+        let mut dbs = vec![compound::Store::at(objects_directory.into(), 0)?];
+
+        let compute_ofs = |db: &compound::Store| db.bundles.iter().map(|p| p.pack.id).max().map(|ofs| ofs + 1);
+        let mut ofs = compute_ofs(&dbs[0]).unwrap_or(0);
+
         for object_path in alternate::resolve(dbs[0].loose.path.clone())?.into_iter() {
-            dbs.push(compound::Store::at(object_path)?);
+            let store = compound::Store::at(object_path, ofs)?;
+            ofs = compute_ofs(&store).unwrap_or(ofs);
+            dbs.push(store);
         }
         assert!(
             !dbs.is_empty(),
diff --git a/git-odb/tests/odb/store/compound/mod.rs b/git-odb/tests/odb/store/compound/mod.rs
index 5b3bb0f4b88..3d3a88cd997 100644
--- a/git-odb/tests/odb/store/compound/mod.rs
+++ b/git-odb/tests/odb/store/compound/mod.rs
@@ -3,7 +3,7 @@ use git_odb::compound::Store;
 use crate::fixture_path;
 
 fn db() -> Store {
-    Store::at(fixture_path("objects")).expect("valid object path")
+    Store::at(fixture_path("objects"), 0).expect("valid object path")
 }
 
 mod init {
diff --git a/git-pack/src/data/mod.rs b/git-pack/src/data/mod.rs
index 794e0c2d432..70f25557fb8 100644
--- a/git-pack/src/data/mod.rs
+++ b/git-pack/src/data/mod.rs
@@ -56,9 +56,8 @@ pub struct File {
     path: std::path::PathBuf,
     /// A hash to represent the `path` field when used with cache lookup, or a way to identify this pack by its location on disk.
     ///
-    /// Note that `path` might not be canonicalized, thus different hashes might actually refer to the same pack on disk. This will
-    /// only lead to less efficient cache usage.
-    /// TODO: remove this intrinsic id as it will henceforth be handled separately by the object store
+    /// These must be unique per pack and must be stable, that is they don't change if the pack doesn't change.
+    /// If the same id is assigned (or reassigned) to different packs, pack creation or cache access will fail in hard-to-debug ways.
     pub id: u32,
     version: Version,
     num_objects: u32,