| Message ID | 20190218184147.7563-1-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | | expand |
>>> Elijah Newren <newren@gmail.com> schrieb am 18.02.2019 um 19:41 in Nachricht <20190218184147.7563-1-newren@gmail.com>: > Hi Ulrich, > > Sorry for the late reply... No problem, thanks for the explanation. I'll "fast forward" directly to the patch below and comment inline: -- Ulrich [...] > ‑‑ 8< ‑‑ > Subject: [PATCH] merge‑options.txt: correct wording of ‑‑no‑commit option > > The former wording implied that ‑‑no‑commit would always cause the > merge operation to abort and allow the user to make further changes I think "abort" is not the perfect word, because the merge would rather be "paused for commit" in my understanding; a "git merge --abort" is different, right? > and/or provide a special commit message for the merge commit. This > is not the case for fast‑forward merges, as there is no merge commit > to create. Without a merge commit, there is no place where it makes > sense to "stop the merge and allow the user to tweak changes"; doing > that would require a full rebase of some sort. ...and before trying the merge it's not always obvious whether the merge will be fast-forward type or not. So actually the outcome of --no-commit depends on the conents being merged, not on the command line options. > > Modify the wording to correctly address fast‑forward cases as well, > and suggest using ‑‑no‑ff with ‑‑no‑commit if the point is to ensure > that the merge aborts. > > Reported‑by: Ulrich Windl <Ulrich.Windl@rz.uni‑regensburg.de> > Signed‑off‑by: Elijah Newren <newren@gmail.com> > ‑‑‑ > Documentation/merge‑options.txt | 12 +++++++++‑‑‑ > 1 file changed, 9 insertions(+), 3 deletions(‑) > > diff ‑‑git a/Documentation/merge‑options.txt b/Documentation/merge‑options.txt > index c2a263ba74..d1061b8cf7 100644 > ‑‑‑ a/Documentation/merge‑options.txt > +++ b/Documentation/merge‑options.txt > @@ ‑3,9 +3,15 @@ > Perform the merge and commit the result. This option can > be used to override ‑‑no‑commit. > + > ‑With ‑‑no‑commit perform the merge but pretend the merge > ‑failed and do not autocommit, to give the user a chance to > ‑inspect and further tweak the merge result before committing. > +With ‑‑no‑commit perform the merge and stop just before creating > +a merge commit, to give the user a chance to inspect and further > +tweak the merge result before committing. > ++ > +Note that fast‑forward updates do not need to create a merge > +commit and therefore there is no way to stop those merges with > +‑‑no‑commit. Thus, if you want to ensure your branch is not > +changed or updated by the merge command, use ‑‑no‑ff with > +‑‑no‑commit. > > ‑‑edit:: > ‑e:: > ‑‑ > 2.21.0.rc1.264.g6c9e06a32d
>>> Junio C Hamano <gitster@pobox.com> schrieb am 19.02.2019 um 20:32 in Nachricht <xmqqk1hv1sms.fsf@gitster-ct.c.googlers.com>: > Elijah Newren <newren@gmail.com> writes: > >> +With ‑‑no‑commit perform the merge and stop just before creating >> +a merge commit, to give the user a chance to inspect and further >> +tweak the merge result before committing. >> ++ >> +Note that fast‑forward updates do not need to create a merge >> +commit and therefore there is no way to stop those merges with >> +‑‑no‑commit. Thus, if you want to ensure your branch is not >> +changed or updated by the merge command, use ‑‑no‑ff with >> +‑‑no‑commit. > > While the above is an improvement (so I'll queue it on 'pu' not to > lose sight of it), I find the use of "do not need to" above somewhat > misleading. It solicits a reaction "ok, we know it does not need > to, but it could prepare to create one to allow us to further muck > with it, no?". > > IOW, a fast‑forward by definition does not create a merge by itself, > so there is nowhere to stop during a creation of a merge. So at > least: > > s/do not need to/do not/ Agree. > > It also may be a good idea to consider detecting this case and be a > bit more helpful, perhaps with end‑user experience looking like... > > $ git checkout master^0 > $ git merge ‑‑no‑commit next > Updating 0d0ac3826a..ee538a81fe > Fast‑forward > ...diffstat follows here... > hint: merge completed without creating a commit. > hint: if you wanted to prepare for a manually tweaked merge, > hint: do "git reset ‑‑keep ORIG_HEAD" followed by > hint: "git merge ‑‑no‑ff ‑‑no‑commit next". > > or even > > $ git checkout master^0 > $ git merge ‑‑no‑commit next > warning: defaulting to ‑‑no‑ff, given a ‑‑no‑commit request > Automatic merge went well; stopped before committing as requested > hint: if you'd rather have a fast‑forward without creating a commit, > hint: do "git reset ‑‑keep next" now. > > I do not have a strong preference among three (the third option > being not doing anything), but if pressed, I'd say that the last one > might be the most user‑friendly, even though it feels a bit too > magical and trying to be smarter than its own good. > > In any case, the hint for the "recovery" procedure needs to be > carefully written. Actually I think if the user specified "--no-commit" and the merge turns out to be fast-forward, the user could be asked whether to continue or not (instead of undoinf afterwards); maybe when entering a response is not possible (batch processing) the merge should be aborted due to "--no-commit" not being possible (well actually there would never be a commit, even without that option). The problem is that without prior inspection of the tree you cannot know whether the merge will be fast-forward or not: fast-forward being an optimization (taht is enabled by default) makes life more complicated here. Regards, Ulrich Windl
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index c2a263ba74..d1061b8cf7 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -3,9 +3,15 @@ Perform the merge and commit the result. This option can be used to override --no-commit. + -With --no-commit perform the merge but pretend the merge -failed and do not autocommit, to give the user a chance to -inspect and further tweak the merge result before committing. +With --no-commit perform the merge and stop just before creating +a merge commit, to give the user a chance to inspect and further +tweak the merge result before committing. ++ +Note that fast-forward updates do not need to create a merge +commit and therefore there is no way to stop those merges with +--no-commit. Thus, if you want to ensure your branch is not +changed or updated by the merge command, use --no-ff with +--no-commit. --edit:: -e::