Re: [PATCH v2 2/5] repack: add --keep-pack option
- Date: Wed, 7 Mar 2018 17:19:50 +0700
- From: Duy Nguyen <pclouds@xxxxxxxxx>
- Subject: Re: [PATCH v2 2/5] repack: add --keep-pack option
On Wed, Mar 7, 2018 at 1:25 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> 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
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
>> 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,