| Message ID | YU4Zse+dAeDg8RqV@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | enabling GIT_REF_PARANOIA by default | expand |
> @@ -277,7 +277,7 @@ test_expect_success 'incremental-repack task' ' > > # Delete refs that have not been repacked in these packs. > git for-each-ref --format="delete %(refname)" \ > - refs/prefetch refs/tags >refs && > + refs/prefetch refs/tags refs/remotes >refs && > git update-ref --stdin <refs && > > # Replace the object directory with this pack layout. > @@ -286,6 +286,10 @@ test_expect_success 'incremental-repack task' ' > ls $packDir/*.pack >packs-before && > test_line_count = 3 packs-before && > > + # make sure we do not have any broken refs that were > + # missed in the deletion above > + git for-each-ref && For what it's worth, I verified that a fatal error is indeed caused in "git for-each-ref" if refs/remotes was not deleted. This patch looks good.
On Mon, Sep 27, 2021 at 10:38:18AM -0700, Jonathan Tan wrote: > > @@ -277,7 +277,7 @@ test_expect_success 'incremental-repack task' ' > > > > # Delete refs that have not been repacked in these packs. > > git for-each-ref --format="delete %(refname)" \ > > - refs/prefetch refs/tags >refs && > > + refs/prefetch refs/tags refs/remotes >refs && > > git update-ref --stdin <refs && > > > > # Replace the object directory with this pack layout. > > @@ -286,6 +286,10 @@ test_expect_success 'incremental-repack task' ' > > ls $packDir/*.pack >packs-before && > > test_line_count = 3 packs-before && > > > > + # make sure we do not have any broken refs that were > > + # missed in the deletion above > > + git for-each-ref && > > For what it's worth, I verified that a fatal error is indeed caused in > "git for-each-ref" if refs/remotes was not deleted. This patch looks > good. Me too. :) It works because for-each-ref's default format will try to show the type of the object, so it complains when the object is missing. We could make this "git fsck" if that's less subtle, but this is a bit cheaper. -Peff
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 36a4218745..31245f6276 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -277,7 +277,7 @@ test_expect_success 'incremental-repack task' ' # Delete refs that have not been repacked in these packs. git for-each-ref --format="delete %(refname)" \ - refs/prefetch refs/tags >refs && + refs/prefetch refs/tags refs/remotes >refs && git update-ref --stdin <refs && # Replace the object directory with this pack layout. @@ -286,6 +286,10 @@ test_expect_success 'incremental-repack task' ' ls $packDir/*.pack >packs-before && test_line_count = 3 packs-before && + # make sure we do not have any broken refs that were + # missed in the deletion above + git for-each-ref && + # the job repacks the two into a new pack, but does not # delete the old ones. git maintenance run --task=incremental-repack &&
The "incremental-repack task" test replaces the object directory with a known state. As a result, some of our refs point to objects that are not included in that state. Commit 3cf5f221be (t7900: clean up some broken refs, 2021-01-19) cleaned up some of those (that were causing warnings to stderr from the maintenance process). But there are a few more that were missed. These aren't hurting anything for now, but it's certainly an unexpected state to leave the test repository in, and it will become a problem if repack ever gets more picky about broken refs. Let's clean up those additional refs (which are all in refs/remotes, with nothing there that isn't broken), and add an extra "for-each-ref" call to assert that we've got everything. Signed-off-by: Jeff King <peff@peff.net> --- t/t7900-maintenance.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)