1 files changed, 156 insertions, 117 deletions

diff --git a/builtin/pull.c b/builtin/pull.c
index 8159c5d7c9..e8927fc2ff 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -87,8 +87,8 @@ static char *opt_verify_signatures;
 static int opt_autostash = -1;
 static int config_autostash;
 static int check_trust_level = 1;
-static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
-static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
+static struct strvec opt_strategies = STRVEC_INIT;
+static struct strvec opt_strategy_opts = STRVEC_INIT;
 static char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
 
@@ -110,7 +110,7 @@ static char *opt_ipv4;
 static char *opt_ipv6;
 static int opt_show_forced_updates = -1;
 static char *set_upstream;
-static struct argv_array opt_fetch = ARGV_ARRAY_INIT;
+static struct strvec opt_fetch = STRVEC_INIT;
 
 static struct option pull_options[] = {
 	/* Shared options */
@@ -142,7 +142,7 @@ static struct option pull_options[] = {
 		N_("add (at most <n>) entries from shortlog to merge commit message"),
 		PARSE_OPT_OPTARG),
 	OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
-		N_("add Signed-off-by:"),
+		N_("add a Signed-off-by trailer"),
 		PARSE_OPT_OPTARG),
 	OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
 		N_("create a single commit instead of doing a merge"),
