Support pack.indexVersion config option (#1594)

dulwich/object_store.py

@@ -402,9 +402,10 @@ class BaseObjectStore:
 
 
 class PackBasedObjectStore(BaseObjectStore):
-    def __init__(self, pack_compression_level=-1) -> None:
+    def __init__(self, pack_compression_level=-1, pack_index_version=None) -> None:
         self._pack_cache: dict[str, Pack] = {}
         self.pack_compression_level = pack_compression_level
+        self.pack_index_version = pack_index_version
 
     def add_pack(self) -> tuple[BytesIO, Callable[[], None], Callable[[], None]]:
         """Add a new pack to this object store."""
@@ -791,6 +792,7 @@ class DiskObjectStore(PackBasedObjectStore):
         path: Union[str, os.PathLike],
         loose_compression_level=-1,
         pack_compression_level=-1,
+        pack_index_version=None,
     ) -> None:
         """Open an object store.
 
@@ -798,13 +800,18 @@ class DiskObjectStore(PackBasedObjectStore):
           path: Path of the object store.
           loose_compression_level: zlib compression level for loose objects
           pack_compression_level: zlib compression level for pack objects
+          pack_index_version: pack index version to use (1, 2, or 3)
         """
-        super().__init__(pack_compression_level=pack_compression_level)
+        super().__init__(
+            pack_compression_level=pack_compression_level,
+            pack_index_version=pack_index_version,
+        )
         self.path = path
         self.pack_dir = os.path.join(self.path, PACKDIR)
         self._alternates = None
         self.loose_compression_level = loose_compression_level
         self.pack_compression_level = pack_compression_level
+        self.pack_index_version = pack_index_version
 
         # Commit graph support - lazy loaded
         self._commit_graph = None
@@ -832,7 +839,13 @@ class DiskObjectStore(PackBasedObjectStore):
             )
         except KeyError:
             pack_compression_level = default_compression_level
-        return cls(path, loose_compression_level, pack_compression_level)
+        try:
+            pack_index_version = int(config.get((b"pack",), b"indexVersion").decode())
+        except KeyError:
+            pack_index_version = None
+        return cls(
+            path, loose_compression_level, pack_compression_level, pack_index_version
+        )
 
     @property
     def alternates(self):
@@ -1019,7 +1032,9 @@ class DiskObjectStore(PackBasedObjectStore):
 
         # Write the index.
         with GitFile(target_index_path, "wb", mask=PACK_MODE) as index_file:
-            write_pack_index(index_file, entries, pack_sha)
+            write_pack_index(
+                index_file, entries, pack_sha, version=self.pack_index_version
+            )
 
         # Add the pack to the store and return it.
         final_pack = Pack(pack_base_name)
@@ -1892,7 +1907,7 @@ class BucketBasedObjectStore(PackBasedObjectStore):
                 max_size=PACK_SPOOL_FILE_MAX_SIZE, prefix="incoming-"
             )
             checksum = p.get_stored_checksum()
-            write_pack_index(idxf, entries, checksum)
+            write_pack_index(idxf, entries, checksum, version=self.pack_index_version)
             idxf.seek(0)
             idx = load_pack_index_file(basename + ".idx", idxf)
             for pack in self.packs:

dulwich/pack.py

@@ -91,6 +91,9 @@ DEFAULT_PACK_DELTA_WINDOW_SIZE = 10
 # Keep pack files under 16Mb in memory, otherwise write them out to disk
 PACK_SPOOL_FILE_MAX_SIZE = 16 * 1024 * 1024
 
+# Default pack index version to use when none is specified
+DEFAULT_PACK_INDEX_VERSION = 2
+
 
 OldUnpackedObject = Union[tuple[Union[bytes, int], list[bytes]], list[bytes]]
 ResolveExtRefFn = Callable[[bytes], tuple[int, OldUnpackedObject]]
@@ -1960,7 +1963,7 @@ def write_pack(
         )
     entries = sorted([(k, v[0], v[1]) for (k, v) in entries.items()])
     with GitFile(filename + ".idx", "wb") as f:
-        return data_sum, write_pack_index_v2(f, entries, data_sum)
+        return data_sum, write_pack_index(f, entries, data_sum)
 
 
 def pack_header_chunks(num_objects):
