On Tue, Feb 04, 2025 at 10:22:36AM -0500, Jeff King wrote: > On Tue, Feb 04, 2025 at 08:00:41AM +0100, Patrick Steinhardt wrote: > > > this small patch series fixes `git repack -ad --keep-unreachable` when > > there aren't any preexisting packfiles. > > > > Changes in v2: > > - Merge tests into t7701. > > - Link to v1: https://lore.kernel.org/r/20250203-b4-pks-repack-unreachable-objects-wo-packfiles-v1-0-7c4d69c5072c@xxxxxx > > This looks good to me. > > One interesting thing I did notice: > > > +test_expect_success 'repack -k packs unreachable loose objects without existing packfiles' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + > > + oid=$(echo would-be-deleted-loose | git hash-object -w --stdin) && > > + objpath=.git/objects/$(echo $sha1 | sed "s,..,&/,") && > > + test_path_is_file $objpath && > > + > > + git repack -ad --keep-unreachable && > > + test_path_is_missing $objpath && > > + git cat-file -p $oid > > + ) > > +' > > In the test in v1, we had reachable commits to pack. And here we don't. > So before your patch, the behavior in the v1 test was that we'd create a > new pack, but it wouldn't pick up the loose object. But the behavior of > this test is that we say "Nothing new to pack". > > I originally thought that output meant that we were not running > pack-objects at all. But looking at builtin/repack.c, we do run it, and > it simply chooses not to make a pack (which makes sense; how would > repack even realize if there was stuff to pack, since pack-objects is > what does the traversal). > > So the two outcomes are both the result of the same bug. In both cases > we do not correctly pack the loose objects, so whether we make a pack is > just a question of whether there was other reachable stuff to pack. And > since your patch is fixing the bug at its root, both outcomes are fixed. > > And when I suggested in my response to v1 that "Nothing new to pack" in > an empty repo was a separate bug, I was just wrong. ;) There is nothing > else to fix after your patch. > > Thanks for finding and fixing. Thanks for your thorough review! Patrick
