repository: address review, build PackReader key from pack_id and detect in-memory packs in iter_headers

src/borg/cache.py

@@ -661,8 +661,7 @@ def build_chunkindex_from_repo(
         # PackReader uses the store directly, so refresh the lock here; a full rebuild can be slow.
         repository._lock_refresh()
         pack_id = hex_to_bin(info.name)
-        pack_name = "packs/" + info.name
-        for chunk_id, obj_offset, obj_size in PackReader(repository.store, pack_name).iter_headers():
+        for chunk_id, obj_offset, obj_size in PackReader(repository.store, pack_id).iter_headers():
             num_chunks += 1
             chunks[chunk_id] = ChunkIndexEntry(
                 flags=init_flags, size=0, pack_id=pack_id, obj_offset=obj_offset, obj_size=obj_size

src/borg/repository.py

@@ -206,33 +206,33 @@ class PackWriter:
 
 
 class PackReader:
-    """Walks the object headers of a pack, the read-side counterpart to PackWriter.
+    """Reads pack files, the read-side counterpart to PackWriter.
 
-    Reads only the fixed headers, not the payloads, so locating every object in a pack
-    costs one short range read per object.
+    Pass pack_id to read from the store, or pack_contents for a pack already in memory.
     """
 
-    def __init__(self, store, key):
+    def __init__(self, store=None, pack_id=None, pack_contents=None):
         self.store = store
-        self.key = key  # "packs/<pack_id hex>"
+        self.pack_id = pack_id
+        self.key = "packs/" + bin_to_hex(pack_id) if pack_id is not None else None
+        self.pack_contents = pack_contents
+
+    def _read(self, offset, size):
+        # read from the in-memory pack if we have it, else range-read from the store
+        if self.pack_contents is not None:
+            return self.pack_contents[offset : offset + size]
+        return self.store.load(self.key, offset=offset, size=size)
 
     def iter_headers(self):
-        """Yield (chunk_id, offset, size) for each object, reading the headers from the store."""
-        return self._iter_headers(lambda offset, size: self.store.load(self.key, offset=offset, size=size))
+        """Yield (chunk_id, offset, size) for each object by walking the fixed object headers.
 
-    @classmethod
-    def from_bytes(cls, pack):
-        """Like iter_headers(), but for a pack already held in memory."""
-        return cls._iter_headers(lambda offset, size: pack[offset : offset + size])
-
-    @staticmethod
-    def _iter_headers(read):
-        # read(offset, size) returns pack bytes from the store or from memory; each header gives the
-        # object size, hence the offset of the next one.
+        Only the headers are read, not the payloads, so locating every object costs one short
+        range read per object (or just a slice, when the pack is already in memory).
+        """
         hdr_size = RepoObj.obj_header.size
         offset = 0
         while True:
-            hdr_data = read(offset, hdr_size)
+            hdr_data = self._read(offset, hdr_size)
             if len(hdr_data) < hdr_size:
                 break  # clean EOF, or trailing partial bytes
             hdr = RepoObj.ObjHeader(*RepoObj.obj_header.unpack(hdr_data))

src/borg/testsuite/repository_test.py

@@ -439,30 +439,31 @@ def test_pack_writer_final_partial_pack_uses_sha256():
     assert pack_id != chunk_id
 
 
-def test_pack_reader_from_bytes_walks_objects():
+def test_pack_reader_in_memory_walks_objects():
     obj1 = fchunk(b"payload-one", meta=b"meta1", chunk_id=H(1))
     obj2 = fchunk(b"d2", meta=b"m2", chunk_id=H(2))
     pack = obj1 + obj2
-    assert list(PackReader.from_bytes(pack)) == [(H(1), 0, len(obj1)), (H(2), len(obj1), len(obj2))]
+    headers = PackReader(pack_contents=pack).iter_headers()
+    assert list(headers) == [(H(1), 0, len(obj1)), (H(2), len(obj1), len(obj2))]
 
 
 def test_pack_reader_empty_pack():
-    assert list(PackReader.from_bytes(b"")) == []
+    assert list(PackReader(pack_contents=b"").iter_headers()) == []
 
 
 def test_pack_reader_stops_on_trailing_partial_header():
     # a truncated trailing header is a clean stop, not an error
     obj = fchunk(b"data", meta=b"meta", chunk_id=H(3))
     pack = obj + b"\x00\x00\x00"
-    assert list(PackReader.from_bytes(pack)) == [(H(3), 0, len(obj))]
+    assert list(PackReader(pack_contents=pack).iter_headers()) == [(H(3), 0, len(obj))]
 
 
 def test_pack_reader_iter_headers_reads_through_store(tmp_path):
     obj1 = fchunk(b"FIRST", chunk_id=H(47))
     obj2 = fchunk(b"SECOND", chunk_id=H(48))
     pack = obj1 + obj2
-    pack_key = "packs/" + bin_to_hex(H(43))
+    pack_id = H(43)
     with Repository(str(tmp_path / "repo"), exclusive=True, create=True) as repository:
-        repository.store_store(pack_key, pack)
-        reader = PackReader(repository.store, pack_key)
+        repository.store_store("packs/" + bin_to_hex(pack_id), pack)
+        reader = PackReader(repository.store, pack_id)
         assert list(reader.iter_headers()) == [(H(47), 0, len(obj1)), (H(48), len(obj1), len(obj2))]