{"thread":{"id":"34605","subject":"git replace: should it check for object type, and can it replace merges?","startedAt":"2013-08-03T15:13:46Z","lastAt":"2013-08-05T16:41:52Z","messageCount":5,"participants":["Philip Oakley","Thomas Rast","Christian Couder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"224524","messageId":"24928554D0C34B4696DAD74F569E2E17@PhilipOakley","threadId":"34605","inReplyTo":null,"subject":"git replace: should it check for object type, and can it replace merges?","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-08-03T15:13:46Z","receivedAt":"2013-08-03T15:13:46Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"A recent comment http://stackoverflow.com/a/18027030/717355 on a \nquestion I asked two years ago about 'grafts' and 'replace' indicates \nthat users think that 'git replace' can't replace a merge commit. The \ndocumentation doesn't have any examples and gives the naive impression \nthat one should only replace a simple commit with another simple commit.\n\nHaving looked at the code, I realised that anything can be replaced with \nanything, which is perhaps not what was intended. A simple thought is \nthat the replace should be like-for-like with regard to object type, \nthough that would not include replacing a sub-module for a tree (and \nvice versa).\n\nShould 'git replace' check the object types to ensure they are sensible?\n\nWould it be reasonable to add examples to indicate the range of \nreplacements, and how to prepare alternative merge commits, or is that a \nhostage to fortune?\n\n\nPhilip Oakley\n"},{"id":"224525","messageId":"87zjsyydya.fsf@hexa.v.cablecom.net","threadId":"34605","inReplyTo":"24928554D0C34B4696DAD74F569E2E17@PhilipOakley","subject":"Re: git replace: should it check for object type, and can it replace merges?","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2013-08-03T15:46:37Z","receivedAt":"2013-08-03T15:46:37Z","isPatch":false,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"\"Philip Oakley\" writes:\n\n> A recent comment http://stackoverflow.com/a/18027030/717355 on a\n> question I asked two years ago about 'grafts' and 'replace' indicates\n> that users think that 'git replace' can't replace a merge commit. The\n> documentation doesn't have any examples and gives the naive impression\n> that one should only replace a simple commit with another simple\n> commit.\n>\n> Having looked at the code, I realised that anything can be replaced\n> with anything, which is perhaps not what was intended. A simple\n> thought is that the replace should be like-for-like with regard to\n> object type, though that would not include replacing a sub-module for\n> a tree (and vice versa).\n\nOff hand I cannot come up with any case where you can replace one object\nwith one of a different type, keeping the history valid.\n\nTo back that up:\n\n* Refs can be \"replaced\" simply by changing the ref itself.\n\n* Annotated tags contain the type of the tagged object.\n\n* The tree/parent lines in commits must be a tree and commits, resp.\n\n* The object types referred to by trees are specified in the 'mode'\n field:\n 100644 and 100755 blob\n 160000 commit\n 040000 tree\n (these are the only valid modes)\n\n* Blobs don't point at anything.\n\nSo yes:\n\n> Should 'git replace' check the object types to ensure they are sensible?\n\nI think that would be a very good thing to check.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"224569","messageId":"CAP8UFD3yHMKfBdjHc6CxeAXSdQ=ua1MddAb5rRXV3TU0ZW7g6w@mail.gmail.com","threadId":"34605","inReplyTo":"24928554D0C34B4696DAD74F569E2E17@PhilipOakley","subject":"Re: git replace: should it check for object type, and can it replace merges?","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2013-08-05T10:57:05Z","receivedAt":"2013-08-05T10:57:05Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi,\n\nOn Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley wrote:\n> A recent comment http://stackoverflow.com/a/18027030/717355 on a question I\n> asked two years ago about 'grafts' and 'replace' indicates that users think\n> that 'git replace' can't replace a merge commit. The documentation doesn't\n> have any examples and gives the naive impression that one should only\n> replace a simple commit with another simple commit.\n\nI am sorry if the documentation gives this impression.\nI'd like to fix it, but I am not sure what should be changed.\nShould adding an example be enough? Or do you want it to state that explicitely?\n\n> Having looked at the code, I realised that anything can be replaced with\n> anything, which is perhaps not what was intended.\n\nThe documentation says in the \"BUGS\" section:\n\n\"And of course things may break if an object of one type is replaced\nby an object of another type (for example a blob replaced by a\ncommit).\"\n\nSo yes it is a know bug.\n\n> A simple thought is that\n> the replace should be like-for-like with regard to object type, though that\n> would not include replacing a sub-module for a tree (and vice versa).\n>\n> Should 'git replace' check the object types to ensure they are sensible?\n\nIt would probably be a good idea to do that, yeah.\nBut I don't know much about submodules, so I can't say if replacing a\nsub-module for a tree (and vice versa) should be allowed.\nOr if there should be a --force-different-objects option for these\nkinds of special cases.\n\n> Would it be reasonable to add examples to indicate the range of\n> replacements, and how to prepare alternative merge commits, or is that a\n> hostage to fortune?\n\nYeah, adding examples would be a good idea. I don't understand what do\nyou mean with \"range of replacements\", though.\n\nI am not sure preparing alternative commits or merge commits should be\nan important part of the examples.\n\nThere are many cases that could be interesting to different users:\n\n- replacing a non merge commit with a merge commit (if someone forgot\nto use --no-ff when merging for example)\n- replacing a merge commit with a non merge commit (if a rebase should\nhave been done)\n- and of course replacing a non merge commit with a non merge commit,\nor a merge commit with a merge commit\n\nSo I think explaining how another commit can be created from existing\ncommits belongs to some other parts of the git documentation.\nPerhaps there could be such examples in the git hash-object and git\nfilter-branch documentation and we could just point to them.\n\nBest,\nChristian.\n"},{"id":"224594","messageId":"2A84813A5DBF494391F94A840C228278@PhilipOakley","threadId":"34605","inReplyTo":"CAP8UFD3yHMKfBdjHc6CxeAXSdQ=ua1MddAb5rRXV3TU0ZW7g6w@mail.gmail.com","subject":"Re: git replace: should it check for object type, and can it replace merges?","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2013-08-05T16:07:37Z","receivedAt":"2013-08-05T16:07:37Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Christian Couder\" \n> Hi,\n> On Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley \n> wrote:\n>> A recent comment http://stackoverflow.com/a/18027030/717355 on a \n>> question I\n>> asked two years ago about 'grafts' and 'replace' indicates that users \n>> think\n>> that 'git replace' can't replace a merge commit. The documentation \n>> doesn't\n>> have any examples and gives the naive impression that one should only\n>> replace a simple commit with another simple commit.\n>\n> I am sorry if the documentation gives this impression.\n> I'd like to fix it, but I am not sure what should be changed.\n> Should adding an example be enough? Or do you want it to state that \n> explicitely?\n>\nI did a quick markup which is shown below (an export of the commit from \nthe gitk viewer)\n\n>> Having looked at the code, I realised that anything can be replaced \n>> with\n>> anything, which is perhaps not what was intended.\n>\n> The documentation says in the \"BUGS\" section:\n>\n> \"And of course things may break if an object of one type is replaced\n> by an object of another type (for example a blob replaced by a\n> commit).\"\n\nI hadn't seen that part, being 'hidden' at the end of the paragraph \n(that is, it's easy to miss;-). If my update was acceptable then that \nsentence could probably be deleted (though it may require the checks to \nactually be in the code, and not just a \"must\" imperative in my \ndocumentation update).\n\n>\n> So yes it is a know bug.\n>\n>> A simple thought is that\n>> the replace should be like-for-like with regard to object type, \n>> though that\n>> would not include replacing a sub-module for a tree (and vice versa).\n>>\n>> Should 'git replace' check the object types to ensure they are \n>> sensible?\n>\n> It would probably be a good idea to do that, yeah.\n> But I don't know much about submodules, so I can't say if replacing a\n> sub-module for a tree (and vice versa) should be allowed.\n> Or if there should be a --force-different-objects option for these\n> kinds of special cases.\n\nAn extra bit of thought made me realise that while a sub-module is \nrepresented as a special symbolic commit, it is still just an element of \na tree object, so would still be a tree <-> tree replacement, so doesn't \nbreak the rule.\n\n>\n>> Would it be reasonable to add examples to indicate the range of\n>> replacements, and how to prepare alternative merge commits, or is \n>> that a\n>> hostage to fortune?\n>\n> Yeah, adding examples would be a good idea. I don't understand what do\n> you mean with \"range of replacements\", though.\n\nThere were in two parts: 1) creating a replacement merge commit, and 2) \ncreating a replacement tree, possibly with a sub-module in it.\n\n>\n> I am not sure preparing alternative commits or merge commits should be\n> an important part of the examples.\n>\n> There are many cases that could be interesting to different users:\n>\n> - replacing a non merge commit with a merge commit (if someone forgot\n> to use --no-ff when merging for example)\n> - replacing a merge commit with a non merge commit (if a rebase should\n> have been done)\n> - and of course replacing a non merge commit with a non merge commit,\n> or a merge commit with a merge commit\n>\n> So I think explaining how another commit can be created from existing\n> commits belongs to some other parts of the git documentation.\n\nYes, I just haven't looked yet. I think there are some examples in the \nplumbing command man pages. They'd just need referencing.\n\n> Perhaps there could be such examples in the git hash-object and git\n> filter-branch documentation and we could just point to them.\n>\n> Best,\n> Christian.\n> --\n\nMy quick markup, based on a local branch.\n---8<---\ncommit c12c03462f8c65a593e702896b461f1c63d67ec5\nAuthor: Philip Oakley \nDate: Sat Aug 3 20:20:05 2013 +0100\n\n Doc: 'replace' the same object type, and mention merge commits\n\n Signed-off-by: Philip Oakley \n\ndiff --git a/Documentation/git-replace.txt \nb/Documentation/git-replace.txt\nindex e0b4057..2ab451c 100644\n--- a/Documentation/git-replace.txt\n+++ b/Documentation/git-replace.txt\n@@ -20,6 +20,10 @@ The name of the 'replace' reference is the SHA-1 of \nthe object that is\n replaced. The content of the 'replace' reference is the SHA-1 of the\n replacement object.\n\n+The type of the replacement object must be the same as that of the\n+object it replaces. Merge commits can be replaced by non-merge commits\n+and vice versa.\n+\n Unless `-f` is given, the 'replace' reference must not yet exist.\n\n Replacement references will be used by default by all Git commands\n--->8---\n\nPhilip \n"},{"id":"224602","messageId":"7v1u68m6nj.fsf@alter.siamese.dyndns.org","threadId":"34605","inReplyTo":"CAP8UFD3yHMKfBdjHc6CxeAXSdQ=ua1MddAb5rRXV3TU0ZW7g6w@mail.gmail.com","subject":"Re: git replace: should it check for object type, and can it replace merges?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-05T16:41:52Z","receivedAt":"2013-08-05T16:41:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Hi,\n>\n> On Sat, Aug 3, 2013 at 5:13 PM, Philip Oakley wrote:\n>> A recent comment http://stackoverflow.com/a/18027030/717355 on a question I\n>> asked two years ago about 'grafts' and 'replace' indicates that users think\n>> that 'git replace' can't replace a merge commit. The documentation doesn't\n>> have any examples and gives the naive impression that one should only\n>> replace a simple commit with another simple commit.\n>\n> I am sorry if the documentation gives this impression.\n> I'd like to fix it, but I am not sure what should be changed.\n> Should adding an example be enough? Or do you want it to state that explicitely?\n>\n>> Having looked at the code, I realised that anything can be replaced with\n>> anything, which is perhaps not what was intended.\n>\n> The documentation says in the \"BUGS\" section:\n>\n> \"And of course things may break if an object of one type is replaced\n> by an object of another type (for example a blob replaced by a\n> commit).\"\n>\n> So yes it is a know bug.\n\nIs that even a BUG? The users are pretty much asking for it if they\nplace an object name of a random wrong object themselves.\n\nI agree that a hand-holding wrapper \"git replace\" could help them to\navoid mistakes, though.\n"}]}