Re: [PATCH 2/4] t5329: test 'git gc --cruft' without '--prune=now'

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:

quoted

-		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:

quoted

+		# 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