@@ -251,25 +251,25 @@ static struct option pull_options[] = {
 /**
  * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
  */
-static void argv_push_verbosity(struct argv_array *arr)
+static void argv_push_verbosity(struct strvec *arr)
 {
 	int verbosity;
 
 	for (verbosity = opt_verbosity; verbosity > 0; verbosity--)
-		argv_array_push(arr, "-v");
+		strvec_push(arr, "-v");
 
 	for (verbosity = opt_verbosity; verbosity < 0; verbosity++)
-		argv_array_push(arr, "-q");
+		strvec_push(arr, "-q");
 }
 
 /**
  * Pushes "-f" switches into arr to match the opt_force level.
  */
-static void argv_push_force(struct argv_array *arr)
+static void argv_push_force(struct strvec *arr)
 {
 	int force = opt_force;
 	while (force-- > 0)
-		argv_array_push(arr, "-f");
+		strvec_push(arr, "-f");
 }
 
 /**
@@ -324,7 +324,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(void)
+static enum rebase_type config_get_rebase(int *rebase_unspecified)
 {
 	struct branch *curr_branch = branch_get("HEAD");
 	const char *value;
@@ -344,21 +344,7 @@ static enum rebase_type config_get_rebase(void)
 	if (!git_config_get_value("pull.rebase", &value))
 		return parse_config_rebase("pull.rebase", value, 1);
 
-	if (opt_verbosity >= 0 &&
-	    (!opt_ff || strcmp(opt_ff, "--ff-only"))) {
-		warning(_("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"
-			"\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"
-			"\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"));
-	}
+	*rebase_unspecified = 1;
 
 	return REBASE_FALSE;
 }
@@ -524,75 +510,75 @@ static void parse_repo_refspecs(int argc, const char **argv, const char **repo,
  */
 static int run_fetch(const char *repo, const char **refspecs)
 {
-	struct argv_array args = ARGV_ARRAY_INIT;
+	struct strvec args = STRVEC_INIT;
 	int ret;
 
-	argv_array_pushl(&args, "fetch", "--update-head-ok", NULL);
+	strvec_pushl(&args, "fetch", "--update-head-ok", NULL);
 
 	/* Shared options */
 	argv_push_verbosity(&args);
 	if (opt_progress)
-		argv_array_push(&args, opt_progress);
+		strvec_push(&args, opt_progress);
 
 	/* Options passed to git-fetch */
 	if (opt_all)
-		argv_array_push(&args, opt_all);
+		strvec_push(&args, opt_all);
 	if (opt_append)
-		argv_array_push(&args, opt_append);
+		strvec_push(&args, opt_append);
 	if (opt_upload_pack)
-		argv_array_push(&args, opt_upload_pack);
+		strvec_push(&args, opt_upload_pack);
 	argv_push_force(&args);
 	if (opt_tags)
-		argv_array_push(&args, opt_tags);
+		strvec_push(&args, opt_tags);
 	if (opt_prune)
-		argv_array_push(&args, opt_prune);
+		strvec_push(&args, opt_prune);
 	if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
 		switch (recurse_submodules) {
 		case RECURSE_SUBMODULES_ON:
-			argv_array_push(&args, "--recurse-submodules=on");
+			strvec_push(&args, "--recurse-submodules=on");
 			break;
 		case RECURSE_SUBMODULES_OFF:
-			argv_array_push(&args, "--recurse-submodules=no");
+			strvec_push(&args, "--recurse-submodules=no");
 			break;
 		case RECURSE_SUBMODULES_ON_DEMAND:
-			argv_array_push(&args, "--recurse-submodules=on-demand");
+			strvec_push(&args, "--recurse-submodules=on-demand");
 			break;
 		default:
 			BUG("submodule recursion option not understood");
 		}
 	if (max_children)
-		argv_array_push(&args, max_children);
+		strvec_push(&args, max_children);
 	if (opt_dry_run)
-		argv_array_push(&args, "--dry-run");
+		strvec_push(&args, "--dry-run");
 	if (opt_keep)
-		argv_array_push(&args, opt_keep);
+		strvec_push(&args, opt_keep);
 	if (opt_depth)
-		argv_array_push(&args, opt_depth);
+		strvec_push(&args, opt_depth);
 	if (opt_unshallow)
-		argv_array_push(&args, opt_unshallow);
+		strvec_push(&args, opt_unshallow);
 	if (opt_update_shallow)
-		argv_array_push(&args, opt_update_shallow);
+		strvec_push(&args, opt_update_shallow);
 	if (opt_refmap)
-		argv_array_push(&args, opt_refmap);
+		strvec_push(&args, opt_refmap);
 	if (opt_ipv4)
-		argv_array_push(&args, opt_ipv4);
+		strvec_push(&args, opt_ipv4);
 	if (opt_ipv6)
-		argv_array_push(&args, opt_ipv6);
+		strvec_push(&args, opt_ipv6);
 	if (opt_show_forced_updates > 0)
-		argv_array_push(&args, "--show-forced-updates");
+		strvec_push(&args, "--show-forced-updates");
 	else if (opt_show_forced_updates == 0)
-		argv_array_push(&args, "--no-show-forced-updates");
+		strvec_push(&args, "--no-show-forced-updates");
 	if (set_upstream)
-		argv_array_push(&args, set_upstream);
-	argv_array_pushv(&args, opt_fetch.argv);
+		strvec_push(&args, set_upstream);
+	strvec_pushv(&args, opt_fetch.v);
 
 	if (repo) {
-		argv_array_push(&args, repo);
-		argv_array_pushv(&args, refspecs);
+		strvec_push(&args, repo);
+		strvec_pushv(&args, refspecs);
 	} else if (*refspecs)
 		BUG("refspecs without repo?");
-	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
-	argv_array_clear(&args);
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
 	return ret;
 }
 
@@ -637,8 +623,8 @@ static int rebase_submodules(void)
 
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;
-	argv_array_pushl(&cp.args, "submodule", "update",
-				   "--recursive", "--rebase", NULL);
+	strvec_pushl(&cp.args, "submodule", "update",
+		     "--recursive", "--rebase", NULL);
 	argv_push_verbosity(&cp.args);
 
 	return run_command(&cp);
@@ -650,8 +636,8 @@ static int update_submodules(void)
 
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;
-	argv_array_pushl(&cp.args, "submodule", "update",
-				   "--recursive", "--checkout", NULL);
+	strvec_pushl(&cp.args, "submodule", "update",
+		     "--recursive", "--checkout", NULL);
 	argv_push_verbosity(&cp.args);
 
 	return run_command(&cp);
@@ -663,48 +649,48 @@ static int update_submodules(void)
 static int run_merge(void)
 {
 	int ret;
-	struct argv_array args = ARGV_ARRAY_INIT;
+	struct strvec args = STRVEC_INIT;
 
-	argv_array_pushl(&args, "merge", NULL);
+	strvec_pushl(&args, "merge", NULL);
 
 	/* Shared options */
 	argv_push_verbosity(&args);
 	if (opt_progress)
-		argv_array_push(&args, opt_progress);
+		strvec_push(&args, opt_progress);
 
 	/* Options passed to git-merge */
 	if (opt_diffstat)
-		argv_array_push(&args, opt_diffstat);
+		strvec_push(&args, opt_diffstat);
 	if (opt_log)
-		argv_array_push(&args, opt_log);
+		strvec_push(&args, opt_log);
 	if (opt_signoff)
-		argv_array_push(&args, opt_signoff);
+		strvec_push(&args, opt_signoff);
 	if (opt_squash)
-		argv_array_push(&args, opt_squash);
+		strvec_push(&args, opt_squash);
 	if (opt_commit)
-		argv_array_push(&args, opt_commit);
+		strvec_push(&args, opt_commit);
 	if (opt_edit)
-		argv_array_push(&args, opt_edit);
+		strvec_push(&args, opt_edit);
 	if (cleanup_arg)
-		argv_array_pushf(&args, "--cleanup=%s", cleanup_arg);
+		strvec_pushf(&args, "--cleanup=%s", cleanup_arg);
 	if (opt_ff)
-		argv_array_push(&args, opt_ff);
+		strvec_push(&args, opt_ff);
 	if (opt_verify_signatures)
-		argv_array_push(&args, opt_verify_signatures);
-	argv_array_pushv(&args, opt_strategies.argv);
-	argv_array_pushv(&args, opt_strategy_opts.argv);
+		strvec_push(&args, opt_verify_signatures);
+	strvec_pushv(&args, opt_strategies.v);
+	strvec_pushv(&args, opt_strategy_opts.v);
 	if (opt_gpg_sign)
-		argv_array_push(&args, opt_gpg_sign);
+		strvec_push(&args, opt_gpg_sign);
 	if (opt_autostash == 0)
-		argv_array_push(&args, "--no-autostash");
+		strvec_push(&args, "--no-autostash");
 	else if (opt_autostash == 1)
-		argv_array_push(&args, "--autostash");
+		strvec_push(&args, "--autostash");
 	if (opt_allow_unrelated_histories > 0)
-		argv_array_push(&args, "--allow-unrelated-histories");
+		strvec_push(&args, "--allow-unrelated-histories");
 
-	argv_array_push(&args, "FETCH_HEAD");
-	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
-	argv_array_clear(&args);
+	strvec_push(&args, "FETCH_HEAD");
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
 	return ret;
 }
 
@@ -801,8 +787,8 @@ static int get_rebase_fork_point(struct object_id *fork_point, const char *repo,
 	if (!remote_branch)
 		return -1;
 
-	argv_array_pushl(&cp.args, "merge-base", "--fork-point",
-			remote_branch, curr_branch->name, NULL);
+	strvec_pushl(&cp.args, "merge-base", "--fork-point",
+		     remote_branch, curr_branch->name, NULL);
 	cp.no_stdin = 1;
 	cp.no_stderr = 1;
 	cp.git_cmd = 1;
@@ -853,60 +839,108 @@ static int get_octopus_merge_base(struct object_id *merge_base,
 
 /**
  * Given the current HEAD oid, the merge head returned from git-fetch and the
- * fork point calculated by get_rebase_fork_point(), runs git-rebase with the
- * appropriate arguments and returns its exit status.
+ * fork point calculated by get_rebase_fork_point(), compute the <newbase> and
+ * <upstream> arguments to use for the upcoming git-rebase invocation.
  */
-static int run_rebase(const struct object_id *curr_head,
+static int get_rebase_newbase_and_upstream(struct object_id *newbase,
+		struct object_id *upstream,
+		const struct object_id *curr_head,
 		const struct object_id *merge_head,
 		const struct object_id *fork_point)
 {
-	int ret;
 	struct object_id oct_merge_base;
-	struct argv_array args = ARGV_ARRAY_INIT;
 
 	if (!get_octopus_merge_base(&oct_merge_base, curr_head, merge_head, fork_point))
 		if (!is_null_oid(fork_point) && oideq(&oct_merge_base, fork_point))
 			fork_point = NULL;
 
-	argv_array_push(&args, "rebase");
+	if (fork_point && !is_null_oid(fork_point))
+		oidcpy(upstream, fork_point);
+	else
+		oidcpy(upstream, merge_head);
+
+	oidcpy(newbase, merge_head);
+
+	return 0;
+}
+
+/**
+ * Given the <newbase> and <upstream> calculated by
+ * get_rebase_newbase_and_upstream(), runs git-rebase with the
+ * appropriate arguments and returns its exit status.
+ */
+static int run_rebase(const struct object_id *newbase,
+		const struct object_id *upstream)
+{
+	int ret;
+	struct strvec args = STRVEC_INIT;
+
+	strvec_push(&args, "rebase");
 
 	/* Shared options */
 	argv_push_verbosity(&args);
 
 	/* Options passed to git-rebase */
 	if (opt_rebase == REBASE_MERGES)
-		argv_array_push(&args, "--rebase-merges");
+		strvec_push(&args, "--rebase-merges");
 	else if (opt_rebase == REBASE_PRESERVE)
-		argv_array_push(&args, "--preserve-merges");
+		strvec_push(&args, "--preserve-merges");
 	else if (opt_rebase == REBASE_INTERACTIVE)
-		argv_array_push(&args, "--interactive");
+		strvec_push(&args, "--interactive");
 	if (opt_diffstat)
-		argv_array_push(&args, opt_diffstat);
-	argv_array_pushv(&args, opt_strategies.argv);
-	argv_array_pushv(&args, opt_strategy_opts.argv);
+		strvec_push(&args, opt_diffstat);
+	strvec_pushv(&args, opt_strategies.v);
+	strvec_pushv(&args, opt_strategy_opts.v);
 	if (opt_gpg_sign)
-		argv_array_push(&args, opt_gpg_sign);
+		strvec_push(&args, opt_gpg_sign);
 	if (opt_autostash == 0)
-		argv_array_push(&args, "--no-autostash");
+		strvec_push(&args, "--no-autostash");
 	else if (opt_autostash == 1)
-		argv_array_push(&args, "--autostash");
+		strvec_push(&args, "--autostash");
 	if (opt_verify_signatures &&
 	    !strcmp(opt_verify_signatures, "--verify-signatures"))
 		warning(_("ignoring --verify-signatures for rebase"));
 
-	argv_array_push(&args, "--onto");
-	argv_array_push(&args, oid_to_hex(merge_head));
+	strvec_push(&args, "--onto");
+	strvec_push(&args, oid_to_hex(newbase));
 
-	if (fork_point && !is_null_oid(fork_point))
-		argv_array_push(&args, oid_to_hex(fork_point));
-	else
-		argv_array_push(&args, oid_to_hex(merge_head));
+	strvec_push(&args, oid_to_hex(upstream));
+
+	ret = run_command_v_opt(args.v, RUN_GIT_CMD);
+	strvec_clear(&args);
+	return ret;
+}
 
-	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
-	argv_array_clear(&args);
+static int get_can_ff(struct object_id *orig_head, struct object_id *orig_merge_head)
+{
+	int ret;
+	struct commit_list *list = NULL;
+	struct commit *merge_head, *head;
+
+	head = lookup_commit_reference(the_repository, orig_head);
+	commit_list_insert(head, &list);
+	merge_head = lookup_commit_reference(the_repository, orig_merge_head);
+	ret = repo_is_descendant_of(the_repository, merge_head, list);
+	free_commit_list(list);
 	return ret;
 }
 
+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"
+		 "\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"
+		 "\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"));
+}
+
 int cmd_pull(int argc, const char **argv, const char *prefix)
 {
 	const char *repo, **refspecs;
@@ -914,6 +948,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	struct object_id orig_head, curr_head;
 	struct object_id rebase_fork_point;
 	int autostash;
+	int rebase_unspecified = 0;
+	int can_ff;
 
 	if (!getenv("GIT_REFLOG_ACTION"))
 		set_reflog_message(argc, argv);
@@ -935,7 +971,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		opt_ff = xstrdup_or_null(config_get_ff());
 
 	if (opt_rebase < 0)
-		opt_rebase = config_get_rebase();
+		opt_rebase = config_get_rebase(&rebase_unspecified);
 
 	if (read_cache_unmerged())
 		die_resolve_conflict("pull");
@@ -1009,33 +1045,36 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (opt_rebase && 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_verbosity >= 0)
+			show_advice_pull_non_ff();
+	}
+
 	if (opt_rebase) {
 		int ret = 0;
 		int ran_ff = 0;
+
+		struct object_id newbase;
+		struct object_id upstream;
+		get_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head,
+						merge_heads.oid, &rebase_fork_point);
+
 		if ((recurse_submodules == RECURSE_SUBMODULES_ON ||
 		     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND) &&
-		    submodule_touches_in_range(the_repository, &rebase_fork_point, &curr_head))
+		    submodule_touches_in_range(the_repository, &upstream, &curr_head))
 			die(_("cannot rebase with locally recorded submodule modifications"));
 		if (!autostash) {
-			struct commit_list *list = NULL;
-			struct commit *merge_head, *head;
-
-			head = lookup_commit_reference(the_repository,
-						       &orig_head);
-			commit_list_insert(head, &list);
-			merge_head = lookup_commit_reference(the_repository,
-							     &merge_heads.oid[0]);
-			if (repo_is_descendant_of(the_repository,
-						  merge_head, list)) {
+			if (can_ff) {
 				/* we can fast-forward this without invoking rebase */
 				opt_ff = "--ff-only";
 				ran_ff = 1;
 				ret = run_merge();
 			}
-			free_commit_list(list);
 		}
 		if (!ran_ff)
-			ret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
+			ret = run_rebase(&newbase, &upstream);
 
 		if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
 			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))