Skip to content

Commit d3fa84d

Browse files
gitstergitster
committed
Merge branch 'fc/pull-merge-rebase'
When a user does not tell "git pull" to use rebase or merge, the command gives a loud message telling a user to choose between rebase or merge but creates a merge anyway, forcing users who would want to rebase to redo the operation. Fix an early part of this problem by tightening the condition to give the message---there is no reason to stop or force the user to choose between rebase or merge if the history fast-forwards. * fc/pull-merge-rebase: pull: display default warning only when non-ff pull: correct condition to trigger non-ff advice pull: get rid of unnecessary global variable pull: give the advice for choosing rebase/merge much later pull: refactor fast-forward check
2 parents 85cf82f + c525de3 commit d3fa84d

File tree

2 files changed

+104
-32
lines changed

2 files changed

+104
-32
lines changed

‎builtin/pull.c

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ static const char *config_get_ff(void)
324324
* looks for the value of "pull.rebase". If both configuration keys do not
325325
* exist, returns REBASE_FALSE.
326326
*/
327-
static enum rebase_type config_get_rebase(void)
327+
static enum rebase_type config_get_rebase(int *rebase_unspecified)
328328
{
329329
struct branch *curr_branch = branch_get("HEAD");
330330
const char *value;
@@ -344,20 +344,7 @@ static enum rebase_type config_get_rebase(void)
344344
if (!git_config_get_value("pull.rebase", &value))
345345
return parse_config_rebase("pull.rebase", value, 1);
346346

347-
if (opt_verbosity >= 0 && !opt_ff) {
348-
advise(_("Pulling without specifying how to reconcile divergent branches is\n"
349-
"discouraged. You can squelch this message by running one of the following\n"
350-
"commands sometime before your next pull:\n"
351-
"\n"
352-
" git config pull.rebase false # merge (the default strategy)\n"
353-
" git config pull.rebase true # rebase\n"
354-
" git config pull.ff only # fast-forward only\n"
355-
"\n"
356-
"You can replace \"git config\" with \"git config --global\" to set a default\n"
357-
"preference for all repositories. You can also pass --rebase, --no-rebase,\n"
358-
"or --ff-only on the command line to override the configured default per\n"
359-
"invocation.\n"));
360-
}
347+
*rebase_unspecified = 1;
361348

362349
return REBASE_FALSE;
363350
}
@@ -924,13 +911,45 @@ static int run_rebase(const struct object_id *newbase,
924911
return ret;
925912
}
926913

