gc: enable cruft packs by default
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v2 Latest10 patches 8 repliesIn review
- v110 patches 18 repliesSuperseded
Patches in v2 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/10 | pack-write.c: plug a leak in stage_tmp_packfiles()pack-write.c | +8 | 3 replies | |
| 2/10 | builtin/repack.c: fix incorrect reference to '-C'builtin/ | +1 | 0 replies | |
| 3/10 | builtin/gc.c: ignore cruft packs with `--keep-largest-pack`Documentation/ Documentation/ builtin/ +1 more | +53 | 0 replies | |
| 4/10 | t/t5304-prune.sh: prepare for `gc --cruft` by defaultt/ | +14 | 0 replies | |
| 5/10 | t/t6501-freshen-objects.sh: prepare for `gc --cruft` by defaultt/ | +5 | 0 replies | |
| 6/10 | t/t6500-gc.sh: refactor cruft pack testst/ | +44 | 0 replies | |
| 7/10 | t/t6500-gc.sh: add additional test casest/ | +4 | 0 replies | |
| 8/10 | t/t9300-fast-import.sh: prepare for `gc --cruft` by defaultt/ | +4 | 0 replies | |
| 9/10 | builtin/gc.c: make `gc.cruftPacks` enabled by defaultDocumentation/ Documentation/ Documentation/ +3 more | +11 | 4 replies | |
| 10/10 | repository.h: drop unused `gc_cruft_packs`repo-settings.c repository.h | +1 | 1 reply |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v2 yet.
Cover letter [PATCH v2 00/10] gc: enable cruft packs by default
Here is a very tiny reroll of my series to graduate gc.cruftPacks out of feature.experimental and enables it by default.
A complete summary of the topic is available in the original cover letter[1], and the changes since last time are limited to test clean-up, patch reorganization, and some touch-ups on the patch messages themselves.
As always, a range-diff is below for convenience.
Thanks for all of the review thus far, and in advance for any review on this round, too.
[1]: https://lore.kernel.org/git/[email protected]/
Taylor Blau (10):
pack-write.c: plug a leak in stage_tmp_packfiles() builtin/repack.c: fix incorrect reference to '-C' builtin/gc.c: ignore cruft packs with --keep-largest-pack t/t5304-prune.sh: prepare for gc --cruft by default t/t6501-freshen-objects.sh: prepare for gc --cruft by default t/t6500-gc.sh: refactor cruft pack tests t/t6500-gc.sh: add additional test cases t/t9300-fast-import.sh: prepare for gc --cruft by default builtin/gc.c: make gc.cruftPacks enabled by default repository.h: drop unused gc_cruft_packs
Documentation/config/feature.txt | 3 - Documentation/config/gc.txt | 12 +-- Documentation/git-gc.txt | 12 +-- Documentation/gitformat-pack.txt | 4 +- builtin/gc.c | 8 +- builtin/repack.c | 2 +- pack-write.c | 14 ++-- repo-settings.c | 4 +- repository.h | 1 - t/t5304-prune.sh | 28 +++---- t/t6500-gc.sh | 135 ++++++++++++++++--------------- t/t6501-freshen-objects.sh | 10 +-- t/t9300-fast-import.sh | 13 +-- 13 files changed, 120 insertions(+), 126 deletions(-)
Range-diff against v1:
1: 65ac7ed9b8 = 1: c477b754e7 pack-write.c: plug a leak in stage_tmp_packfiles() 2: fbc8d15032 = 2: 52fb61fa9c builtin/repack.c: fix incorrect reference to '-C' 3: 796df920ad ! 3: 63b9ee8e2e builtin/gc.c: ignore cruft packs with `--keep-largest-pack`
@@ Commit message
- The same is true for gc.bigPackThreshold, if the size of the cruft pack exceeds the limit set by the caller.
- Ignore cruft packs in the common implementation for both of these
- options, and add a pair of tests to prevent any future regressions here.
+ In the future, it is possible that gc.bigPackThreshold could be used
+ to write a separate cruft pack containing any new unreachable objects
+ that entered the repository since the last time a cruft pack was
+ written.
+
+ There are some complexities to doing so, mainly around handling
+ pruning objects that are in an existing cruft pack that is above the
+ threshold (which would either need to be rewritten, or else delay
+ pruning). Rewriting a substantially similar cruft pack isn't ideal, but
+ it is significantly better than the status-quo.
+
+ If users have large cruft packs that they don't want to rewrite, they
+ can mark them as *.keep packs. But in general, if a repository has a
+ cruft pack that is so large it is slowing down GC's, it should probably
+ be pruned anyway.
+
+ In the meantime, ignore cruft packs in the common implementation for
+ both of these options, and add a pair of tests to prevent any future
+ regressions here.
@@ Documentation/git-gc.txt: be performed as well.
- All packs except the largest pack and those marked with a
- .keep files are consolidated into a single pack. When this
- option is used, gc.bigPackThreshold is ignored.
-+ All packs except the largest pack, any packs marked with a
-+ .keep file, and any cruft pack(s) are consolidated into a
-+ single pack. When this option is used, gc.bigPackThreshold is
-+ ignored.
++ All packs except the largest non-cruft pack, any packs marked
++ with a .keep file, and any cruft pack(s) are consolidated into
++ a single pack. When this option is used, gc.bigPackThreshold
++ is ignored.
AGGRESSIVE
----------
4: 44006da959 = 4: 905eeb9027 t/t5304-prune.sh: prepare for `gc --cruft` by default 8: 4ccc525c39 ! 5: fa6eafb1fe t/t6501-freshen-objects.sh: prepare for `gc --cruft` by default
@@ Commit message
cover the case of freshening loose objects not using cruft packs.
We could run this test twice, once with --cruft and once with
- --no-cruft, but doing so is unnecessary, since the object rescuing and
- freshening behavior is already extensively tested via t5329.
+ --no-cruft, but doing so is unnecessary, since we already test object
+ rescuing, freshening, and dealing with corrupt parts of the unreachable
+ object graph extensively via t5329.
6: 56a965e517 = 6: e6270d75fa t/t6500-gc.sh: refactor cruft pack tests 7: 6957e54f51 ! 7: 9db3fa9e36 t/t6500-gc.sh: add additional test cases
@@ Commit message
which enumerates all possible combinations of arguments that will produce (or not produce) a cruft pack.
- This prepares us for the following commit, which will change the default
+ This prepares us for a future commit which will change the default value
of gc.cruftPacks by ensuring that we understand which invocations do and do not change as a result.
@@ t/t6500-gc.sh: do
done
for argv in \
-+ "gc --no-cruft" \
++ "gc" \
+ "-c gc.cruftPacks=false gc" \
+ "-c gc.cruftPacks=true gc --no-cruft" \
"-c feature.expiremental=true -c gc.cruftPacks=false gc" \
5: 1b07eb83fe ! 8: 894cf176ea t/t9300-fast-import.sh: prepare for gc --cruft by default
@@ Metadata
## Commit message ##
t/t9300-fast-import.sh: prepare for gc --cruft by default
- In a similar fashion as the previous commit, adjust the fast-import
- tests to prepare for "git gc" generating a cruft pack by default.
+ In a similar fashion as previous commits, adjust the fast-import tests
+ to prepare for "git gc" generating a cruft pack by default.
This adjustment is slightly different, however. Instead of relying on us writing out the objects loose, and then calling git prune to remove
@@ Commit message
one git gc --prune, which handles pruning both loose objects, and objects that would otherwise be written to a cruft pack.
+ Likely this pattern of "git gc && git prune" started all the way back in
+ 03db4525d3 (Support gitlinks in fast-import., 2008-07-19), which
+ happened after deprecating git gc --prune in 9e7d501990 (builtin-gc.c:
+ deprecate --prune, it now really has no effect, 2008-05-09).
+
+ After --prune was un-deprecated in 58e9d9d472 (gc: make --prune useful
+ again by accepting an optional parameter, 2009-02-14), this script got a
+ handful of new "git gc && git prune" instances via via 4cedb78cb5
+ (fast-import: add input format tests, 2011-08-11). These could have been
+ git gc --prune, but weren't (likely taking after 03db4525d3).
+
## t/t9300-fast-import.sh ##
9: bfda40a21d ! 9: b6784ddfe2 builtin/gc.c: make gc.cruftPacks enabled by default
@@ t/t6500-gc.sh: assert_no_cruft_packs () {
}
for argv in \
+- "gc --cruft" \
+ "gc" \
- "gc --cruft" \
"-c gc.cruftPacks=true gc" \
- "-c gc.cruftPacks=false gc --cruft" \
- "-c feature.experimental=true gc" \
@@ t/t6500-gc.sh: assert_no_cruft_packs () {
do
test_expect_success "git $argv generates a cruft pack" '
test_when_finished "rm -fr repo" &&
-@@ t/t6500-gc.sh: done
+@@ t/t6500-gc.sh: do
+ done
+
for argv in \
- "gc --no-cruft" \
+- "gc" \
++ "gc --no-cruft" \
"-c gc.cruftPacks=false gc" \
- "-c gc.cruftPacks=true gc --no-cruft" \
- "-c feature.expiremental=true -c gc.cruftPacks=false gc" \10: fa15125454 = 10: c67ee7c2ff repository.h: drop unused gc_cruft_packs
Signature · 1 line
2.40.0.362.gc67ee7c2ff