Re: [PATCH v4 1/9] builtin/receive-pack: properly clean up keep files
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 20, 2026, 06:46 UTC
- Message-ID
- <aoaixMF1biKYhWN2@pks.im>
- In-Reply-To
- <20260819215311.3880274-2-jltobler@gmail.com>
On Wed, Aug 19, 2026 at 04:53:03PM -0500, Justin Tobler wrote:
Show 20 quoted lines
> diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh > index 0798ddab02..3da253cc1a 100755 > --- a/t/t5547-push-quarantine.sh > +++ b/t/t5547-push-quarantine.sh > @@ -70,4 +70,26 @@ test_expect_success 'updating a ref from quarantine is forbidden' ' > git -C update.git fsck > ' > > +test_expect_success '.keep file is removed after push' ' > + test_when_finished rm -rf keep.git && > + git init --bare keep.git && > + > + git -C keep.git config set receive.unpackLimit 0 && > + > + # While incoming objects are still quarantined, validate that the keep > + # lockfile does indeed exist. > + test_hook -C keep.git pre-receive <<-\EOF && > + keep="$(ls "$GIT_QUARANTINE_PATH"/pack/pack-*.keep)" && > + test -f "$keep" > + EOF
Good. So we know that the file exists while the transaction is running...
Show 8 quoted lines
> + test_commit foo &&
> + git push keep.git HEAD &&
> + pack="$(ls keep.git/objects/pack/pack-*.pack)" &&
> + keep="${pack%.pack}.keep" &&
> +
> + test_path_is_file "$pack" &&
> + test_path_is_missing "$keep"
> +'... and we know that the packfile exists without its ".keep" file once the transaction has been committed.
What we don't verify is that the ".keep" file is getting migrated to the target repository and stays intact while we're updating references. So do we maybe want to add the following diff so that we test for the full lifecycle of the ".keep" file?
diff --git a/t/t5547-push-quarantine.sh b/t/t5547-push-quarantine.sh index 3da253cc1a..a722a01e8d 100755 --- a/t/t5547-push-quarantine.sh +++ b/t/t5547-push-quarantine.sh @@ -83,11 +83,19 @@ test_expect_success '.keep file is removed after push' ' test -f "$keep" EOF + # And when updating references the keep-file should have been migrated + # to the actual repository. + test_hook -C keep.git reference-transaction <<-\EOF && + keep="$(ls objects/pack/pack-*.keep)" && + test -f "$keep" + EOF + test_commit foo && git push keep.git HEAD && + + # Once done, there should be no ".keep" files anywhere anymore. pack="$(ls keep.git/objects/pack/pack-*.pack)" && keep="${pack%.pack}.keep" && - test_path_is_file "$pack" && test_path_is_missing "$keep" ' Patrick