914+
static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_head)
915+
{
916+
int ret;
917+
struct commit_list *list = NULL;
918+
struct commit *merge_head, *head;
919+
920+
head = lookup_commit_reference(the_repository, orig_head);
921+
commit_list_insert(head, &list);
922+
merge_head = lookup_commit_reference(the_repository, orig_merge_head);
923+
ret = repo_is_descendant_of(the_repository, merge_head, list);
924+
free_commit_list(list);
925+
return ret;
926+
}
927+
928+
static void show_advice_pull_non_ff(void)
929+
{
930+
advise(_("Pulling without specifying how to reconcile divergent branches is\n"
931+
"discouraged. You can squelch this message by running one of the following\n"
932+
"commands sometime before your next pull:\n"
933+
"\n"
934+
" git config pull.rebase false # merge (the default strategy)\n"
935+
" git config pull.rebase true # rebase\n"
936+
" git config pull.ff only # fast-forward only\n"
937+
"\n"
938+
"You can replace \"git config\" with \"git config --global\" to set a default\n"
939+
"preference for all repositories. You can also pass --rebase, --no-rebase,\n"
940+
"or --ff-only on the command line to override the configured default per\n"
941+
"invocation.\n"));
942+
}
943+
927944
int cmd_pull(int argc, const char **argv, const char *prefix)
928945
{
929946
const char *repo, **refspecs;
930947
struct oid_array merge_heads = OID_ARRAY_INIT;
931948
struct object_id orig_head, curr_head;
932949
struct object_id rebase_fork_point;
933950
int autostash;
951+
int rebase_unspecified = 0;
952+
int can_ff;
934953

935954
if (!getenv("GIT_REFLOG_ACTION"))
936955
set_reflog_message(argc, argv);
@@ -952,7 +971,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
952971
opt_ff = xstrdup_or_null(config_get_ff());
953972

954973
if (opt_rebase < 0)
955-
opt_rebase = config_get_rebase();
974+
opt_rebase = config_get_rebase(&rebase_unspecified);
956975

957976
if (read_cache_unmerged())
958977
die_resolve_conflict("pull");
@@ -1026,6 +1045,13 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
10261045
if (opt_rebase && merge_heads.nr > 1)
10271046
die(_("Cannot rebase onto multiple branches."));
10281047

1048+
can_ff = get_can_ff(&orig_head, &merge_heads.oid[0]);
1049+
1050+
if (rebase_unspecified && !opt_ff && !can_ff) {
1051+
if (opt_verbosity >= 0)
1052+
show_advice_pull_non_ff();
1053+
}
1054+
10291055
if (opt_rebase) {
10301056
int ret = 0;
10311057
int ran_ff = 0;
@@ -1040,22 +1066,12 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
10401066
submodule_touches_in_range(the_repository, &upstream, &curr_head))
10411067
die(_("cannot rebase with locally recorded submodule modifications"));
10421068
if (!autostash) {
1043-
struct commit_list *list = NULL;
1044-
struct commit *merge_head, *head;
1045-
1046-
head = lookup_commit_reference(the_repository,
1047-
&orig_head);
1048-
commit_list_insert(head, &list);
1049-
merge_head = lookup_commit_reference(the_repository,
1050-
&merge_heads.oid[0]);
1051-
if (repo_is_descendant_of(the_repository,
1052-
merge_head, list)) {
1069+
if (can_ff) {
10531070
/* we can fast-forward this without invoking rebase */
10541071
opt_ff = "--ff-only";
10551072
ran_ff = 1;
10561073
ret = run_merge();
10571074
}
1058-
free_commit_list(list);
10591075
}
10601076
if (!ran_ff)
10611077
ret = run_rebase(&newbase, &upstream);

‎t/t7601-merge-pull-config.sh

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,8 @@ test_expect_success 'setup' '
2929

3030
test_expect_success 'pull.rebase not set' '
3131
git reset --hard c0 &&
32-
git -c color.advice=always pull . c1 2>err &&
33-
test_decode_color <err >decoded &&
34-
test_i18ngrep "<YELLOW>hint: " decoded &&
35-
test_i18ngrep "Pulling without specifying how to reconcile" decoded
36-
32+
git pull . c1 2>err &&
33+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
3734
'
3835

3936
test_expect_success 'pull.rebase not set and pull.ff=true' '
@@ -87,6 +84,65 @@ test_expect_success 'pull.rebase not set and --ff-only given' '
8784
test_i18ngrep ! "Pulling without specifying how to reconcile" err
8885
'
8986

87+
test_expect_success 'pull.rebase not set (not-fast-forward)' '
88+
git reset --hard c2 &&
89+
git -c color.advice=always pull . c1 2>err &&
90+
test_decode_color <err >decoded &&
91+
test_i18ngrep "<YELLOW>hint: " decoded &&
92+
test_i18ngrep "Pulling without specifying how to reconcile" decoded
93+
'
94+
95+
test_expect_success 'pull.rebase not set and pull.ff=true (not-fast-forward)' '
96+
git reset --hard c2 &&
97+
test_config pull.ff true &&
98+
git pull . c1 2>err &&
99+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
100+
'
101+
102+
test_expect_success 'pull.rebase not set and pull.ff=false (not-fast-forward)' '
103+
git reset --hard c2 &&
104+
test_config pull.ff false &&
105+
git pull . c1 2>err &&
106+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
107+
'
108+
109+
test_expect_success 'pull.rebase not set and pull.ff=only (not-fast-forward)' '
110+
git reset --hard c2 &&
111+
test_config pull.ff only &&
112+
test_must_fail git pull . c1 2>err &&
113+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
114+
'
115+
116+
test_expect_success 'pull.rebase not set and --rebase given (not-fast-forward)' '
117+
git reset --hard c2 &&
118+
git pull --rebase . c1 2>err &&
119+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
120+
'
121+
122+
test_expect_success 'pull.rebase not set and --no-rebase given (not-fast-forward)' '
123+
git reset --hard c2 &&
124+
git pull --no-rebase . c1 2>err &&
125+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
126+
'
127+
128+
test_expect_success 'pull.rebase not set and --ff given (not-fast-forward)' '
129+
git reset --hard c2 &&
130+
git pull --ff . c1 2>err &&
131+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
132+
'
133+
134+
test_expect_success 'pull.rebase not set and --no-ff given (not-fast-forward)' '
135+
git reset --hard c2 &&
136+
git pull --no-ff . c1 2>err &&
137+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
138+
'
139+
140+
test_expect_success 'pull.rebase not set and --ff-only given (not-fast-forward)' '
141+
git reset --hard c2 &&
142+
test_must_fail git pull --ff-only . c1 2>err &&
143+
test_i18ngrep ! "Pulling without specifying how to reconcile" err
144+
'
145+
90146
test_expect_success 'merge c1 with c2' '
91147
git reset --hard c1 &&
92148
test -f c0.c &&

0 commit comments

Comments
 (0)