{"thread":{"id":"57343","subject":"[PATCH 0/3] reftable related test tweaks","startedAt":"2022-01-31T17:50:24Z","lastAt":"2022-02-08T14:58:37Z","messageCount":21,"participants":["Han-Wen Nienhuys via GitGitGadget","Taylor Blau","Junio C Hamano","Han-Wen Nienhuys","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"447351","messageId":"pull.1209.git.git.1643651420.gitgitgadget@gmail.com","threadId":"57343","inReplyTo":null,"subject":"[PATCH 0/3] reftable related test tweaks","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-31T17:50:17Z","receivedAt":"2022-01-31T17:50:24Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"These are 3 assorted fixes from my reftable-backend branch.\n\nHan-Wen Nienhuys (3):\n t1405: explictly delete reflogs for reftable\n t1405: mark test that checks existence as REFFILES\n t5312: prepare for reftable\n\n t/t1405-main-ref-store.sh | 8 +++++++-\n t/t5312-prune-corruption.sh | 10 +++++-----\n 2 files changed, 12 insertions(+), 6 deletions(-)\n\n\nbase-commit: 5d01301f2b865aa8dba1654d3f447ce9d21db0b5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1209%2Fhanwen%2Ftest-tweaks-20220130-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1209/hanwen/test-tweaks-20220130-v1\nPull-Request: https://github.com/git/git/pull/1209\n-- \ngitgitgadget\n"},{"id":"447352","messageId":"299451d317f83b908ee4ba750405302238209103.1643651420.git.gitgitgadget@gmail.com","threadId":"57343","inReplyTo":"pull.1209.git.git.1643651420.gitgitgadget@gmail.com","subject":"[PATCH 1/3] t1405: explictly delete reflogs for reftable","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-31T17:50:18Z","receivedAt":"2022-01-31T17:50:30Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nDeleting a ref in reftable just records a (ObjectID => ZeroID)\ntransaction in the reflog. To ensure 'for_each_reflog()' test below\nworks, explictly delete reflogs for deleted refs.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t1405-main-ref-store.sh | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 1a3ee8845d6..62e5e9d1b0a 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -40,6 +40,12 @@ test_expect_success 'delete_refs(FOO, refs/tags/new-tag)' '\n \ttest_must_fail git rev-parse refs/tags/new-tag --\n '\n \n+# In reftable, we keep the reflogs around for deleted refs.\n+test_expect_success !REFFILES 'delete-reflog(FOO, refs/tags/new-tag)' '\n+\t$RUN delete-reflog FOO &&\n+\t$RUN delete-reflog refs/tags/new-tag\n+'\n+\n test_expect_success 'rename_refs(main, new-main)' '\n \tgit rev-parse main >expected &&\n \t$RUN rename-ref refs/heads/main refs/heads/new-main &&\n-- \ngitgitgadget\n\n"},{"id":"447353","messageId":"1ded69d89709d23147b29f67122b659293414405.1643651420.git.gitgitgadget@gmail.com","threadId":"57343","inReplyTo":"pull.1209.git.git.1643651420.gitgitgadget@gmail.com","subject":"[PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-31T17:50:19Z","receivedAt":"2022-01-31T17:50:35Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nThe reftable backend doesn't support mere existence of reflogs.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t1405-main-ref-store.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh\nindex 62e5e9d1b0a..51f82916281 100755\n--- a/t/t1405-main-ref-store.sh\n+++ b/t/t1405-main-ref-store.sh\n@@ -111,7 +111,7 @@ test_expect_success 'delete_reflog(HEAD)' '\n \ttest_must_fail git reflog exists HEAD\n '\n \n-test_expect_success 'create-reflog(HEAD)' '\n+test_expect_success REFFILES 'create-reflog(HEAD)' '\n \t$RUN create-reflog HEAD &&\n \tgit reflog exists HEAD\n '\n-- \ngitgitgadget\n\n"},{"id":"447354","messageId":"b2c6e14c7e7752c9e42cb38372edc8971895932f.1643651420.git.gitgitgadget@gmail.com","threadId":"57343","inReplyTo":"pull.1209.git.git.1643651420.gitgitgadget@gmail.com","subject":"[PATCH 3/3] t5312: prepare for reftable","fromName":"Han-Wen Nienhuys via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-01-31T17:50:20Z","receivedAt":"2022-01-31T17:50:44Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"From: Han-Wen Nienhuys \n\nMark some tests as REFFILES if they rely on packed refs. Use ref-store\nhelper to create bogus refs.\n\nSigned-off-by: Han-Wen Nienhuys \n---\n t/t5312-prune-corruption.sh | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh\nindex ea889c088a5..9d8e249ae8b 100755\n--- a/t/t5312-prune-corruption.sh\n+++ b/t/t5312-prune-corruption.sh\n@@ -22,8 +22,8 @@ test_expect_success 'disable reflogs' '\n '\n \n create_bogus_ref () {\n-\ttest_when_finished 'rm -f .git/refs/heads/bogus..name' &&\n-\techo $bogus >.git/refs/heads/bogus..name\n+\ttest-tool ref-store main update-ref msg \"refs/heads/bogus..name\" $bogus $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&\n+\ttest_when_finished \"test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/bogus..name\"\n }\n \n test_expect_success 'create history reachable only from a bogus-named ref' '\n@@ -113,7 +113,7 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '\n # we do not want to count on running pack-refs to\n # actually pack it, as it is perfectly reasonable to\n # skip processing a broken ref\n-test_expect_success 'create packed-refs file with broken ref' '\n+test_expect_success REFFILES 'create packed-refs file with broken ref' '\n \trm -f .git/refs/heads/main &&\n \tcat >.git/packed-refs <<-EOF &&\n \t$missing refs/heads/main\n@@ -124,13 +124,13 @@ test_expect_success 'create packed-refs file with broken ref' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'pack-refs does not silently delete broken packed ref' '\n+test_expect_success REFFILES 'pack-refs does not silently delete broken packed ref' '\n \tgit pack-refs --all --prune &&\n \tgit rev-parse refs/heads/main >actual &&\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'pack-refs does not drop broken refs during deletion' '\n+test_expect_success REFFILES 'pack-refs does not drop broken refs during deletion' '\n \tgit update-ref -d refs/heads/other &&\n \tgit rev-parse refs/heads/main >actual &&\n \ttest_cmp expect actual\n-- \ngitgitgadget\n"},{"id":"447362","messageId":"YfhUIJuO70va6gr8@nand.local","threadId":"57343","inReplyTo":"1ded69d89709d23147b29f67122b659293414405.1643651420.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-01-31T21:26:56Z","receivedAt":"2022-01-31T21:27:08Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jan 31, 2022 at 05:50:19PM +0000, Han-Wen Nienhuys via GitGitGadget wrote:\n> From: Han-Wen Nienhuys \n>\n> The reftable backend doesn't support mere existence of reflogs.\n\nPerhaps I'm missing something obvious, but this and the previous patch\nseem to be conflicting each other.\n\nMy understanding of the previous change is that you wanted a reflog\nentry when the REFFILES prerequisite isn't met. But this patch says what\nmatches my understanding is that reftable and reflogs do not play\ntogether.\n\nIf reflogs do not interact with the reftable backend, then what does\nthis patch do?\n\nThanks,\nTaylor\n"},{"id":"447375","messageId":"xmqqzgnbh7rv.fsf@gitster.g","threadId":"57343","inReplyTo":"YfhUIJuO70va6gr8@nand.local","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-01-31T22:15:00Z","receivedAt":"2022-01-31T22:15:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> On Mon, Jan 31, 2022 at 05:50:19PM +0000, Han-Wen Nienhuys via GitGitGadget wrote:\n>> From: Han-Wen Nienhuys \n>>\n>> The reftable backend doesn't support mere existence of reflogs.\n>\n> Perhaps I'm missing something obvious, but this and the previous patch\n> seem to be conflicting each other.\n>\n> My understanding of the previous change is that you wanted a reflog\n> entry when the REFFILES prerequisite isn't met. But this patch says what\n> matches my understanding is that reftable and reflogs do not play\n> together.\n>\n> If reflogs do not interact with the reftable backend, then what does\n> this patch do?\n\nOne difference between the files and the reftable backend is that\nwith the files backend, you can say \"I am not adding any entry yet,\nbut remember that reflog is enabled for this ref, while all other\nrefs reflog is not enabled\", and the way to do so is to touch the\n\"$GIT_DIR/logs/refs/heads/frotz\" file---this enables reflog for the\n\"frotz\" branch, even if core.logAllRefUpdates is not set.\n\nBecause there is no generic reflog API that says \"enable log for\nthis ref\", a test that checks this feature with files backend would\ndo \"touch .git/refs/heads/frotz\".\n\nI didn't look at \"this patch\", but it is understandable if such a\ntest needs to be skipped via REFFILES prerequisite, because the\nreftable backend lacks this feature.\n"},{"id":"447453","messageId":"CAFQ2z_OFRJh9cwxnbDzrshYPGOvJC6Rz1eHTF-aKURno+41Cvw@mail.gmail.com","threadId":"57343","inReplyTo":"xmqqzgnbh7rv.fsf@gitster.g","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-01T20:06:08Z","receivedAt":"2022-02-01T20:06:22Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Jan 31, 2022 at 11:15 PM Junio C Hamano wrote:\n>\n> Taylor Blau writes:\n>\n> > On Mon, Jan 31, 2022 at 05:50:19PM +0000, Han-Wen Nienhuys via GitGitGadget wrote:\n> >> From: Han-Wen Nienhuys \n> >>\n> >> The reftable backend doesn't support mere existence of reflogs.\n> >\n> > Perhaps I'm missing something obvious, but this and the previous patch\n> > seem to be conflicting each other.\n> >\n> > My understanding of the previous change is that you wanted a reflog\n> > entry when the REFFILES prerequisite isn't met. But this patch says what\n> > matches my understanding is that reftable and reflogs do not play\n> > together.\n> >\n> > If reflogs do not interact with the reftable backend, then what does\n> > this patch do?\n>\n> One difference between the files and the reftable backend is that\n> with the files backend, you can say \"I am not adding any entry yet,\n> but remember that reflog is enabled for this ref, while all other\n> refs reflog is not enabled\", and the way to do so is to touch the\n> \"$GIT_DIR/logs/refs/heads/frotz\" file---this enables reflog for the\n> \"frotz\" branch, even if core.logAllRefUpdates is not set.\n>\n> Because there is no generic reflog API that says \"enable log for\n> this ref\", a test that checks this feature with files backend would\n> do \"touch .git/refs/heads/frotz\".\n\nThere is refs_create_reflog(), so the generic reflog API exists. The\nproblem is that there is no sensible way to implement it in reftable.\n\nOne option is (reflog exists == there exists at least one reflog entry\nfor the ref). This messes up the test from this patch, because it\ncreates a reflog, but because it doesn't populate the reflog, so we\nreturn false for git-reflog-exists.\n\nIt also turns out to mess up the tests in t3420, as follows:\n\n++ git stash show -p\nerror: refs/stash@{0} is not a valid reference\n\nI get\n\n reflog_exists: refs/stash: 0\n\nand \"git stash show -p\" aborts with \"error: refs/stash@{0} is not a\nvalid reference\".\n\nSo I now went with the other option, ie. (reflog exists == true), ie.\nevery conceivable ref has a reflog (but most are empty). This makes\nt3420 pass.\n\nThis behavior also confuses t1405, because in\n\n $RUN delete-reflog HEAD &&\n test_must_fail git reflog exists HEAD\n\nthe last command now always returns true.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447461","messageId":"xmqqa6facn9i.fsf@gitster.g","threadId":"57343","inReplyTo":"CAFQ2z_OFRJh9cwxnbDzrshYPGOvJC6Rz1eHTF-aKURno+41Cvw@mail.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-01T21:03:53Z","receivedAt":"2022-02-01T21:03:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> Because there is no generic reflog API that says \"enable log for\n>> this ref\", a test that checks this feature with files backend would\n>> do \"touch .git/refs/heads/frotz\".\n>\n> There is refs_create_reflog(), so the generic reflog API exists. The\n> problem is that there is no sensible way to implement it in reftable.\n\nAh, yes, that's correct.\n\n> One option is (reflog exists == there exists at least one reflog entry\n> for the ref).\n\nBecause the current callers of refs_create_reflog() does want a\nreflog created that does not give any entry when iterated, I agree\nwith you that adding a \"fake\" reflog entry alone is not a sufficient\nemulation of the API. I think these are all ...\n\n> This messes up the test from this patch, because it\n> creates a reflog, but because it doesn't populate the reflog, so we\n> return false for git-reflog-exists.\n>\n> It also turns out to mess up the tests in t3420, as follows:\n>\n> ++ git stash show -p\n> error: refs/stash@{0} is not a valid reference\n>\n> I get\n>\n> reflog_exists: refs/stash: 0\n>\n> and \"git stash show -p\" aborts with \"error: refs/stash@{0} is not a\n> valid reference\".\n\n... indications of hat.\n\nI wonder if it is simple and easy to add a new reflog entry type\nused as an implementation detail of the reftable. If we can do so,\nthen, the reftable backend integrated to the ref API can do these\nthings:\n\n - reflog_exists() can say yes when one reflog entry of any type\n (internal to the reftable implementation) exists for the ref;\n\n - create_reflog() can add a reflog entry of the \"fake\" type\n (internal to the reftable implementation);\n\n - for_each_reflog_ent() and its reverse can learn to skip such a\n fake reflog entry.\n\nAs there is no way to ask, via the API, the number of the existing\nreflog entries, the ref API callers would not be able to tell such\nan implementation detail that with reftable backend, create_reflog()\ndoes not create an empty reflog. To them, a reflog created with the\nAPI call would truly be empty as iterators will not return anything.\n\nOr do we have a list of refs kept somewhere in the reftable data\nstructure in a separate chunk? Do we have a bit for each of these\nrefs to record if the log is enabled for it? Then instead of the\nfake reflog entry, we could implement the necessary semantics a lot\nmore cleanly:\n\n - reflog_exists() can just peek the bit.\n\n - create_reflog() can just flip the bit.\n\n - there is no need to touch the iterators.\n\n - the equivalent to files_log_ref_write() can decide based on the\n bit (i.e. what reflog_exists() says) whether to log changes to\n the ref.\n\nIt is probably a lot more sensible to fail refs_create_reflog() and\nsafe_create_reflog() (which is a thin wrapper around the former), if\nwe cannot implement \"a reflog can exist and have no entries yet\"\nsemantics.\n\nOutside the test helper, the only place the helper is used is\n\"checkout -l\" when should_autocreate_reflog() returns false, which\nshould be rare (as we are updating a branch, it should be either a\ndetached HEAD or a ref under refs/heads/), so it would not be a huge\npractical downside if we cannot prepare an empty reflog anyway, I\nwould think.\n\nThanks.\n\n"},{"id":"447463","messageId":"220201.865ypy9te7.gmgdl@evledraar.gmail.com","threadId":"57343","inReplyTo":"b2c6e14c7e7752c9e42cb38372edc8971895932f.1643651420.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] t5312: prepare for reftable","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-01T21:17:34Z","receivedAt":"2022-02-01T21:19:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jan 31 2022, Han-Wen Nienhuys via GitGitGadget wrote:\n\n> From: Han-Wen Nienhuys \n>\n> Mark some tests as REFFILES if they rely on packed refs. Use ref-store\n> helper to create bogus refs.\n>\n> Signed-off-by: Han-Wen Nienhuys \n> ---\n> t/t5312-prune-corruption.sh | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh\n> index ea889c088a5..9d8e249ae8b 100755\n> --- a/t/t5312-prune-corruption.sh\n> +++ b/t/t5312-prune-corruption.sh\n> @@ -22,8 +22,8 @@ test_expect_success 'disable reflogs' '\n> '\n> \n> create_bogus_ref () {\n> -\ttest_when_finished 'rm -f .git/refs/heads/bogus..name' &&\n> -\techo $bogus >.git/refs/heads/bogus..name\n> +\ttest-tool ref-store main update-ref msg \"refs/heads/bogus..name\" $bogus $ZERO_OID REF_SKIP_REFNAME_VERIFICATION &&\n> +\ttest_when_finished \"test-tool ref-store main delete-refs REF_NO_DEREF msg refs/heads/bogus..name\"\n> }\n> \n> test_expect_success 'create history reachable only from a bogus-named ref' '\n> @@ -113,7 +113,7 @@ test_expect_success 'pack-refs does not silently delete broken loose ref' '\n> # we do not want to count on running pack-refs to\n> # actually pack it, as it is perfectly reasonable to\n> # skip processing a broken ref\n> -test_expect_success 'create packed-refs file with broken ref' '\n> +test_expect_success REFFILES 'create packed-refs file with broken ref' '\n> \trm -f .git/refs/heads/main &&\n> \tcat >.git/packed-refs <<-EOF &&\n> \t$missing refs/heads/main\n> @@ -124,13 +124,13 @@ test_expect_success 'create packed-refs file with broken ref' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success 'pack-refs does not silently delete broken packed ref' '\n> +test_expect_success REFFILES 'pack-refs does not silently delete broken packed ref' '\n> \tgit pack-refs --all --prune &&\n> \tgit rev-parse refs/heads/main >actual &&\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_success 'pack-refs does not drop broken refs during deletion' '\n> +test_expect_success REFFILES 'pack-refs does not drop broken refs during deletion' '\n> \tgit update-ref -d refs/heads/other &&\n> \tgit rev-parse refs/heads/main >actual &&\n> \ttest_cmp expect actual\n\nThe setup for these is reffiles-specific, but it seems to me this is\nsomething we'd really like to test with reftable rather than skipping it\nentirely.\n\nI.e. the scenario described in the \"we create...\" comment in this file\nis something that might happen with reftable too, no?\n"},{"id":"447465","messageId":"220201.861r0m9t8n.gmgdl@evledraar.gmail.com","threadId":"57343","inReplyTo":"xmqqa6facn9i.fsf@gitster.g","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-01T21:22:00Z","receivedAt":"2022-02-01T21:23:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Feb 01 2022, Junio C Hamano wrote:\n\n> Han-Wen Nienhuys writes:\n>\n>>> Because there is no generic reflog API that says \"enable log for\n>>> this ref\", a test that checks this feature with files backend would\n>>> do \"touch .git/refs/heads/frotz\".\n>>\n>> There is refs_create_reflog(), so the generic reflog API exists. The\n>> problem is that there is no sensible way to implement it in reftable.\n>\n> Ah, yes, that's correct.\n>\n>> One option is (reflog exists == there exists at least one reflog entry\n>> for the ref).\n>\n> Because the current callers of refs_create_reflog() does want a\n> reflog created that does not give any entry when iterated, I agree\n> with you that adding a \"fake\" reflog entry alone is not a sufficient\n> emulation of the API. I think these are all ...\n>\n>> This messes up the test from this patch, because it\n>> creates a reflog, but because it doesn't populate the reflog, so we\n>> return false for git-reflog-exists.\n>>\n>> It also turns out to mess up the tests in t3420, as follows:\n>>\n>> ++ git stash show -p\n>> error: refs/stash@{0} is not a valid reference\n>>\n>> I get\n>>\n>> reflog_exists: refs/stash: 0\n>>\n>> and \"git stash show -p\" aborts with \"error: refs/stash@{0} is not a\n>> valid reference\".\n>\n> ... indications of hat.\n>\n> I wonder if it is simple and easy to add a new reflog entry type\n> used as an implementation detail of the reftable. If we can do so,\n> then, the reftable backend integrated to the ref API can do these\n> things:\n>\n> - reflog_exists() can say yes when one reflog entry of any type\n> (internal to the reftable implementation) exists for the ref;\n>\n> - create_reflog() can add a reflog entry of the \"fake\" type\n> (internal to the reftable implementation);\n>\n> - for_each_reflog_ent() and its reverse can learn to skip such a\n> fake reflog entry.\n>\n> As there is no way to ask, via the API, the number of the existing\n> reflog entries, the ref API callers would not be able to tell such\n> an implementation detail that with reftable backend, create_reflog()\n> does not create an empty reflog. To them, a reflog created with the\n> API call would truly be empty as iterators will not return anything.\n\nWe could surely add magic record types, but how would such a dance be\nperformed while keeping compatibility with existing JGit clients?\n"},{"id":"447472","messageId":"xmqqsft2b5jl.fsf@gitster.g","threadId":"57343","inReplyTo":"220201.861r0m9t8n.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-01T22:11:58Z","receivedAt":"2022-02-01T22:12:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We could surely add magic record types, but how would such a dance be\n> performed while keeping compatibility with existing JGit clients?\n\nYes. It is exactly the point of the question I asked. If it is\nsimple and easy to add such a new type that is ignored/skipped by\nexisting clients, then we can go that route. If it is simple and\neasy to add a new bit per ref that existing clients would not barf,\nwe can use that as an alternative implementation strategy.\n\nAnd if neither is possible, and there is no other viable third way,\nthen what I wrote in the part you omitted from your quote still\nstands, which was:\n\n>> It is probably a lot more sensible to fail refs_create_reflog() and\n>> safe_create_reflog() (which is a thin wrapper around the former), if\n>> we cannot implement \"a reflog can exist and have no entries yet\"\n>> semantics.\n"},{"id":"447610","messageId":"CAFQ2z_OUqMx7WiTYHGrb5A0K1d_zNVTspM+6trw+u2rqRPjYwA@mail.gmail.com","threadId":"57343","inReplyTo":"220201.865ypy9te7.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 3/3] t5312: prepare for reftable","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-03T14:24:12Z","receivedAt":"2022-02-03T14:24:27Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Feb 1, 2022 at 10:19 PM Ævar Arnfjörð Bjarmason\n wrote:\n\n> > -test_expect_success 'pack-refs does not drop broken refs during deletion' '\n> > +test_expect_success REFFILES 'pack-refs does not drop broken refs during deletion' '\n> > git update-ref -d refs/heads/other &&\n> > git rev-parse refs/heads/main >actual &&\n> > test_cmp expect actual\n>\n> The setup for these is reffiles-specific, but it seems to me this is\n> something we'd really like to test with reftable rather than skipping it\n> entirely.\n>\n> I.e. the scenario described in the \"we create...\" comment in this file\n> is something that might happen with reftable too, no?\n\nThat is tested in the 3 tests right above the ones I marked with\nREFFILES ('pack-refs does not silently delete broken loose ref'). The\ntests at the bottom check what happens if you have a missing SHA1 in a\npacked-refs file. The reftable backend does not have a packed-refs, so\nthere is nothing to test.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447616","messageId":"CAFQ2z_NSCvRbj1bxirxhqSWD+LadzCa8VNOsxGCmFCNT3GUU0g@mail.gmail.com","threadId":"57343","inReplyTo":"xmqqsft2b5jl.fsf@gitster.g","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-03T16:02:47Z","receivedAt":"2022-02-03T16:03:01Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Feb 1, 2022 at 11:12 PM Junio C Hamano wrote:\n>\n> Ævar Arnfjörð Bjarmason writes:\n>\n> > We could surely add magic record types, but how would such a dance be\n> > performed while keeping compatibility with existing JGit clients?\n>\n> Yes. It is exactly the point of the question I asked. If it is\n> simple and easy to add such a new type that is ignored/skipped by\n> existing clients, then we can go that route. If it is simple and\n> easy to add a new bit per ref that existing clients would not barf,\n> we can use that as an alternative implementation strategy.\n\nI'm not sure that there are any JGit clients: I committed reftable\nsupport at the end of 2019. Before that time, we were running it\ninternally at Google, but only ref storage, and without the posix\npart. Reflogs were never stored in refable, and I actually found a\ncouple of bugs in Shawn's Java code.\n\nGerrit has increasingly started using Git as a database, and the\npacked/loose system is just not a very good database, so that\nmotivates the work reftable in general. But the folks who run Gerrit\non a POSIX filesystem want to be sure that isn't a fringe feature, so\nthey only want to start using it once Git itself supports it. So there\nis a chicken & egg problem.\n\nIt's sad that we have to introduce an existence bit to make things\nwork, but overall it is probably easier for me to do than trying to\nmake sense of sequencer.c and how it uses refs/stash@{0}.\n\nTechnically, the only obstacle I see is that we'd need to treat an\nexistence entry especially for the purpose of compaction/gc: we can\ndiscard older entries, but we shouldn't discard the existence bit, no\nmatter how old it is.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447626","messageId":"220203.867dab6dmp.gmgdl@evledraar.gmail.com","threadId":"57343","inReplyTo":"CAFQ2z_NSCvRbj1bxirxhqSWD+LadzCa8VNOsxGCmFCNT3GUU0g@mail.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-02-03T17:39:29Z","receivedAt":"2022-02-03T17:54:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Feb 03 2022, Han-Wen Nienhuys wrote:\n\n> On Tue, Feb 1, 2022 at 11:12 PM Junio C Hamano wrote:\n>>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>> > We could surely add magic record types, but how would such a dance be\n>> > performed while keeping compatibility with existing JGit clients?\n>>\n>> Yes. It is exactly the point of the question I asked. If it is\n>> simple and easy to add such a new type that is ignored/skipped by\n>> existing clients, then we can go that route. If it is simple and\n>> easy to add a new bit per ref that existing clients would not barf,\n>> we can use that as an alternative implementation strategy.\n>\n> I'm not sure that there are any JGit clients: I committed reftable\n> support at the end of 2019. Before that time, we were running it\n> internally at Google, but only ref storage, and without the posix\n> part. Reflogs were never stored in refable, and I actually found a\n> couple of bugs in Shawn's Java code.\n>\n> Gerrit has increasingly started using Git as a database, and the\n> packed/loose system is just not a very good database, so that\n> motivates the work reftable in general. But the folks who run Gerrit\n> on a POSIX filesystem want to be sure that isn't a fringe feature, so\n> they only want to start using it once Git itself supports it. So there\n> is a chicken & egg problem.\n>\n> It's sad that we have to introduce an existence bit to make things\n> work, but overall it is probably easier for me to do than trying to\n> make sense of sequencer.c and how it uses refs/stash@{0}.\n>\n> Technically, the only obstacle I see is that we'd need to treat an\n> existence entry especially for the purpose of compaction/gc: we can\n> discard older entries, but we shouldn't discard the existence bit, no\n> matter how old it is.\n\nAh, that's very informative. I had been assuming (or misremembered) that\nreftable was already seeing production use at Google. Perhaps I\nremembering the now-dead Google Code (or whatever it was called). Maybe\nnot.\n\nIn any case, not being locked into the format as specified is very\nnice. So is it basically seeing no (production) use anywhere as far as\nyou know? Whether that's in production at Google, or some third parties\nvia JGit-something (maybe as editor libraries?).\n\nTaking a bit of a step back.\n\nI do think that generally speaking parts of this series are putting the\ncart before the horse in seemingly trying to get the test suite clean\nbefore we have the integration in-tree.\n\nNot everything you have here, but some of it.\n\nI know I'm the one who started encouraging you to work towards getting\nthe test mode passing, but I think that while it's good to mark some\nobviously file-only tests beforehand, anything where we have different\nbehavior under reftable should really come after.\n\nBecause then we can positively assert what we do differently, not just\nskip an existing test.\n\nAnd yes, for many tests that will require rewriting their setup, because\nthey conflate things that are backend-independent, such as the general\nquestion of \"can we ask about reflog existence?\" with the implementation\ndetail of the test setup, which oftentimes is file-backend specific.\n\nOf course that will mean we'll have some interim period where our test\nsuite is a dumpster fire under GIT_TEST_REFTABLE=true, and I think\nthat's fine, as long as we work towards getting it passing, and as long\nas the non-stability of the nascent backend is very prominently\nadvertised in the interim.\n\nI.e. I think *the* issue with the original series you had in this regard\nwas that git-init.txt (or whatever it was) basically just discussed\nenabling reftable matter-of-factly, when we were still failing\ntens/hundreds of tests, which is just setting up a big bear trap for\nusers to step into.\n\nBut if we just changed those docs a bit to note \"!!WARNING WARNING!!\nEXPERIMENTAL AND UNSTABLE !!WARNING WARNING!!\" or whatever we could\nmerge the API integration parts sooner than later, even with a lot of\nknown-broken tests.\n\nWe could then whitelist the broken parts, and work on narrowing that set\ndown. Similar what the SANITIZE=leak mode is currently doing for memory\nleaks.\n\nI think that would make things a lot easier when reviewing submissions\nlike these, in that we have reftable/* in-tree already, but with the\n\"real\" integration we could check how files/reftable backends behave,\nadd the diverging behavior to tests etc.\n\nWhat do you think?\n"},{"id":"447629","messageId":"CAFQ2z_MkZBtjViTsDNuKLYUXzFXJM6sPLOvXdiRAZrs84pggUw@mail.gmail.com","threadId":"57343","inReplyTo":"220203.867dab6dmp.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-03T18:10:43Z","receivedAt":"2022-02-03T18:10:57Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Thu, Feb 3, 2022 at 6:53 PM Ævar Arnfjörð Bjarmason wrote:\n> >> Yes. It is exactly the point of the question I asked. If it is\n> >> simple and easy to add such a new type that is ignored/skipped by\n> >> existing clients, then we can go that route. If it is simple and\n> >> easy to add a new bit per ref that existing clients would not barf,\n> >> we can use that as an alternative implementation strategy.\n> >\n> > I'm not sure that there are any JGit clients: I committed reftable\n> > support at the end of 2019. Before that time, we were running it\n> > internally at Google, but only ref storage, and without the posix\n> > part. Reflogs were never stored in refable, and I actually found a\n> > couple of bugs in Shawn's Java code.\n> >\n> > Gerrit has increasingly started using Git as a database, and the\n> > packed/loose system is just not a very good database, so that\n> > motivates the work reftable in general. But the folks who run Gerrit\n> > on a POSIX filesystem want to be sure that isn't a fringe feature, so\n> > they only want to start using it once Git itself supports it. So there\n> > is a chicken & egg problem.\n> >\n> > It's sad that we have to introduce an existence bit to make things\n> > work, but overall it is probably easier for me to do than trying to\n> > make sense of sequencer.c and how it uses refs/stash@{0}.\n> >\n> > Technically, the only obstacle I see is that we'd need to treat an\n> > existence entry especially for the purpose of compaction/gc: we can\n> > discard older entries, but we shouldn't discard the existence bit, no\n> > matter how old it is.\n>\n> Ah, that's very informative. I had been assuming (or misremembered) that\n> reftable was already seeing production use at Google. Perhaps I\n> remembering the now-dead Google Code (or whatever it was called). Maybe\n> not.\n\nWe use the format (the JGit code) at Google, but we only use it to\nstore refs, that is, the refname => {SHA1, symref, tag} mapping. We\ncurrently don't store reflog data in reftable, and the bugs I found\nwere just in the reflog parts.\n\nWe store the tables in bigtable (among others), so the part that does\nthe POSIX file locking is new (basically, everything in\nreftable/stack.c and its equivalent in JGit).\n\nSo we are locked into the format to some degree.\n\nFor the existence bit, I think I could simply record a $zeroid =>\n$zeroid ref update in ref log and treat that specially.\n\n> In any case, not being locked into the format as specified is very\n> nice. So is it basically seeing no (production) use anywhere as far as\n> you know? Whether that's in production at Google, or some third parties\n> via JGit-something (maybe as editor libraries?).\n\nI think our friends at Gerritforge have been experimenting with it,\nbut not in a production setting, AFAIK. Luca might confirm.\n\n> Taking a bit of a step back.\n>\n> I do think that generally speaking parts of this series are putting the\n> cart before the horse in seemingly trying to get the test suite clean\n> before we have the integration in-tree.\n>\n> Not everything you have here, but some of it.\n>\n> I know I'm the one who started encouraging you to work towards getting\n> the test mode passing, but I think that while it's good to mark some\n> obviously file-only tests beforehand, anything where we have different\n> behavior under reftable should really come after.\n\n(I can't parse your last sentence)\n\n> Of course that will mean we'll have some interim period where our test\n> suite is a dumpster fire under GIT_TEST_REFTABLE=true, and I think\n\nIt's actually not that bad. By my last count, there were 38 files with\ntest failures.\n\n> that's fine, as long as we work towards getting it passing, and as long\n> as the non-stability of the nascent backend is very prominently\n> advertised in the interim.\n>\n> I.e. I think *the* issue with the original series you had in this regard\n> was that git-init.txt (or whatever it was) basically just discussed\n> enabling reftable matter-of-factly, when we were still failing\n> tens/hundreds of tests, which is just setting up a big bear trap for\n> users to step into.\n\nI read that comment, and I removed that long ago. Right now the only\nway to get a reftable is to say GIT_TEST_REFTABLE=1 on init.\n\n> But if we just changed those docs a bit to note \"!!WARNING WARNING!!\n> EXPERIMENTAL AND UNSTABLE !!WARNING WARNING!!\" or whatever we could\n> merge the API integration parts sooner than later, even with a lot of\n> known-broken tests.\n>\n> We could then whitelist the broken parts, and work on narrowing that set\n> down. Similar what the SANITIZE=leak mode is currently doing for memory\n> leaks.\n>\n> I think that would make things a lot easier when reviewing submissions\n> like these, in that we have reftable/* in-tree already, but with the\n> \"real\" integration we could check how files/reftable backends behave,\n> add the diverging behavior to tests etc.\n>\n> What do you think?\n\nSounds more fun than the current model :-)\n\nThe latest version of the code is here:\nhttps://github.com/hanwen/git/tree/merged-seen-20220117\n\nWhat do you need besides the \"RFC: reftable backend\" commit?\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447635","messageId":"xmqqk0ebvm35.fsf@gitster.g","threadId":"57343","inReplyTo":"CAFQ2z_OUqMx7WiTYHGrb5A0K1d_zNVTspM+6trw+u2rqRPjYwA@mail.gmail.com","subject":"Re: [PATCH 3/3] t5312: prepare for reftable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-03T18:31:10Z","receivedAt":"2022-02-03T18:31:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> On Tue, Feb 1, 2022 at 10:19 PM Ævar Arnfjörð Bjarmason\n> wrote:\n>\n>> > -test_expect_success 'pack-refs does not drop broken refs during deletion' '\n>> > +test_expect_success REFFILES 'pack-refs does not drop broken refs during deletion' '\n>> > git update-ref -d refs/heads/other &&\n>> > git rev-parse refs/heads/main >actual &&\n>> > test_cmp expect actual\n>>\n>> The setup for these is reffiles-specific, but it seems to me this is\n>> something we'd really like to test with reftable rather than skipping it\n>> entirely.\n>>\n>> I.e. the scenario described in the \"we create...\" comment in this file\n>> is something that might happen with reftable too, no?\n>\n> That is tested in the 3 tests right above the ones I marked with\n> REFFILES ('pack-refs does not silently delete broken loose ref'). The\n> tests at the bottom check what happens if you have a missing SHA1 in a\n> packed-refs file. The reftable backend does not have a packed-refs, so\n> there is nothing to test.\n\nYup. The patch posted looked good to me.\n\nThanks for writing and reviewing.\n"},{"id":"447681","messageId":"xmqq4k5fr1mh.fsf@gitster.g","threadId":"57343","inReplyTo":"CAFQ2z_NSCvRbj1bxirxhqSWD+LadzCa8VNOsxGCmFCNT3GUU0g@mail.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-03T23:06:46Z","receivedAt":"2022-02-03T23:06:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n> Technically, the only obstacle I see is that we'd need to treat an\n> existence entry especially for the purpose of compaction/gc: we can\n> discard older entries, but we shouldn't discard the existence bit, no\n> matter how old it is.\n\nI was hoping that we already have a type of block that can be used\nto record an attribute on the ref (other than its value) and it\nwould be just the matter of stealing one unused bit from such a\nrecord per ref to say \"when answering 'does this ref have reflog?'\nsay yes even when there is no log record for that refname\". Or the\ntable format is extensible enough that we can add such a block\nwithout breaking existing clients.\n\nIf we have to implement the \"this ref has (enabled) reflog, even\nthough there may not be any log record for it right now\" bit as a\nfake log record, then yes, we'd have to teach the log iterator to\nskip such an entry and we'd have to teach the expiry logic that they\nare not to be expired. It certainly is a sad design than being able\nto express the bit in a more direct way (like file backend does,\nwhich is \"presence of the reflog file gives the precense of the log,\ncontents of the reflog file are the actual logs).\n\n"},{"id":"447873","messageId":"CAFQ2z_Nb=wY_+B1ub0XDgZnvgCHGmFu1rjMuKgbFFir0=1PHtw@mail.gmail.com","threadId":"57343","inReplyTo":"xmqq4k5fr1mh.fsf@gitster.g","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-07T09:48:47Z","receivedAt":"2022-02-07T10:05:05Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Fri, Feb 4, 2022 at 12:06 AM Junio C Hamano wrote:\n>\n> Han-Wen Nienhuys writes:\n>\n> > Technically, the only obstacle I see is that we'd need to treat an\n> > existence entry especially for the purpose of compaction/gc: we can\n> > discard older entries, but we shouldn't discard the existence bit, no\n> > matter how old it is.\n>\n> I was hoping that we already have a type of block that can be used\n> to record an attribute on the ref (other than its value) and it\n> would be just the matter of stealing one unused bit from such a\n> record per ref to say \"when answering 'does this ref have reflog?'\n> say yes even when there is no log record for that refname\". Or the\n> table format is extensible enough that we can add such a block\n> without breaking existing clients.\n\nThat place doesn't exist, unfortunately, but even if it did, having a\nspecial reflog entry indicating existence is a better solution all\naround, I think. A separate per-ref bit allows for data\ninconsistencies: what if the bit says \"there is no reflog\", but we\nactually do have reflog entries in the 'g' section?\n\nIt also has less chances of creating complicated control flows\n(especially in JGit which wasn't designed for this bit from the\nstart): the tables have to be written in lexicographic order, so you\nonly can write this bit after you know if reflog entries were written\nfor a certain ref.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447894","messageId":"CAFQ2z_PE_ERoocVjUGCqcFxTDUy79PFbkCVh-y+At7KvXx8TtQ@mail.gmail.com","threadId":"57343","inReplyTo":"CAFQ2z_Nb=wY_+B1ub0XDgZnvgCHGmFu1rjMuKgbFFir0=1PHtw@mail.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-07T16:52:36Z","receivedAt":"2022-02-07T17:03:48Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Mon, Feb 7, 2022 at 10:48 AM Han-Wen Nienhuys wrote:\n>\n> On Fri, Feb 4, 2022 at 12:06 AM Junio C Hamano wrote:\n> >\n> > Han-Wen Nienhuys writes:\n> >\n> > > Technically, the only obstacle I see is that we'd need to treat an\n> > > existence entry especially for the purpose of compaction/gc: we can\n> > > discard older entries, but we shouldn't discard the existence bit, no\n> > > matter how old it is.\n> >\n> > I was hoping that we already have a type of block that can be used\n> > to record an attribute on the ref (other than its value) and it\n> > would be just the matter of stealing one unused bit from such a\n> > record per ref to say \"when answering 'does this ref have reflog?'\n> > say yes even when there is no log record for that refname\". Or the\n> > table format is extensible enough that we can add such a block\n> > without breaking existing clients.\n>\n> That place doesn't exist, unfortunately, but even if it did, having a\n> special reflog entry indicating existence is a better solution all\n> around, I think. A separate per-ref bit allows for data\n> inconsistencies: what if the bit says \"there is no reflog\", but we\n> actually do have reflog entries in the 'g' section?\n>\n> It also has less chances of creating complicated control flows\n> (especially in JGit which wasn't designed for this bit from the\n> start): the tables have to be written in lexicographic order, so you\n> only can write this bit after you know if reflog entries were written\n> for a certain ref.\n\nCorrection. I wish the table blocks were written in lexicographic\norder, but they are written in order 'g', ['i',] 'o', ['i'], 'g',\n['i']. Since the 'g' block is last within a table, we could add a new\nsection at the end. My point that this is considerable work to think\nthrough how to make this work with JGit still stands, though.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"447925","messageId":"xmqq35kugs9j.fsf@gitster.g","threadId":"57343","inReplyTo":"CAFQ2z_PE_ERoocVjUGCqcFxTDUy79PFbkCVh-y+At7KvXx8TtQ@mail.gmail.com","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-02-07T23:40:24Z","receivedAt":"2022-02-08T01:06:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Han-Wen Nienhuys writes:\n\n>> It also has less chances of creating complicated control flows\n>> (especially in JGit which wasn't designed for this bit from the\n>> start): the tables have to be written in lexicographic order, so you\n>> only can write this bit after you know if reflog entries were written\n>> for a certain ref.\n>\n> Correction. I wish the table blocks were written in lexicographic\n> order, but they are written in order 'g', ['i',] 'o', ['i'], 'g',\n> ['i']. Since the 'g' block is last within a table, we could add a new\n> section at the end. My point that this is considerable work to think\n> through how to make this work with JGit still stands, though.\n\nAs long as a fake/NULL entry in the reflog is invisible to iterators\nand does not count as part of numbered entries when reflog@{23}\nnotation is used, I think it is perfectly fine to take that\napproach, instead of \"separate bit\". I brought it up only as a\npossible alternative (i.e. \"if bit is on or any entry exists, we do\nhave log for the ref\") in case ignoring the fake entry is impossible.\n\n"},{"id":"447991","messageId":"CAFQ2z_P3MNTSDwqjfCDNU8jStCFtqXsP5vpcJsxr25B0JLxP=g@mail.gmail.com","threadId":"57343","inReplyTo":"xmqq35kugs9j.fsf@gitster.g","subject":"Re: [PATCH 2/3] t1405: mark test that checks existence as REFFILES","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2022-02-08T14:58:21Z","receivedAt":"2022-02-08T14:58:37Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Tue, Feb 8, 2022 at 12:40 AM Junio C Hamano wrote:\n> >> It also has less chances of creating complicated control flows\n> >> (especially in JGit which wasn't designed for this bit from the\n> >> start): the tables have to be written in lexicographic order, so you\n> >> only can write this bit after you know if reflog entries were written\n> >> for a certain ref.\n> >\n> > Correction. I wish the table blocks were written in lexicographic\n> > order, but they are written in order 'g', ['i',] 'o', ['i'], 'g',\n> > ['i']. Since the 'g' block is last within a table, we could add a new\n> > section at the end. My point that this is considerable work to think\n> > through how to make this work with JGit still stands, though.\n>\n> As long as a fake/NULL entry in the reflog is invisible to iterators\n> and does not count as part of numbered entries when reflog@{23}\n> notation is used, I think it is perfectly fine to take that\n> approach, instead of \"separate bit\". I brought it up only as a\n> possible alternative (i.e. \"if bit is on or any entry exists, we do\n> have log for the ref\") in case ignoring the fake entry is impossible.\n\nI implemented this. It was very clean and easy; I didn't yet check if\nJGit can handle it, but if it doesn't, it should be easy to fix.\n\nYou can drop patch 2/3 (ie. this subject line).\n\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"}]}