{"thread":{"id":"59890","subject":"[PATCH] t7701: make annotated tag unreachable","startedAt":"2023-06-21T10:21:26Z","lastAt":"2023-06-27T07:09:56Z","messageCount":4,"participants":["Taylor Blau","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"478620","messageId":"48d3c2c1871c8122d22fbce7c256ca65582fcd67.1687342818.git.me@ttaylorr.com","threadId":"59890","inReplyTo":null,"subject":"[PATCH] t7701: make annotated tag unreachable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-21T10:21:10Z","receivedAt":"2023-06-21T10:21:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In 4dc16e2cb0 (gc: introduce `gc.recentObjectsHook`, 2023-06-07), we\nadded tests to ensure that prune-able (i.e. unreachable and with mtime\nolder than the cutoff) objects which are marked as recent via the new\n`gc.recentObjectsHook` configuration are unpacked as loose with\n`--unpack-unreachable`.\n\nIn that test, we also ensure that objects which are reachable from other\nunreachable objects which were *not* pruned are kept as well, regardless\nof their mtimes. For this, we use an annotated tag pointing at a blob\n($obj2) which would otherwise be pruned.\n\nBut after pruning, that object is kept around for two reasons. One, the\ntag object's mtime wasn't adjusted to be beyond the 1-hour cutoff, so it\nwould be kept as due to its recency regardless. The other reason is\nbecause the tag itself is reachable.\n\nUse mktag to write the tag object directly without pointing a reference\nat it, and adjust the mtime of the tag object to be older than the\ncutoff to ensure that our `gc.recentObjectsHook` configuration is\nworking as intended.\n\nNoticed-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\nPeff pointed this out to me after the gc.recentObjectsHook stuff was in\n'next', but this should go on top. This patch is based off of the tip of\n\"tb/gc-recent-object-hook\".\n\n t/t7701-repack-unpack-unreachable.sh | 14 +++++++++++++-\n 1 file changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex ba428c18a8..ceb4e805d2 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -126,8 +126,19 @@ test_expect_success 'gc.recentObjectsHook' '\n \tgit cat-file -p $obj2 &&\n \tgit cat-file -p $obj3 &&\n\n+\t# make an unreachable annotated tag object to ensure we rescue objects\n+\t# which are reachable from non-pruned unreachable objects\n \tgit tag -a -m tag obj2-tag $obj2 &&\n-\tobj2_tag=\"$(git rev-parse obj2-tag)\" &&\n+\tobj2_tag=\"$(git mktag <<-EOF\n+\tobject $obj2\n+\ttype blob\n+\ttag obj2-tag\n+\ttagger T A Gger <tagger@example.com> 1234567890 -0000\n+\tEOF\n+\t)\" &&\n+\n+\tobj2_tag_pack=\"$(echo $obj2_tag | git pack-objects .git/objects/pack/pack)\" &&\n+\tgit prune-packed &&\n\n \twrite_script precious-objects <<-EOF &&\n \techo $obj2_tag\n@@ -136,6 +147,7 @@ test_expect_success 'gc.recentObjectsHook' '\n\n \ttest-tool chmtime =-86400 .git/objects/pack/pack-$pack2.pack &&\n \ttest-tool chmtime =-86400 .git/objects/pack/pack-$pack3.pack &&\n+\ttest-tool chmtime =-86400 .git/objects/pack/pack-$obj2_tag_pack.pack &&\n \tgit repack -A -d --unpack-unreachable=1.hour.ago &&\n\n \tgit cat-file -p $obj1 &&\n--\n2.40.1.478.g4dc16e2cb0\n"},{"id":"478763","messageId":"20230624043826.GA104835@coredump.intra.peff.net","threadId":"59890","inReplyTo":"48d3c2c1871c8122d22fbce7c256ca65582fcd67.1687342818.git.me@ttaylorr.com","subject":"Re: [PATCH] t7701: make annotated tag unreachable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-24T04:38:26Z","receivedAt":"2023-06-24T04:38:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 21, 2023 at 06:21:10AM -0400, Taylor Blau wrote:\n\n> But after pruning, that object is kept around for two reasons. One, the\n> tag object's mtime wasn't adjusted to be beyond the 1-hour cutoff, so it\n> would be kept as due to its recency regardless. The other reason is\n> because the tag itself is reachable.\n> \n> Use mktag to write the tag object directly without pointing a reference\n> at it, and adjust the mtime of the tag object to be older than the\n> cutoff to ensure that our `gc.recentObjectsHook` configuration is\n> working as intended.\n\nThanks, this should fix the problem (which you can notice by making the\nrecentObjectsHook in the test a noop and seeing that it still passes).\n\nBut there's one more thing...\n\n> diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\n> index ba428c18a8..ceb4e805d2 100755\n> --- a/t/t7701-repack-unpack-unreachable.sh\n> +++ b/t/t7701-repack-unpack-unreachable.sh\n> @@ -126,8 +126,19 @@ test_expect_success 'gc.recentObjectsHook' '\n>  \tgit cat-file -p $obj2 &&\n>  \tgit cat-file -p $obj3 &&\n> \n> +\t# make an unreachable annotated tag object to ensure we rescue objects\n> +\t# which are reachable from non-pruned unreachable objects\n>  \tgit tag -a -m tag obj2-tag $obj2 &&\n> -\tobj2_tag=\"$(git rev-parse obj2-tag)\" &&\n> +\tobj2_tag=\"$(git mktag <<-EOF\n> +\tobject $obj2\n> +\ttype blob\n> +\ttag obj2-tag\n> +\ttagger T A Gger <tagger@example.com> 1234567890 -0000\n> +\tEOF\n> +\t)\" &&\n\nSince we are using \"mktag\" here, the call to \"git tag\" can go away\nentirely. It's not only redundant, but it erroneously keeps reachability\nto $obj2. The test still does something useful (it makes sure that\n$obj2_tag was saved), but it is failing to test what it wanted to about\n$obj2, namely that it is kept because the hook mentioned an object that\npoints to it.\n\nSo we'd want to squash in:\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex ceb4e805d2..fe6c3e77a3 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -128,7 +128,6 @@ test_expect_success 'gc.recentObjectsHook' '\n \n \t# make an unreachable annotated tag object to ensure we rescue objects\n \t# which are reachable from non-pruned unreachable objects\n-\tgit tag -a -m tag obj2-tag $obj2 &&\n \tobj2_tag=\"$(git mktag <<-EOF\n \tobject $obj2\n \ttype blob\n\n-Peff\n"},{"id":"478769","messageId":"259b1b559114ab1a9a0bd7f1ad29a4cba2612ae0.1687617197.git.me@ttaylorr.com","threadId":"59890","inReplyTo":"20230624043826.GA104835@coredump.intra.peff.net","subject":"[PATCH v2] t7701: make annotated tag unreachable","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2023-06-24T14:33:47Z","receivedAt":"2023-06-24T14:33:56Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In 4dc16e2cb0 (gc: introduce `gc.recentObjectsHook`, 2023-06-07), we\nadded tests to ensure that prune-able (i.e. unreachable and with mtime\nolder than the cutoff) objects which are marked as recent via the new\n`gc.recentObjectsHook` configuration are unpacked as loose with\n`--unpack-unreachable`.\n\nIn that test, we also ensure that objects which are reachable from other\nunreachable objects which were *not* pruned are kept as well, regardless\nof their mtimes. For this, we use an annotated tag pointing at a blob\n($obj2) which would otherwise be pruned.\n\nBut after pruning, that object is kept around for two reasons. One, the\ntag object's mtime wasn't adjusted to be beyond the 1-hour cutoff, so it\nwould be kept as due to its recency regardless. The other reason is\nbecause the tag itself is reachable.\n\nUse mktag to write the tag object directly without pointing a reference\nat it, and adjust the mtime of the tag object to be older than the\ncutoff to ensure that our `gc.recentObjectsHook` configuration is\nworking as intended.\n\nNoticed-by: Jeff King <peff@peff.net>\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\nFixes a trivial oversight from an earlier version of this patch noticed\nby Peff.\n\nRange-diff against v1:\n1:  48d3c2c187 ! 1:  259b1b5591 t7701: make annotated tag unreachable\n    @@ t/t7701-repack-unpack-unreachable.sh: test_expect_success 'gc.recentObjectsHook'\n      \tgit cat-file -p $obj2 &&\n      \tgit cat-file -p $obj3 &&\n\n    +-\tgit tag -a -m tag obj2-tag $obj2 &&\n    +-\tobj2_tag=\"$(git rev-parse obj2-tag)\" &&\n     +\t# make an unreachable annotated tag object to ensure we rescue objects\n     +\t# which are reachable from non-pruned unreachable objects\n    - \tgit tag -a -m tag obj2-tag $obj2 &&\n    --\tobj2_tag=\"$(git rev-parse obj2-tag)\" &&\n     +\tobj2_tag=\"$(git mktag <<-EOF\n     +\tobject $obj2\n     +\ttype blob\n\n t/t7701-repack-unpack-unreachable.sh | 15 +++++++++++++--\n 1 file changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh\nindex ba428c18a8..fe6c3e77a3 100755\n--- a/t/t7701-repack-unpack-unreachable.sh\n+++ b/t/t7701-repack-unpack-unreachable.sh\n@@ -126,8 +126,18 @@ test_expect_success 'gc.recentObjectsHook' '\n \tgit cat-file -p $obj2 &&\n \tgit cat-file -p $obj3 &&\n\n-\tgit tag -a -m tag obj2-tag $obj2 &&\n-\tobj2_tag=\"$(git rev-parse obj2-tag)\" &&\n+\t# make an unreachable annotated tag object to ensure we rescue objects\n+\t# which are reachable from non-pruned unreachable objects\n+\tobj2_tag=\"$(git mktag <<-EOF\n+\tobject $obj2\n+\ttype blob\n+\ttag obj2-tag\n+\ttagger T A Gger <tagger@example.com> 1234567890 -0000\n+\tEOF\n+\t)\" &&\n+\n+\tobj2_tag_pack=\"$(echo $obj2_tag | git pack-objects .git/objects/pack/pack)\" &&\n+\tgit prune-packed &&\n\n \twrite_script precious-objects <<-EOF &&\n \techo $obj2_tag\n@@ -136,6 +146,7 @@ test_expect_success 'gc.recentObjectsHook' '\n\n \ttest-tool chmtime =-86400 .git/objects/pack/pack-$pack2.pack &&\n \ttest-tool chmtime =-86400 .git/objects/pack/pack-$pack3.pack &&\n+\ttest-tool chmtime =-86400 .git/objects/pack/pack-$obj2_tag_pack.pack &&\n \tgit repack -A -d --unpack-unreachable=1.hour.ago &&\n\n \tgit cat-file -p $obj1 &&\n--\n2.40.1.479.g48d3c2c187.dirty\n"},{"id":"478867","messageId":"20230627070942.GC1226768@coredump.intra.peff.net","threadId":"59890","inReplyTo":"259b1b559114ab1a9a0bd7f1ad29a4cba2612ae0.1687617197.git.me@ttaylorr.com","subject":"Re: [PATCH v2] t7701: make annotated tag unreachable","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-06-27T07:09:42Z","receivedAt":"2023-06-27T07:09:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 24, 2023 at 10:33:47AM -0400, Taylor Blau wrote:\n\n> In 4dc16e2cb0 (gc: introduce `gc.recentObjectsHook`, 2023-06-07), we\n> added tests to ensure that prune-able (i.e. unreachable and with mtime\n> older than the cutoff) objects which are marked as recent via the new\n> `gc.recentObjectsHook` configuration are unpacked as loose with\n> `--unpack-unreachable`.\n> \n> In that test, we also ensure that objects which are reachable from other\n> unreachable objects which were *not* pruned are kept as well, regardless\n> of their mtimes. For this, we use an annotated tag pointing at a blob\n> ($obj2) which would otherwise be pruned.\n> \n> But after pruning, that object is kept around for two reasons. One, the\n> tag object's mtime wasn't adjusted to be beyond the 1-hour cutoff, so it\n> would be kept as due to its recency regardless. The other reason is\n> because the tag itself is reachable.\n> \n> Use mktag to write the tag object directly without pointing a reference\n> at it, and adjust the mtime of the tag object to be older than the\n> cutoff to ensure that our `gc.recentObjectsHook` configuration is\n> working as intended.\n> \n> Noticed-by: Jeff King <peff@peff.net>\n> Signed-off-by: Taylor Blau <me@ttaylorr.com>\n> ---\n> Fixes a trivial oversight from an earlier version of this patch noticed\n> by Peff.\n\nThanks, this looks great to me.\n\n-Peff\n"}]}