{"thread":{"id":"55130","subject":"[PATCH] reflog expire --stale-fix: be generous about missing objects","startedAt":"2021-02-10T16:12:57Z","lastAt":"2021-02-12T19:36:47Z","messageCount":5,"participants":["Johannes Schindelin via GitGitGadget","Junio C Hamano","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"416639","messageId":"pull.873.git.1612973499110.gitgitgadget@gmail.com","threadId":"55130","inReplyTo":null,"subject":"[PATCH] reflog expire --stale-fix: be generous about missing objects","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-10T16:11:38Z","receivedAt":"2021-02-10T16:12:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhenever a user runs `git reflog expire --stale-fix`, the most likely\nreason is that their repository is at least _somewhat_ corrupt. Which\nmeans that it is more than just possible that some objects are missing.\n\nIf that is the case, that can currently let the command abort through\nthe phase where it tries to mark all reachable objects.\n\nInstead of adding insult to injury, let's be gentle and continue as best\nas we can in such a scenario, simply by ignoring the missing objects and\nmoving on.\n\nSigned-off-by: Johannes Schindelin \n---\n Make git reflog expire --stale-fix a lot more useful\n \n Yesterday, I tried to run a quick test to find out whether master's\n version of git repack prevents .bitmap files from being deleted by still\n having them mmap()ed. Since I do not have a build of master lying around\n just like that, I checked it out, built the thing, and then ran\n \n ./bin-wrappers/git -c alias.c='!(cd /path/to/directory && ./test-whether-bitmaps-are-mmaped-during-repack.sh) c\n \n \n Do NOT try this at home! The problem with this invocation is that the\n alias will still have GIT_DIR set, therefore the git init in that script\n will not create a new Git directory, and the git repack -ad in that\n script will remove all kinds of precious objects from the Git checkout.\n Even though I interrupted the run as quickly as I realized that things\n were going wrong, my repository was corrupted in a major way, and it\n took me many hours to get back to a healthy state.\n \n It made matters worse that git reflog expire --stale-fix was less\n helpful than it could have been, and this patch is the result of my\n directed emotional energy.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-873%2Fdscho%2Freflog-expire-with-missing-objects-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-873/dscho/reflog-expire-with-missing-objects-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/873\n\n builtin/reflog.c | 3 +++\n t/t1410-reflog.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex ca1d8079f320..09541d1c8048 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -602,6 +602,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t */\n \tif (cb.cmd.stalefix) {\n \t\trepo_init_revisions(the_repository, &cb.cmd.revs, prefix);\n+\t\tcb.cmd.revs.do_not_die_on_missing_tree = 1;\n+\t\tcb.cmd.revs.ignore_missing = 1;\n+\t\tcb.cmd.revs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n \t\tmark_reachable_objects(&cb.cmd.revs, 0, 0, NULL);\ndiff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\nindex ecccaa063453..27b9080251a9 100755\n--- a/t/t1410-reflog.sh\n+++ b/t/t1410-reflog.sh\n@@ -158,6 +158,32 @@ test_expect_success 'reflog expire' '\n \tcheck_fsck \"dangling commit $K\"\n '\n \n+test_expect_success '--stale-fix handles missing objects generously' '\n+\tgit -c core.logAllRefUpdates=false fast-import --date-format=now <<-EOS &&\n+\tcommit refs/heads/stale-fix\n+\tmark :1\n+\tcommitter Author now\n+\tdata < now\n+\tdata <\nwrites:\n\n> From: Johannes Schindelin \n>\n> Whenever a user runs `git reflog expire --stale-fix`, the most likely\n> reason is that their repository is at least _somewhat_ corrupt. Which\n> means that it is more than just possible that some objects are missing.\n>\n> If that is the case, that can currently let the command abort through\n> the phase where it tries to mark all reachable objects.\n>\n> Instead of adding insult to injury, let's be gentle and continue as best\n> as we can in such a scenario, simply by ignoring the missing objects and\n> moving on.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nI am of two minds.\n\nI appreciate an effort of making it looser and less likely to get in\ntrouble when running in a corrupt repository, but --stale-fix is a\nbit special and should probably be removed.\n\nThe only reason the option was added was because we needed to have\nan easy way to recover from a specific kind of reflog corruption\nthat would have resulted by a (then known) bug in \"git reflog\" in\nthe released version of Git that came immediately before the version\nof Git that added the \"fix\" option, while the root cause of the\ncorruption got fixed. \n\nBack when 1389d9dd (reflog expire --fix-stale, 2007-01-06) was\nwritten, it was very useful to have a way to recover from the\ncorruption likely to have happened with the version of Git that came\nbefore it. But it no longer is relevant after this many years.\nThere may be other ways to break the reflog entries, but --stale-fix\nwas never designed to deal with anything but a specific way the\nreflog gets corrupted (namely, by the old version of Git that\ncorrupted reflog in a specific way), so keeping it would not be very\nuseful.\n\n\n"},{"id":"416741","messageId":"YCUQkqXNXFzBs7aA@coredump.intra.peff.net","threadId":"55130","inReplyTo":"xmqqy2fv1x98.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] reflog expire --stale-fix: be generous about missing objects","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-02-11T11:10:10Z","receivedAt":"2021-02-11T11:14:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 10, 2021 at 12:30:27PM -0800, Junio C Hamano wrote:\n\n> I appreciate an effort of making it looser and less likely to get in\n> trouble when running in a corrupt repository, but --stale-fix is a\n> bit special and should probably be removed.\n> \n> The only reason the option was added was because we needed to have\n> an easy way to recover from a specific kind of reflog corruption\n> that would have resulted by a (then known) bug in \"git reflog\" in\n> the released version of Git that came immediately before the version\n> of Git that added the \"fix\" option, while the root cause of the\n> corruption got fixed.\n> \n> Back when 1389d9dd (reflog expire --fix-stale, 2007-01-06) was\n> written, it was very useful to have a way to recover from the\n> corruption likely to have happened with the version of Git that came\n> before it. But it no longer is relevant after this many years.\n> There may be other ways to break the reflog entries, but --stale-fix\n> was never designed to deal with anything but a specific way the\n> reflog gets corrupted (namely, by the old version of Git that\n> corrupted reflog in a specific way), so keeping it would not be very\n> useful.\n\nFWIW, I have used --stale-fix for cases where a repo's reflog was\n\"corrupted\" by its alternate pruning objects.\n\nE.g., if you do something like this:\n\n git clone -s orig.git new.git\n\nyou're now at risk of \"git gc\" in orig.git corrupting new.git, because\nits reachability check doesn't know anything about those refs. You can\nmitigate that by keeping a copy of new.git's refs in orig.git. Something\nlike:\n\n git -C orig.git fetch ../new.git refs/*:refs/preserve/*\n git -C orig.git gc\n\nBut that doesn't know about reflogs at all! It will still corrupt them\n(but only sometimes; depending how often you do that fetch, you might\ncatch the same values in orig.git's reflog).\n\nPossibly the correct answer here is \"turn off reflogs in new.git\", but\nthey are sometimes useful, and things _mostly_ work (for history that\ndoesn't rewind, or where the rewound bits are specific to new.git). So\nit's useful to be able to run something like \"reflog expire --stale-fix\"\nto clear out the occasional problem.\n\n(A careful reader will note that objects mentioned only in the index\nhave a similar problem; but again, those tend to be local to new.git,\nand don't exist at all in a server context).\n\n-Peff\n"},{"id":"416799","messageId":"nycvar.QRO.7.76.6.2102121702120.29765@tvgsbejvaqbjf.bet","threadId":"55130","inReplyTo":"YCUQkqXNXFzBs7aA@coredump.intra.peff.net","subject":"Re: [PATCH] reflog expire --stale-fix: be generous about missing objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-02-12T16:04:08Z","receivedAt":"2021-02-12T16:06:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 11 Feb 2021, Jeff King wrote:\n\n> On Wed, Feb 10, 2021 at 12:30:27PM -0800, Junio C Hamano wrote:\n>\n> > I appreciate an effort of making it looser and less likely to get in\n> > trouble when running in a corrupt repository, but --stale-fix is a\n> > bit special and should probably be removed.\n> >\n> > The only reason the option was added was because we needed to have\n> > an easy way to recover from a specific kind of reflog corruption\n> > that would have resulted by a (then known) bug in \"git reflog\" in\n> > the released version of Git that came immediately before the version\n> > of Git that added the \"fix\" option, while the root cause of the\n> > corruption got fixed.\n> >\n> > Back when 1389d9dd (reflog expire --fix-stale, 2007-01-06) was\n> > written, it was very useful to have a way to recover from the\n> > corruption likely to have happened with the version of Git that came\n> > before it. But it no longer is relevant after this many years.\n> > There may be other ways to break the reflog entries, but --stale-fix\n> > was never designed to deal with anything but a specific way the\n> > reflog gets corrupted (namely, by the old version of Git that\n> > corrupted reflog in a specific way), so keeping it would not be very\n> > useful.\n\nThank you for the original historical context.\n\n> FWIW, I have used --stale-fix for cases where a repo's reflog was\n> \"corrupted\" by its alternate pruning objects.\n>\n> E.g., if you do something like this:\n>\n> git clone -s orig.git new.git\n>\n> you're now at risk of \"git gc\" in orig.git corrupting new.git, because\n> its reachability check doesn't know anything about those refs. You can\n> mitigate that by keeping a copy of new.git's refs in orig.git. Something\n> like:\n>\n> git -C orig.git fetch ../new.git refs/*:refs/preserve/*\n> git -C orig.git gc\n>\n> But that doesn't know about reflogs at all! It will still corrupt them\n> (but only sometimes; depending how often you do that fetch, you might\n> catch the same values in orig.git's reflog).\n>\n> Possibly the correct answer here is \"turn off reflogs in new.git\", but\n> they are sometimes useful, and things _mostly_ work (for history that\n> doesn't rewind, or where the rewound bits are specific to new.git). So\n> it's useful to be able to run something like \"reflog expire --stale-fix\"\n> to clear out the occasional problem.\n>\n> (A careful reader will note that objects mentioned only in the index\n> have a similar problem; but again, those tend to be local to new.git,\n> and don't exist at all in a server context).\n\nI want to add the experience with that half year during which `git gc`\nwith worktrees was broken, as it would happily ignore the reflogs of the\nworktree-specific `HEAD`s, all except the one from the worktree from which\n`git gc` was run. That was some fun time, and `--stale-fix` was a life\nsaver.\n\nWith this _additional_ historical context, I would deem it wise to keep\n`--stale-fix` around.\n\nCiao,\nDscho\n"},{"id":"416802","messageId":"xmqqy2ftuli9.fsf@gitster.c.googlers.com","threadId":"55130","inReplyTo":"nycvar.QRO.7.76.6.2102121702120.29765@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] reflog expire --stale-fix: be generous about missing objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-12T19:35:58Z","receivedAt":"2021-02-12T19:36:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> Possibly the correct answer here is \"turn off reflogs in new.git\", but\n>> they are sometimes useful, and things _mostly_ work (for history that\n>> doesn't rewind, or where the rewound bits are specific to new.git). So\n>> it's useful to be able to run something like \"reflog expire --stale-fix\"\n>> to clear out the occasional problem.\n>>\n>> (A careful reader will note that objects mentioned only in the index\n>> have a similar problem; but again, those tend to be local to new.git,\n>> and don't exist at all in a server context).\n>\n> I want to add the experience with that half year during which `git gc`\n> with worktrees was broken, as it would happily ignore the reflogs of the\n> worktree-specific `HEAD`s, all except the one from the worktree from which\n> `git gc` was run. That was some fun time, and `--stale-fix` was a life\n> saver.\n\nThe option was invented for a specific case, but if its \"fix\"\napplies for breakages caused by more recent bugs and user induced\nactions, I would agree that that it gives us a good reason to keep\nit around.\n\nI have to wonder if the explanation in the documentation for the\noption needs to be made less specific to the originally motivating\ncase, though.\n\nThanks.\n"}]}