# [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch 6 messages from 2011-12-01 to 2011-12-01. Participants: Samuel Bronson, Junio C Hamano. Thread: https://gitlist.dev/t/29058 ## Samuel Bronson, 2011-12-01 01:38 Subject: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch Message-ID: <1322703537-3914-1-git-send-email-naesten@gmail.com> URL: https://gitlist.dev/e/1322703537-3914-1-git-send-email-naesten%40gmail.com ``` Without this, pull becomes unusable for merging branches when the config option `merge.ff` is set to `only`. Signed-off-by: Samuel Bronson --- git-pull.sh | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/git-pull.sh b/git-pull.sh index 9868a0b..a09fcbc 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -76,6 +76,8 @@ do no_ff=--no-ff ;; --ff-only) ff_only=--ff-only ;; + --no-ff-only) + ff_only=--no-ff-only ;; -s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\ --strateg=*|--strategy=*|\ -s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy) -- 1.7.7.3 ``` ## Samuel Bronson, 2011-12-01 01:38 Subject: [PATCH/RFC 2/2] merge, pull: Document the --no-ff-only merge option Message-ID: <1322703537-3914-2-git-send-email-naesten@gmail.com> URL: https://gitlist.dev/e/1322703537-3914-2-git-send-email-naesten%40gmail.com In-Reply-To: <1322703537-3914-1-git-send-email-naesten@gmail.com> ``` Now that --no-ff-only will work with pull and not just with merge, we should probably tell the users about it. Signed-off-by: Samuel Bronson --- Documentation/merge-options.txt | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 6bd0b04..2adc4f5 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -56,9 +56,13 @@ With --no-squash perform the merge and commit the result. This option can be used to override --squash. --ff-only:: +--no-ff-only:: Refuse to merge and exit with a non-zero status unless the current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. ++ +With --no-ff-only, don't refuse. This is can be used to override +--ff-only, or the "`only`" value for the `merge.ff` configuration option. -s :: --strategy=:: -- 1.7.7.3 ``` ## Junio C Hamano, 2011-12-01 04:58 Subject: Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch Message-ID: <7vborsq45x.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vborsq45x.fsf%40alter.siamese.dyndns.org In-Reply-To: <1322703537-3914-1-git-send-email-naesten@gmail.com> ``` Samuel Bronson writes: > Without this, pull becomes unusable for merging branches when the config > option `merge.ff` is set to `only`. > > Signed-off-by: Samuel Bronson I wonder why you need this. We have "git config --unset merge.ff" after all. From purely mechanstic point of view, being able to temporarily defeat a configuration variable makes perfect sense, but from the point of view of workflow, I am not sure if it is a good thing to even allow it in the first place in this particular case. Setting merge.ff to 'only' means you are following along other people's work and making nothing new on your own in this particular repository, no? Hence you won't be asking the upstream to pull from this repository, which in turn means that even if you made a merge by temporarily defeating the configuration setting with this patch, your future pulls will no longer fast forward, until somehow the upstream gets hold of your merge commit. By the way (this is a digression), I also have to say --no-ff-only is too *ugly* as a UI element, even though I know "git merge" already allows it by accident. "ff" is a tristate. By default, fast-forward is done when appropriate, and people can _refuse_ to fast-forward and force Git to create an extra merge commit. Or if you are strictly following along, you can say you _require_ fast-forward and reject anything else. So it may make the UI saner if we updated the UI to allow users to say: --ff=normal the default --ff=never same as --no-ff that forces an extra merge commit --ff=required same as --ff-only while keeping the current --ff-only and --no-ff as backward compatibility wart. I dunno. ``` ## Samuel Bronson, 2011-12-01 17:18 Subject: Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch Message-ID: URL: https://gitlist.dev/e/CAJYzjmep7sKxiSNhMzAX2DRYJhANDQkPL5pX4HOZ9CssJxcWbw%40mail.gmail.com In-Reply-To: <7vborsq45x.fsf@alter.siamese.dyndns.org> ``` On Wed, Nov 30, 2011 at 11:58 PM, Junio C Hamano wrote: > Samuel Bronson writes: > >> Without this, pull becomes unusable for merging branches when the config >> option `merge.ff` is set to `only`. >> >> Signed-off-by: Samuel Bronson > > I wonder why you need this. We have "git config --unset merge.ff" after > all. From purely mechanstic point of view, being able to temporarily > defeat a configuration variable makes perfect sense, but from the point of > view of workflow, I am not sure if it is a good thing to even allow it in > the first place in this particular case. > > Setting merge.ff to 'only' means you are following along other people's > work and making nothing new on your own in this particular repository, no? > Hence you won't be asking the upstream to pull from this repository, which > in turn means that even if you made a merge by temporarily defeating the > configuration setting with this patch, your future pulls will no longer > fast forward, until somehow the upstream gets hold of your merge commit. Actually, I wanted to set merge.ff to only avoid *accidentally* merging origin/master onto my local master, when I would likely to prefer to either rebase my work onto it, or retroactively put my work in a branch and merge that *into* the real master branch; I would then override only when I explicitly did want a merge. (I actually have commit access to the repository in question, but wish to avoid making others' commits to master look as if they were on side branches.) > By the way (this is a digression), I also have to say --no-ff-only is too > *ugly* as a UI element, even though I know "git merge" already allows it > by accident. > > "ff" is a tristate. By default, fast-forward is done when appropriate, and > people can _refuse_ to fast-forward and force Git to create an extra merge > commit. Or if you are strictly following along, you can say you _require_ > fast-forward and reject anything else. So it may make the UI saner if we > updated the UI to allow users to say: > >        --ff=normal     the default >        --ff=never      same as --no-ff that forces an extra merge commit >        --ff=required   same as --ff-only > > while keeping the current --ff-only and --no-ff as backward compatibility > wart. Hmm, yes, I had noticed that it was a tristate (merge.ff clearly is), and I guess --no-ff-only is a pretty ugly flag. I do have to ask, though: why give --ff these new values? Wouldn't it make more sense to reuse the values accepted by merge.ff; namely, 'true' (the implied default), 'false', and 'only'? Otherwise, this looks like a very nice way to implement what I want: I guess it is probably a mistake that the existing (documented) flags do not behave in this way? ``` ## Junio C Hamano, 2011-12-01 18:06 Subject: Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch Message-ID: <7vvcq0np35.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvcq0np35.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Samuel Bronson writes: > Hmm, yes, I had noticed that it was a tristate (merge.ff clearly is), > and I guess --no-ff-only is a pretty ugly flag. I do have to ask, > though: why give --ff these new values? Wouldn't it make more sense to > reuse the values accepted by merge.ff; namely, 'true' (the implied > default), 'false', and 'only'? The 'true' and 'false' values to merge.ff are carry-over from the days when it was a boolean, _not_ a tristate. If we were to make the UI more rational by making it clear that this is not a boolean, it is a good time for us to aim a bit higher than merely repeating the mistakes we made in the past due to historical accident. In other words, we could add a synonym for the "default" mode in addition to "--ff=true" (and for the "always merge" mode in addition to "--ff=false") that makes it clear that the value is _not_ a boolean [*1*]. If we were to go the "--ff=" route, we have to add support for other ways to spell boolean 'true' (e.g. 'yes', '1', and 'on') anyway, so it is not that much extra work to do so, I would think. > Otherwise, this looks like a very nice way to implement what I want: I > guess it is probably a mistake that the existing (documented) flags do > not behave in this way? Yeah, right now if you say "merge --ff-only --no-ff", we say these are mutually exclusive (which is true), but if you think about the tristate nature of the 'ff' option and spell it differently in your head, i.e. "merge --ff=only --ff=never", it is reasonable to argue that we should apply the usual "last one overrides" rule and behave as if "merge --no-ff" were given (for the purpose of "last one overrides", the configured defaults can be treated as if they come very early on the command line). After all "merge --no-ff --ff" does seem to use the "last one overrides" rule. [Footnote] *1* Perhaps 'allowed' instead of 'normal' (which I wrote out of thin-air; I do not have any strong preference on the actual values) may be a better choice for such a "this is not a boolean" spelling for the default mode. ``` ## Samuel Bronson, 2011-12-01 18:59 Subject: Re: [PATCH/RFC 1/2] pull: pass the --no-ff-only flag through to merge, not fetch Message-ID: URL: https://gitlist.dev/e/CAJYzjmcePWriGLr5a0oe_%3D6qUkfKp5OyFGGEabj6S8vy%2Bhb4%2Bg%40mail.gmail.com In-Reply-To: <7vvcq0np35.fsf@alter.siamese.dyndns.org> ``` On Thu, Dec 1, 2011 at 1:06 PM, Junio C Hamano wrote: > Samuel Bronson writes: > >> Hmm, yes, I had noticed that it was a tristate (merge.ff clearly is), >> and I guess --no-ff-only is a pretty ugly flag. I do have to ask, >> though: why give --ff these new values? Wouldn't it make more sense to >> reuse the values accepted by merge.ff; namely, 'true' (the implied >> default), 'false', and 'only'? > > The 'true' and 'false' values to merge.ff are carry-over from the days > when it was a boolean, _not_ a tristate. If we were to make the UI more > rational by making it clear that this is not a boolean, it is a good time > for us to aim a bit higher than merely repeating the mistakes we made in > the past due to historical accident. In other words, we could add a > synonym for the "default" mode in addition to "--ff=true" (and for the > "always merge" mode in addition to "--ff=false") that makes it clear that > the value is _not_ a boolean [*1*]. If we were to go the "--ff=" > route, we have to add support for other ways to spell boolean 'true' > (e.g. 'yes', '1', and 'on') anyway, so it is not that much extra work to > do so, I would think. Sure, that makes sense. I was just a little worried that you might be (accidentally) proposing that --ff use a different set of names than merge.ff for a moment there... >> Otherwise, this looks like a very nice way to implement what I want: I >> guess it is probably a mistake that the existing (documented) flags do >> not behave in this way? > > Yeah, right now if you say "merge --ff-only --no-ff", we say these are > mutually exclusive (which is true), but if you think about the tristate > nature of the 'ff' option and spell it differently in your head, i.e. > "merge --ff=only --ff=never", it is reasonable to argue that we should > apply the usual "last one overrides" rule and behave as if "merge --no-ff" > were given (for the purpose of "last one overrides", the configured > defaults can be treated as if they come very early on the command line). > After all "merge --no-ff --ff" does seem to use the "last one overrides" > rule. Yes, I totally agree that it would make more sense that way; I certainly tried that before I even began to look at any of the code. > [Footnote] > > *1* Perhaps 'allowed' instead of 'normal' (which I wrote out of thin-air; > I do not have any strong preference on the actual values) may be a better > choice for such a "this is not a boolean" spelling for the default mode. ```