repository: always name packs by sha256(pack)

.github/workflows/ci.yml

@@ -703,57 +703,3 @@ jobs:
           report_type: coverage
           env_vars: OS,python
           files: coverage.xml
-
-  sha256_pack_id_tests:
-    name: sha256 pack-id (informational)
-    needs: [lint]
-    runs-on: ubuntu-24.04
-    timeout-minutes: 90
-    continue-on-error: true
-    concurrency:
-      group: sha256-pack-id-${{ github.head_ref || github.ref }}
-      cancel-in-progress: false
-
-    steps:
-    - uses: actions/checkout@v6
-      with:
-        fetch-depth: 0
-        fetch-tags: true
-
-    - name: Set up Python 3.12
-      uses: actions/setup-python@v6
-      with:
-        python-version: "3.12"
-
-    - name: Cache pip
-      uses: actions/cache@v5
-      with:
-        path: ~/.cache/pip
-        key: ${{ runner.os }}-${{ runner.arch }}-pip-sha256-pack-id-${{ hashFiles('requirements.d/development.lock.txt') }}
-        restore-keys: |
-          ${{ runner.os }}-${{ runner.arch }}-pip-
-
-    - name: Cache tox environments
-      uses: actions/cache@v5
-      with:
-        path: .tox
-        key: ${{ runner.os }}-${{ runner.arch }}-tox-sha256-pack-id-${{ hashFiles('requirements.d/development.lock.txt', 'pyproject.toml') }}
-        restore-keys: |
-          ${{ runner.os }}-${{ runner.arch }}-tox-sha256-pack-id-
-
-    - name: Install Linux packages
-      run: |
-        sudo apt-get update
-        sudo apt-get install -y pkg-config build-essential
-        sudo apt-get install -y libssl-dev libacl1-dev liblz4-dev
-
-    - name: Install Python requirements
-      run: |
-        python -m pip install --upgrade pip setuptools wheel
-        pip install -r requirements.d/development.lock.txt
-
-    - name: Install borgbackup
-      run: pip install -ve ".[cockpit,s3,sftp,rclone]"
-
-    - name: Run tests with sha256 pack-ids
-      run: tox -e sha256-pack-id

docs/internals/packs.rst

@@ -71,12 +71,13 @@ Blobs follow one another contiguously with no padding::
 Pack ID
 ~~~~~~~
 
-The pack ID equals the ``chunk_id`` of the blob it contains::
+The pack ID is the SHA-256 of the pack file's bytes::
 
-    pack_id = chunk_id
+    pack_id = sha256(pack_bytes)
 
-Since ``chunk_id`` is the ID hash of the plaintext, the filename commits to the
-content. ``borg check`` can detect silent corruption without decrypting any blob.
+Content-addressing the file by its own bytes makes the name commit to the
+content, so borgstore can verify and cache it and ``borg check`` can detect
+silent corruption of the stored file.
 
 Namespace
 ~~~~~~~~~

pyproject.toml

@@ -257,11 +257,6 @@ extras = ["pyfuse3", "sftp", "s3", "rclone"]
 set_env = {BORG_FUSE_IMPL = "mfusepy"}
 extras = ["mfusepy", "sftp", "s3", "rclone"]
 
-# Informational env: forces sha256 pack_ids even with max_count=1 to expose
-# code that still assumes pack_id == chunk_id. Run: tox -e sha256-pack-id
-[tool.tox.env.sha256-pack-id]
-set_env = {BORG_TESTONLY_SHA256_PACK_ID = "1"}
-
 [tool.tox.env.ruff]
 skip_install = true
 deps = ["ruff"]

src/borg/conftest.py

@@ -80,9 +80,6 @@ def pytest_sessionfinish(session, exitstatus):
 @pytest.fixture(autouse=True)
 def clean_env(tmpdir_factory, monkeypatch):
     # also avoid to use anything from the outside environment:
