"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted hunk
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 54048306095..e70ed99e408 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -7,12 +7,13 @@ pull.ff::
line). When set to `only`, only such fast-forward merges are
allowed (equivalent to giving the `--ff-only` option from the
command line). This setting overrides `merge.ff` when pulling.
+ Incompatible with pull.rebase.
This update may mean well, but I sense that the description needs to
be tightened up a bit more. Unless you mean to say that I am not
allowed to say "[pull] rebase=no" when I set "[pull] ff", that is.
Or do you mean pull.ff=only is incompatible with any setting of
pull.rebase?
Or do you mean pull.ff=only is incompatible with any setting of
pull.rebase other than false?
Or do you mean any setting of pull.ff is imcompatible with any
setting of pull.rebase other than false?
(You do not have to answer---the above questions just demonstrate
that the description is unnecessarily loose).
pull.rebase::
When true, rebase branches on top of the fetched branch, instead
of merging the default branch from the default remote when "git
pull" is run. See "branch.<name>.rebase" for setting this on a
- per-branch basis.
+ per-branch basis. Incompatible with pull.ff.
Likewise.
I think it technically is OK to say "only ff update is allowed" or
"ff update is allowed when able" while saying pull.rebase=yes.
- For those who say ff=only, the actual value of pull.rebase would
not matter (i.e. the integration immediately finishes by updating
to what was obtained from the upstream because there is no new
development on our own on top of theirs to either replay or
merge).
- For those who merely allow ff, an update may fast-forward even
when pull.rebase is set to true (or any non-false value), while
we'll replay our own development on top of theirs when their
history is not descendent of ours.
So I do not see need to declare these combinations "incompatible".
But perhaps I am missing some subtle cases?
quoted hunk
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c014..03e8694e146 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -121,6 +121,9 @@ When false, merge the current branch into the upstream branch.
+
When `interactive`, enable the interactive mode of rebase.
+
+Note that these flags are incompatible with --no-ff and --ff-only; if
+such incompatible flags are given, the last one will take precedence.
++
I am not sure about these, either. Again, --ff-only (whether it is
combined with --rebase or --rebase=no) would mean that the operation
would fail when we have our own development on top of their history,
and we repoint the tip of our history to theirs when we do not.
We see "--no-ff" is explained to "always create an unnecessary extra
merge", bit I am not sure it is the right mental model to apply when
the user prefers rebasing. The merge workflow is to treat our
history as the primary and merging theirs in, so when theirs is a
descendant (i.e. we have no new development of our own), some people
may want to mark "we were there before we updated from them" with an
extra merge. Without --no-ff, the resulting history would be quite
different in the merge workflow if you have or do not have your own
development. But the rebase workflow is to treat their history as
the primary and replay our own development on top of theirs, and the
shape of the resulting history would be the same whether you have
your own development on top of theirs. We start from their tip and
then replay our own development on top (which could be an empty
set), and there is nothing "--no-ff" would need to do differently to
keep the resulting history similar to cases where we do have
something of our own. IOW, "--no-ff" becoming a no-op in a "rebase"
workflow is a natural consequence, and there is no strong reason to
say it is incompatible.
Junio C Hamano wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
quoted
pull.rebase::
When true, rebase branches on top of the fetched branch, instead
of merging the default branch from the default remote when "git
pull" is run. See "branch.<name>.rebase" for setting this on a
- per-branch basis.
+ per-branch basis. Incompatible with pull.ff.
Likewise.
I think it technically is OK to say "only ff update is allowed" or
"ff update is allowed when able" while saying pull.rebase=yes.
- For those who say ff=only, the actual value of pull.rebase would
not matter (i.e. the integration immediately finishes by updating
to what was obtained from the upstream because there is no new
development on our own on top of theirs to either replay or
merge).
- For those who merely allow ff, an update may fast-forward even
when pull.rebase is set to true (or any non-false value), while
we'll replay our own development on top of theirs when their
history is not descendent of ours.
So I do not see need to declare these combinations "incompatible".
But perhaps I am missing some subtle cases?
Doing such a thing would be wrong. As I already explained multiple
times, pull.rebase can be overriden by the command line. When doing
--merge we want to honor pull.ff, when we don't we want to ignore it.
Since both Elijah and Junio have muted me, and they keep making these
mistakes, can anybody else forward this point to them?
Cheers.
--
Felipe Contreras
On Thu, Jul 15, 2021 at 10:33 AM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 54048306095..e70ed99e408 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -7,12 +7,13 @@ pull.ff::
line). When set to `only`, only such fast-forward merges are
allowed (equivalent to giving the `--ff-only` option from the
command line). This setting overrides `merge.ff` when pulling.
+ Incompatible with pull.rebase.
This update may mean well, but I sense that the description needs to
be tightened up a bit more. Unless you mean to say that I am not
allowed to say "[pull] rebase=no" when I set "[pull] ff", that is.
Or do you mean pull.ff=only is incompatible with any setting of
pull.rebase?
Or do you mean pull.ff=only is incompatible with any setting of
pull.rebase other than false?
Or do you mean any setting of pull.ff is imcompatible with any
setting of pull.rebase other than false?
(You do not have to answer---the above questions just demonstrate
that the description is unnecessarily loose).
Fair enough.
quoted
pull.rebase::
When true, rebase branches on top of the fetched branch, instead
of merging the default branch from the default remote when "git
pull" is run. See "branch.<name>.rebase" for setting this on a
- per-branch basis.
+ per-branch basis. Incompatible with pull.ff.
Likewise.
I think it technically is OK to say "only ff update is allowed" or
"ff update is allowed when able" while saying pull.rebase=yes.
- For those who say ff=only, the actual value of pull.rebase would
not matter (i.e. the integration immediately finishes by updating
to what was obtained from the upstream because there is no new
development on our own on top of theirs to either replay or
merge).
- For those who merely allow ff, an update may fast-forward even
when pull.rebase is set to true (or any non-false value), while
we'll replay our own development on top of theirs when their
history is not descendent of ours.
So I do not see need to declare these combinations "incompatible".
But perhaps I am missing some subtle cases?
Let me ask two questions:
1. When is it beneficial for users to set both pull.ff and pull.rebase?
2. Is it harmful to users for us to allow both to be set when we will
just ignore one?
I believe the answer to (1) is "never", and the answer to (2) is "yes".
For the second question in particular, I can think of two example cases:
2a) Users start with pull.ff=only, perhaps suggested by someone else
and left in their config for a long time. When users hit a case that
can't fast-forward and they either ask for help or find a post on
stack overflow that suggests setting pull.rebase=true, they do so and
then get no warning that the setting they just added is being ignored.
2b) Perhaps users start with pull.rebase=true (suggested by a
colleague and forgot about it as they are more of a tester than a
developer and thus usually only see fast-forwards). Then at some
point they need to function as an integrator, and they read the docs
and determine that pull.ff=false should do what they want to create
merge commits. But then they get shocked that they've rebased public
commits (and perhaps also pushed them out) when they wanted merges.
Our docs have pretty clearly stated that pull.ff=false and --no-ff
create merges.
Additionally, pull.rebase=interactive seems like a case that interacts
poorly with pull.ff. It may be that if you disagree that pull.rebase
and pull.ff are more generally incompatible, then we may still need to
micro-document individual incompatibilities. (And does that list grow
and get confusing?)
quoted
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c014..03e8694e146 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -121,6 +121,9 @@ When false, merge the current branch into the upstream branch.
+
When `interactive`, enable the interactive mode of rebase.
+
+Note that these flags are incompatible with --no-ff and --ff-only; if
+such incompatible flags are given, the last one will take precedence.
++
I am not sure about these, either. Again, --ff-only (whether it is
combined with --rebase or --rebase=no) would mean that the operation
would fail when we have our own development on top of their history,
and we repoint the tip of our history to theirs when we do not.
We see "--no-ff" is explained to "always create an unnecessary extra
merge", bit I am not sure it is the right mental model to apply when
the user prefers rebasing. The merge workflow is to treat our
history as the primary and merging theirs in, so when theirs is a
descendant (i.e. we have no new development of our own), some people
may want to mark "we were there before we updated from them" with an
extra merge. Without --no-ff, the resulting history would be quite
different in the merge workflow if you have or do not have your own
development. But the rebase workflow is to treat their history as
the primary and replay our own development on top of theirs, and the
shape of the resulting history would be the same whether you have
your own development on top of theirs. We start from their tip and
then replay our own development on top (which could be an empty
set), and there is nothing "--no-ff" would need to do differently to
keep the resulting history similar to cases where we do have
something of our own. IOW, "--no-ff" becoming a no-op in a "rebase"
workflow is a natural consequence, and there is no strong reason to
say it is incompatible.
I think you're overlooking a variety of ways this could harm the user.
But first let me check if I understand you correctly: I think you're
suggesting that
* if --ff-only is given along with any --rebase[=*] flags, then we
silently ignore the --rebase[=*] flags.
* if --no-ff is given along with any --rebase flag other than
--rebase=false, then we silently ignore --no-ff.
Let me ask the same two questions as for the config: Does it benefit
the users to allow them to specify both flags? Does it hurt some
users, especially if undocumented that one of the flags will be
ignored?
I think the same usecases from the config settings apply, though the
fact that both flags are present simultaneously on the command line at
least makes it less problematic in the case of --ff-only (users will
more quickly discover why their --rebase=<whatever> flag was ignored).
But there's an additional reason why treating --no-ff as a no-op is
harmful when --rebase is present:
git rebase has a --no-ff flag (a synonym for --force-rebase). Some
users may intend for git pull --rebase --no-ff to run `git fetch
origin && git rebase --no-ff FETCH_HEAD`. The fact that --no-ff will
be ignored and sometimes results in a fast-forward when the user
explicitly requested no fast forward seems buggy.
I thought this was the biggest hole in my proposal: there is both a
git merge --no-ff and a git rebase --no-ff, and they avoid
fast-forwards in different ways with different results. It _might_ be
beneficial to allow both to be triggered directly from the pull
command. However, allowing both has some backward compatibility
problems because (1) --no-ff is documented as making a merge, and (2)
people could have pull.rebase=true set and forget about it and then
get surprised when they add --no-ff to the command line and get a
rebase --no-ff (or even a plain rebase without --no-ff) rather than
the documented merge --no-ff. I think the split config/parameters of
git pull make it really hard for us to determine whether users intend
to get a merge --no-ff or rebase --no-ff.
I think silently ignoring and not even documenting these pitfalls is
problematic. We should at least document them.