Junio C Hamano wrote:

Felipe Contreras [off-list ref] writes:

quoted

It's clear --ff doesn't imply a merge, so we shouldn't act as if it was.

Do you specifically mean --ff, or do you talk collectively about
anything that goes in opt_ff in the C code?

I meant --ff, but the rationale can be extended to all of opt_ff.

The "--ff" option means "we are allowing fast-forward, so please do
not make new commit object unnecessarily, but it is just we are
allowing---we are not limiting ourselves to fast-forard; feel free
to create a merge commit if necessary".

Yes. *If* a rebase is not specified.

So it does imply that the user prefers to merge and does not want to
rebase.

We could imply that, but currently it doesn't.

Currently this does not do a merge:

  git config pull.rebase true
  git pull --ff

If you meant what opt_ff can relay, then there are "--no-ff" and
"--ff-only" to consider:

 - "--no-ff" says "we do not allow fast-forward; when the other side
   is pure descendant of ours, create a merge commit to make them
   the second parent, so that our side of the history stays to be
   the first-parent chain that merged them as a side topic."  It may
   not say what should happen when the history does not
   fast-forward, and it _is_ possible to argue, for the sake of
   argument, that it asks to rebase if not fast-forward (so that
   their history becomes the primary and we build on top of them)
   while asking to merge if fast-forward (so that our history stays
   the primary and we absorb their work as a side branch), but that
   is a behavior that does not make much sense.

I agree it doesn't make much sense; if the user wants a rebase in case
of non-fast-forward, --no-ff is the only way.

   It is much easier to reason about if we accept that the user who
   says "--no-ff" expects a merge to happen, not a rebase.

Yes, but currently that's not the case.

Currently this doesn't do a merge:

  git config pull.rebase true
  git pull --no-ff

We would need to change the semantics.

 - "--ff-only" says "when their history is pure descendant of ours,
   just fast-forward our branch to match their history, and
   otherwise fail."  This one does not have to imply either merge or
   rebase, as both would give us identical result (i.e. merge would
   fast-forward and rebase would replay *no* work of our own on top
   of theirs.  Either case, the result is that our branch tip now
   points at the tip of their history).

   The topic under discussion is based on the "we do not have to
   give advice between merge and rebase if the history
   fast-forwards", and anybody in support of the topic would be in
   agreement with this case.

Yes.

In any case, I think what we have in 'seen' already is a good
stopping point for this cycle.

It's not a bad stopping point.

But the next patches are needed too. Up to the first 6 patches should be
uncontroversial.

We are not erroring out any new case and simply not showing an advice
in a situation that it would not apply---the question "does --ff imply
merge?" does not have to be answered in order to evaluate the 5-patch
series we have.

Not my patches.

The patch you introduced regarding rebase_unspecified does depend on
what happens next. If we decide to change the semantics of --ff* and
imply a merge, then my patch to add REBASE_DEFAULT is needed, and as you
can see in another patch series [1], that basically has to revert your patch.

Cheers.

[1] https://lore.kernel.org/git/20201218211026.1937168-8-felipe.contreras@gmail.com/ (local)

-- 
Felipe Contreras