-    # note: BORG_TESTONLY_SHA256_PACK_ID is intentionally NOT exempted here. The repository
-    # module captures it at import time (repository.FORCE_SHA256_PACK_ID), before this fixture
-    # runs, so wiping it per test is harmless and the env stays fully isolated.
     keys = [key for key in os.environ if key.startswith("BORG_") and key not in ("BORG_FUSE_IMPL",)]
     for key in keys:
         monkeypatch.delenv(key, raising=False)

src/borg/repository.py

@@ -102,14 +102,6 @@ def build_rest_backend(location):
     return REST(base_url="http://stdio-backend", command=rest_serve_command(location))
 
 
-# Test-only switch: force sha256 pack_ids even at N=1, to expose code that still assumes
-# pack_id == chunk_id. Read once here, at import time, on purpose:
-#  - it happens before the per-test clean_env fixture wipes BORG_* vars, so the tox env that
-#    sets this var does not need a clean_env exemption (the captured value survives the wipe);
-#  - flush() then checks a plain bool instead of touching os.environ on every write.
-FORCE_SHA256_PACK_ID = os.environ.get("BORG_TESTONLY_SHA256_PACK_ID") == "1"
-
-
 class PackWriter:
     """Buffers chunks into a pack file and writes to the store when full.
 
@@ -123,10 +115,8 @@ class PackWriter:
     uses that repository's single, authoritative index (see the chunks property), so
     there is never a second copy to keep in sync.  Unit tests pass an explicit index.
 
-    At max_count=1 (N=1 phase) each put() maps exactly one chunk to one pack,
-    so pack_id == chunk_id and the naming scheme is unchanged from before.
-    Raising max_count later (N>1 phase) enables real packing without touching
-    this class's interface.
+    max_count bounds how many chunks a pack accumulates before flush() writes it.
+    Raising it produces larger packs without changing this class's interface.
     """
 
     def __init__(self, store, *, max_count=1, chunks=None, repository=None):
@@ -179,17 +169,9 @@ def flush(self):
         # that incremental string concatenation would cause in Python).
         pack_data = b"".join(cdata for _, cdata in self._pieces)
 
-        # Determine pack_id.
-        # N=1: the pack contains exactly one chunk, so we keep pack_id == chunk_id
-        #      (backward-compatible file naming: packs/{chunk_id_hex}).
-        # N>1: the pack contains multiple chunks; use SHA256(pack_bytes) so the
-        #      file is content-addressed and borgstore can verify/cache it.
-        # BORG_TESTONLY_SHA256_PACK_ID (see FORCE_SHA256_PACK_ID): always use sha256 even at
-        #      N=1, exposing code that still assumes pack_id == chunk_id.
-        if self.max_count == 1 and not FORCE_SHA256_PACK_ID:
-            pack_id = self._pieces[0][0]  # N=1: pack_id == chunk_id
-        else:
-            pack_id = sha256(pack_data).digest()
+        # Name the pack by the SHA-256 of its bytes: the name commits to the stored content,
+        # so borgstore can verify and cache the file.
+        pack_id = sha256(pack_data).digest()
 
         # Record (chunk_id, pack_id, obj_offset, obj_size) for every piece.
         results = []
@@ -675,10 +657,8 @@ def check_pack(pack):
                     # add all existing objects to the index.
                     # borg check: the index may have corrupted objects (we did not delete them)
                     # borg check --repair: the index will only have non-corrupted objects.
