Linus Torvalds <[email protected]> writes:
> Soes my use pattern of "git gc --prune=now" make sense? Maybe not. But
> it's what I've gotten used to, and it's at least not entirely insane.
FWIW, my end-of-day ritual is to do repack -a -d -f with a large
window and a small depth, followed by prune, which boils down to
about the same. So I'd hope that is not entirely insane. I however
do not think I bother with an explicit --expire=now when running the
prune, though.
In any case, that makes two of us, and the suggested patch protects
only one of the two ;-)
> But at least once now, I've done that "git gc" at the end of the day, and
> a new pull request comes in, so I do the "git pull" without even thinking
> about the fact that "git gc" is still running.
*That* is something I don't do. After all, I am fully aware that I
have started end-of-day ritual by that time, so I won't even look at
a new patch (or a pull request for that matter).
> So I actually would much prefer that foir git gc, "--prune=now" means
>
> (a) "now"
>
> (b) now at the _start_ of the "git gc" operation, not the time at
> the _end_ of the operation when we've already spent a minute or
> two doing repacking and are now doing the final pruning.
>
> anyway, with that explanation in mind, I'm appending a patch that is
> pretty small and does that. It's a bit hacky, but I think it still makes
> sense.
>
> Comments?
Closing the possiblity of racing a running "gc" and new object
creation like the above generally makes sense, I would think,
whether the creation is due to 'pull/fetch', 'add', or even 'push'.
I however have to wonder if there are opposite "oops" end-user
operation we also need to worry about, i.e. we are doing a large-ish
fetch, and get bored and run a gc fron another terminal. Perhaps
*that* is a bit too stupid to worry about? Auto-gc deliberately
does not use 'now' because it wants to leave a grace period to avoid
exactly that kind of race.
> diff --git a/builtin/gc.c b/builtin/gc.c
> index c4777b244..98368c8b5 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -535,8 +535,12 @@ int cmd_gc(int argc, const char **argv, const char
> *prefix)
> if (argc > 0)
> usage_with_options(builtin_gc_usage, builtin_gc_options);
>
> - if (prune_expire && parse_expiry_date(prune_expire, &dummy))
> - die(_("failed to parse prune expiry value %s"), prune_expire);
> + if (prune_expire) {
> + if (!strcmp(prune_expire, "now"))
> + prune_expire = show_date(time(NULL), 0,
> DATE_MODE(ISO8601));
> + if (parse_expiry_date(prune_expire, &dummy))
> + die(_("failed to parse prune expiry value %s"),
> prune_expire);
> + }
>
> if (aggressive) {
> argv_array_push(&repack, "-f");
