merge: handle --ff/--no-ff/--ff-only as a tri-state option

These three options mean "favor fast-forwarding when possible, without creating an unnecessary merge", "never fast-forward and always create a merge commit even when the commit being merged is a strict descendant", and "we do not want to create any merge commit; update only when the merged commit is a strict descendant". They are "pick one out of these three possibilities" options, and correspond to "merge.ff" configuration that is tri-state (yes, no and only). However, the implementation did not follow the usual convention for the command line options (later one wins, and command line overrides what is in the configuration). Fix this by consolidating two variables (fast_forward_only and allow_fast_forward) used in the implementation into one enum that can take one of the three possible values. Signed-off-by: Miklos Vajna <vmiklos@suse.cz> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Miklos Vajna committed Jul 2, 2013 at 16:47 UTC a54841e96b78203598dea6b31b9618f40f107e7b
2 files changed +42 -25
builtin/merge.c
+33 -22
@@ -47,8 +47,8 @@ static const char * const builtin_merge_usage[] = {
47 };
48
49 static int show_diffstat = 1, shortlog_len = -1, squash;
50 -static int option_commit = 1, allow_fast_forward = 1;
51 -static int fast_forward_only, option_edit = -1;
50 +static int option_commit = 1;
51 +static int option_edit = -1;
52 static int allow_trivial = 1, have_message, verify_signatures;
53 static int overwrite_ignore = 1;
54 static struct strbuf merge_msg = STRBUF_INIT;
@@ -76,6 +76,14 @@ static struct strategy all_strategy[] = {
76
77 static const char *pull_twohead, *pull_octopus;
78
79 +enum ff_type {
80 + FF_NO,
81 + FF_ALLOW,
82 + FF_ONLY
83 +};
84 +
85 +static enum ff_type fast_forward = FF_ALLOW;
86 +
87 static int option_parse_message(const struct option *opt,
88 const char *arg, int unset)
89 {
@@ -178,6 +186,13 @@ static int option_parse_n(const struct option *opt,
186 return 0;
187 }
188
189 +static int option_parse_ff_only(const struct option *opt,
190 + const char *arg, int unset)
191 +{
192 + fast_forward = FF_ONLY;
193 + return 0;
194 +}
195 +
196 static struct option builtin_merge_options[] = {
197 { OPTION_CALLBACK, 'n', NULL, NULL, NULL,
198 N_("do not show a diffstat at the end of the merge"),
@@ -194,10 +209,10 @@ static struct option builtin_merge_options[] = {
209 N_("perform a commit if the merge succeeds (default)")),
210 OPT_BOOL('e', "edit", &option_edit,
211 N_("edit message before committing")),
197 - OPT_BOOLEAN(0, "ff", &allow_fast_forward,
198 - N_("allow fast-forward (default)")),
199 - OPT_BOOLEAN(0, "ff-only", &fast_forward_only,
200 - N_("abort if fast-forward is not possible")),
212 + OPT_SET_INT(0, "ff", &fast_forward, N_("allow fast-forward (default)"), FF_ALLOW),
213 + { OPTION_CALLBACK, 0, "ff-only", NULL, NULL,
214 + N_("abort if fast-forward is not possible"),
215 + PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
216 OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
217 OPT_BOOL(0, "verify-signatures", &verify_signatures,
218 N_("Verify that the named commit has a valid GPG signature")),
@@ -581,10 +596,9 @@ static int git_merge_config(const char *k, const char *v, void *cb)
596 else if (!strcmp(k, "merge.ff")) {
597 int boolval = git_config_maybe_bool(k, v);
598 if (0 <= boolval) {
584 - allow_fast_forward = boolval;
599 + fast_forward = boolval ? FF_ALLOW : FF_NO;
600 } else if (v && !strcmp(v, "only")) {
586 - allow_fast_forward = 1;
587 - fast_forward_only = 1;
601 + fast_forward = FF_ONLY;
602 } /* do not barf on values from future versions of git */
603 return 0;
604 } else if (!strcmp(k, "merge.defaulttoupstream")) {
@@ -863,7 +877,7 @@ static int finish_automerge(struct commit *head,
877
878 free_commit_list(common);
879 parents = remoteheads;
866 - if (!head_subsumed || !allow_fast_forward)
880 + if (!head_subsumed || fast_forward == FF_NO)
881 commit_list_insert(head, &parents);
882 strbuf_addch(&merge_msg, '\n');
883 prepare_to_commit(remoteheads);
@@ -1008,7 +1022,7 @@ static void write_merge_state(struct commit_list *remoteheads)
1022 if (fd < 0)
1023 die_errno(_("Could not open '%s' for writing"), filename);
1024 strbuf_reset(&buf);
1011 - if (!allow_fast_forward)
1025 + if (fast_forward == FF_NO)
1026 strbuf_addf(&buf, "no-ff");
1027 if (write_in_full(fd, buf.buf, buf.len) != buf.len)
1028 die_errno(_("Could not write to '%s'"), filename);
@@ -1157,14 +1171,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1171 show_diffstat = 0;
1172
1173 if (squash) {
1160 - if (!allow_fast_forward)
1174 + if (fast_forward == FF_NO)
1175 die(_("You cannot combine --squash with --no-ff."));
1176 option_commit = 0;
1177 }
1178
1165 - if (!allow_fast_forward && fast_forward_only)
1166 - die(_("You cannot combine --no-ff with --ff-only."));
1167 -
1179 if (!abort_current_merge) {
1180 if (!argc) {
1181 if (default_to_upstream)
@@ -1206,7 +1217,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1217 "empty head"));
1218 if (squash)
1219 die(_("Squash commit into empty head not supported yet"));
1209 - if (!allow_fast_forward)
1220 + if (fast_forward == FF_NO)
1221 die(_("Non-fast-forward commit does not make sense into "
1222 "an empty head"));
1223 remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
@@ -1294,11 +1305,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1305 sha1_to_hex(commit->object.sha1));
1306 setenv(buf.buf, merge_remote_util(commit)->name, 1);
1307 strbuf_reset(&buf);
1297 - if (!fast_forward_only &&
1308 + if (fast_forward != FF_ONLY &&
1309 merge_remote_util(commit) &&
1310 merge_remote_util(commit)->obj &&
1311 merge_remote_util(commit)->obj->type == OBJ_TAG)
1301 - allow_fast_forward = 0;
1312 + fast_forward = FF_NO;
1313 }
1314
1315 if (option_edit < 0)
@@ -1315,7 +1326,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1326
1327 for (i = 0; i < use_strategies_nr; i++) {
1328 if (use_strategies[i]->attr & NO_FAST_FORWARD)
1318 - allow_fast_forward = 0;
1329 + fast_forward = FF_NO;
1330 if (use_strategies[i]->attr & NO_TRIVIAL)
1331 allow_trivial = 0;
1332 }
@@ -1345,7 +1356,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1356 */
1357 finish_up_to_date("Already up-to-date.");
1358 goto done;
1348 - } else if (allow_fast_forward && !remoteheads->next &&
1359 + } else if (fast_forward != FF_NO && !remoteheads->next &&
1360 !common->next &&
1361 !hashcmp(common->item->object.sha1, head_commit->object.sha1)) {
1362 /* Again the most common case of merging one remote. */
@@ -1392,7 +1403,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1403 * only one common.
1404 */
1405 refresh_cache(REFRESH_QUIET);
1395 - if (allow_trivial && !fast_forward_only) {
1406 + if (allow_trivial && fast_forward != FF_ONLY) {
1407 /* See if it is really trivial. */
1408 git_committer_info(IDENT_STRICT);
1409 printf(_("Trying really trivial in-index merge...\n"));
@@ -1433,7 +1444,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
1444 }
1445 }
1446
1436 - if (fast_forward_only)
1447 + if (fast_forward == FF_ONLY)
1448 die(_("Not possible to fast-forward, aborting."));
1449
1450 /* We are going to make a new commit. */
t/t7600-merge.sh
+9 -3
@@ -497,9 +497,15 @@ test_expect_success 'combining --squash and --no-ff is refused' '
497 test_must_fail git merge --no-ff --squash c1
498 '
499
500 -test_expect_success 'combining --ff-only and --no-ff is refused' '
501 - test_must_fail git merge --ff-only --no-ff c1 &&
502 - test_must_fail git merge --no-ff --ff-only c1
500 +test_expect_success 'option --ff-only overwrites --no-ff' '
501 + git merge --no-ff --ff-only c1 &&
502 + test_must_fail git merge --no-ff --ff-only c2
503 +'
504 +
505 +test_expect_success 'option --ff-only overwrites merge.ff=only config' '
506 + git reset --hard c0 &&
507 + test_config merge.ff only &&
508 + git merge --no-ff c1
509 '
510
511 test_expect_success 'merge c0 with c1 (ff overrides no-ff)' '