-                    # the pack file name is the pack_id (sha256(pack) at N>1 or with the
-                    # BORG_TESTONLY_SHA256_PACK_ID switch), which is not the chunk_id, so recover
-                    # each object's real (chunk_id, offset, size) from its on-disk header rather
-                    # than assuming pack file name == chunk_id.
+                    # the pack file name is the pack_id; each object's chunk_id, offset and size
+                    # come from its on-disk header, so scan the headers to rebuild the index.
                     pack_id = hex_to_bin(info.name)
                     for chunk_id, obj_offset, obj_size in RepoObj.iter_object_headers(pack):
                         chunks[chunk_id] = ChunkIndexEntry(

src/borg/testsuite/archiver/check_cmd_test.py

@@ -449,7 +449,7 @@ def test_empty_repository(archivers, request):
     with Repository(archiver.repository_location, exclusive=True) as repository:
         # empty the repo by dropping every pack file directly via the store. We iterate the actual
         # packs/ listing (the file names are the pack_ids), so this does not depend on what list()
-        # yields or on pack_id == chunk_id.
+        # yields.
         for info in repository.store_list("packs"):
             repository.store_delete("packs/" + info.name)
     cmd(archiver, "check", exit_code=1)

src/borg/testsuite/repository_test.py

@@ -5,7 +5,7 @@
 import pytest
 from ..helpers import IntegrityError, Location, bin_to_hex
 from ..hashindex import ChunkIndex
-from ..repository import Repository, MAX_DATA_SIZE, rest_serve_command, PackWriter, FORCE_SHA256_PACK_ID
+from ..repository import Repository, MAX_DATA_SIZE, rest_serve_command, PackWriter
 from ..repoobj import RepoObj, OBJ_MAGIC, OBJ_VERSION
 from .hashindex_test import H
 
@@ -55,8 +55,8 @@ def reopen(repository, exclusive: bool | None = True, create=False):
 
 def fchunk(data, meta=b"", chunk_id=b"\x00" * 32):
     # Build a raw chunk with a valid RepoObj layout but no encryption or compression. Pass a unique
-    # chunk_id when objects must not share a pack: identical bytes hash to the same sha256 pack id,
-    # so under BORG_TESTONLY_SHA256_PACK_ID they would otherwise collapse into one pack.
+    # chunk_id when objects must not share a pack: identical bytes hash to the same sha256 pack id
+    # and would otherwise collapse into one pack.
     hdr = RepoObj.obj_header.pack(OBJ_MAGIC, OBJ_VERSION, chunk_id, len(meta), len(data))
     assert isinstance(data, bytes)
     chunk = hdr + meta + data
@@ -222,10 +222,7 @@ def test_pack_writer_n1_flush():
     assert len(results) == 1
     stored_id, pack_id, obj_offset, obj_size = results[0]
     assert stored_id == chunk_id
-    if FORCE_SHA256_PACK_ID:
-        assert pack_id == sha256(cdata).digest()  # sha256 switch: pack is named by its content
-    else:
-        assert pack_id == chunk_id  # N=1: pack_id == chunk_id
+    assert pack_id == sha256(cdata).digest()
     assert obj_offset == 0
     assert obj_size == len(cdata)
 
@@ -344,11 +341,7 @@ def test_put_marks_id_in_chunk_index(tmp_path):
         repository.put(id1, fchunk(b"ZEROS"))
         entry = repository._chunks.get(id1)
         assert entry is not None
-        if FORCE_SHA256_PACK_ID:
-            # sha256 switch: the pack is named by its content, not by the chunk_id.
-            assert entry.pack_id == sha256(fchunk(b"ZEROS")).digest()
-        else:
-            assert entry.pack_id == id1  # N=1: pack_id == chunk_id, set by update_pack_info in put()
+        assert entry.pack_id == sha256(fchunk(b"ZEROS")).digest()
         assert entry.size == 0  # uncompressed size filled in by cache layer
 
 
@@ -372,8 +365,7 @@ def test_check_detects_corruption_in_later_object(tmp_path):
 
 
 def test_pack_writer_final_partial_pack_uses_sha256():
-    # When max_count > 1, a final flush with only 1 piece must still use SHA256,
-    # not the N=1 pack_id == chunk_id hack.
+    # A final flush with fewer pieces than max_count must still use SHA256(pack_bytes).
     store = MockStore()
     chunk_id = b"d" * 32
     cdata = b"solo"