fix: validate BORG_PACK_MAX_SIZE and BORG_PACK_MAX_COUNT

[mvanhorn]

Description

A repository opened with BORG_PACK_MAX_SIZE at or above 4 GiB, or with only BORG_PACK_MAX_COUNT set, could build a pack whose object offsets the chunk index cannot store; the backup then aborted with struct.error after that pack was already stored. A non-numeric value aborted with ValueError, and zero or a negative value made every chunk its own pack. Repository.open passed both variables through int() with no range check, and when only the count was set it gave PackWriter max_size=None, so pack growth had no byte ceiling.

Repository.init now parses and validates both variables, so a bad value fails before the store is created, opened or locked (for borg repo-create, no store is created and destroyed again). Each variable must be a positive integer, and BORG_PACK_MAX_SIZE must be below MAX_PACK_SIZE_LIMIT, which is 2126512128 (2 GiB - 20 MiB, i.e. 2 GiB minus MAX_OBJECT_SIZE). PackWriter.add keeps the object that crosses the cap, so this keeps every pack below 2 GiB, clear of OS bugs with files of 2 GiB or more. A rejected value raises Error naming the variable and the value.

With only BORG_PACK_MAX_COUNT set, the writer still gets MAX_PACK_SIZE_LIMIT - 1 as its byte ceiling. pack_max_size returns the user's BORG_PACK_MAX_SIZE, or DEFAULT_PACK_MAX_SIZE when it is unset, so compaction keeps targeting that value rather than the safety ceiling.

The environment-variable help and docs/internals/packs.rst state the limit as 2126512128 (2 GiB - 20 MiB). Repository tests cover rejected values (including that repo creation leaves nothing behind), the size one below the limit, count-only mode, the unset defaults, and both variables set together.

Fixes #10485

Checklist

• PR is against master (or maintenance branch if only applicable there)
• New code has tests and docs where appropriate
• Tests pass (run tox or the relevant test subset)
• Commit messages are clean and reference related issues

AI was used for assistance.

[codecov]

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.08%. Comparing base (5eef8df) to head (52aee23).
⚠️ Report is 48 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/borg/repository.py	80.00%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10488      +/-   ##
==========================================
+ Coverage   88.80%   89.08%   +0.28%     
==========================================
  Files         103      103              
  Lines       19509    19775     +266     
  Branches     3049     3098      +49     
==========================================
+ Hits        17324    17616     +292     
+ Misses       1515     1496      -19     
+ Partials      670      663       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

[ThomasWaldmann]

Thanks, looks good overall. Some suggestions:

1. Validate before touching the repository: the env vars are parsed near the end of Repository.open(), after the store was opened and the lock acquired (and for borg repo-create, after the store was created, which then gets destroyed again). Please move the parsing/validation to the top of open() (or into __init__, where BORG_PACK_CACHE_SIZE is read), so a bad value fails before anything is done with the repository.

2. Stale comment in open(): it still gives "the last obj_offset has to fit in uint32" as the reason for the limit. Since fdd76a5 the actual reason is staying below 2 GiB to avoid OS bugs; the uint32 limit is far away now.

3. Commits: the first commit message (and the PR description) still say 2**32 - MAX_OBJECT_SIZE, and the second commit does not reference #10485. Please squash both into one commit with an updated message.

4. Help text: "below 2 GiB minus one maximum object" and "capped at one byte below 2 GiB minus one maximum object" assume the reader knows MAX_OBJECT_SIZE. That is exactly 20 MiB, so the limit is 2126512128 (2 GiB - 20 MiB). Suggestion:

   • BORG_PACK_MAX_SIZE: "must be below 2126512128 (2 GiB - 20 MiB)"
   • BORG_PACK_MAX_COUNT: "if BORG_PACK_MAX_SIZE is not set, packs still stay below 2 GiB"

   The "just under 2 GiB" in docs/internals/packs.rst could give the exact number, too.

[mvanhorn]

Thanks @ThomasWaldmann, all four addressed in 52aee23:

• Validation moved into Repository.__init__, next to BORG_PACK_CACHE_SIZE. I went with __init__ rather than the top of open() because __enter__ calls create() first, so a bad value would still create and then destroy the store on borg repo-create. Added a test that repo creation leaves nothing behind.
• Stale uint32 comment fixed.
• Help text now says "must be below 2126512128 (2 GiB - 20 MiB)", and docs/internals/packs.rst has the exact number.
• Squashed into one commit referencing BORG_PACK_MAX_SIZE / BORG_PACK_MAX_COUNT are not validated, packs >= 4 GiB break the chunk index #10485, without the 2**32 - MAX_OBJECT_SIZE wording. Updated the PR description to match.

repository_test.py passes locally (279), plus the compact, help and repo-create archiver tests.

[ThomasWaldmann]

Thanks for the fixes. One last nit (not caused by your PR, but it could be also fixed here, while we are dealing with that stuff):

Empty values: BORG_PACK_MAX_SIZE= (empty) is now an error, whereas BORG_PACK_CACHE_SIZE treats an empty value as unset. It crashed before the PR too, so this isn't a regression, just a small inconsistency.