{"thread":{"id":"42099","subject":"'next'ed --allow-unrelated-histories could cause lots of grief","startedAt":"2016-04-21T16:10:43Z","lastAt":"2016-04-21T23:17:49Z","messageCount":8,"participants":["Yaroslav Halchenko","Junio C Hamano","Joey Hess"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"284042","messageId":"20160421161043.GK7907@onerussian.com","threadId":"42099","inReplyTo":null,"subject":"'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-04-21T16:10:43Z","receivedAt":"2016-04-21T16:10:43Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"Dear Git Gurus,\n\nI guess whenever it rains it indeed pours, so it is me whining again.\n\nI have decided to try 2.8.1.369.geae769a available from debian\nexperimental and through our (datalad) tests failing I became\naware of the \n\n https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18\n merge: refuse to create too cool a merge by default\n\nwhich is planned for the next release. I guess it is indeed a\nworthwhile accident-prevention measure BUT not sure if it is so\nimportant as to cause a change in behavior on which some projects using\ngit through the cmdline interface might have been relying upon for\nyears!\n\nGiven that git is quite 'self-healing', i.e. if someone has managed to\nmake a merge he didn't intend to, there is always a way back (e.g., as\nsimple as git reset --hard HEAD^), I am really not sure how valuable\nsuch change of default behavior would be? Could it may be made into a\nwarning instead? or reversed option \"--dont-allow-unrelated-histories\"?\n\nMoreover, it was explicitly stated that \"no configuration variable to\nenable this by default exists and will not be added\", which would cause\n3rd party scripts/code/projects relying on previous behavior to provide\nversion specific handling (either to add that\n--allow-unrelated-histories or not)... very cumbersome! If nothing else\nremains, could there at least be a config option which we could\nthen use regardless of the version of git we are using for such merges?\n\nP.S. Please maintain CC list\n\nThank you in advance for your consideration,\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834 Fax: +1 (603) 646-1419\nWWW: http://www.linkedin.com/in/yarik \n"},{"id":"284046","messageId":"xmqqbn52ud6r.fsf@gitster.mtv.corp.google.com","threadId":"42099","inReplyTo":"20160421161043.GK7907@onerussian.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-21T16:41:32Z","receivedAt":"2016-04-21T16:41:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yaroslav Halchenko writes:\n\n> I have decided to try 2.8.1.369.geae769a available from debian\n> experimental and through our (datalad) tests failing I became\n> aware of the \n>\n> https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18\n> merge: refuse to create too cool a merge by default\n>\n> which is planned for the next release.\n\nSee http://thread.gmane.org/gmane.linux.kernel.gpio/15365/focus=15401\nfor the backstory.\n\nAs this is to allow maintainers at higher levels of hierarchy not to\nhave to worry about stupid mistakes happen at maintainers at lower\nlevels, I'm afraid that turning this into an opt-in safety would\ndefeat the point of the change in a major way.\n\n> ... BUT not sure if it is so\n> important as to cause a change in behavior on which some projects using\n> git through the cmdline interface might have been relying upon for\n> years!\n\nIt is not very productive to make such an emotional statement\nwithout substantiating _why_ a merge that adds a new root, which was\ndeclared in the thread above as \"crap\" (in the context of the kernel\nproject), is necessary and is a good idea in \"some projects\". Maybe\nthere is a valid use case that those from the kernel land didn't\nthink about.\n\n> Given that git is quite 'self-healing', i.e. if someone has managed to\n> make a merge he didn't intend to, there is always a way back (e.g., as\n> simple as git reset --hard HEAD^),\n\nThat is only true if people notice. A mere warning would not be an\neffective prevention measure for a user who has to perform dozens of\nmerges a day.\n\nI am personally on the fence, but right now I am on the side of\nkeeping the behaviour as implemented and documented, simply because\nI haven't heard anything concrete to convince me why some people\nneed to regularly do a \"crap\" merge (in other words, in what context\nthese are not \"crap\" merges but ability to create them is a valuable\npart of everyday workflow).\n"},{"id":"284064","messageId":"20160421181917.GA3628@kitenet.net","threadId":"42099","inReplyTo":"20160421161043.GK7907@onerussian.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-04-21T18:19:17Z","receivedAt":"2016-04-21T18:19:17Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Yaroslav Halchenko wrote:\n> which is planned for the next release. I guess it is indeed a\n> worthwhile accident-prevention measure BUT not sure if it is so\n> important as to cause a change in behavior on which some projects using\n> git through the cmdline interface might have been relying upon for\n> years!\n\nNot only through the command line interface. The git-annex webapp has\ncommon use cases that will be broken by this change.\n\n> Moreover, it was explicitly stated that \"no configuration variable to\n> enable this by default exists and will not be added\", which would cause\n> 3rd party scripts/code/projects relying on previous behavior to provide\n> version specific handling (either to add that\n> --allow-unrelated-histories or not)... very cumbersome!\n\nAgreed, a configuration setting that could be passed via -c would be\nmuch less cumbersome than checking the version of git in order to only\npass the option to git versions that understand it. This would also\nprovide a way to get git pull to allow such merges.\n\nCompare with, for example, the change to default to an interactive\nmerge, where GIT_MERGE_AUTOEDIT=no was provided to ease compatability.\n\n-- \nsee shy jo\n"},{"id":"284065","messageId":"20160421185528.GJ23764@onerussian.com","threadId":"42099","inReplyTo":"xmqqbn52ud6r.fsf@gitster.mtv.corp.google.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-04-21T18:55:28Z","receivedAt":"2016-04-21T18:55:28Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"\nOn Thu, 21 Apr 2016, Junio C Hamano wrote:\n\n> Yaroslav Halchenko writes:\n\n> > I have decided to try 2.8.1.369.geae769a available from debian\n> > experimental and through our (datalad) tests failing I became\n> > aware of the \n\n> > https://github.com/git/git/pull/158/commits/e379fdf34fee96cd205be83ff4e71699bdc32b18\n> > merge: refuse to create too cool a merge by default\n\n> > which is planned for the next release.\n\n> See http://thread.gmane.org/gmane.linux.kernel.gpio/15365/focus=15401\n> for the backstory.\n\nTHANKS for the link!\n\n> As this is to allow maintainers at higher levels of hierarchy not to\n> have to worry about stupid mistakes happen at maintainers at lower\n> levels, I'm afraid that turning this into an opt-in safety would\n> defeat the point of the change in a major way.\n\n> > ... BUT not sure if it is so\n> > important as to cause a change in behavior on which some projects using\n> > git through the cmdline interface might have been relying upon for\n> > years!\n\n> It is not very productive to make such an emotional statement\n> without substantiating _why_ a merge that adds a new root, which was\n> declared in the thread above as \"crap\" (in the context of the kernel\n> project),\n\nSorry if my statement sounded too emotional ;) I will outline some of\nthe use-cases below.\n\nMeanwhile, I could argue that 'crap' in the above thread refers to the\nentirety of the branch and thus more to the originating detached\nroot commit. Action of creating of such a branch was also described as\nsomeone 'has done something TOTALLY INSANE'. And as \"maintainers at\nhigher levels\" expressed: \"You actually have to *work* at making shit\nlike this\".\n\nI do see now the reason for introduced change of behavior clearer BUT I\nwould still argue that it should better be supported somehow by at least\na configuration option to not make poor mortals like yours truly\nto sweat to stay compatible with multiple versions of git.\n\n> is necessary and is a good idea in \"some projects\". Maybe\n> there is a valid use case that those from the kernel land didn't\n> think about.\n\nFWIW\n\n- for git-annex (Joey was CCed from the beginning, not sure if annex\n would be affected though), it would be merging of git-annex\n branches while joining multiple annexes for the sync (e.g. by git\n annex assistant). \n\n- In our (datalad) case, repository is initialized with stock 'master'\n branch which carries configuration, which then used by the 'crawler'\n to initiate 'incoming' branch to contain (only) incoming data, which\n is later branched into 'incoming-processed' and later merged into\n 'master', which is where such problematic merge would happen. With\n such a workflow we can easily maintain custom changes within master,\n while automate changes to the crawled and processed data within\n those other branches being merged into master for final consumption.\n\n As a somewhat ugly workaround, we could probably artificially\n create an empty initial commit (forgot even how to do this magic) and\n branch-off it I guess, but I see other unit-tests failing as well, so\n I guess the situation is more chronic.\n\n- In Debian packaging world, people often use 'debian' overlay branch\n which has no common ancestor with 'upstream' sources, but which need\n to be merged for 'in-tree' work. I don't remember though if any of\n the tools rely on this feature (gbp iirc overlays not through the\n merge), but I know that I have used it quite a few times\n (interactively though, so yes -- could add a flag).\n\n> > Given that git is quite 'self-healing', i.e. if someone has managed to\n> > make a merge he didn't intend to, there is always a way back (e.g., as\n> > simple as git reset --hard HEAD^),\n\n> That is only true if people notice. A mere warning would not be an\n> effective prevention measure for a user who has to perform dozens of\n> merges a day.\n\nCould be argued... Generally git's warnings is not something to be\nignored IMHO. Do you ignore a balk of them on a daily basis? if so --\nthey might better be downgraded to some kind of 'info' level msg then.\n\n> I am personally on the fence, but right now I am on the side of\n> keeping the behaviour as implemented and documented, simply because\n> I haven't heard anything concrete to convince me why some people\n> need to regularly do a \"crap\" merge (in other words, in what context\n> these are not \"crap\" merges but ability to create them is a valuable\n> part of everyday workflow).\n\nonce again -- IMHO it wasn't a 'merge' which was a crap, but the state\nof the branch to be merged, and getting to that stage was non-trivial\nendeavor as well ;)\n\nSince the referenced situation happened only in 2016, I think that such\nmerges leading to undesired outcomes were quite a rare incident. On the\nother hand I do not have any statistic on how many of similar\nmerges were intensional, but I guess there could easily be thousands of\nsuch merges intended at least in the git-annex world alone. The point\nis that it would be change in behavior require custom version-dependent\nhandling.\n\nIf it is more of a policy to be enforced per project, then IMHO it would\nbe better if disallowing such merges was a configuration option which\ndevelopers of adhering to such a policy project would configure for\ntheir environments.\n\nCheers and sorry for a long post (again).\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834 Fax: +1 (603) 646-1419\nWWW: http://www.linkedin.com/in/yarik \n"},{"id":"284073","messageId":"xmqqinzasqgv.fsf@gitster.mtv.corp.google.com","threadId":"42099","inReplyTo":"20160421185528.GJ23764@onerussian.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-21T19:37:36Z","receivedAt":"2016-04-21T19:37:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yaroslav Halchenko writes:\n\n>> It is not very productive to make such an emotional statement\n>> without substantiating _why_ a merge that adds a new root, which was\n>> declared in the thread above as \"crap\" (in the context of the kernel\n>> project),\n>\n> Sorry if my statement sounded too emotional ;) I will outline some of\n> the use-cases below.\n\nThanks. Emotional is fine, as long as you _also_ have useful\ninformation.\n"},{"id":"284075","messageId":"20160421203606.GM23764@onerussian.com","threadId":"42099","inReplyTo":"xmqqinzasqgv.fsf@gitster.mtv.corp.google.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Yaroslav Halchenko","fromEmail":"yoh@onerussian.com","sentAt":"2016-04-21T20:36:06Z","receivedAt":"2016-04-21T20:36:06Z","isPatch":false,"sender":{"key":"yoh@onerussian.com","avatar":"https://gravatar.com/avatar/8901b82415ae451a83aea49409708912726e53620e3ac92320bf1f86548d97e9?d=mp&s=160"},"body":"\nOn Thu, 21 Apr 2016, Junio C Hamano wrote:\n> >> It is not very productive to make such an emotional statement\n> >> without substantiating _why_ a merge that adds a new root, which was\n> >> declared in the thread above as \"crap\" (in the context of the kernel\n> >> project),\n\n> > Sorry if my statement sounded too emotional ;) I will outline some of\n> > the use-cases below.\n\n> Thanks. Emotional is fine, as long as you _also_ have useful\n> information.\n\nGotcha: I will follow \"emotional + useful == fine\" advice closer\nnext time ;)\n\nThank you a lot for the suggested patch with the env variable\nworkaround!\n-- \nYaroslav O. Halchenko\nCenter for Open Neuroscience http://centerforopenneuroscience.org\nDartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755\nPhone: +1 (603) 646-9834 Fax: +1 (603) 646-1419\nWWW: http://www.linkedin.com/in/yarik \n"},{"id":"284085","messageId":"20160421225739.GA7508@kitenet.net","threadId":"42099","inReplyTo":"20160421185528.GJ23764@onerussian.com","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Joey Hess","fromEmail":"id@joeyh.name","sentAt":"2016-04-21T22:57:39Z","receivedAt":"2016-04-21T22:57:39Z","isPatch":false,"sender":{"key":"id@joeyh.name","avatar":"https://avatars.githubusercontent.com/u/16392?v=4"},"body":"Yaroslav Halchenko wrote:\n> - for git-annex (Joey was CCed from the beginning, not sure if annex\n> would be affected though), it would be merging of git-annex\n> branches while joining multiple annexes for the sync (e.g. by git\n> annex assistant). \n\nNot entirely accurate (git-annex merges its git-annex branches using a\ncustom merge method and not involving git-merge). The actual use case is\ntwo users (or one user with two devices) each with a git-annex\nrepository who decide to share their files by combining the two\nrepositories. This is pretty far from the kernel world, so it's not like\nbisection is something they care about.\n\nHowever, I also see --allow-unrelated-histories as very useful to\nprevent many foot-shooting maneuvers. Especially when a repository has\nspecial-purpose branches, like git-annex's git-annex branch, or other\nbranches that are never intended to be merged into master. It's a not\nentirely uncommon mistake for users to merge in such a branch, and the\nusers who make such a mistake often don't know enough git to easily\nrecover from it.\n\nJunio C Hamano wrote:\n> merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment\n\nI hope this patch lands, it will save me a lot of bother.\n\n-- \nsee shy jo\n"},{"id":"284086","messageId":"xmqqk2jqr1pe.fsf@gitster.mtv.corp.google.com","threadId":"42099","inReplyTo":"20160421225739.GA7508@kitenet.net","subject":"Re: 'next'ed --allow-unrelated-histories could cause lots of grief","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-21T23:17:49Z","receivedAt":"2016-04-21T23:17:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Joey Hess writes:\n\n> Junio C Hamano wrote:\n>> merge: GIT_MERGE_ALLOW_UNRELATED_HISTORIES environment\n>\n> I hope this patch lands, it will save me a lot of bother.\n\nYup, it is somewhat ugly but the least bad option among command line\noption (which would break with older versions of Git), configuration\nvariables (which would encourage users to opt out of safety\nunconditionallly) and environment (which allows scripts to opt-in),\nI would say.\n"}]}