On Wed, Mar 7, 2018 at 1:25 AM, Junio C Hamano <gits...@pobox.com> wrote: > Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes: > >> +--keep-pack=<pack name>:: >> + Ignore the given pack. This is the equivalent of having >> + `.keep` file on the pack. Implies `--honor-pack-keep`. >> + > > A few questions I am not sure how I would answer: > > - Do we want to have this listed in the SYNOPSIS section, too? > > - We would want to make the SP in "<pack name>" consistent with > the dash in "<missing-action>" in the same document; which way do > we make it uniform?
Probably the latter. > > - Is this description clear enough to convey that we allow more > than one instance of this option specified, and the pack names > accumulate? If a question is raised, it's probably not clear. > - Are there use cases where we want to _ignore_ on-disk ".keep" and > only honor the ones given via the "--keep-pack" options? I can't think of one. These .keep files are originally lock files and ignoring them sounds like a bad idea. Perhaps we could add --no-keep-pack later to explicit not keep a pack, ignoring .keep file if present? >> diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh >> index 38247afbec..553d907d34 100755 >> --- a/t/t7700-repack.sh >> +++ b/t/t7700-repack.sh >> @@ -196,5 +196,24 @@ test_expect_success 'objects made unreachable by grafts >> only are kept' ' >> git cat-file -t $H1 >> ' >> >> +test_expect_success 'repack --keep-pack' ' >> + test_create_repo keep-pack && >> + ( >> + cd keep-pack && >> + for cmit in one two three four; do >> + test_commit $cmit && >> + git repack -d >> + done && > > Style: replace "; " before do with LF followed by a few HT. > > This 'for' loop would not exit and report error if an early > test_commit or "git repack -d" fails, would it? Yeah. I guess I'll just unroll the loop. >> + git repack -a -d --keep-pack $KEEP1 --keep-pack $KEEP4 && >> + ls .git/objects/pack/*.pack >new-counts && >> + test_line_count = 3 new-counts && >> + git fsck > > One important invariant for this new feature is that $KEEP1 and > $KEEP4 will both appear in new-counts file, no? Rename new-counts > to new-pack-list and inspect the contents, not just line count, > perhaps? OK -- Duy
