Re: [PATCH] gc: make --prune useful again by accepting an optional parameter
- From
Thomas Rast <trast@student.ethz.ch>
- Date
- Feb 14, 2009, 21:33 UTC
- Message-ID
- <200902142233.07190.trast@student.ethz.ch>
- In-Reply-To
- <alpine.DEB.1.00.0902141959060.10279@pacific.mpi-cbg.de>
Johannes Schindelin wrote:
> With this patch, "git gc --no-prune" will not prune any loose (and > dangling) object, and "git gc --prune=5.minutes.ago" will prune > all loose objects older than 5 minutes.
Prompted by discussion on IRC, I just spent some time staring at the code, and for the code part and FWIW,
Acked-by: Thomas Rast <trast@student.ethz.ch>
Meaning that for all I can see, it handles every combination of config, --prune and --no-prune correctly.
I have some non-code nits however:
> +--prune=<date>:: > + Prune loose objects older than date (default is 2 weeks ago). > + This option is on by default.
This isn't really true; to the user, it defaults to the value of 'gc.pruneExpire'. That this is done by defaulting the state of --prune to 2 weeks ago is an implementation detail.
> + { OPTION_STRING, 0, "prune", &prune_expire, "date",
> + "prune unreferenced objects (deprecated)",
> + PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },You still call the option "deprecated" :-)
> diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
[...]
Show 20 quoted lines
> +test_expect_success 'gc --no-prune && gc --prune=<date>' ' > + > + before=$(git count-objects | sed "s/ .*//") && > + BLOB=$(echo aleph_0 | git hash-object -w --stdin) && > + BLOB_FILE=.git/objects/$(echo $BLOB | sed "s/^../&\//") && > + test $((1 + $before)) = $(git count-objects | sed "s/ .*//") && > + test -f $BLOB_FILE && > + test-chmtime =-$((86400*5001)) $BLOB_FILE && > + git gc --no-prune && > + test 1 = $(git count-objects | sed "s/ .*//") && > + test -f $BLOB_FILE && > + git gc --prune=5002.days.ago && > + test 1 = $(git count-objects | sed "s/ .*//") && > + test -f $BLOB_FILE && > + git gc --prune=5000.days.ago && > + test 0 = $(git count-objects | sed "s/ .*//") && > + test ! -f $BLOB_FILE > + > +' > +
I'm not a huge fan of such contiguous blocks, as a failure is harder to pin to any of the several tests inside, especially with 'test' not showing any output when it fails.
Also, it does test the interaction between having gc.pruneExpire set, but passing --no-prune: the tests above leave gc.pruneExpire at "2.days.ago". This wasn't immediately obvious however, and you should probably set the config again just in case someone decides to insert a test with a different value.
--
Thomas Rast
trast@{inf,student}.ethz.ch