* [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
@ 2025-02-05 3:06 D. Ben Knoble
2025-02-05 13:08 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-02-05 3:06 UTC (permalink / raw)
To: git
Cc: D. Ben Knoble, Alex Henrie,
Ævar Arnfjörð Bjarmason, Felipe Contreras,
Patrick Steinhardt, Junio C Hamano, Elijah Newren
When running "git pull" with the following configuration options, we
fail to merge divergent branches:
- pull.ff=only
- pull.rebase (unset)
- branch.<current_branch>.rebase=true
Yet it seems that the user intended to make rebase the default for the
current branch while using --ff-only for non-rebase pulls. Since this
case appears uncovered by existing tests, changing the behavior here
might be safe: it makes what was an error into a successful rebase.
Add a test for the behavior and make it pass: this requires knowing from
where the rebase was requested. Previous commits (e4dc25ed49 (pull:
since --ff-only overrides, handle it first, 2021-07-22), adc27d6a93
(pull: make --rebase and --no-rebase override pull.ff=only, 2021-07-22))
took care to differentiate that --rebase overrides pull.ff=only, but
don't distinguish which config setting requests the rebase. Split
config_get_rebase into 2 parts so that we know where the rebase comes
from, since we only want to allow branch-config to override pull.ff=only
(like --rebase does); pull.rebase should still be overridden by
pull.ff=only or --ff-only.
Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
---
Notes:
- I also looked at ea1954af77 (pull: should be noop when already-up-to-date,
2021-11-17) when trying to understand how some options override others,
but it didnt' seem germane to the final version.
- I think I've got the right test script, since it's the one that started
failing before I added the "else" branch to the new code (which also
confirms that it's necessary to preserve current behavior); the only new
behavior should be the one mentioned by the new test.
- A possible #leftoverbits: it would be good to document more clearly the
interplay of --ff[-only], --rebase, pull.ff, pull.rebase, and
branch.<name>.rebase, particularly when they override each other.
Confusingly, branch.<name>.merge has nothing to do with whether pull will
merge or rebase ;) lest you think I'd forgotten something that _looks_
parallel to pull.rebase.
builtin/pull.c | 39 ++++++++++++++++++++++++++++--------
t/t7601-merge-pull-config.sh | 8 ++++++++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 9c4a00620a..c30f233dcc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -326,13 +326,13 @@ static const char *config_get_ff(void)
}
/**
- * Returns the default configured value for --rebase. It first looks for the
+ * Returns the default configured value for --rebase. It looks for the
* value of "branch.$curr_branch.rebase", where $curr_branch is the current
* branch, and if HEAD is detached or the configuration key does not exist,
- * looks for the value of "pull.rebase". If both configuration keys do not
- * exist, returns REBASE_FALSE.
+ * considers the result unspecified. Follow up by checking
+ * config_get_rebase_pull.
*/
-static enum rebase_type config_get_rebase(int *rebase_unspecified)
+static enum rebase_type config_get_rebase_branch(int *rebase_unspecified)
{
struct branch *curr_branch = branch_get("HEAD");
const char *value;
@@ -349,11 +349,22 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)
free(key);
}
+ *rebase_unspecified = 1;
+ return REBASE_INVALID;
+}
+
+/*
+ * Looks for the value of "pull.rebase". If it does not exist, returns
+ * REBASE_FALSE.
+ */
+static enum rebase_type config_get_rebase_pull(int *rebase_unspecified)
+{
+ const char *value;
+
if (!git_config_get_value("pull.rebase", &value))
return parse_config_rebase("pull.rebase", value, 1);
*rebase_unspecified = 1;
-
return REBASE_FALSE;
}
@@ -1026,7 +1037,7 @@ int cmd_pull(int argc,
* are relying on the next if-condition happening before
* the config_get_rebase() call so that an explicit
* "--rebase" can override a config setting of
- * pull.ff=only.
+ * pull.ff=only. [continued…]
*/
if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only")) {
free(opt_ff);
@@ -1034,8 +1045,20 @@ int cmd_pull(int argc,
}
}
- if (opt_rebase < 0)
- opt_rebase = config_get_rebase(&rebase_unspecified);
+ if (opt_rebase < 0) {
+ /*
+ * […continued] But, if the config requests rebase *for this
+ * branch*, override --ff-only, which otherwise takes precedence
+ * over pull.rebase=true.
+ */
+ opt_rebase = config_get_rebase_branch(&rebase_unspecified);
+ if (opt_rebase >= 0 && opt_ff && !strcmp(opt_ff, "--ff-only")) {
+ free(opt_ff);
+ opt_ff = xstrdup("--ff");
+ } else {
+ opt_rebase = config_get_rebase_pull(&rebase_unspecified);
+ }
+ }
if (repo_read_index_unmerged(the_repository))
die_resolve_conflict("pull");
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 199a1d5db3..fd99f46aad 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -113,6 +113,14 @@
test_grep ! "You have divergent branches" err
'
+test_expect_success 'pull.rebase not set and pull.ff=only and branch.<name>.rebase=true (not-fast-forward)' '
+ git reset --hard c2 &&
+ test_config pull.ff only &&
+ git switch -c bc2 &&
+ test_config branch.bc2.rebase true &&
+ git pull . c1
+'
+
test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
git reset --hard c2 &&
git pull --rebase . c1 2>err &&
--
2.47.0
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 3:06 [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only D. Ben Knoble
@ 2025-02-05 13:08 ` Junio C Hamano
2025-02-05 16:36 ` D. Ben Knoble
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2025-02-05 13:08 UTC (permalink / raw)
To: D. Ben Knoble
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> When running "git pull" with the following configuration options, we
> fail to merge divergent branches:
>
> - pull.ff=only
> - pull.rebase (unset)
> - branch.<current_branch>.rebase=true
>
> Yet it seems that the user intended to make rebase the default for the
> current branch while using --ff-only for non-rebase pulls. Since this
> case appears uncovered by existing tests, changing the behavior here
> might be safe: it makes what was an error into a successful rebase.
Hmph, to me it looks more like with pull.ff, the user, no matter
what other variables say and which mode between merge and rebase a
pull consolidates the histories, wanted to make sure they will never
accept anything other than fast-forwarding of the history, because
the end-user expects that they will pull only after they push out
everything, i.e., the expectation is that the other side is a strict
fast-forward or the user wants to examine the situation before
making further damage to the local history.
With that understanding, I am not sure "even though pull.ff tells
us to stop unless the other side is a descendant of our history, if
we are rebasing, it is OK if they have something we have never seen"
is a good thing to do.
So, I dunno.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 13:08 ` Junio C Hamano
@ 2025-02-05 16:36 ` D. Ben Knoble
2025-02-05 17:42 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-02-05 16:36 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Wed, Feb 5, 2025 at 8:09 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>
> > When running "git pull" with the following configuration options, we
> > fail to merge divergent branches:
> >
> > - pull.ff=only
> > - pull.rebase (unset)
> > - branch.<current_branch>.rebase=true
> >
> > Yet it seems that the user intended to make rebase the default for the
> > current branch while using --ff-only for non-rebase pulls. Since this
> > case appears uncovered by existing tests, changing the behavior here
> > might be safe: it makes what was an error into a successful rebase.
>
> Hmph, to me it looks more like with pull.ff, the user, no matter
> what other variables say and which mode between merge and rebase a
> pull consolidates the histories, wanted to make sure they will never
> accept anything other than fast-forwarding of the history, because
> the end-user expects that they will pull only after they push out
> everything, i.e., the expectation is that the other side is a strict
> fast-forward or the user wants to examine the situation before
> making further damage to the local history.
That's certainly one way to understand --ff-only, but I can't find it
supported by existing docs (though it's what current code says,
excepting lack of test for interaction with branch.name.merge). For
example, `git help pull`:
--ff-only
Only update to the new history if there is no divergent local
history. This is the default when no method for reconciling divergent
histories is provided (via the --rebase=* flags).
and `git help config`:
pull.ff
By default, Git does not create an extra merge commit when merging a
commit that is a descendant of the current commit. Instead, the tip
of the current branch is fast-forwarded. When set to false, this
variable tells Git to create an extra merge commit in such a case
(equivalent to giving the --no-ff option from the command 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.
[…]
branch.autoSetupRebase
When a new branch is created with git branch, git switch or git
checkout that tracks another branch, this variable tells Git to set
up pull to rebase instead of merge (see "branch.<name>.rebase"). When
never, rebase is never automatically set to true. When local, rebase
is set to true for tracked branches of other local branches. When
remote, rebase is set to true for tracked branches of remote-tracking
branches. When always, rebase will be set to true for all tracking
branches. See "branch.autoSetupMerge" for details on how to set up a
branch to track another branch. This option defaults to never.
[…]
branch.<name>.rebase
When true, rebase the branch <name> on top of the fetched branch,
instead of merging the default branch from the default remote when
"git pull" is run. See "pull.rebase" for doing this in a non
branch-specific manner.
[snip]
NOTE: this is a possibly dangerous operation; do not use it unless
you understand the implications (see git-rebase(1) for details).
So I would tend to read branch.name.rebase as "you opted in to this,
you know what you're doing" and let it override --ff-only.
Granted, it's not clear just from reading the various git-config files
which sections and variables override which, so I'm perhaps
overly-reliant on the documentation to understand when those overrides
happen (see "notes" in original post).
>
> With that understanding, I am not sure "even though pull.ff tells
> us to stop unless the other side is a descendant of our history, if
> we are rebasing, it is OK if they have something we have never seen"
> is a good thing to do.
>
> So, I dunno.
Agreed that if pull.ff=only is supposed to override all other options
(except those on the command-line), this might be wrong. And `git pull
--rebase` works in the scenario I described.
I think that `pull.ff=only` + `branch.name.rebase=true` is a useful
combination to say "unless I'm asking to rebase [via --rebase or
branch settings], only permit fast-forward pulls." For example, my
main or master branch is typically fast-forward only, while I want my
topic branches to be rebased; preferably, all of those things happen
for just "git pull."
But maybe the intended way to accomplish what I want is pull.ff=true
(the default?), which doesn't prevent accidental merges in the cases I
want it to without setting branch.name.mergeOptions for each branch I
want to protect from accidental pull-merges. (I'm in the habit of
using fetch + merge as needed and mostly use pull to shortcut things
when I'm confident, and obviously I can undo the accidental merge… but
not having it in the first place is nice, too.)
LMK if something in my position is not clear—my overreliance on
parentheticals can be confusing.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 16:36 ` D. Ben Knoble
@ 2025-02-05 17:42 ` Junio C Hamano
2025-02-05 21:14 ` D. Ben Knoble
0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2025-02-05 17:42 UTC (permalink / raw)
To: D. Ben Knoble
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>> So, I dunno.
>
> Agreed that if pull.ff=only is supposed to override all other options
> (except those on the command-line), this might be wrong. And `git pull
> --rebase` works in the scenario I described.
Yeah, I view --ff-only as a safety measure for the user to say "my
workflow is to make sure I do not have anything locally cooking on
my branch when integrating with the other side, and stop me if I
somehow made a mistake". If rebase or other options override, the
folks in the rebasing camp, unlike in the merging camp, cannot
benefit from such safety measure, which worries me.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 17:42 ` Junio C Hamano
@ 2025-02-05 21:14 ` D. Ben Knoble
2025-02-07 2:35 ` Alex Henrie
2025-04-22 19:58 ` D. Ben Knoble
0 siblings, 2 replies; 13+ messages in thread
From: D. Ben Knoble @ 2025-02-05 21:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Wed, Feb 5, 2025 at 12:42 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>
> >> So, I dunno.
> >
> > Agreed that if pull.ff=only is supposed to override all other options
> > (except those on the command-line), this might be wrong. And `git pull
> > --rebase` works in the scenario I described.
>
> Yeah, I view --ff-only as a safety measure for the user to say "my
> workflow is to make sure I do not have anything locally cooking on
> my branch when integrating with the other side, and stop me if I
> somehow made a mistake". If rebase or other options override, the
> folks in the rebasing camp, unlike in the merging camp, cannot
> benefit from such safety measure, which worries me.
Is there, then, an existing combination that means roughly to treat
`git pull` with no other options like this:
- if not rebasing, forbid merging and be equivalent to --ff-only
- if rebasing is requested (because of branch.name.rebase or --rebase
or …?), allow it
In other words, something like a pull.merge=ff (or ff-only) meaning to
apply the rules I've attempted to describe, in which case I would
leave pull.ff unset?
I suppose pull.rebase=true is close, but is not quite the same for me
(I'd like to be warned when this would imply a non-fast-forward for a
main branch, though the "rebasing" logs might be sufficient)…
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 21:14 ` D. Ben Knoble
@ 2025-02-07 2:35 ` Alex Henrie
2025-02-10 20:26 ` D. Ben Knoble
2025-04-22 19:58 ` D. Ben Knoble
1 sibling, 1 reply; 13+ messages in thread
From: Alex Henrie @ 2025-02-07 2:35 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Tue, Feb 4, 2025 at 8:11 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
>
> When running "git pull" with the following configuration options, we
> fail to merge divergent branches:
>
> - pull.ff=only
> - pull.rebase (unset)
> - branch.<current_branch>.rebase=true
>
> Yet it seems that the user intended to make rebase the default for the
> current branch while using --ff-only for non-rebase pulls.
You make an interesting point. The idea is that more specific options
override less specific options. In this case, "fast-forward only" is
more specific than "rebase" (because rebasing might or might not
fast-forward), but "my branch" is also more specific than "all
branches". So which option should win? 🤔
On Wed, Feb 5, 2025 at 2:14 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
> Is there, then, an existing combination that means roughly to treat
> `git pull` with no other options like this:
> - if not rebasing, forbid merging and be equivalent to --ff-only
> - if rebasing is requested (because of branch.name.rebase or --rebase
> or …?), allow it
I think what we're missing is a branch.<name>.ffOnly option to make a
particular branch fast-forward only. Such an option would be
especially useful for the master branch, but you could set it on all
of your branches except the ones that you want to rebase. We could
even have a branch.autoSetupFfOnly option to turn on ffOnly
automatically for new branches.
-Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-07 2:35 ` Alex Henrie
@ 2025-02-10 20:26 ` D. Ben Knoble
2025-02-11 6:55 ` Alex Henrie
0 siblings, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-02-10 20:26 UTC (permalink / raw)
To: Alex Henrie
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Thu, Feb 6, 2025 at 9:36 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
>
> On Tue, Feb 4, 2025 at 8:11 PM D. Ben Knoble
> <ben.knoble+github@gmail.com> wrote:
> >
> > When running "git pull" with the following configuration options, we
> > fail to merge divergent branches:
> >
> > - pull.ff=only
> > - pull.rebase (unset)
> > - branch.<current_branch>.rebase=true
> >
> > Yet it seems that the user intended to make rebase the default for the
> > current branch while using --ff-only for non-rebase pulls.
>
> You make an interesting point. The idea is that more specific options
> override less specific options. In this case, "fast-forward only" is
> more specific than "rebase" (because rebasing might or might not
> fast-forward), but "my branch" is also more specific than "all
> branches". So which option should win? 🤔
Precisely! I think "my branch" is most specific here, but Junio's
argument is (if I understand it) that pull.ff=only is _stronger_,
regardless of specificity.
>
> On Wed, Feb 5, 2025 at 2:14 PM D. Ben Knoble
> <ben.knoble+github@gmail.com> wrote:
>
> > Is there, then, an existing combination that means roughly to treat
> > `git pull` with no other options like this:
> > - if not rebasing, forbid merging and be equivalent to --ff-only
> > - if rebasing is requested (because of branch.name.rebase or --rebase
> > or …?), allow it
>
> I think what we're missing is a branch.<name>.ffOnly option to make a
> particular branch fast-forward only. Such an option would be
> especially useful for the master branch, but you could set it on all
> of your branches except the ones that you want to rebase. We could
> even have a branch.autoSetupFfOnly option to turn on ffOnly
> automatically for new branches.
That is probably something that is missing, and might solve the
problem, but I don't know that these in particular are something I
need (read: want to implement).
How do you (and Junio, and others) feel about
pull.ff=onlyUnlessOverridden? The meaning would be "like --ff-only
except when branch.<name>.rebase says otherwise."
The name of the value can be workshopped (I initially thought of
"override" as a short value, but it may be too short to convey its
intended meaning). Perhaps "onlyOr[Branch]Rebase"?
I think this would be a smaller change that meets my needs without
changing the meaning of ff=only.
>
> -Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-10 20:26 ` D. Ben Knoble
@ 2025-02-11 6:55 ` Alex Henrie
2025-04-22 20:03 ` D. Ben Knoble
0 siblings, 1 reply; 13+ messages in thread
From: Alex Henrie @ 2025-02-11 6:55 UTC (permalink / raw)
To: D. Ben Knoble
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Mon, Feb 10, 2025 at 1:26 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
>
> On Thu, Feb 6, 2025 at 9:36 PM Alex Henrie <alexhenrie24@gmail.com> wrote:
> >
> > On Tue, Feb 4, 2025 at 8:11 PM D. Ben Knoble
> > <ben.knoble+github@gmail.com> wrote:
> > >
> > > When running "git pull" with the following configuration options, we
> > > fail to merge divergent branches:
> > >
> > > - pull.ff=only
> > > - pull.rebase (unset)
> > > - branch.<current_branch>.rebase=true
> > >
> > > Yet it seems that the user intended to make rebase the default for the
> > > current branch while using --ff-only for non-rebase pulls.
> >
> > You make an interesting point. The idea is that more specific options
> > override less specific options. In this case, "fast-forward only" is
> > more specific than "rebase" (because rebasing might or might not
> > fast-forward), but "my branch" is also more specific than "all
> > branches". So which option should win? 🤔
>
> Precisely! I think "my branch" is most specific here, but Junio's
> argument is (if I understand it) that pull.ff=only is _stronger_,
> regardless of specificity.
I can see it both ways here, though in general when the user's intent
is ambiguous, I think Git should default to the more conservative
operation.
> > On Wed, Feb 5, 2025 at 2:14 PM D. Ben Knoble
> > <ben.knoble+github@gmail.com> wrote:
> >
> > > Is there, then, an existing combination that means roughly to treat
> > > `git pull` with no other options like this:
> > > - if not rebasing, forbid merging and be equivalent to --ff-only
> > > - if rebasing is requested (because of branch.name.rebase or --rebase
> > > or …?), allow it
> >
> > I think what we're missing is a branch.<name>.ffOnly option to make a
> > particular branch fast-forward only. Such an option would be
> > especially useful for the master branch, but you could set it on all
> > of your branches except the ones that you want to rebase. We could
> > even have a branch.autoSetupFfOnly option to turn on ffOnly
> > automatically for new branches.
>
> That is probably something that is missing, and might solve the
> problem, but I don't know that these in particular are something I
> need (read: want to implement).
>
> How do you (and Junio, and others) feel about
> pull.ff=onlyUnlessOverridden? The meaning would be "like --ff-only
> except when branch.<name>.rebase says otherwise."
>
> The name of the value can be workshopped (I initially thought of
> "override" as a short value, but it may be too short to convey its
> intended meaning). Perhaps "onlyOr[Branch]Rebase"?
>
> I think this would be a smaller change that meets my needs without
> changing the meaning of ff=only.
In my opinion, the matrix of which pull options override which pull
options is already too hard to understand. Rather than add a new
dimension to pull.ff, I would much prefer to fill in the gap that is
the lack of a per-branch fast-forward setting. It might be more work
in the short term, but it's an investment:
pull.ff=onlyUnlessOverridden would only address your particular use
case, but a per-branch setting could address many others. For example,
the user could set branch.autoSetupRebase=true to make every branch
rebase by default, but override it with branch.master.ff=only to make
the master branch fast-forward only. Or the user could have
branch.<name>.rebase set to either true or false as appropriate for
each branch, but temporarily set branch.<name>.ff=only when they are
in the middle of work on a branch and don't want to accidentally bring
in upstream changes that would interrupt their work.
If you think that you can write the patch to implement
pull.ff=onlyUnlessOverridden on your own, I think you're capable of
implementing branch.<name>.ff=(true|false|only) and
branch.autoSetupFf=(true|false|only). Use the code for the existing
branch.<name>.rebase and branch.autoSetupRebase options as a guide,
and people like me are available on the mailing list to support you.
-Alex
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-05 21:14 ` D. Ben Knoble
2025-02-07 2:35 ` Alex Henrie
@ 2025-04-22 19:58 ` D. Ben Knoble
2025-04-22 20:05 ` D. Ben Knoble
1 sibling, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-04-22 19:58 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Wed, Feb 5, 2025 at 4:14 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
>
> On Wed, Feb 5, 2025 at 12:42 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> >
> > >> So, I dunno.
> > >
> > > Agreed that if pull.ff=only is supposed to override all other options
> > > (except those on the command-line), this might be wrong. And `git pull
> > > --rebase` works in the scenario I described.
> >
> > Yeah, I view --ff-only as a safety measure for the user to say "my
> > workflow is to make sure I do not have anything locally cooking on
> > my branch when integrating with the other side, and stop me if I
> > somehow made a mistake". If rebase or other options override, the
> > folks in the rebasing camp, unlike in the merging camp, cannot
> > benefit from such safety measure, which worries me.
>
> Is there, then, an existing combination that means roughly to treat
> `git pull` with no other options like this:
> - if not rebasing, forbid merging and be equivalent to --ff-only
> - if rebasing is requested (because of branch.name.rebase or --rebase
> or …?), allow it
>
> In other words, something like a pull.merge=ff (or ff-only) meaning to
> apply the rules I've attempted to describe, in which case I would
> leave pull.ff unset?
>
> I suppose pull.rebase=true is close, but is not quite the same for me
> (I'd like to be warned when this would imply a non-fast-forward for a
> main branch, though the "rebasing" logs might be sufficient)…
FWIW, I found some tests that indicate, to me, that I should use
pull.rebase=true (or merges) + branch.<name>.rebase=false for the case
I described: https://github.com/git/git/blob/08bdfd453584e489d5a551aecbdcb77584e1b958/t/t5520-pull.sh#L505-L514
So it turns out my itch was already scratched.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-02-11 6:55 ` Alex Henrie
@ 2025-04-22 20:03 ` D. Ben Knoble
0 siblings, 0 replies; 13+ messages in thread
From: D. Ben Knoble @ 2025-04-22 20:03 UTC (permalink / raw)
To: Alex Henrie
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Tue, Feb 11, 2025 at 1:56 AM Alex Henrie <alexhenrie24@gmail.com> wrote:
[snip]
> > > On Wed, Feb 5, 2025 at 2:14 PM D. Ben Knoble
> > > <ben.knoble+github@gmail.com> wrote:
> > >
> > > > Is there, then, an existing combination that means roughly to treat
> > > > `git pull` with no other options like this:
> > > > - if not rebasing, forbid merging and be equivalent to --ff-only
> > > > - if rebasing is requested (because of branch.name.rebase or --rebase
> > > > or …?), allow it
> > >
> > > I think what we're missing is a branch.<name>.ffOnly option to make a
> > > particular branch fast-forward only. Such an option would be
> > > especially useful for the master branch, but you could set it on all
> > > of your branches except the ones that you want to rebase. We could
> > > even have a branch.autoSetupFfOnly option to turn on ffOnly
> > > automatically for new branches.
> >
> > That is probably something that is missing, and might solve the
> > problem, but I don't know that these in particular are something I
> > need (read: want to implement).
> >
> > How do you (and Junio, and others) feel about
> > pull.ff=onlyUnlessOverridden? The meaning would be "like --ff-only
> > except when branch.<name>.rebase says otherwise."
> >
> > The name of the value can be workshopped (I initially thought of
> > "override" as a short value, but it may be too short to convey its
> > intended meaning). Perhaps "onlyOr[Branch]Rebase"?
> >
> > I think this would be a smaller change that meets my needs without
> > changing the meaning of ff=only.
>
> In my opinion, the matrix of which pull options override which pull
> options is already too hard to understand. Rather than add a new
> dimension to pull.ff, I would much prefer to fill in the gap that is
> the lack of a per-branch fast-forward setting. It might be more work
> in the short term, but it's an investment:
> pull.ff=onlyUnlessOverridden would only address your particular use
> case, but a per-branch setting could address many others. For example,
> the user could set branch.autoSetupRebase=true to make every branch
> rebase by default, but override it with branch.master.ff=only to make
> the master branch fast-forward only. Or the user could have
> branch.<name>.rebase set to either true or false as appropriate for
> each branch, but temporarily set branch.<name>.ff=only when they are
> in the middle of work on a branch and don't want to accidentally bring
> in upstream changes that would interrupt their work.
>
> If you think that you can write the patch to implement
> pull.ff=onlyUnlessOverridden on your own, I think you're capable of
> implementing branch.<name>.ff=(true|false|only) and
> branch.autoSetupFf=(true|false|only). Use the code for the existing
> branch.<name>.rebase and branch.autoSetupRebase options as a guide,
> and people like me are available on the mailing list to support you.
>
> -Alex
I actually did start working on this by first writing documentation; I
got about as far as saying that branch.<name>.rebase overrides
branch.<name>.ff when pulling unless it is only and that
branch.<name>.ff overrides merge.ff before I realized that I was
constructing a complex decision-matrix of how config and CLI options
affect what happens, and it's already overwhelming enough…
It would actually be nice to spell out the matrix somewhere, but I can
do that in a blog post if I ever find time. I'll leave it to others to
increase the complexity of that matrix :)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-04-22 19:58 ` D. Ben Knoble
@ 2025-04-22 20:05 ` D. Ben Knoble
2025-04-22 20:07 ` D. Ben Knoble
0 siblings, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-04-22 20:05 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Alex Henrie, Ævar Arnfjörð Bjarmason,
Felipe Contreras, Patrick Steinhardt, Elijah Newren
On Tue, Apr 22, 2025 at 3:58 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
>
> On Wed, Feb 5, 2025 at 4:14 PM D. Ben Knoble
> <ben.knoble+github@gmail.com> wrote:
> >
> > On Wed, Feb 5, 2025 at 12:42 PM Junio C Hamano <gitster@pobox.com> wrote:
> > >
> > > "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> > >
> > > >> So, I dunno.
> > > >
> > > > Agreed that if pull.ff=only is supposed to override all other options
> > > > (except those on the command-line), this might be wrong. And `git pull
> > > > --rebase` works in the scenario I described.
> > >
> > > Yeah, I view --ff-only as a safety measure for the user to say "my
> > > workflow is to make sure I do not have anything locally cooking on
> > > my branch when integrating with the other side, and stop me if I
> > > somehow made a mistake". If rebase or other options override, the
> > > folks in the rebasing camp, unlike in the merging camp, cannot
> > > benefit from such safety measure, which worries me.
> >
> > Is there, then, an existing combination that means roughly to treat
> > `git pull` with no other options like this:
> > - if not rebasing, forbid merging and be equivalent to --ff-only
> > - if rebasing is requested (because of branch.name.rebase or --rebase
> > or …?), allow it
> >
> > In other words, something like a pull.merge=ff (or ff-only) meaning to
> > apply the rules I've attempted to describe, in which case I would
> > leave pull.ff unset?
> >
> > I suppose pull.rebase=true is close, but is not quite the same for me
> > (I'd like to be warned when this would imply a non-fast-forward for a
> > main branch, though the "rebasing" logs might be sufficient)…
>
> FWIW, I found some tests that indicate, to me, that I should use
> pull.rebase=true (or merges) + branch.<name>.rebase=false for the case
> I described: https://github.com/git/git/blob/08bdfd453584e489d5a551aecbdcb77584e1b958/t/t5520-pull.sh#L505-L514
>
> So it turns out my itch was already scratched.
I left out the commit reference, whose message described what I think
I originally wanted:
> my main or master branch is typically fast-forward only, while I want my
> topic branches to be rebased; preferably, all of those things happen
> for just "git pull."
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-04-22 20:05 ` D. Ben Knoble
@ 2025-04-22 20:07 ` D. Ben Knoble
2025-04-22 20:30 ` Junio C Hamano
0 siblings, 1 reply; 13+ messages in thread
From: D. Ben Knoble @ 2025-04-22 20:07 UTC (permalink / raw)
To: git
On Tue, Apr 22, 2025 at 4:05 PM D. Ben Knoble
<ben.knoble+github@gmail.com> wrote:
>
> On Tue, Apr 22, 2025 at 3:58 PM D. Ben Knoble
> <ben.knoble+github@gmail.com> wrote:
> >
> > On Wed, Feb 5, 2025 at 4:14 PM D. Ben Knoble
> > <ben.knoble+github@gmail.com> wrote:
> > >
> > > On Wed, Feb 5, 2025 at 12:42 PM Junio C Hamano <gitster@pobox.com> wrote:
> > > >
> > > > "D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
> > > >
> > > > >> So, I dunno.
> > > > >
> > > > > Agreed that if pull.ff=only is supposed to override all other options
> > > > > (except those on the command-line), this might be wrong. And `git pull
> > > > > --rebase` works in the scenario I described.
> > > >
> > > > Yeah, I view --ff-only as a safety measure for the user to say "my
> > > > workflow is to make sure I do not have anything locally cooking on
> > > > my branch when integrating with the other side, and stop me if I
> > > > somehow made a mistake". If rebase or other options override, the
> > > > folks in the rebasing camp, unlike in the merging camp, cannot
> > > > benefit from such safety measure, which worries me.
> > >
> > > Is there, then, an existing combination that means roughly to treat
> > > `git pull` with no other options like this:
> > > - if not rebasing, forbid merging and be equivalent to --ff-only
> > > - if rebasing is requested (because of branch.name.rebase or --rebase
> > > or …?), allow it
> > >
> > > In other words, something like a pull.merge=ff (or ff-only) meaning to
> > > apply the rules I've attempted to describe, in which case I would
> > > leave pull.ff unset?
> > >
> > > I suppose pull.rebase=true is close, but is not quite the same for me
> > > (I'd like to be warned when this would imply a non-fast-forward for a
> > > main branch, though the "rebasing" logs might be sufficient)…
> >
> > FWIW, I found some tests that indicate, to me, that I should use
> > pull.rebase=true (or merges) + branch.<name>.rebase=false for the case
> > I described: https://github.com/git/git/blob/08bdfd453584e489d5a551aecbdcb77584e1b958/t/t5520-pull.sh#L505-L514
> >
> > So it turns out my itch was already scratched.
>
> I left out the commit reference, whose message described what I think
> I originally wanted:
>
> > my main or master branch is typically fast-forward only, while I want my
> > topic branches to be rebased; preferably, all of those things happen
> > for just "git pull."
Since I apparently hit Send too fast, dropped the CC list to just add
the reference I repeatedly forgot to paste:
6b37dff17f (pull: introduce a pull.rebase option to enable --rebase, 2011-11-06)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only
2025-04-22 20:07 ` D. Ben Knoble
@ 2025-04-22 20:30 ` Junio C Hamano
0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2025-04-22 20:30 UTC (permalink / raw)
To: D. Ben Knoble; +Cc: git
"D. Ben Knoble" <ben.knoble+github@gmail.com> writes:
>> > So it turns out my itch was already scratched.
>> ...
>> I left out the commit reference, whose message described what I think
>> I originally wanted:
>
> 6b37dff17f (pull: introduce a pull.rebase option to enable --rebase, 2011-11-06)
Good to know that your itch was already scratched ;-)
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-04-22 20:30 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-05 3:06 [PATCH] pull: allow branch.<name>.rebase to override pull.ff=only D. Ben Knoble
2025-02-05 13:08 ` Junio C Hamano
2025-02-05 16:36 ` D. Ben Knoble
2025-02-05 17:42 ` Junio C Hamano
2025-02-05 21:14 ` D. Ben Knoble
2025-02-07 2:35 ` Alex Henrie
2025-02-10 20:26 ` D. Ben Knoble
2025-02-11 6:55 ` Alex Henrie
2025-04-22 20:03 ` D. Ben Knoble
2025-04-22 19:58 ` D. Ben Knoble
2025-04-22 20:05 ` D. Ben Knoble
2025-04-22 20:07 ` D. Ben Knoble
2025-04-22 20:30 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).