Junio C Hamano wrote: > Felipe Contreras <felipe.contreras@xxxxxxxxx> writes: > > > 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@xxxxxxxxx/ -- Felipe Contreras
