{"thread":{"id":"55411","subject":"Bug report: git branch behaves as if --no-replace-objects is passed","startedAt":"2021-03-30T06:06:22Z","lastAt":"2021-03-31T00:33:21Z","messageCount":10,"participants":["Elijah Newren","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"420555","messageId":"CABPp-BEAbN05+hCtK=xhGg5uZFqbUvH9hMcCNMcBWp5JWLqzPw@mail.gmail.com","threadId":"55411","inReplyTo":null,"subject":"Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-30T06:05:05Z","receivedAt":"2021-03-30T06:06:22Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nForwarding a report from\nhttps://github.com/newren/git-filter-repo/issues/229, with a few more\ndetails...\n\nlog, diff, etc. seem to all support replace objects nicely:\n\n    git init -b main whatever\n    cd whatever/\n    >empty\n    git add empty\n    git commit -m initial\n    git replace -f deadbeefdeadbeefdeadbeefdeadbeefdeadbeef $(git\nrev-parse main)\n\n    echo stuff >>empty\n\nNow, If I try to use this replace object:\n\n    $ git diff --stat deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n     empty | 1 +\n     1 file changed, 1 insertion(+)\n    $ git log --oneline deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n    deadbee (replaced) initial\n\nit all works well.  BUT, if I try to use it with branch it doesn't work:\n\n    $ git branch --contains deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n    $\n\nand possibly worse, if I create a new branch based on it and use it:\n\n    $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n    $ git checkout foobar\n    $ echo stuff >empty\n    $ git add empty\n    $ git commit -m more\n\nthen it's clear that branch created foobar pointing to the replaced\nobject rather than the replacement object -- despite the fact that the\nreplaced object doesn't even exist within this repo:\n\n    $ git cat-file -p HEAD\n    tree 18108bae26dc91af2055bc66cc9fea278012dbd3\n    parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n    author Elijah Newren <newren@gmail.com> 1617083739 -0700\n    committer Elijah Newren <newren@gmail.com> 1617083739 -0700\n\n    more\n\nI poked around in the code a little but it is not at all clear to me\nwhy some parts of the code (log, diff) translate replace refs\ncorrectly, while others (branch) don't.  It is clear from the output\nthat log is aware that the refs are replaced, which makes me wonder if\nevery caller needs to be aware of replace refs for them to work\ncorrectly everywhere, because I couldn't find a missing environment\nsetup for \"branch\".\n"},{"id":"420557","messageId":"YGLNBFJv8NKmrbvz@coredump.intra.peff.net","threadId":"55411","inReplyTo":"CABPp-BEAbN05+hCtK=xhGg5uZFqbUvH9hMcCNMcBWp5JWLqzPw@mail.gmail.com","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-03-30T07:02:28Z","receivedAt":"2021-03-30T07:03:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 29, 2021 at 11:05:05PM -0700, Elijah Newren wrote:\n\n> it all works well.  BUT, if I try to use it with branch it doesn't work:\n> \n>     $ git branch --contains deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n>     $\n\nI'm not surprised here. The replace mechanism is usually \"if you are\ntrying to access object X, then access the contents of Y instead\". But I\ndon't think we generally rewrite references from other objects. So in\nthat sense, no ref \"contains\" deadbeef, because nobody points to it as\nan ancestor.\n\nI guess \"branch --contains\" could convert the mention of the replaced\nobject on the command-line, something like this:\n\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex 4542d4d3f9..200be4e3d2 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -6,6 +6,7 @@\n #include \"string-list.h\"\n #include \"strvec.h\"\n #include \"oid-array.h\"\n+#include \"replace-object.h\"\n \n /*----- some often used options -----*/\n \n@@ -84,6 +85,7 @@ int parse_opt_verbosity_cb(const struct option *opt, const char *arg,\n int parse_opt_commits(const struct option *opt, const char *arg, int unset)\n {\n \tstruct object_id oid;\n+\tconst struct object_id *replace;\n \tstruct commit *commit;\n \n \tBUG_ON_OPT_NEG(unset);\n@@ -92,7 +94,9 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)\n \t\treturn -1;\n \tif (get_oid(arg, &oid))\n \t\treturn error(\"malformed object name %s\", arg);\n-\tcommit = lookup_commit_reference(the_repository, &oid);\n+\n+\treplace = lookup_replace_object(the_repository, &oid);\n+\tcommit = lookup_commit_reference(the_repository, replace);\n \tif (!commit)\n \t\treturn error(\"no such commit %s\", arg);\n \tcommit_list_insert(commit, opt->value);\n\nthough if we go that route, I suspect we ought to be adding both the\noriginal _and_ the replacement.\n\nI'm not entirely sure this is a good direction, though.\n\n> and possibly worse, if I create a new branch based on it and use it:\n> \n>     $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n>     $ git checkout foobar\n>     $ echo stuff >empty\n>     $ git add empty\n>     $ git commit -m more\n> \n> then it's clear that branch created foobar pointing to the replaced\n> object rather than the replacement object -- despite the fact that the\n> replaced object doesn't even exist within this repo:\n> \n>     $ git cat-file -p HEAD\n>     tree 18108bae26dc91af2055bc66cc9fea278012dbd3\n>     parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n>     author Elijah Newren <newren@gmail.com> 1617083739 -0700\n>     committer Elijah Newren <newren@gmail.com> 1617083739 -0700\n> \n>     more\n\nYeah, that's pretty horrible. I do think you're using replace objects in\na way they weren't really intended for, in two ways:\n\n  - usually you'd actually have deadbeef, and you just want to rewrite\n    it to something else\n\n  - you wouldn't usually work directly with the replace object on the\n    command line like this. Usually the intent is to patch some old\n    section of history to get a different view.\n\nNone of which is an excuse exactly. But just to say that I'm not too\nsurprised that the \"replace\" mechanism is a bit half-baked, and there\nare probably a lot of weird implications nobody has thought through.\n\nPatches welcome, of course, though I suspect it may be a rabbit hole\nthat isn't worth your time. :)\n\n> I poked around in the code a little but it is not at all clear to me\n> why some parts of the code (log, diff) translate replace refs\n> correctly, while others (branch) don't.  It is clear from the output\n> that log is aware that the refs are replaced, which makes me wonder if\n> every caller needs to be aware of replace refs for them to work\n> correctly everywhere, because I couldn't find a missing environment\n> setup for \"branch\".\n\nRight. To work as you expect above, basically everything that is going\nto look at an oid would need to consider whether to use a replace ref.\nShoving the lookup into get_oid() would \"fix\" that, but I suspect would\nconfuse some other callers. Right now the rule is \"when you access the\nobject contents, pretend as if it had X instead of Y\", so we can enforce\nthat at the layer of accessing the object database.\n\n-Peff\n"},{"id":"420639","messageId":"xmqq35wceae6.fsf@gitster.g","threadId":"55411","inReplyTo":"YGLNBFJv8NKmrbvz@coredump.intra.peff.net","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T18:58:41Z","receivedAt":"2021-03-30T18:59:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... though if we go that route, I suspect we ought to be adding both the\n> original _and_ the replacement.\n\nSo \"branch --contains X\" would ask \"which of these branches reach X\nor its replacement?\" and \"branch --no-contains X\" would ask \"which\nof these do not reach X nor its replacement?\" --- I guess the result\nis still internally consistent (meaning: any and all branches fall\ninto either \"--contains X\" or \"--no-contains X\" camp).\n\n> I'm not entirely sure this is a good direction, though.\n>\n>> and possibly worse, if I create a new branch based on it and use it:\n>> \n>>     $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n>>     $ git checkout foobar\n>>     $ echo stuff >empty\n>>     $ git add empty\n>>     $ git commit -m more\n>> \n>> then it's clear that branch created foobar pointing to the replaced\n>> object rather than the replacement object -- despite the fact that the\n>> replaced object doesn't even exist within this repo:\n>> \n>>     $ git cat-file -p HEAD\n>>     tree 18108bae26dc91af2055bc66cc9fea278012dbd3\n>>     parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n>>     author Elijah Newren <newren@gmail.com> 1617083739 -0700\n>>     committer Elijah Newren <newren@gmail.com> 1617083739 -0700\n>> \n>>     more\n>\n> Yeah, that's pretty horrible.\n\nI am not sure.  As you analize below, the replace mechanism is about\ntelling Git: when anybody refers to deadbeef, use its replacement if\ndefined instead.\n\nAnd one of the points in the mechanism is to allow to do so even\nretroactively, so the HEAD object there may be referring to deadbeef\nthat may not exist does not matter, as long as the object that is to\nreplace deadbeef is available.  If not, that is a repository\ncorruption.  After all, the commit object you cat-file'ed may have\nbeen created by somebody else in a separate repository that had\ndeadbeef before they were told by Elijah that the object is obsolete\nand to be replaced by something else (Git supports distributed\ndevelopment) and then pulled into Elijah's repository, and we should\nbe prepared to seeing \"parent deadbeef\" in such a commit.  As long as\nreplacement happens when accessing the contents, that would be OK.\n\nSo, I do not see it as \"pretty horrible\", but I may be missing\nsomething.\n\n\n\n"},{"id":"420652","messageId":"CABPp-BEE0eksCJSAviDh5GyqsOu=i_mW3VY6SEULa4kx0NsUMg@mail.gmail.com","threadId":"55411","inReplyTo":"xmqq35wceae6.fsf@gitster.g","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-30T21:19:35Z","receivedAt":"2021-03-30T21:20:47Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 30, 2021 at 11:58 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Jeff King <peff@peff.net> writes:\n>\n> > ... though if we go that route, I suspect we ought to be adding both the\n> > original _and_ the replacement.\n>\n> So \"branch --contains X\" would ask \"which of these branches reach X\n> or its replacement?\" and \"branch --no-contains X\" would ask \"which\n> of these do not reach X nor its replacement?\" --- I guess the result\n> is still internally consistent (meaning: any and all branches fall\n> into either \"--contains X\" or \"--no-contains X\" camp).\n\nI'm not so sure about this interpretation.  Based on the documentation\nin git-replace(1):\n\n       Replacement references will be used by default by all Git commands\n       except those doing reachability traversal (prune, pack transfer and\n       fsck).\n\nI would have thought that\n\n* \"branch --contains X\" would ask \"which of these branches reach X's\nreplacement?\"\n* \"git --no-replace-objects branch --contains X\" would ask \"which of\nthese branches reach X?\"\n\nand if folks really wanted to check whether either X or its\nreplacement were reachable then they'd need to run both commands.\n\nThe only place outside of reachability traversal where I think it\nmakes sense for a command to distinguish between X being a replace ref\nfor Y and Y itself is in `git log` where it can show the \"replaced\"\nmoniker.\n\n> > I'm not entirely sure this is a good direction, though.\n> >\n> >> and possibly worse, if I create a new branch based on it and use it:\n> >>\n> >>     $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n> >>     $ git checkout foobar\n> >>     $ echo stuff >empty\n> >>     $ git add empty\n> >>     $ git commit -m more\n> >>\n> >> then it's clear that branch created foobar pointing to the replaced\n> >> object rather than the replacement object -- despite the fact that the\n> >> replaced object doesn't even exist within this repo:\n> >>\n> >>     $ git cat-file -p HEAD\n> >>     tree 18108bae26dc91af2055bc66cc9fea278012dbd3\n> >>     parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n> >>     author Elijah Newren <newren@gmail.com> 1617083739 -0700\n> >>     committer Elijah Newren <newren@gmail.com> 1617083739 -0700\n> >>\n> >>     more\n> >\n> > Yeah, that's pretty horrible.\n>\n> I am not sure.  As you analize below, the replace mechanism is about\n> telling Git: when anybody refers to deadbeef, use its replacement if\n> defined instead.\n>\n> And one of the points in the mechanism is to allow to do so even\n> retroactively, so the HEAD object there may be referring to deadbeef\n> that may not exist does not matter, as long as the object that is to\n> replace deadbeef is available.  If not, that is a repository\n> corruption.  After all, the commit object you cat-file'ed may have\n> been created by somebody else in a separate repository that had\n> deadbeef before they were told by Elijah that the object is obsolete\n> and to be replaced by something else (Git supports distributed\n> development) and then pulled into Elijah's repository, and we should\n> be prepared to seeing \"parent deadbeef\" in such a commit.  As long as\n> replacement happens when accessing the contents, that would be OK.\n>\n> So, I do not see it as \"pretty horrible\", but I may be missing\n> something.\n\nI think you're focusing on git commit, or perhaps on git checkout.\nI'm focusing on git branch; what it did does not seem fine to me.\nUsing your own words:\n\n\"the replace mechanism is about telling Git: when anybody refers to\ndeadbeef, use its replacement if defined instead.\"\n\ngit branch didn't do that; it put deadbeef into refs/heads/foobar.\n"},{"id":"420655","messageId":"CABPp-BFewHGOx-RCVtDKhn3=0QC9YWdA--Wtbb_MTHQbu3FQcw@mail.gmail.com","threadId":"55411","inReplyTo":"CABPp-BEE0eksCJSAviDh5GyqsOu=i_mW3VY6SEULa4kx0NsUMg@mail.gmail.com","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-30T21:30:56Z","receivedAt":"2021-03-30T21:32:13Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 30, 2021 at 2:19 PM Elijah Newren <newren@gmail.com> wrote:\n>\n> On Tue, Mar 30, 2021 at 11:58 AM Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > Jeff King <peff@peff.net> writes:\n> >\n> > > ... though if we go that route, I suspect we ought to be adding both the\n> > > original _and_ the replacement.\n> >\n> > So \"branch --contains X\" would ask \"which of these branches reach X\n> > or its replacement?\" and \"branch --no-contains X\" would ask \"which\n> > of these do not reach X nor its replacement?\" --- I guess the result\n> > is still internally consistent (meaning: any and all branches fall\n> > into either \"--contains X\" or \"--no-contains X\" camp).\n>\n> I'm not so sure about this interpretation.  Based on the documentation\n> in git-replace(1):\n>\n>        Replacement references will be used by default by all Git commands\n>        except those doing reachability traversal (prune, pack transfer and\n>        fsck).\n>\n> I would have thought that\n>\n> * \"branch --contains X\" would ask \"which of these branches reach X's\n> replacement?\"\n> * \"git --no-replace-objects branch --contains X\" would ask \"which of\n> these branches reach X?\"\n>\n> and if folks really wanted to check whether either X or its\n> replacement were reachable then they'd need to run both commands.\n>\n> The only place outside of reachability traversal where I think it\n> makes sense for a command to distinguish between X being a replace ref\n> for Y and Y itself is in `git log` where it can show the \"replaced\"\n> moniker.\n>\n> > > I'm not entirely sure this is a good direction, though.\n> > >\n> > >> and possibly worse, if I create a new branch based on it and use it:\n> > >>\n> > >>     $ git branch foobar deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n> > >>     $ git checkout foobar\n> > >>     $ echo stuff >empty\n> > >>     $ git add empty\n> > >>     $ git commit -m more\n> > >>\n> > >> then it's clear that branch created foobar pointing to the replaced\n> > >> object rather than the replacement object -- despite the fact that the\n> > >> replaced object doesn't even exist within this repo:\n> > >>\n> > >>     $ git cat-file -p HEAD\n> > >>     tree 18108bae26dc91af2055bc66cc9fea278012dbd3\n> > >>     parent deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n> > >>     author Elijah Newren <newren@gmail.com> 1617083739 -0700\n> > >>     committer Elijah Newren <newren@gmail.com> 1617083739 -0700\n> > >>\n> > >>     more\n> > >\n> > > Yeah, that's pretty horrible.\n> >\n> > I am not sure.  As you analize below, the replace mechanism is about\n> > telling Git: when anybody refers to deadbeef, use its replacement if\n> > defined instead.\n> >\n> > And one of the points in the mechanism is to allow to do so even\n> > retroactively, so the HEAD object there may be referring to deadbeef\n> > that may not exist does not matter, as long as the object that is to\n> > replace deadbeef is available.  If not, that is a repository\n> > corruption.  After all, the commit object you cat-file'ed may have\n> > been created by somebody else in a separate repository that had\n> > deadbeef before they were told by Elijah that the object is obsolete\n> > and to be replaced by something else (Git supports distributed\n> > development) and then pulled into Elijah's repository, and we should\n> > be prepared to seeing \"parent deadbeef\" in such a commit.  As long as\n> > replacement happens when accessing the contents, that would be OK.\n> >\n> > So, I do not see it as \"pretty horrible\", but I may be missing\n> > something.\n>\n> I think you're focusing on git commit, or perhaps on git checkout.\n> I'm focusing on git branch; what it did does not seem fine to me.\n> Using your own words:\n>\n> \"the replace mechanism is about telling Git: when anybody refers to\n> deadbeef, use its replacement if defined instead.\"\n>\n> git branch didn't do that; it put deadbeef into refs/heads/foobar.\n\nPerhaps I should also add why it not only breaks expectations, but why\nthat broken expectation causes problems:\n\n* People tend to have commit hashes stored in lots of weird placed --\nbug trackers, reports, emails, etc.  These tend to be important for a\nshort time period, but the number of these references make it harder\nfor folks who want to rewrite history to fix various past issues (very\nlarge binary blobs and other misdeeds).\n\n* filter-repo uses replace refs to provide users with a way to access\nnew commits using old commit hashes, to help them through this\ntransition period.\n\n* Additional refs (especially one for every commit) will cause some\nslowness.  So it's nice to be able to provide these replace refs for\nshort term transition, but tell users they can simply delete the\nreplace refs when they no longer need them without consequence.\n\n\nThe fact that git branch puts deadbeef into refs/heads/foobar, leads\nto a chain where new commits now rely on replacement refs.  In the\nbest case, others will not be able to pull from this user and the user\nwill not be able to push the new commits anywhere -- and that user\nwill have some work to do to rewrite (rebase?) the commits\nappropriately.  In the worst case, the users do succeed in\ndistributing this new history, and now all users everywhere will be\nmandated to keep all replace refs for all time (or at least until the\nnext major repository rewrite)...\n"},{"id":"420658","messageId":"xmqqa6qkcnqa.fsf@gitster.g","threadId":"55411","inReplyTo":"CABPp-BEE0eksCJSAviDh5GyqsOu=i_mW3VY6SEULa4kx0NsUMg@mail.gmail.com","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T21:53:33Z","receivedAt":"2021-03-30T21:54:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> Using your own words:\n>\n> \"the replace mechanism is about telling Git: when anybody refers to\n> deadbeef, use its replacement if defined instead.\"\n\n\"When anybody wants the contents of deadbeef, give the contents of\nits replacement\" is what the replace (graft, too) is.\n\n> git branch didn't do that; it put deadbeef into refs/heads/foobar.\n\nYes, but refs/heads/foobar having deadbeef does not have a problem,\nas long as you get the contents of the replacement object when you\nask for the contents of deadbeef (or \"the object referred to by\nthe refs/heads/foobar ref\").\n\n"},{"id":"420659","messageId":"xmqq5z18cngd.fsf@gitster.g","threadId":"55411","inReplyTo":"CABPp-BFewHGOx-RCVtDKhn3=0QC9YWdA--Wtbb_MTHQbu3FQcw@mail.gmail.com","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T21:59:30Z","receivedAt":"2021-03-30T22:00:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> On Tue, Mar 30, 2021 at 2:19 PM Elijah Newren <newren@gmail.com> wrote:\n>>\n>> On Tue, Mar 30, 2021 at 11:58 AM Junio C Hamano <gitster@pobox.com> wrote:\n>> >\n>> > Jeff King <peff@peff.net> writes:\n>> >\n>> > > ... though if we go that route, I suspect we ought to be adding both the\n>> > > original _and_ the replacement.\n>> >\n>> > So \"branch --contains X\" would ask \"which of these branches reach X\n>> > or its replacement?\" and \"branch --no-contains X\" would ask \"which\n>> > of these do not reach X nor its replacement?\" --- I guess the result\n>> > is still internally consistent (meaning: any and all branches fall\n>> > into either \"--contains X\" or \"--no-contains X\" camp).\n>>\n>> I'm not so sure about this interpretation.  Based on the documentation\n>> in git-replace(1):\n>>\n>>        Replacement references will be used by default by all Git commands\n>>        except those doing reachability traversal (prune, pack transfer and\n>>        fsck).\n\nThis \"rechability\" sidenote is primarily so that we won't result in\na corrupt repository when replacement is lifted.  When object A is\nreplaced by object B, and somebody makes A reachable (e.g. a ref\npoints at deadbeef), we mark both A (and the objects A refers to)\nand B reachable, so that \"prune\" won't lose A.  It would allow the\nreplacement lifted after \"prune\".\n\nTweaking \"branch --contains X\" to list branches that reach either X\nor its replacement would probably have the same effect, so I would\nthink it would be a good change (and fix to your original issue).\n\nThe \"the result is still internally consistent\" comment was the\nresult of my attempt to make sure such a change would not introduce\ngross incoherency to the resulting system.\n\n"},{"id":"420662","messageId":"CABPp-BEVFi+EsGMCv=qEhe_OGwmWwXF-mg9rjWYNuF0tDZnASg@mail.gmail.com","threadId":"55411","inReplyTo":"xmqqa6qkcnqa.fsf@gitster.g","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-30T22:43:32Z","receivedAt":"2021-03-30T22:44:40Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 30, 2021 at 2:53 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> writes:\n>\n> > Using your own words:\n> >\n> > \"the replace mechanism is about telling Git: when anybody refers to\n> > deadbeef, use its replacement if defined instead.\"\n>\n> \"When anybody wants the contents of deadbeef, give the contents of\n> its replacement\" is what the replace (graft, too) is.\n>\n> > git branch didn't do that; it put deadbeef into refs/heads/foobar.\n>\n> Yes, but refs/heads/foobar having deadbeef does not have a problem,\n> as long as you get the contents of the replacement object when you\n> ask for the contents of deadbeef (or \"the object referred to by\n> the refs/heads/foobar ref\").\n\nYour \"as long as\" is I think the assumption that's violated in the\nworkflow in question.  You may have the replace ref defined, but\nothers don't[1].  Neither party has the actual original deadbeef\ncommit[2].  Having deadbeef in refs/heads/foobar leads eventually to\ncreating commits with deadbeef as an explicit parent, as we discussed\nabove.  While that's internally consistent, as you point out, can you\npush your new commit elsewhere without pushing the replace refs too?\n\n* If you can, isn't that repository corruption?\n\n* If you can't, how are users supposed to understand, debug, and\ncorrect the fact that their history is unshareable?\n\nWhy does `git branch` (in conjunction with one user deciding to fetch\nreplace refs) make it so easy to create a branch that cannot readily\nbe shared with others?\n\n\n[1] Everyone can get the replace refs, but fetch/clone does not grab\nthem by default, and push does not include them by default, so it's\nmultiple manual steps.\n[2] For the rewrite at $DAYJOB, we'll make it as hard as possible for\nusers to access the original deadbeef commit(s).  People who cloned\nthe old unrewritten repo in the past might still have it somewhere if\nthey didn't follow instructions to delete old clones and reclone, but\nnew hires won't have deadbeef and won't have a way to get it.\n"},{"id":"420664","messageId":"xmqqlfa4b5we.fsf@gitster.g","threadId":"55411","inReplyTo":"CABPp-BEVFi+EsGMCv=qEhe_OGwmWwXF-mg9rjWYNuF0tDZnASg@mail.gmail.com","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-30T23:04:01Z","receivedAt":"2021-03-30T23:04:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> Your \"as long as\" is I think the assumption that's violated in the\n> workflow in question.  You may have the replace ref defined, but\n> others don't[1].  Neither party has the actual original deadbeef\n> commit[2].  Having deadbeef in refs/heads/foobar leads eventually to\n> creating commits with deadbeef as an explicit parent, as we discussed\n> above.  While that's internally consistent, as you point out, can you\n> push your new commit elsewhere without pushing the replace refs too?\n\nI think the change to \"branch --contains\" would be an improvement\nwhether you actually have deadbeef or not, but in any case, defining\n(eh, rather, being able to define) a replacement for something you\ndo not have is the ultimate source of the problem.  And that \"bug\"\nhas not very much specific to how \"branch --contains\" should behave.\n\n> Why does `git branch` (in conjunction with one user deciding to fetch\n> replace refs) make it so easy to create a branch that cannot readily\n> be shared with others?\n\nIn other words, I do not think it is \"git branch\" or \"git checkout -b\"\nthat brought your repository into a broken state.  The \"replace\"\nmechanism may have room for improvement to avoid such a corruption.\n\nIIRC, the original \"graft\" mechanism did not even have any UI, so it\nwas pretty much \"you can graft any parent to any child, and if you\nbreak the repository you can keep both halves\".  Now \"replace\" has a\ndedicated UI component in the form of \"git replace\" command, we should\nbe able to teach it how to record replacement more safely.\n\n"},{"id":"420679","messageId":"CABPp-BG+xKr10BpziijMB4j9+F=PCozAFQRdJ1DVBuGGjir40Q@mail.gmail.com","threadId":"55411","inReplyTo":"xmqqlfa4b5we.fsf@gitster.g","subject":"Re: Bug report: git branch behaves as if --no-replace-objects is passed","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-03-31T00:32:18Z","receivedAt":"2021-03-31T00:33:21Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Tue, Mar 30, 2021 at 4:04 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Elijah Newren <newren@gmail.com> writes:\n>\n> > Your \"as long as\" is I think the assumption that's violated in the\n> > workflow in question.  You may have the replace ref defined, but\n> > others don't[1].  Neither party has the actual original deadbeef\n> > commit[2].  Having deadbeef in refs/heads/foobar leads eventually to\n> > creating commits with deadbeef as an explicit parent, as we discussed\n> > above.  While that's internally consistent, as you point out, can you\n> > push your new commit elsewhere without pushing the replace refs too?\n>\n> I think the change to \"branch --contains\" would be an improvement\n> whether you actually have deadbeef or not, but in any case, defining\n\nGreat, we agree that branch --contains can be improved.\n\n> (eh, rather, being able to define) a replacement for something you\n> do not have is the ultimate source of the problem.  And that \"bug\"\n> has not very much specific to how \"branch --contains\" should behave.\n\nOkay, perhaps you consider the ability to create a replacement for an\nobject that doesn't exist as a bug.  How do we handle this bug,\nthough?  That's not at all clear to me.  Is it documentation updates?\nMore error messages from various commands?  Code changes to handle\nthese cases better?  Something else?  This behavior has been allowed\nfor over a decade, the refs can be created outside of \"git replace\"\nand the replacement mechanism comes with\n* documentation claiming there are only two restrictions on\nreplacement or replaced refs, both bypassable[1]\n* documentation claims of robustness for the replace mechanism[2]\n* documentation claiming that all non-reachability traversal related\ncommands will translate replacement refs to the real commit IDs[3]\n* user-facing UI to support creating/updating/deleting replace refs\n\nBased on the above, filter-repo has been creating replacement refs for\nyears now, one for every commit in the repository that wasn't filtered\nout.  And I thought it all worked great without any problems, until\nthe recent report.  I guess I only ever used the old commit IDs to\npass to things like diff, log, etc., and I guess most users either\ndidn't use the replace refs at all or only used them that way\ntoo...until this week. However, I'm planning to do a very big testcase\nof history rewriting (ancient and huge binary blobs) at $DAYJOB later\nthis year including replace refs for some of the release-team folks,\nso I'm kind of concerned about what I need to fix in git or at least\nwhat I need to document.\n\n[1] See the part up to \"...There is no other restriction on the\nreplaced and replacement objects.\", from git-replace(1)\n[2] \"Note that the grafts mechanism is outdated and can lead to\nproblems...see git-replace(1) for a more flexible and robust system\",\nfrom gitrepository-layout(1)\n[3] \"Replacement references will be used by default by all Git\ncommands except those doing reachability traversal\", from\ngit-replace(1)\n\n> > Why does `git branch` (in conjunction with one user deciding to fetch\n> > replace refs) make it so easy to create a branch that cannot readily\n> > be shared with others?\n>\n> In other words, I do not think it is \"git branch\" or \"git checkout -b\"\n> that brought your repository into a broken state.  The \"replace\"\n> mechanism may have room for improvement to avoid such a corruption.\n>\n> IIRC, the original \"graft\" mechanism did not even have any UI, so it\n> was pretty much \"you can graft any parent to any child, and if you\n> break the repository you can keep both halves\".  Now \"replace\" has a\n> dedicated UI component in the form of \"git replace\" command, we should\n> be able to teach it how to record replacement more safely.\n\nI would like to avoid corruption, and I'm happy to have alternate\nsolutions.  I don't understand how this could be fixed in \"git\nreplace\", though, especially since \"git replace\" was not even invoked\nin the real use case under consideration (git filter-repo uses \"git\nupdate-ref --stdin\" to create the replace objects).\n\nAre there other alternative ways to fix this outside of \"git\nbranch\"/\"git checkout -b\"?\n\nIf not, are there technical reasons that \"git branch <branchname>\n<replace-hash>\" and other commands like it could not be adjusted to\nwrite the replacement hash rather than the replaced hash to the new\nbranch?\n\nEven if there are no alternative fixes and there are technical reasons\nthat \"git branch\" and other commands like it cannot be adjusted, I\nstill feel that translate-old-commit-hashes via\nreplacement-of-non-existent-commits feature is valuable enough to keep\nin filter-repo anyway.  It'd make me a little uneasy, but I'd continue\ncreating the replace refs and just document the drawbacks in that\nscenario.\n"}]}