{"thread":{"id":"60542","subject":"reftable: How to represent deleted referees of symrefs in the reflog?","startedAt":"2023-11-21T13:46:59Z","lastAt":"2023-12-28T08:14:59Z","messageCount":3,"participants":["Patrick Steinhardt","Han-Wen Nienhuys"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"485033","messageId":"ZVy0zKcmc8tjmgzs@tanuki","threadId":"60542","inReplyTo":null,"subject":"reftable: How to represent deleted referees of symrefs in the reflog?","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-11-21T13:46:52Z","receivedAt":"2023-11-21T13:46:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nI'm currently trying to fully understand how exactly reflogs and reflog\nentries are supposed to be deleted in the reftable backend. For ref\nrecords it's easy enough, as explained in the technical documentation\nfor reftables that is part of our tree:\n\n> Deletion of any reference can be explicitly stored by setting the type\n> to 0x0 and omitting the value field of the ref_record. This serves as\n> a tombstone, overriding any assertions about the existence of the\n> reference from earlier files in the stack.\n\nSo you simply create a new ref record with type 0x0 and are done.\n\nFor log records it seems to be a bit of a different story though.\nAgain, quoting the technical documentation:\n\n> The log_type = 0x0 is mostly useful for git stash drop, removing an\n> entry from the reflog of refs/stash in a transaction file (below),\n> without needing to rewrite larger files. Readers reading a stack of\n> reflogs must treat this as a deletion.\n\nTo me it seems like deletions in this case only delete a particular log\nentry instead of the complete log for a particular reference. And some\nolder discussion [1] seems to confirm my hunch that a complete reflog is\ndeleted not with `log_type = 0x0`, but instead by writing the null\nobject ID as new ID.\n\nSo: a single entry is deleted with `log_type = 0x0`, the complete reflog\nentry is deleted with the null object ID as new ID. Fair enough, even\nthough the documentation could be updated to make this easier to\nunderstand. I'll happily do so if my understanding is correct here.\n\nIn any case though, this proposed behaviour is not sufficient to cover\nall cases that the files-based reflog supports. The following case may\nbe weird, but we do have tests for it in t1400:\n\n```\n $ git init repo\nInitialized empty Git repository in /tmp/repo/.git/\n $ cd repo/\n $ git commit --allow-empty --message initial-commit\n[main (root-commit) 9f10b3f] initial-commit\n # Everything looks as expected up to this point.\n $ git reflog show HEAD\n9f10b3f (HEAD -> main) HEAD@{0}: commit (initial): initial-commit\n\n # This behaviour is a bit more on the weird side. We delete the\n # referee, and that causes the files backend to claim that the reflog\n # for HEAD is gone, as well. The reflog still exists though, as\n # demonstrated in the next command.\n $ git update-ref -m delete-main -d refs/heads/main\n $ git reflog show HEAD\nfatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n\n # We now re-create the referee, which revives the reflog _including_\n # the old entry.\n $ git update-ref -m recreate-main refs/heads/main 9f10b3f\n $ git reflog show HEAD\n9f10b3f (HEAD -> main) HEAD@{0}: recreate-main\n9f10b3f (HEAD -> main) HEAD@{2}: commit (initial): initial-commit\n\n $ cat .git/logs/HEAD\n0000000000000000000000000000000000000000 9f10b3f9b20962690fdeff76cd592722fbf57deb Patrick Steinhardt 1700573003 +0100\tcommit (initial): initial-commit\n9f10b3f9b20962690fdeff76cd592722fbf57deb 0000000000000000000000000000000000000000 Patrick Steinhardt 1700573060 +0100\tdelete-main\n0000000000000000000000000000000000000000 9f10b3f9b20962690fdeff76cd592722fbf57deb Patrick Steinhardt 1700573078 +0100\trecreate-main\n```\n\nIt kind of feels like the second step in the files backend where the\nreflog is claimed to not exist is buggy -- I'd have expected to still\nsee the reflog, as the HEAD reference exists quite alright and has never\nstopped to exist. And in the third step, I'd have expected to see three\nreflog entries, including the deletion that is indeed still present in\nthe on-disk logfile.\n\nBut with the reftable backend the problem becomes worse: we cannot even\nrepresent the fact that the reflog still exists, but that the deletion\nof the referee has caused the HEAD to point to the null OID, because the\nnull OID indicates complete deletion of the reflog. Consequentially, if\nwe wrote the null OID, we'd only be able to see the last log entry here.\n\nIt may totally be that I'm missing something obvious here. But if not,\nit leaves us with a couple of options for how to fix it:\n\n 1. Disregard this edge case and accept that the reftable backend\n does not do exactly the same thing as the files backend in very\n specific situations like this.\n\n 2. Change the reftable format so that it can discern these cases,\n e.g. by indicating deletion via a new log type.\n\n 3. Change the files backend to adapt.\n\nNone of these alternatives feel particularly great to me. In my opinion\n(2) is likely the best option, but would require us to change the format\nformat that's already in use by Google in the context of multiple\nprojects. So I'm not quite sure how thrilled you folks at Google and\nother users of the reftable library are with this prospect.\n\nAnyway, happy to hear about alternative takes or corrections.\n\nPatrick\n\n[1]: \n"},{"id":"485882","messageId":"CAOw_e7aDPTjDj_K3yyyatWkw3R-oT+_u1LgZyzQEE=6LnH+pmg@mail.gmail.com","threadId":"60542","inReplyTo":"ZVy0zKcmc8tjmgzs@tanuki","subject":"Re: reftable: How to represent deleted referees of symrefs in the reflog?","fromName":"Han-Wen Nienhuys","fromEmail":"hanwenn@gmail.com","sentAt":"2023-12-20T19:03:00Z","receivedAt":"2023-12-20T19:03:12Z","isPatch":false,"sender":{"key":"hanwenn@gmail.com","avatar":"https://gravatar.com/avatar/058832acb8d613baeb6ce9d21b009d9772424a309b9a521330423daef909a27a?d=mp&s=160"},"body":"On Tue, Nov 21, 2023 at 2:46 PM Patrick Steinhardt wrote:\n> To me it seems like deletions in this case only delete a particular log\n> entry instead of the complete log for a particular reference. And some\n> older discussion [1] seems to confirm my hunch that a complete reflog is\n> deleted not with `log_type = 0x0`, but instead by writing the null\n> object ID as new ID.\n\nNo, writing a null OID (more precisely a transition from $SHA1 =>\n$ZERO) means that a branch was at $SHA1, and then was deleted. The\nreflog continues to exist, and new entries may be added by reviving\nthe branch. That would add a $ZERO => $NEWSHA transition, but the\nhistory of the branch prior to its deletion is retained.\n\n> # This behaviour is a bit more on the weird side. We delete the\n> # referee, and that causes the files backend to claim that the reflog\n> # for HEAD is gone, as well. The reflog still exists though, as\n> # demonstrated in the next command.\n> $ git update-ref -m delete-main -d refs/heads/main\n> $ git reflog show HEAD\n> fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.\n\nThis looks wrong to me. HEAD has a history, and that history didn't go\naway because the current branch happened to be deleted.\n\n> It kind of feels like the second step in the files backend where the\n> reflog is claimed to not exist is buggy -- I'd have expected to still\n\nright, I agree.\n\n> see the reflog, as the HEAD reference exists quite alright and has never\n> stopped to exist. And in the third step, I'd have expected to see three\n> reflog entries, including the deletion that is indeed still present in\n> the on-disk logfile.\n>\n> But with the reftable backend the problem becomes worse: we cannot even\n> represent the fact that the reflog still exists, but that the deletion\n> of the referee has caused the HEAD to point to the null OID, because the\n> null OID indicates complete deletion of the reflog.\n\nThis doesn't match my recollection. See\nhttps://github.com/git/git/pull/1215, or more precisely\nhttps://github.com/git/git/blob/3b2c5ddb28fa42a8c944373bea2ca756b1723908/refs/reftable-backend.c#L1582\n\nRemoving the entire reflog means removing all the individual entries\nusing tombstones.\n\n> Consequentially, if\n> we wrote the null OID, we'd only be able to see the last log entry here.\n>\n> It may totally be that I'm missing something obvious here. But if not,\n> it leaves us with a couple of options for how to fix it:\n>\n> 1. Disregard this edge case and accept that the reftable backend\n> does not do exactly the same thing as the files backend in very\n> specific situations like this.\n\nI remember discussing with Jun that it would be acceptable to have\nslight semantic differences if unavoidable for the reflogs, and there\nshould be a record of this in the list. I think there will always be\nsome differences: for example, dropping reflogs because of a dir/file\nconflict seems like a bug rather than a feature.\n\n> 2. Change the reftable format so that it can discern these cases,\n> e.g. by indicating deletion via a new log type.\n\nThis will be a bit messy, because it means that every read of the\nreflog has to special case the \"deletion marker\" to make sure it is\nabsent, and on every reflog write, you have to create a tombstone for\nthe deletion marker, to make sure the reflog exists. Also, what if you\nhave something that looks like:\n\nrefs/main@1 : 0000... => 345....\nrefs/main@0xfffffff: DELETION\n\nthe reflog doesn't exists (\"DELETION\"), and now it is recreated, ie.\nthe following is added:\n\nrefs/main@0xfffffff: TOMBSTONE DELETION (make sure reflog exists)\nrefs/main@2 : 0000... => abc.... (the entry we want to write)\n\nthe result is that the reflog would also surface the first entry (@1 ,\n000... => 345... ) again. To prevent that from happening, you have to\nwrite tombstones for all entries if you \"delete the reflog\", but then\ndo you really need a separate existence marker?\n\n> None of these alternatives feel particularly great to me. In my opinion\n> (2) is likely the best option, but would require us to change the format\n> format that's already in use by Google in the context of multiple\n> projects. So I'm not quite sure how thrilled you folks at Google and\n> other users of the reftable library are with this prospect.\n\nGoogle uses reftable for datacenter-local serving. The reflog is\nstored in a database, because it's never involved in serving traffic.\nWhen I implemented reftable-on-filesystem, I found a few bugs in the\nreflog code because it was never exercised at Google. Terry/Jonathan\nmay correct me if my knowledge is outdated.\n\nLuca manages large Gerrit installations. He might know if anyone has\nbeen using reftable in the wild.\n\nHTH.\n\n-- \nHan-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen\n"},{"id":"486089","messageId":"ZY0ufWTW0W9DSyzl@tanuki","threadId":"60542","inReplyTo":"CAOw_e7aDPTjDj_K3yyyatWkw3R-oT+_u1LgZyzQEE=6LnH+pmg@mail.gmail.com","subject":"Re: reftable: How to represent deleted referees of symrefs in the reflog?","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2023-12-28T08:14:53Z","receivedAt":"2023-12-28T08:14:59Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Dec 20, 2023 at 08:03:00PM +0100, Han-Wen Nienhuys wrote:\n> On Tue, Nov 21, 2023 at 2:46 PM Patrick Steinhardt wrote:\n> > To me it seems like deletions in this case only delete a particular log\n> > entry instead of the complete log for a particular reference. And some\n> > older discussion [1] seems to confirm my hunch that a complete reflog is\n> > deleted not with `log_type = 0x0`, but instead by writing the null\n> > object ID as new ID.\n> \n> No, writing a null OID (more precisely a transition from $SHA1 =>\n> $ZERO) means that a branch was at $SHA1, and then was deleted. The\n> reflog continues to exist, and new entries may be added by reviving\n> the branch. That would add a $ZERO => $NEWSHA transition, but the\n> history of the branch prior to its deletion is retained.\n> \n> > # This behaviour is a bit more on the weird side. We delete the\n> > # referee, and that causes the files backend to claim that the reflog\n> > # for HEAD is gone, as well. The reflog still exists though, as\n> > # demonstrated in the next command.\n> > $ git update-ref -m delete-main -d refs/heads/main\n> > $ git reflog show HEAD\n> > fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.\n> \n> This looks wrong to me. HEAD has a history, and that history didn't go\n> away because the current branch happened to be deleted.\n> \n> > It kind of feels like the second step in the files backend where the\n> > reflog is claimed to not exist is buggy -- I'd have expected to still\n> \n> right, I agree.\n> \n> > see the reflog, as the HEAD reference exists quite alright and has never\n> > stopped to exist. And in the third step, I'd have expected to see three\n> > reflog entries, including the deletion that is indeed still present in\n> > the on-disk logfile.\n> >\n> > But with the reftable backend the problem becomes worse: we cannot even\n> > represent the fact that the reflog still exists, but that the deletion\n> > of the referee has caused the HEAD to point to the null OID, because the\n> > null OID indicates complete deletion of the reflog.\n> \n> This doesn't match my recollection. See\n> https://github.com/git/git/pull/1215, or more precisely\n> https://github.com/git/git/blob/3b2c5ddb28fa42a8c944373bea2ca756b1723908/refs/reftable-backend.c#L1582\n> \n> Removing the entire reflog means removing all the individual entries\n> using tombstones.\n\nYeah, indeed. I was misinterpreting older discussions here, and now use\nindividual tombstones which does work as expected. It's a bit\nunfortunate that deletion of the reflog thus scales with its size, but\nfor now I think that's okay. We can still iterate on this if this ever\nproves to become a problem.\n\n> > Consequentially, if\n> > we wrote the null OID, we'd only be able to see the last log entry here.\n> >\n> > It may totally be that I'm missing something obvious here. But if not,\n> > it leaves us with a couple of options for how to fix it:\n> >\n> > 1. Disregard this edge case and accept that the reftable backend\n> > does not do exactly the same thing as the files backend in very\n> > specific situations like this.\n> \n> I remember discussing with Jun that it would be acceptable to have\n> slight semantic differences if unavoidable for the reflogs, and there\n> should be a record of this in the list. I think there will always be\n> some differences: for example, dropping reflogs because of a dir/file\n> conflict seems like a bug rather than a feature.\n\nInitially I'm aiming for matching semantics, because this will make it a\nton easier to land the initial implementation of the backend. But I do\nagree that eventually we may want to iterate and allow for diverging\nbehaviour between the backends.\n\nThanks!\n\nPatrick\n"}]}