Re: [PATCH 1/2] prepare_packed_git(): refactor garbage reporting in pack directory

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 13, 2016, 20:08 UTC
Message-ID: <xmqqvb6xmedw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAEtYS8Qs2B3rP1PDGhoWGAgcj2c_pOTpt=s8qj9tWMjkLLFyhQ@mail.gmail.com>

Doug Kelly <dougk.ff7@gmail.com> writes:

Show 6 quoted lines

> Yeah, I know I never got to adding the mtime logic, but for a simple (naive,
> hard-coded) case, I did come up with a basic patch today.  I think this could
> be extended to a configuration option(?) which would allow a default longer
> than 10 seconds (an hour? a day?), then during the regression tests, we
> could provide a shorter timeout to ensure the guarding both works and also
> not wait forever for tests to complete.  Thoughts?

Please do not sleep in the tests. Instead, please try to see if you can use test-chmtime to set the timestamps of these files to the necessary ages for the purpose of your tests.

Thanks.

Show 52 quoted lines

>
> ---
>  builtin/gc.c     | 14 ++++++++++++--
>  t/t5304-prune.sh |  2 ++
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 79e9886..a4ce616 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -51,8 +51,18 @@ static struct string_list pack_garbage =
> STRING_LIST_INIT_DUP;
>  static void clean_pack_garbage(void)
>  {
>   int i;
> - for (i = 0; i < pack_garbage.nr; i++)
> - unlink_or_warn(pack_garbage.items[i].string);
> + /* Define a cutoff time for "new" garbage to prevent race conditions */
> + time_t cutoff = time(NULL) - 10;
> + for (i = 0; i < pack_garbage.nr; i++) {
> + struct stat s;
> + char *garbage = pack_garbage.items[i].string;
> + if (!stat(garbage, &s)) {
> + if (s.st_mtime < cutoff)
> + unlink_or_warn(garbage);
> + } else
> + fprintf(stderr, _("stat failed on pack garbage: %s"),
> + garbage);
> + }
>   string_list_clear(&pack_garbage, 0);
>  }
>
> diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
> index cbcc0c0..7b4650f 100755
> --- a/t/t5304-prune.sh
> +++ b/t/t5304-prune.sh
> @@ -272,6 +272,7 @@ test_expect_success 'clean pack garbage with gc' '
>   : >.git/objects/pack/fake6.keep &&
>   : >.git/objects/pack/fake6.bitmap &&
>   : >.git/objects/pack/fake6.idx &&
> + sleep 10 &&
>   git gc &&
>   git count-objects -v 2>stderr &&
>   grep "^warning:" stderr | sort >actual &&
> @@ -291,6 +292,7 @@ test_expect_success 'ensure unknown garbage kept with gc' '
>   : >.git/objects/pack/foo.keep &&
>   : >.git/objects/pack/fake.pack &&
>   : >.git/objects/pack/fake2.foo &&
> + sleep 10 &&
>   git gc &&
>   git count-objects -v 2>stderr &&
>   grep "^warning:" stderr | sort >actual &&