From: Derrick Stolee <derrickstolee@github.com>
Date: Wed, 15 Jun 2022 13:28:07 GMT
Subject: Re: [PATCH 2/4] t5329: test 'git gc --cruft' without '--prune=now'
Message-ID: <5c05094a-5563-d55a-ea12-7dd74cd22fca@github.com>
In-Reply-To: <YqkcEhQX4qm7fSer@nand.local>

On 6/14/22 7:38 PM, Taylor Blau wrote:
> On Tue, Jun 14, 2022 at 09:27:48PM +0000, Derrick Stolee via GitGitGadget wrote:

>> -		git repack --cruft -d &&
>> +		# Write a cruft pack instead of deleting files.
>> +		git gc --cruft &&
> 
> These ("git repack --cruft -d" and "git gc --cruft") do the same thing,
> so this transformation makes sense.
> 
> It may be slightly clearer to refer to "objects" instead of "files",
> perhaps like:
> 
>     # Write a cruft pack containing all unreachable objects
> 
> and then replace:
> 
>> +		# Ignore the cruft pack and delete every unreachable object.
>>  		git gc --cruft --prune=now &&
> 
> with:
> 
>     # Prune all unreachable objects from the cruft pack
> 
> But I don't think the current wording is a problem, either, so feel free
> to take or leave these suggestions.

I like your suggestions. I'll update them if there are other reasons
to do a v2.
 
> Quoting from your original coverage report, this should take care of:
> 
> Taylor Blau     5b92477f builtin/gc.c: conditionally avoid pruning objects via loose
> builtin/gc.c
> 5b92477f 337) strvec_push(&repack, "--cruft");
> 5b92477f 338) if (prune_expire)

These lines, yes.

> 5b92477f 339) strvec_pushf(&repack, "--cruft-expiration=%s", prune_expire);

This one requires "git gc --cruft --prune=<time>" where <time> is
not "now". I didn't want to jump too much into the exact expire
time as I feared that could cause some issues, but I suppose
passing --prune="01-01-1980" would provide a non-zero expiration
and then lead to us testing "git repack --cruft --cruft-expiration"
as well. (It doesn't actually test the exact expiration time, but
does test that the options are sent down the pipe.)

Thanks,
-Stolee