@@ -2672,7 +2675,32 @@ def write_pack_index_v3(
     return f.write_sha()
 
 
-write_pack_index = write_pack_index_v2
+def write_pack_index(
+    index_filename, entries, pack_checksum, progress=None, version=None
+):
+    """Write a pack index file.
+
+    Args:
+      index_filename: Index filename.
+      entries: List of (checksum, offset, crc32) tuples
+      pack_checksum: Checksum of the pack file.
+      progress: Progress function (not currently used)
+      version: Pack index version to use (1, 2, or 3). If None, defaults to DEFAULT_PACK_INDEX_VERSION.
+
+    Returns:
+      SHA of the written index file
+    """
+    if version is None:
+        version = DEFAULT_PACK_INDEX_VERSION
+
+    if version == 1:
+        return write_pack_index_v1(index_filename, entries, pack_checksum)
+    elif version == 2:
+        return write_pack_index_v2(index_filename, entries, pack_checksum)
+    elif version == 3:
+        return write_pack_index_v3(index_filename, entries, pack_checksum)
+    else:
+        raise ValueError(f"Unsupported pack index version: {version}")
 
 
 class Pack:

dulwich/porcelain.py

@@ -2004,6 +2004,7 @@ def pack_objects(
     delta_window_size=None,
     deltify=None,
     reuse_deltas=True,
+    pack_index_version=None,
 ) -> None:
     """Pack objects into a file.
 
@@ -2016,6 +2017,7 @@ def pack_objects(
                          Set to None for default window size.
       deltify: Whether to deltify objects
       reuse_deltas: Allow reuse of existing deltas while deltifying
+      pack_index_version: Pack index version to use (1, 2, or 3). If None, uses default version.
     """
     with open_repo_closing(repo) as r:
         entries, data_sum = write_pack_from_container(
@@ -2028,7 +2030,7 @@ def pack_objects(
         )
     if idxf is not None:
         entries = sorted([(k, v[0], v[1]) for (k, v) in entries.items()])
-        write_pack_index(idxf, entries, data_sum)
+        write_pack_index(idxf, entries, data_sum, version=pack_index_version)
 
 
 def ls_tree(

tests/test_object_store.py

@@ -341,6 +341,80 @@ class DiskObjectStoreTests(PackBasedObjectStoreTests, TestCase):
             self.assertEqual([], entries)
             o.add_thin_pack(f.read, None)
 
+    def test_pack_index_version_config(self) -> None:
+        # Test that pack.indexVersion configuration is respected
+        from dulwich.config import ConfigDict
+        from dulwich.pack import load_pack_index
+
+        # Create config with pack.indexVersion = 1
+        config = ConfigDict()
+        config[(b"pack",)] = {b"indexVersion": b"1"}
+
+        # Create object store with config
+        store_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, store_dir)
+        os.makedirs(os.path.join(store_dir, "pack"))
+        store = DiskObjectStore.from_config(store_dir, config)
+        self.addCleanup(store.close)
+
+        # Create some objects to pack
+        b1 = make_object(Blob, data=b"blob1")
+        b2 = make_object(Blob, data=b"blob2")
+        store.add_objects([(b1, None), (b2, None)])
+
+        # Add a pack
+        f, commit, abort = store.add_pack()
+        try:
+            # build_pack expects (type_num, data) tuples
+            objects_spec = [
+                (b1.type_num, b1.as_raw_string()),
+                (b2.type_num, b2.as_raw_string()),
+            ]
+            build_pack(f, objects_spec, store=store)
+            commit()
+        except:
+            abort()
+            raise
+
+        # Find the created pack index
+        pack_dir = os.path.join(store_dir, "pack")
+        idx_files = [f for f in os.listdir(pack_dir) if f.endswith(".idx")]
+        self.assertEqual(1, len(idx_files))
+
+        # Load and verify it's version 1
+        idx_path = os.path.join(pack_dir, idx_files[0])
+        idx = load_pack_index(idx_path)
+        self.assertEqual(1, idx.version)
+
+        # Test version 3
+        config[(b"pack",)] = {b"indexVersion": b"3"}
+        store_dir2 = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, store_dir2)
+        os.makedirs(os.path.join(store_dir2, "pack"))
+        store2 = DiskObjectStore.from_config(store_dir2, config)
+        self.addCleanup(store2.close)
+
+        b3 = make_object(Blob, data=b"blob3")
+        store2.add_objects([(b3, None)])
+
+        f2, commit2, abort2 = store2.add_pack()
+        try:
+            objects_spec2 = [(b3.type_num, b3.as_raw_string())]
+            build_pack(f2, objects_spec2, store=store2)
+            commit2()
+        except:
+            abort2()
+            raise
+
+        # Find and verify version 3 index
+        pack_dir2 = os.path.join(store_dir2, "pack")
+        idx_files2 = [f for f in os.listdir(pack_dir2) if f.endswith(".idx")]
+        self.assertEqual(1, len(idx_files2))
+
+        idx_path2 = os.path.join(pack_dir2, idx_files2[0])
+        idx2 = load_pack_index(idx_path2)
+        self.assertEqual(3, idx2.version)
+
 
 class TreeLookupPathTests(TestCase):
     def setUp(self) -> None:

tests/test_pack.py

@@ -967,6 +967,87 @@ class TestPackIndexWritingv3(TestCase, BaseTestFilePackIndexWriting):
         self.assertIn("wrong length", str(cm.exception))
 
 
+class WritePackIndexTests(TestCase):
+    """Tests for the configurable write_pack_index function."""
+
+    def test_default_pack_index_version_constant(self) -> None:
+        from dulwich.pack import DEFAULT_PACK_INDEX_VERSION
+
+        # Ensure the constant is set to version 2 (current Git default)
+        self.assertEqual(2, DEFAULT_PACK_INDEX_VERSION)
+
+    def test_write_pack_index_defaults_to_v2(self) -> None:
+        import tempfile
+
+        from dulwich.pack import (
+            DEFAULT_PACK_INDEX_VERSION,
+            load_pack_index,
+            write_pack_index,
+        )
+
+        tempdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tempdir)
+
+        entries = [(b"1" * 20, 42, zlib.crc32(b"data"))]
+        filename = os.path.join(tempdir, "test_default.idx")
+
+        with GitFile(filename, "wb") as f:
+            write_pack_index(f, entries, b"P" * 20)
+
+        idx = load_pack_index(filename)
+        self.assertEqual(DEFAULT_PACK_INDEX_VERSION, idx.version)
+
+    def test_write_pack_index_version_1(self) -> None:
+        import tempfile
+
+        from dulwich.pack import load_pack_index, write_pack_index
+
+        tempdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tempdir)
+
+        entries = [(b"1" * 20, 42, zlib.crc32(b"data"))]
+        filename = os.path.join(tempdir, "test_v1.idx")
+
+        with GitFile(filename, "wb") as f:
+            write_pack_index(f, entries, b"P" * 20, version=1)
+
+        idx = load_pack_index(filename)
+        self.assertEqual(1, idx.version)
+
+    def test_write_pack_index_version_3(self) -> None:
+        import tempfile
+
+        from dulwich.pack import load_pack_index, write_pack_index
+
+        tempdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tempdir)
+
+        entries = [(b"1" * 20, 42, zlib.crc32(b"data"))]
+        filename = os.path.join(tempdir, "test_v3.idx")
+
+        with GitFile(filename, "wb") as f:
+            write_pack_index(f, entries, b"P" * 20, version=3)
+
+        idx = load_pack_index(filename)
+        self.assertEqual(3, idx.version)
+
+    def test_write_pack_index_invalid_version(self) -> None:
+        import tempfile
+
+        from dulwich.pack import write_pack_index
+
+        tempdir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, tempdir)
+
+        entries = [(b"1" * 20, 42, zlib.crc32(b"data"))]
+        filename = os.path.join(tempdir, "test_invalid.idx")
+
+        with self.assertRaises(ValueError) as cm:
+            with GitFile(filename, "wb") as f:
+                write_pack_index(f, entries, b"P" * 20, version=99)
+        self.assertIn("Unsupported pack index version: 99", str(cm.exception))
+
+
 class MockFileWithoutFileno:
     """Mock file-like object without fileno method."""