git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Emily Shaffer <emilyshaffer@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 1/2] gc: add tests for --cruft and friends
Date: Wed, 03 Aug 2022 14:56:04 -0700	[thread overview]
Message-ID: <xmqqr11x800b.fsf@gitster.g> (raw)
In-Reply-To: <20220803205721.3686361-2-emilyshaffer@google.com> (Emily Shaffer's message of "Wed, 3 Aug 2022 13:57:20 -0700")

Emily Shaffer <emilyshaffer@google.com> writes:

> In 5b92477f89 (builtin/gc.c: conditionally avoid pruning objects via
> loose, 2022-05-20) gc learned to respect '--cruft' and 'gc.cruftPacks'.
> '--cruft' is exercised in t5329-pack-objects-cruft.sh, but in a way that
> doesn't check whether a lone gc run generates these cruft packs.
> 'gc.cruftPacks' is never exercised.
>
> Add some tests to exercise these options to gc in the gc test suite.
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> ---
>  t/t6500-gc.sh | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
> index cd6c53360d..e4c2c3583d 100755
> --- a/t/t6500-gc.sh
> +++ b/t/t6500-gc.sh
> @@ -202,6 +202,42 @@ test_expect_success 'one of gc.reflogExpire{Unreachable,}=never does not skip "e
>  	grep -E "^trace: (built-in|exec|run_command): git reflog expire --" trace.out
>  '
>  
> +test_expect_success 'gc --cruft generates a cruft pack' '
> +	git init crufts &&
> +	test_when_finished "rm -fr crufts" &&
> +	(
> +		cd crufts &&
> +		test_commit base &&
> +
> +		test_commit --no-tag foo &&
> +		test_commit --no-tag bar &&
> +		git reset HEAD^^ &&
> +
> +		git gc --cruft &&
> +
> +		cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&

What guarantees that we will have one pack-*.mtimes?  

I do not mind if we reliably diagnosed it as an error when "git gc
--cruft" created two cruft packs, but I do mind if this call to
basename receives two files plus .mtimes suffix and misbehaves.

Is the fact that it is accompanied by a .mtimes file the only clue
that a pack is a "cruft" pack?  Given that the usefulness of mtimes
based expiration approach is doubted, do we want to rely on it (and
having to redesign the test)?

I think the right test would be to

 * make a list of all "in use" objects;

 * see if there is one (or more) packfile that does not contain any
   "in use" objects (look at their .idx file).

If all packfiles are packs with objects that are still in use, then
we did not create a cruft pack.

> +		test_path_is_file .git/objects/pack/$cruft.pack

DQuote the whole thing, i.e.

		test_path_is_file ".git/objects/pack/$cruft.pack"


  reply	other threads:[~2022-08-03 21:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-03 20:57 [PATCH 0/2] let feature.experimental imply gc.cruftPacks=true Emily Shaffer
2022-08-03 20:57 ` [PATCH 1/2] gc: add tests for --cruft and friends Emily Shaffer
2022-08-03 21:56   ` Junio C Hamano [this message]
2022-08-04  7:48   ` Ævar Arnfjörð Bjarmason
2022-08-04 16:09     ` Junio C Hamano
2022-08-03 20:57 ` [PATCH 2/2] config: let feature.experimental imply gc.cruftPacks=true Emily Shaffer
2022-08-03 22:05   ` Junio C Hamano
2022-08-04 13:05   ` Derrick Stolee
2022-08-04 16:10     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr11x800b.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).