{"thread":{"id":"20925","subject":"merge ignores --no-commit in fast-forward case","startedAt":"2009-09-13T06:40:05Z","lastAt":"2009-09-20T14:04:40Z","messageCount":6,"participants":["Tomas Carnecky","Junio C Hamano","Greg Price","Björn Steinbrink","Paolo Bonzini"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"123036","messageId":"A754ACF1-77C2-4036-A15C-8949E76BD2D5@dbservice.com","threadId":"20925","inReplyTo":null,"subject":"merge ignores --no-commit in fast-forward case","fromName":"Tomas Carnecky","fromEmail":"tom@dbservice.com","sentAt":"2009-09-13T06:40:05Z","receivedAt":"2009-09-13T06:40:05Z","isPatch":false,"sender":{"key":"tom@dbservice.com","avatar":"https://gravatar.com/avatar/900a300bdd1a8bbe086008ad78210bbee2ad2803b7d50a5cba04c1e9404bd6d2?d=mp&s=160"},"body":"Bug or feature?\n\nThree possible solutions that I see:\n\n 2) Refuse to do anything if user gives --no-commit and merge is fast- \nforward\n 3) Document this behavior in the manpage\n 4) Quietly set deny_non_fast_forward when --no-commit was given\n\nA fourth solution would be to not change anything, but users usually \nhave a reason for using --no-commit, and if git commits behind their \nbacks anyway that's just not nice.\n\ntom\n"},{"id":"123037","messageId":"7vocpfz4gm.fsf@alter.siamese.dyndns.org","threadId":"20925","inReplyTo":"A754ACF1-77C2-4036-A15C-8949E76BD2D5@dbservice.com","subject":"Re: merge ignores --no-commit in fast-forward case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-13T07:30:49Z","receivedAt":"2009-09-13T07:30:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tomas Carnecky writes:\n\n> Bug or feature?\n\nFeature, perhaps a misleading one. As a fast-forward merge does not _create_\na commit, and --no-commit is about not creating a commit, it is logically\nconsistent. But it is not useful nor intuitive to be logically consistent\nin this case.\n\n> Three possible solutions that I see:\n>\n> 2) Refuse to do anything if user gives --no-commit and merge is fast- \n> forward\n> 3) Document this behavior in the manpage\n> 4) Quietly set deny_non_fast_forward when --no-commit was given\n\nHeh, this is new. People laugh at me when I number my bullets starting\nfrom zero, like all good computer scientists do ;-)\n\nSeriously, we should at least do #3, and as a backward incompatible change\nat least _consider_ doing #2 (I think #4 is merely an implementation detail\nof #2), and if list reaches concensus in favor of such a change, come up\nwith a transition plan and do so in the 1.7.0 release.\n"},{"id":"123503","messageId":"20090918194717.GA18999@dr-wily.mit.edu","threadId":"20925","inReplyTo":"7vocpfz4gm.fsf@alter.siamese.dyndns.org","subject":"Re: merge ignores --no-commit in fast-forward case","fromName":"Greg Price","fromEmail":"price@ksplice.com","sentAt":"2009-09-18T19:47:18Z","receivedAt":"2009-09-18T19:47:18Z","isPatch":false,"sender":{"key":"price@mit.edu","avatar":"https://avatars.githubusercontent.com/u/28173?v=4"},"body":"I've hit this issue in scripts where someone has written\n\n if ! git merge some-branch --no-commit; then\n # deal or have the user deal\n fi\n\n git commit -m '...'\n\nintending that if the merge succeeds it has left the changes from\nsome-branch uncommitted. And the script works, until for some reason\nsome-branch is on top of HEAD. So Tomas is not the only person who\nwas surprised or confused by the present behavior. =)\n\nIn this case the workaround is to add --no-ff to the command, which is\neffectively what the proposed change would do automatically.\n\nSuppose we were to make such a change in 1.7.0 or a later version.\nWhat would the transition plan need to accomplish? For instance,\nwe could print a warning every time 'merge --no-commit' does a\nfast-forward, and make the change in a later version -- might that suffice?\n\nCheers,\nGreg\n\n\nPS - Here's one version of a documentation patch for #3.\n\n>From 83282bbbd0c918016e71e4ff6fd8a823315fed0e Mon Sep 17 00:00:00 2001\nFrom: Greg Price \nDate: Fri, 18 Sep 2009 15:34:19 -0400\nSubject: [PATCH] Document behavior of --no-commit on fast forward.\n\nThis behavior can be surprising, so document it.\n\nSigned-off-by: Greg Price \n---\n Documentation/merge-options.txt | 5 ++++-\n 1 files changed, 4 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/merge-options.txt\nb/Documentation/merge-options.txt\nindex adadf8e..6015e5d 100644\n--- a/Documentation/merge-options.txt\n+++ b/Documentation/merge-options.txt\n@@ -31,7 +31,10 @@\n --no-commit::\n Perform the merge but pretend the merge failed and do\n not autocommit, to give the user a chance to inspect and\n- further tweak the merge result before committing.\n+ further tweak the merge result before committing. If the\n+ merge resolved as a fast-forward, the branch pointer will\n+ be updated as usual; --no-ff can be combined with this\n+ option to always preserve the branch pointer.\n \n --commit::\n Perform the merge and commit the result. This option can\n-- \n1.6.3.1.499.ge7b8da\n"},{"id":"123509","messageId":"7viqff97l4.fsf@alter.siamese.dyndns.org","threadId":"20925","inReplyTo":"20090918194717.GA18999@dr-wily.mit.edu","subject":"Re: merge ignores --no-commit in fast-forward case","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-18T23:05:59Z","receivedAt":"2009-09-18T23:05:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Price writes:\n\n> Suppose we were to make such a change in 1.7.0 or a later version.\n> What would the transition plan need to accomplish? For instance,\n> we could print a warning every time 'merge --no-commit' does a\n> fast-forward, and make the change in a later version -- might that\n> suffice?\n\nI guess so, but right now I don't want to (nor have bandwidth to) think\nabout things beyond the release we are currently in pre-release freeze\nfor. Helps from other people's braincells are very welcome.\n\n> From: Greg Price \n> Date: Fri, 18 Sep 2009 15:34:19 -0400\n> Subject: [PATCH] Document behavior of --no-commit on fast forward.\n>\n> This behavior can be surprising, so document it.\n\nIf \"surprising\" is the problem (and it certainly is), perhaps it would be\nbetter to rewrite it not to be so surprising, like:\n\n --no-commit::\n Do not create a new commit. If your current branch\n fast-forwards to the branch you are merging, this option\n has no effect (use --no-ff for that). Otherwise, a merge\n is done and the index and the work tree files are updated,\n but the command stops before creating a new commit that\n records the result, in order to give the user a chance to\n inspect and tweak the merge result before committing.\n\nI dunno.\n\n> Signed-off-by: Greg Price \n> ---\n> Documentation/merge-options.txt | 5 ++++-\n> 1 files changed, 4 insertions(+), 1 deletions(-)\n>\n> diff --git a/Documentation/merge-options.txt\n> b/Documentation/merge-options.txt\n> index adadf8e..6015e5d 100644\n> --- a/Documentation/merge-options.txt\n> +++ b/Documentation/merge-options.txt\n> @@ -31,7 +31,10 @@\n> --no-commit::\n> Perform the merge but pretend the merge failed and do\n> not autocommit, to give the user a chance to inspect and\n> - further tweak the merge result before committing.\n> + further tweak the merge result before committing. If the\n> + merge resolved as a fast-forward, the branch pointer will\n> + be updated as usual; --no-ff can be combined with this\n> + option to always preserve the branch pointer.\n"},{"id":"123512","messageId":"20090919080903.GA16316@atjola.homenet","threadId":"20925","inReplyTo":"7vocpfz4gm.fsf@alter.siamese.dyndns.org","subject":"Re: merge ignores --no-commit in fast-forward case","fromName":"Björn Steinbrink","fromEmail":"b.steinbrink@gmx.de","sentAt":"2009-09-19T08:09:03Z","receivedAt":"2009-09-19T08:09:03Z","isPatch":false,"sender":{"key":"b.steinbrink@gmx.de","avatar":"https://avatars.githubusercontent.com/u/230962?v=4"},"body":"On 2009.09.13 00:30:49 -0700, Junio C Hamano wrote:\n> Tomas Carnecky writes:\n> > Three possible solutions that I see:\n> >\n> > 2) Refuse to do anything if user gives --no-commit and merge is fast- \n> > forward\n> > 3) Document this behavior in the manpage\n> > 4) Quietly set deny_non_fast_forward when --no-commit was given\n> \n> Heh, this is new. People laugh at me when I number my bullets starting\n> from zero, like all good computer scientists do ;-)\n> \n> Seriously, we should at least do #3, and as a backward incompatible change\n> at least _consider_ doing #2 (I think #4 is merely an implementation detail\n> of #2), and if list reaches concensus in favor of such a change, come up\n> with a transition plan and do so in the 1.7.0 release.\n\nHm, I always treated --no-commit as a way of saying: I think this merge\nmight cause semantic problems, please let me fix those directly, instead\nof having to deal with 'commit --amend' and 'diff HEAD^ HEAD'.\n\nObviously, in case of a fast-forward such semantic problems aren't to be\nexpected, and I've just been wrong in my expectations. And then I'm\nhappy with the fast-forward. I'd _not_ be happy if a merge commit would\nbe forced (that's what #4 was about, right? deny_non_fast_forward only\nappears in builtin-receive-pack.c, so I guess allow_fast_forward=0\nwas meant...).\n\n#2 would be ok with me, I guess. I expected the wrong thing from the\nmerge, so git may complain, though it means more typing ;-)\n\nBjörn\n"},{"id":"123539","messageId":"4AB63678.6010500@gnu.org","threadId":"20925","inReplyTo":"7viqff97l4.fsf@alter.siamese.dyndns.org","subject":"Re: merge ignores --no-commit in fast-forward case","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-09-20T14:04:40Z","receivedAt":"2009-09-20T14:04:40Z","isPatch":false,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 09/19/2009 01:05 AM, Junio C Hamano wrote:\n> If \"surprising\" is the problem (and it certainly is), perhaps it would be\n> better to rewrite it not to be so surprising, like:\n>\n> --no-commit::\n> Do not create a new commit.\n\nDo not create a new _merge_ commit, and it's perfect.\n\nPaolo\n"}]}