* [PATCH v2 01/13] doc: pull: explain what is a fast-forward
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 02/13] pull: improve default warning Felipe Contreras
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
We want users to know what is a fast-forward in order to understand the
default warning.
Let's expand the explanation in order to cover both the simple, and the
complex cases with as much detail as possible.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/git-pull.txt | 41 ++++++++++++++++++++++++++++++++------
1 file changed, 35 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 5c3fb67c01..142df1c4a1 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -41,16 +41,41 @@ Assume the following history exists and the current branch is
------------
A---B---C master on origin
/
- D---E---F---G master
+ D---E master
^
origin/master in your repository
------------
Then "`git pull`" will fetch and replay the changes from the remote
`master` branch since it diverged from the local `master` (i.e., `E`)
-until its current commit (`C`) on top of `master` and record the
-result in a new commit along with the names of the two parent commits
-and a log message from the user describing the changes.
+until its current commit (`C`) on top of `master`.
+
+After the remote changes have been synchronized, the local `master` will
+be fast-forwarded to the same commit as the remote one, therefore
+creating a linear history.
+
+------------
+ D---E---A---B---C master, origin/master
+------------
+
+However, a non-fast-forward case looks very different:
+
+------------
+ A---B---C origin/master
+ /
+ D---E---F---G master
+------------
+
+If there are additional changes in the local `master`, it's
+not possible to fast-forward, so a decision must be made how to
+synchronize the local, and remote brances.
+
+In these situations `git pull` will warn you about your possible
+options, which are either merge (`--no-rebase`), or rebase (`--rebase`).
+However, by default it will continue doing a merge.
+
+A merge will create a new commit with two parent commits (`G` and `C`)
+and a log message describing the changes, which you can edit.
------------
A---B---C origin/master
@@ -58,8 +83,11 @@ and a log message from the user describing the changes.
D---E---F---G---H master
------------
+Once the merge commit is created (`H`), your local `master` branch has
+incorporated the changes of the remote `master` branch.
+
See linkgit:git-merge[1] for details, including how conflicts
-are presented and handled.
+are presented and handled, and also linkgit:git-rebase[1].
In Git 1.7.0 or later, to cancel a conflicting merge, use
`git reset --merge`. *Warning*: In older versions of Git, running 'git pull'
@@ -248,7 +276,8 @@ version.
SEE ALSO
--------
-linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-config[1]
+linkgit:git-fetch[1], linkgit:git-merge[1], linkgit:git-rebase[1],
+linkgit:git-config[1]
GIT
---
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 02/13] pull: improve default warning
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 01/13] doc: pull: explain what is a fast-forward Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 03/13] pull: cleanup autostash check Felipe Contreras
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
According to feedback from GitHub trainers [1], most newcomers don't
understand what a rebase is. So in the default warning we want to
provide our users with a command that does the most sensible thing,
fixes the divergence, gets rid of the warning, with the minimum mental
effort, and happens to be the default:
git pull --no-rebase (later --merge)
In addition, we don't want to start by recommending a permanent
configuration, but a temporary solution so they start training their
fingers and maybe learn how to do a rebase. So we start with the commands.
Also, we need to be clear about what we mean by "specifying"; merge, or
rebase.
Moreover, thanks to the previous patch now "git pull --help" explains
what a fast-forward is, let's mention that reference.
And finally, use --global in the configuration commands like we did with
push.default.
[1] https://lore.kernel.org/git/20130909201751.GA14437@sigill.intra.peff.net/
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index e8927fc2ff..a766d9762c 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -927,18 +927,20 @@ static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_
static void show_advice_pull_non_ff(void)
{
- advise(_("Pulling without specifying how to reconcile divergent branches is\n"
- "discouraged. You can squelch this message by running one of the following\n"
- "commands sometime before your next pull:\n"
+ advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n"
+ "you need to specify if you want a merge, or a rebase.\n"
"\n"
- " git config pull.rebase false # merge (the default strategy)\n"
- " git config pull.rebase true # rebase\n"
- " git config pull.ff only # fast-forward only\n"
+ " git pull --no-rebase # the default (merge)\n"
+ " git pull --rebase\n"
"\n"
- "You can replace \"git config\" with \"git config --global\" to set a default\n"
- "preference for all repositories. You can also pass --rebase, --no-rebase,\n"
- "or --ff-only on the command line to override the configured default per\n"
- "invocation.\n"));
+ "You can squelch this message by running one of the following commands:\n"
+ "\n"
+ " git config --global pull.rebase false # merge\n"
+ " git config --global pull.rebase true # rebase\n"
+ " git config --global pull.ff only # fast-forward only\n"
+ "\n"
+ "If unsure, run \"git pull --no-rebase\".\n"
+ "Read \"git pull --help\" for more information."));
}
int cmd_pull(int argc, const char **argv, const char *prefix)
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 03/13] pull: cleanup autostash check
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 01/13] doc: pull: explain what is a fast-forward Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 02/13] pull: improve default warning Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 04/13] pull: trivial cleanup Felipe Contreras
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Currently "git pull --rebase" takes a shortcut in the case a
fast-forward merge is possible; run_merge() is called with --ff-only.
However, "git merge" didn't have an --autostash option, so, when "git
pull --rebase --autostash" was called *and* the fast-forward merge
shortcut was taken, then the pull failed.
This was fixed in commit f15e7cf5cc (pull: ff --rebase --autostash
works in dirty repo, 2017-06-01) by simply skipping the fast-forward
merge shortcut.
Later on "git merge" learned the --autostash option [a03b55530a
(merge: teach --autostash option, 2020-04-07)], and so did "git pull"
[d9f15d37f1 (pull: pass --autostash to merge, 2020-04-07)].
Therefore it's not necessary to skip the fast-forward merge shortcut
anymore when called with --rebase --autostash.
Let's always take the fast-forward merge shortcut by essentially
reverting f15e7cf5cc.
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index a766d9762c..42cd6c38d8 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -949,7 +949,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
struct oid_array merge_heads = OID_ARRAY_INIT;
struct object_id orig_head, curr_head;
struct object_id rebase_fork_point;
- int autostash;
int rebase_unspecified = 0;
int can_ff;
@@ -984,8 +983,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (get_oid("HEAD", &orig_head))
oidclr(&orig_head);
- autostash = config_autostash;
if (opt_rebase) {
+ int autostash = config_autostash;
if (opt_autostash != -1)
autostash = opt_autostash;
@@ -1067,13 +1066,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
submodule_touches_in_range(the_repository, &upstream, &curr_head))
die(_("cannot rebase with locally recorded submodule modifications"));
- if (!autostash) {
- if (can_ff) {
- /* we can fast-forward this without invoking rebase */
- opt_ff = "--ff-only";
- ran_ff = 1;
- ret = run_merge();
- }
+
+ if (can_ff) {
+ /* we can fast-forward this without invoking rebase */
+ opt_ff = "--ff-only";
+ ran_ff = 1;
+ ret = run_merge();
}
if (!ran_ff)
ret = run_rebase(&newbase, &upstream);
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 04/13] pull: trivial cleanup
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (2 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 03/13] pull: cleanup autostash check Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 05/13] pull: trivial whitespace style fix Felipe Contreras
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
There's no need to store ran_ff. Now it's obvious from the conditionals.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 42cd6c38d8..21089e5a29 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1055,7 +1055,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (opt_rebase) {
int ret = 0;
- int ran_ff = 0;
struct object_id newbase;
struct object_id upstream;
@@ -1070,11 +1069,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (can_ff) {
/* we can fast-forward this without invoking rebase */
opt_ff = "--ff-only";
- ran_ff = 1;
ret = run_merge();
- }
- if (!ran_ff)
+ } else {
ret = run_rebase(&newbase, &upstream);
+ }
if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 05/13] pull: trivial whitespace style fix
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (3 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 04/13] pull: trivial cleanup Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 06/13] pull: introduce --merge option Felipe Contreras
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Two spaces unaligned to anything is not part of the coding-style. A
single tab is.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 21089e5a29..48e25a5061 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -126,9 +126,9 @@ static struct option pull_options[] = {
/* Options passed to git-merge or git-rebase */
OPT_GROUP(N_("Options related to merging")),
OPT_CALLBACK_F('r', "rebase", &opt_rebase,
- "(false|true|merges|preserve|interactive)",
- N_("incorporate changes by rebasing rather than merging"),
- PARSE_OPT_OPTARG, parse_opt_rebase),
+ "(false|true|merges|preserve|interactive)",
+ N_("incorporate changes by rebasing rather than merging"),
+ PARSE_OPT_OPTARG, parse_opt_rebase),
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
N_("do not show a diffstat at the end of the merge"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 06/13] pull: introduce --merge option
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (4 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 05/13] pull: trivial whitespace style fix Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 07/13] rebase: add REBASE_DEFAULT Felipe Contreras
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Previously --no-rebase (which still works for backwards compatibility).
Now we can update the default warning, and the git-pull(1) man page to
use --merge instead of the non-intuitive --no-rebase.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/git-pull.txt | 11 +++++++----
builtin/pull.c | 6 ++++--
t/t7601-merge-pull-config.sh | 8 ++++----
3 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 142df1c4a1..195496e63d 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -71,8 +71,8 @@ not possible to fast-forward, so a decision must be made how to
synchronize the local, and remote brances.
In these situations `git pull` will warn you about your possible
-options, which are either merge (`--no-rebase`), or rebase (`--rebase`).
-However, by default it will continue doing a merge.
+options, which are either `--merge`, or `--rebase`. However, by default
+it will continue doing a merge.
A merge will create a new commit with two parent commits (`G` and `C`)
and a log message describing the changes, which you can edit.
@@ -159,8 +159,11 @@ It rewrites history, which does not bode well when you
published that history already. Do *not* use this option
unless you have read linkgit:git-rebase[1] carefully.
---no-rebase::
- Override earlier --rebase.
+-m::
+--merge::
+ Force a merge.
++
+Previously this was --no-rebase, but that usage has been deprecated.
Options related to fetching
~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/builtin/pull.c b/builtin/pull.c
index 48e25a5061..1336b59b21 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -129,6 +129,8 @@ static struct option pull_options[] = {
"(false|true|merges|preserve|interactive)",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase),
+ OPT_SET_INT('m', "merge", &opt_rebase,
+ N_("incorporate changes by merging"), REBASE_FALSE),
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
N_("do not show a diffstat at the end of the merge"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -930,7 +932,7 @@ static void show_advice_pull_non_ff(void)
advise(_("Pulling without specifying how to reconcile divergent branches is discouraged;\n"
"you need to specify if you want a merge, or a rebase.\n"
"\n"
- " git pull --no-rebase # the default (merge)\n"
+ " git pull --merge # the default\n"
" git pull --rebase\n"
"\n"
"You can squelch this message by running one of the following commands:\n"
@@ -939,7 +941,7 @@ static void show_advice_pull_non_ff(void)
" git config --global pull.rebase true # rebase\n"
" git config --global pull.ff only # fast-forward only\n"
"\n"
- "If unsure, run \"git pull --no-rebase\".\n"
+ "If unsure, run \"git pull --merge\".\n"
"Read \"git pull --help\" for more information."));
}
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 52e8ccc933..6d03e0b9fe 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -60,9 +60,9 @@ test_expect_success 'pull.rebase not set and --rebase given' '
test_i18ngrep ! "Pulling without specifying how to reconcile" err
'
-test_expect_success 'pull.rebase not set and --no-rebase given' '
+test_expect_success 'pull.rebase not set and --merge given' '
git reset --hard c0 &&
- git pull --no-rebase . c1 2>err &&
+ git pull --merge . c1 2>err &&
test_i18ngrep ! "Pulling without specifying how to reconcile" err
'
@@ -119,9 +119,9 @@ test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)'
test_i18ngrep ! "Pulling without specifying how to reconcile" err
'
-test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward)' '
+test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
git reset --hard c2 &&
- git pull --no-rebase . c1 2>err &&
+ git pull --merge . c1 2>err &&
test_i18ngrep ! "Pulling without specifying how to reconcile" err
'
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 07/13] rebase: add REBASE_DEFAULT
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (5 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 06/13] pull: introduce --merge option Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 08/13] pull: move configurations fetches Felipe Contreras
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
By introducing a default we can distinguish when the user has forced an
option.
Therefore there's no need pass around an extra variable variable (it's
the same as opt_rebase == REBASE_DEFAULT), nor is there any need to
initialize opt_rebase to an invalid value.
Additionally this will allow us to override the default with a
configuration, and subsequently the configuration with arguments.
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 27 ++++++++++++---------------
rebase.h | 3 ++-
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 1336b59b21..c0a90fa741 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -74,7 +74,7 @@ static char *opt_progress;
static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
/* Options passed to git-merge or git-rebase */
-static enum rebase_type opt_rebase = -1;
+static enum rebase_type opt_rebase;
static char *opt_diffstat;
static char *opt_log;
static char *opt_signoff;
@@ -326,7 +326,7 @@ static const char *config_get_ff(void)
* looks for the value of "pull.rebase". If both configuration keys do not
* exist, returns REBASE_FALSE.
*/
-static enum rebase_type config_get_rebase(int *rebase_unspecified)
+static enum rebase_type config_get_rebase(void)
{
struct branch *curr_branch = branch_get("HEAD");
const char *value;
@@ -346,9 +346,7 @@ static enum rebase_type config_get_rebase(int *rebase_unspecified)
if (!git_config_get_value("pull.rebase", &value))
return parse_config_rebase("pull.rebase", value, 1);
- *rebase_unspecified = 1;
-
- return REBASE_FALSE;
+ return REBASE_DEFAULT;
}
/**
@@ -443,7 +441,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
const char *remote = curr_branch ? curr_branch->remote_name : NULL;
if (*refspecs) {
- if (opt_rebase)
+ if (opt_rebase >= REBASE_TRUE)
fprintf_ln(stderr, _("There is no candidate for rebasing against among the refs that you just fetched."));
else
fprintf_ln(stderr, _("There are no candidates for merging among the refs that you just fetched."));
@@ -456,7 +454,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
repo);
} else if (!curr_branch) {
fprintf_ln(stderr, _("You are not currently on a branch."));
- if (opt_rebase)
+ if (opt_rebase >= REBASE_TRUE)
fprintf_ln(stderr, _("Please specify which branch you want to rebase against."));
else
fprintf_ln(stderr, _("Please specify which branch you want to merge with."));
@@ -471,7 +469,7 @@ static void NORETURN die_no_merge_candidates(const char *repo, const char **refs
remote_name = _("<remote>");
fprintf_ln(stderr, _("There is no tracking information for the current branch."));
- if (opt_rebase)
+ if (opt_rebase >= REBASE_TRUE)
fprintf_ln(stderr, _("Please specify which branch you want to rebase against."));
else
fprintf_ln(stderr, _("Please specify which branch you want to merge with."));
@@ -951,7 +949,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
struct oid_array merge_heads = OID_ARRAY_INIT;
struct object_id orig_head, curr_head;
struct object_id rebase_fork_point;
- int rebase_unspecified = 0;
int can_ff;
if (!getenv("GIT_REFLOG_ACTION"))
@@ -973,8 +970,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (!opt_ff)
opt_ff = xstrdup_or_null(config_get_ff());
- if (opt_rebase < 0)
- opt_rebase = config_get_rebase(&rebase_unspecified);
+ if (!opt_rebase)
+ opt_rebase = config_get_rebase();
if (read_cache_unmerged())
die_resolve_conflict("pull");
@@ -985,7 +982,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
if (get_oid("HEAD", &orig_head))
oidclr(&orig_head);
- if (opt_rebase) {
+ if (opt_rebase >= REBASE_TRUE) {
int autostash = config_autostash;
if (opt_autostash != -1)
autostash = opt_autostash;
@@ -1045,17 +1042,17 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
die(_("Cannot merge multiple branches into empty head."));
return pull_into_void(merge_heads.oid, &curr_head);
}
- if (opt_rebase && merge_heads.nr > 1)
+ if (opt_rebase >= REBASE_TRUE && merge_heads.nr > 1)
die(_("Cannot rebase onto multiple branches."));
can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
- if (rebase_unspecified && !opt_ff && !can_ff) {
+ if (!opt_rebase && !opt_ff && !can_ff) {
if (opt_verbosity >= 0)
show_advice_pull_non_ff();
}
- if (opt_rebase) {
+ if (opt_rebase >= REBASE_TRUE) {
int ret = 0;
struct object_id newbase;
diff --git a/rebase.h b/rebase.h
index cc723d4748..34d4acfd74 100644
--- a/rebase.h
+++ b/rebase.h
@@ -3,7 +3,8 @@
enum rebase_type {
REBASE_INVALID = -1,
- REBASE_FALSE = 0,
+ REBASE_DEFAULT = 0,
+ REBASE_FALSE,
REBASE_TRUE,
REBASE_PRESERVE,
REBASE_MERGES,
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 08/13] pull: move configurations fetches
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (6 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 07/13] rebase: add REBASE_DEFAULT Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 09/13] pull: show warning with --ff options Felipe Contreras
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Now that we have FETCH_DEFAULT we can fetch the configuration before
parsing the argument options.
The options will override the configuration, and if they don't;
opt_rebase will remain being FETCH_DEFAULT.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index c0a90fa741..2fd5e44e03 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -951,6 +951,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
struct object_id rebase_fork_point;
int can_ff;
+ opt_ff = xstrdup_or_null(config_get_ff());
+ opt_rebase = config_get_rebase();
+
if (!getenv("GIT_REFLOG_ACTION"))
set_reflog_message(argc, argv);
@@ -967,12 +970,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
parse_repo_refspecs(argc, argv, &repo, &refspecs);
- if (!opt_ff)
- opt_ff = xstrdup_or_null(config_get_ff());
-
- if (!opt_rebase)
- opt_rebase = config_get_rebase();
-
if (read_cache_unmerged())
die_resolve_conflict("pull");
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 09/13] pull: show warning with --ff options
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (7 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 08/13] pull: move configurations fetches Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 10/13] pull: add pull.mode Felipe Contreras
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
We want the user to specify either --merge or --rebase, if she doesn't
we throw a warning.
Using --ff, --no-ff, or --ff-only does not make the merge explicit.
For example, if the user has the following configuration:
git config pull.rebase true
git pull --no-ff
A merge is not implied.
We should be consistent and either imply a merge; in which case a
previous "pull.rebase=true" configuration is overridden, or don't; in
which case the warning should be thrown.
Cc: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 2 +-
t/t7601-merge-pull-config.sh | 12 ++++++------
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index 2fd5e44e03..f1a03ccd14 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1044,7 +1044,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
- if (!opt_rebase && !opt_ff && !can_ff) {
+ if (!opt_rebase && !can_ff) {
if (opt_verbosity >= 0)
show_advice_pull_non_ff();
}
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 6d03e0b9fe..7c4607191a 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -96,21 +96,21 @@ test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
git reset --hard c2 &&
test_config pull.ff true &&
git pull . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'pull.rebase not set and pull.ff=false (not-fast-forward)' '
git reset --hard c2 &&
test_config pull.ff false &&
git pull . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'pull.rebase not set and pull.ff=only (not-fast-forward)' '
git reset --hard c2 &&
test_config pull.ff only &&
test_must_fail git pull . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
@@ -128,19 +128,19 @@ test_expect_success 'pull.rebase not set and --merge given (not-fast-forward)' '
test_expect_success 'pull.rebase not set and --ff given (not-fast-forward)' '
git reset --hard c2 &&
git pull --ff . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'pull.rebase not set and --no-ff given (not-fast-forward)' '
git reset --hard c2 &&
git pull --no-ff . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'pull.rebase not set and --ff-only given (not-fast-forward)' '
git reset --hard c2 &&
test_must_fail git pull --ff-only . c1 2>err &&
- test_i18ngrep ! "Pulling without specifying how to reconcile" err
+ test_i18ngrep "Pulling without specifying how to reconcile" err
'
test_expect_success 'merge c1 with c2' '
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 10/13] pull: add pull.mode
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (8 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 09/13] pull: show warning with --ff options Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 11/13] pull: add pull.mode=fast-forward Felipe Contreras
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras,
Theodore Ts'o
The evolution of pull options has somewhat served most users, however,
they have been found lacking for a very needed trio: merge / rebase /
fast-forward-only.
Another thing that is missing is the possibility to specify pull options
on a per-repository basis, as Theodore Ts'o suggested long time ago
[1].
This patch adds a pull.mode option with two possible values (for now);
merge and rebase. If set, it overrides what the user has specified in
pull.rebase, and it's updated with either --merge, or --rebase.
In addition to pull.mode, a 'remote.<name>.pullmode' configuration is
introduced, so the user can specify the desired options based on the
source of the pull, rather than the destination.
And of course; new modes like fast-forward-only can be introduced later
on.
[1] https://lore.kernel.org/git/20130312212027.GE14792@thunk.org
Cc: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/config/pull.txt | 6 +++
Documentation/config/remote.txt | 6 +++
Documentation/git-pull.txt | 3 +-
builtin/pull.c | 81 +++++++++++++++++++++++++++++++--
rebase.c | 10 ++++
rebase.h | 9 ++++
t/t5520-pull.sh | 54 ++++++++++++++++++++++
t/t7601-merge-pull-config.sh | 14 ++++++
8 files changed, 178 insertions(+), 5 deletions(-)
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index 5404830609..f4385cde33 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -29,6 +29,12 @@ mode.
it unless you understand the implications (see linkgit:git-rebase[1]
for details).
+pull.mode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the fetched branch. The possible values are 'merge',
+ and 'rebase'. See "branch.<name>.pullmode" for setting this on a
+ per-branch basis.
+
pull.octopus::
The default merge strategy to use when pulling multiple branches
at once.
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index a8e6437a90..a732c92cf5 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -84,3 +84,9 @@ remote.<name>.promisor::
remote.<name>.partialclonefilter::
The filter that will be applied when fetching from this
promisor remote.
+
+remote.<name>.pullmode::
+ When "git pull" is run, this determines if it would either merge or
+ rebase the branches from this remote. The possible values are 'merge', and
+ 'rebase'. See "pull.mode" for doing this in a non
+ repository-specific manner.
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 195496e63d..65f8b16a7d 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -72,7 +72,8 @@ synchronize the local, and remote brances.
In these situations `git pull` will warn you about your possible
options, which are either `--merge`, or `--rebase`. However, by default
-it will continue doing a merge.
+it will continue doing a merge (you can change that with the `pull.mode`
+configuration).
A merge will create a new commit with two parent commits (`G` and `C`)
and a log message describing the changes, which you can edit.
diff --git a/builtin/pull.c b/builtin/pull.c
index f1a03ccd14..bfadd585c7 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -27,6 +27,8 @@
#include "commit-reach.h"
#include "sequencer.h"
+static enum pull_mode_type mode;
+
/**
* Parses the value of --rebase. If value is a false value, returns
* REBASE_FALSE. If value is a true value, returns REBASE_TRUE. If value is
@@ -49,6 +51,14 @@ static enum rebase_type parse_config_rebase(const char *key, const char *value,
return REBASE_INVALID;
}
+static enum pull_mode_type parse_config_pull_mode(const char *key, const char *value)
+{
+ enum pull_mode_type v = pull_mode_parse_value(value);
+ if (v == PULL_MODE_INVALID)
+ die(_("Invalid value for %s: %s"), key, value);
+ return v;
+}
+
/**
* Callback for --rebase, which parses arg with parse_config_rebase().
*/
@@ -60,9 +70,21 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset
*value = parse_config_rebase("--rebase", arg, 0);
else
*value = unset ? REBASE_FALSE : REBASE_TRUE;
+
+ if (*value > 0)
+ mode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
+
return *value == REBASE_INVALID ? -1 : 0;
}
+static int parse_opt_merge(const struct option *opt, const char *arg, int unset)
+{
+ enum rebase_type *value = opt->value;
+ mode = PULL_MODE_MERGE;
+ *value = REBASE_FALSE;
+ return 0;
+}
+
static const char * const pull_usage[] = {
N_("git pull [<options>] [<repository> [<refspec>...]]"),
NULL
@@ -129,8 +151,9 @@ static struct option pull_options[] = {
"(false|true|merges|preserve|interactive)",
N_("incorporate changes by rebasing rather than merging"),
PARSE_OPT_OPTARG, parse_opt_rebase),
- OPT_SET_INT('m', "merge", &opt_rebase,
- N_("incorporate changes by merging"), REBASE_FALSE),
+ OPT_CALLBACK_F('m', "merge", &opt_rebase, NULL,
+ N_("incorporate changes by merging"),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_merge),
OPT_PASSTHRU('n', NULL, &opt_diffstat, NULL,
N_("do not show a diffstat at the end of the merge"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG),
@@ -349,6 +372,31 @@ static enum rebase_type config_get_rebase(void)
return REBASE_DEFAULT;
}
+static enum pull_mode_type config_get_pull_mode(const char *repo)
+{
+ const char *value;
+ struct remote *remote;
+
+ remote = remote_get(repo);
+
+ if (remote) {
+ char *key = xstrfmt("remote.%s.pullmode", remote->name);
+
+ if (!git_config_get_value(key, &value)) {
+ enum pull_mode_type ret = parse_config_pull_mode(key, value);
+ free(key);
+ return ret;
+ }
+
+ free(key);
+ }
+
+ if (!git_config_get_value("pull.mode", &value))
+ return parse_config_pull_mode("pull.mode", value);
+
+ return PULL_MODE_DEFAULT;
+}
+
/**
* Read config variables.
*/
@@ -935,8 +983,8 @@ static void show_advice_pull_non_ff(void)
"\n"
"You can squelch this message by running one of the following commands:\n"
"\n"
- " git config --global pull.rebase false # merge\n"
- " git config --global pull.rebase true # rebase\n"
+ " git config --global pull.mode merge\n"
+ " git config --global pull.mode rebase\n"
" git config --global pull.ff only # fast-forward only\n"
"\n"
"If unsure, run \"git pull --merge\".\n"
@@ -970,6 +1018,31 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
parse_repo_refspecs(argc, argv, &repo, &refspecs);
+ /*
+ * If the user has not specified --merge or --rebase, fetch pull.mode to override
+ * pull.rename.
+ */
+ if (!mode) {
+ mode = config_get_pull_mode(repo);
+
+ switch (mode) {
+ case PULL_MODE_MERGE:
+ opt_rebase = REBASE_FALSE;
+ break;
+ case PULL_MODE_REBASE:
+ /* Do not oeverride other rebase modes */
+ if (opt_rebase < REBASE_TRUE)
+ opt_rebase = REBASE_TRUE;
+ break;
+ case PULL_MODE_DEFAULT:
+ if (opt_rebase > 0)
+ mode = opt_rebase >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
+ break;
+ default:
+ break;
+ }
+ }
+
if (read_cache_unmerged())
die_resolve_conflict("pull");
diff --git a/rebase.c b/rebase.c
index f8137d859b..bdfca49886 100644
--- a/rebase.c
+++ b/rebase.c
@@ -33,3 +33,13 @@ enum rebase_type rebase_parse_value(const char *value)
return REBASE_INVALID;
}
+
+enum pull_mode_type pull_mode_parse_value(const char *value)
+{
+ if (!strcmp(value, "merge") || !strcmp(value, "m"))
+ return PULL_MODE_MERGE;
+ else if (!strcmp(value, "rebase") || !strcmp(value, "r"))
+ return PULL_MODE_REBASE;
+
+ return PULL_MODE_INVALID;
+}
diff --git a/rebase.h b/rebase.h
index 34d4acfd74..5ab8f4ddd5 100644
--- a/rebase.h
+++ b/rebase.h
@@ -13,4 +13,13 @@ enum rebase_type {
enum rebase_type rebase_parse_value(const char *value);
+enum pull_mode_type {
+ PULL_MODE_INVALID = -1,
+ PULL_MODE_DEFAULT = 0,
+ PULL_MODE_MERGE,
+ PULL_MODE_REBASE
+};
+
+enum pull_mode_type pull_mode_parse_value(const char *value);
+
#endif /* REBASE */
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 9fae07cdfa..59799ac4d5 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -449,6 +449,16 @@ test_expect_success 'pull.rebase' '
test_cmp expect actual
'
+test_expect_success 'pull.mode rebase' '
+ git reset --hard before-rebase &&
+ test_config pull.mode rebase &&
+ git pull . copy &&
+ test_cmp_rev HEAD^ copy &&
+ echo new >expect &&
+ git show HEAD:file2 >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'pull --autostash & pull.rebase=true' '
test_config pull.rebase true &&
test_pull_autostash 1 --autostash
@@ -480,6 +490,18 @@ test_expect_success 'branch.to-rebase.rebase should override pull.rebase' '
test_cmp expect actual
'
+test_expect_success 'remote..pullmode' '
+ git reset --hard before-rebase &&
+ git remote add test_remote . &&
+ test_when_finished "git remote remove test_remote" &&
+ test_config remote.test_remote.pullmode rebase &&
+ git pull test_remote copy &&
+ test_cmp_rev HEAD^ copy &&
+ echo new >expect &&
+ git show HEAD:file2 >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'pull --rebase warns on --verify-signatures' '
git reset --hard before-rebase &&
git pull --rebase --verify-signatures . copy 2>err &&
@@ -523,6 +545,17 @@ test_expect_success 'pull.rebase=false create a new merge commit' '
test_cmp expect actual
'
+test_expect_success 'pull.mode=merge create a new merge commit' '
+ git reset --hard before-preserve-rebase &&
+ test_config pull.mode merge &&
+ git pull . copy &&
+ test_cmp_rev HEAD^1 before-preserve-rebase &&
+ test_cmp_rev HEAD^2 copy &&
+ echo file3 >expect &&
+ git show HEAD:file3.t >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'pull.rebase=true flattens keep-merge' '
git reset --hard before-preserve-rebase &&
test_config pull.rebase true &&
@@ -552,6 +585,16 @@ test_expect_success REBASE_P \
test_cmp_rev HEAD^2 keep-merge
'
+test_expect_success REBASE_P \
+ 'pull.rebase=preserve rebases and merges keep-merge with pull.mode' '
+ git reset --hard before-preserve-rebase &&
+ test_config pull.mode rebase &&
+ test_config pull.rebase preserve &&
+ git pull . copy &&
+ test_cmp_rev HEAD^^ copy &&
+ test_cmp_rev HEAD^2 keep-merge
+'
+
test_expect_success 'pull.rebase=interactive' '
write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
echo I was here >fake.out &&
@@ -593,6 +636,17 @@ test_expect_success '--rebase=false create a new merge commit' '
test_cmp expect actual
'
+test_expect_success '--rebase=false create a new merge commit with pull.mode' '
+ git reset --hard before-preserve-rebase &&
+ test_config pull.mode rebase &&
+ git pull --rebase=false . copy &&
+ test_cmp_rev HEAD^1 before-preserve-rebase &&
+ test_cmp_rev HEAD^2 copy &&
+ echo file3 >expect &&
+ git show HEAD:file3.t >actual &&
+ test_cmp expect actual
+'
+
test_expect_success '--rebase=true rebases and flattens keep-merge' '
git reset --hard before-preserve-rebase &&
test_config pull.rebase preserve &&
diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
index 7c4607191a..47fd2e2d05 100755
--- a/t/t7601-merge-pull-config.sh
+++ b/t/t7601-merge-pull-config.sh
@@ -33,6 +33,13 @@ test_expect_success 'pull.rebase not set' '
test_i18ngrep ! "Pulling without specifying how to reconcile" err
'
+test_expect_success 'pull.mode set' '
+ git reset --hard c0 &&
+ test_config pull.mode merge &&
+ git pull . c1 2>err &&
+ test_i18ngrep ! "Pulling without specifying how to reconcile" err
+'
+
test_expect_success 'pull.rebase not set and pull.ff=true' '
git reset --hard c0 &&
test_config pull.ff true &&
@@ -92,6 +99,13 @@ test_expect_success 'pull.rebase not set (not-fast-forward)' '
test_i18ngrep "Pulling without specifying how to reconcile" decoded
'
+test_expect_success 'pull.mode set' '
+ git reset --hard c2 &&
+ test_config pull.mode merge &&
+ git pull . c1 2>err &&
+ test_i18ngrep ! "Pulling without specifying how to reconcile" err
+'
+
test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
git reset --hard c2 &&
test_config pull.ff true &&
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 11/13] pull: add pull.mode=fast-forward
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (9 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 10/13] pull: add pull.mode Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 12/13] pull: reorganize mode conditionals Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 13/13] pull: improve --rebase and pull.rebase interaction Felipe Contreras
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
It is very typical for Git newcomers to inadvertently create merges and
worse; pushing them. This is one of the reasons many experienced users
prefer to avoid 'git pull', and recommend newcomers to avoid it as well.
To escape these problems--and keep 'git pull' useful--it has been
suggested that 'git pull' barfs by default if the merge is
non-fast-forward, which unfortunately would break backwards
compatibility.
This patch leaves everything in place to enable this new mode, but it
only gets enabled if the user specifically configures it:
pull.mode = fast-forward
Later on this mode can be enabled by default.
For *some* of the long discussions you can read:
https://lore.kernel.org/git/20130522115042.GA20649@inner.h.apk.li
https://lore.kernel.org/git/1377988690-23460-1-git-send-email-felipe.contreras@gmail.com
https://lore.kernel.org/4ay6w9i74cygt6ii1b0db7wg.1398433713382@email.android.com
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Documentation/config/pull.txt | 4 +--
Documentation/config/remote.txt | 4 +--
builtin/pull.c | 6 ++++-
rebase.c | 2 ++
rebase.h | 3 ++-
t/t5520-pull.sh | 44 +++++++++++++++++++++++++++++++++
6 files changed, 57 insertions(+), 6 deletions(-)
diff --git a/Documentation/config/pull.txt b/Documentation/config/pull.txt
index f4385cde33..2b5e813259 100644
--- a/Documentation/config/pull.txt
+++ b/Documentation/config/pull.txt
@@ -32,8 +32,8 @@ for details).
pull.mode::
When "git pull" is run, this determines if it would either merge or
rebase the fetched branch. The possible values are 'merge',
- and 'rebase'. See "branch.<name>.pullmode" for setting this on a
- per-branch basis.
+ 'rebase', and 'fast-forward'. See "branch.<name>.pullmode" for setting
+ this on a per-branch basis.
pull.octopus::
The default merge strategy to use when pulling multiple branches
diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index a732c92cf5..e7db1d46c1 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -87,6 +87,6 @@ remote.<name>.partialclonefilter::
remote.<name>.pullmode::
When "git pull" is run, this determines if it would either merge or
- rebase the branches from this remote. The possible values are 'merge', and
- 'rebase'. See "pull.mode" for doing this in a non
+ rebase the branches from this remote. The possible values are 'merge',
+ 'rebase', and 'fast-forward'. See "pull.mode" for doing this in a non
repository-specific manner.
diff --git a/builtin/pull.c b/builtin/pull.c
index bfadd585c7..ef23d8a52f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -985,7 +985,7 @@ static void show_advice_pull_non_ff(void)
"\n"
" git config --global pull.mode merge\n"
" git config --global pull.mode rebase\n"
- " git config --global pull.ff only # fast-forward only\n"
+ " git config --global pull.mode fast-forward\n"
"\n"
"If unsure, run \"git pull --merge\".\n"
"Read \"git pull --help\" for more information."));
@@ -1027,6 +1027,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
switch (mode) {
case PULL_MODE_MERGE:
+ case PULL_MODE_FAST_FORWARD:
opt_rebase = REBASE_FALSE;
break;
case PULL_MODE_REBASE:
@@ -1117,6 +1118,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
+ if (mode == PULL_MODE_FAST_FORWARD && !can_ff)
+ die(_("The pull was not fast-forward, please either merge or rebase.\n"));
+
if (!opt_rebase && !can_ff) {
if (opt_verbosity >= 0)
show_advice_pull_non_ff();
diff --git a/rebase.c b/rebase.c
index bdfca49886..9fe99b5b16 100644
--- a/rebase.c
+++ b/rebase.c
@@ -40,6 +40,8 @@ enum pull_mode_type pull_mode_parse_value(const char *value)
return PULL_MODE_MERGE;
else if (!strcmp(value, "rebase") || !strcmp(value, "r"))
return PULL_MODE_REBASE;
+ else if (!strcmp(value, "fast-forward") || !strcmp(value, "f"))
+ return PULL_MODE_FAST_FORWARD;
return PULL_MODE_INVALID;
}
diff --git a/rebase.h b/rebase.h
index 5ab8f4ddd5..e66a73feb4 100644
--- a/rebase.h
+++ b/rebase.h
@@ -17,7 +17,8 @@ enum pull_mode_type {
PULL_MODE_INVALID = -1,
PULL_MODE_DEFAULT = 0,
PULL_MODE_MERGE,
- PULL_MODE_REBASE
+ PULL_MODE_REBASE,
+ PULL_MODE_FAST_FORWARD
};
enum pull_mode_type pull_mode_parse_value(const char *value);
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 59799ac4d5..45d818065f 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -873,4 +873,48 @@ test_expect_success 'git pull --rebase against local branch' '
test_cmp expect file2
'
+setup_other () {
+ test_when_finished "git checkout master && git branch -D other test" &&
+ git checkout -b other $1 &&
+ >new &&
+ git add new &&
+ git commit -m new &&
+ git checkout -b test -t other &&
+ git reset --hard master
+}
+
+setup_ff () {
+ setup_other master
+}
+
+setup_non_ff () {
+ setup_other master^
+}
+
+test_expect_success 'fast-forward (pull.mode=fast-forward)' '
+ setup_ff &&
+ git -c pull.mode=fast-forward pull
+'
+
+test_expect_success 'non-fast-forward (pull.mode=fast-forward)' '
+ setup_non_ff &&
+ test_must_fail git -c pull.mode=fast-forward pull
+'
+
+test_expect_success 'non-fast-forward with merge (pull.mode=fast-forward)' '
+ setup_non_ff &&
+ git -c pull.mode=fast-forward pull --merge
+'
+
+test_expect_success 'non-fast-forward with rebase (pull.mode=fast-forward)' '
+ setup_non_ff &&
+ git -c pull.mode=fast-forward pull --rebase
+'
+
+test_expect_success 'non-fast-forward error message (pull.mode=fast-forward)' '
+ setup_non_ff &&
+ test_must_fail git -c pull.mode=fast-forward pull 2> error &&
+ test_i18ngrep "The pull was not fast-forward" error
+'
+
test_done
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 12/13] pull: reorganize mode conditionals
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (10 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 11/13] pull: add pull.mode=fast-forward Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
2020-12-18 21:10 ` [PATCH v2 13/13] pull: improve --rebase and pull.rebase interaction Felipe Contreras
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Now that everything is in place we can shuffle around the conditionals
so it's clearer what we are trying to do.
No functional changes.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index ef23d8a52f..ad8afabe9b 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1118,12 +1118,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
- if (mode == PULL_MODE_FAST_FORWARD && !can_ff)
- die(_("The pull was not fast-forward, please either merge or rebase.\n"));
-
- if (!opt_rebase && !can_ff) {
- if (opt_verbosity >= 0)
+ if (!can_ff) {
+ if (!mode && opt_verbosity >= 0)
show_advice_pull_non_ff();
+
+ if (mode == PULL_MODE_FAST_FORWARD)
+ die(_("The pull was not fast-forward, please either merge or rebase.\n"));
}
if (opt_rebase >= REBASE_TRUE) {
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v2 13/13] pull: improve --rebase and pull.rebase interaction
2020-12-18 21:10 [PATCH v2 00/13] pull: pull mode part 2 Felipe Contreras
` (11 preceding siblings ...)
2020-12-18 21:10 ` [PATCH v2 12/13] pull: reorganize mode conditionals Felipe Contreras
@ 2020-12-18 21:10 ` Felipe Contreras
12 siblings, 0 replies; 14+ messages in thread
From: Felipe Contreras @ 2020-12-18 21:10 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Elijah Newren, Felipe Contreras
Currently --rebase without argument overrides pull.rebase:
git config pull.rebase merges
git pull --rebase
Up until now this hasn't been a big issue, since user has not been
forced to specify a merge, or a rebase. But with the introduction of
--merge and pull.mode, the user could in theory have the following
configuration:
git config pull.mode merge
git config pull.rebase merges
In such case, the user would expect:
git pull --rebase
To be the same as:
git pull --rebase=merges
If the user wants to override the configuration, she can do:
git pull --rebase=true
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
builtin/pull.c | 10 ++++++++--
t/t5520-pull.sh | 10 ++++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/builtin/pull.c b/builtin/pull.c
index ad8afabe9b..4e8e9d85a2 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -68,8 +68,14 @@ static int parse_opt_rebase(const struct option *opt, const char *arg, int unset
if (arg)
*value = parse_config_rebase("--rebase", arg, 0);
- else
- *value = unset ? REBASE_FALSE : REBASE_TRUE;
+ else {
+ if (!unset) {
+ /* --rebase shouldn't override pull.rebase=merges (and others) */
+ if (*value < REBASE_TRUE)
+ *value = REBASE_TRUE;
+ } else
+ *value = REBASE_FALSE;
+ }
if (*value > 0)
mode = *value >= REBASE_TRUE ? PULL_MODE_REBASE : PULL_MODE_MERGE;
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 45d818065f..d0f5722eab 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -595,6 +595,16 @@ test_expect_success REBASE_P \
test_cmp_rev HEAD^2 keep-merge
'
+test_expect_success REBASE_P \
+ 'pull.rebase=preserve interacts correctly with pull.mode and --rebase' '
+ git reset --hard before-preserve-rebase &&
+ test_config pull.mode merge &&
+ test_config pull.rebase preserve &&
+ git pull --rebase . copy &&
+ test_cmp_rev HEAD^^ copy &&
+ test_cmp_rev HEAD^2 keep-merge
+'
+
test_expect_success 'pull.rebase=interactive' '
write_script "$TRASH_DIRECTORY/fake-editor" <<-\EOF &&
echo I was here >fake.out &&
--
2.30.0.rc0
^ permalink raw reply related [flat|nested] 14+ messages in thread