Merge branch 'pb/pull-rebase-recurse-submodules'

"git pull --rebase --recurse-submodules" checked for local changes
in a wrong range and failed to run correctly when it should.

* pb/pull-rebase-recurse-submodules:
  pull: check for local submodule modifications with the right range
  t5572: describe '--rebase' tests a little more
  t5572: add notes on a peculiar test
  pull --rebase: compute rebase arguments in separate function

1 files changed, 36 insertions, 12 deletions

diff --git a/builtin/pull.c b/builtin/pull.c
index 1034372..aa56ebc 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -852,21 +852,42 @@ 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 strvec args = STRVEC_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;
 
+	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 */
@@ -894,12 +915,9 @@ static int run_rebase(const struct object_id *curr_head,
 		warning(_("ignoring --verify-signatures for rebase"));
 
 	strvec_push(&args, "--onto");
-	strvec_push(&args, oid_to_hex(merge_head));
+	strvec_push(&args, oid_to_hex(newbase));
 
-	if (fork_point && !is_null_oid(fork_point))
-		strvec_push(&args, oid_to_hex(fork_point));
-	else
-		strvec_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);
@@ -1011,9 +1029,15 @@ 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;
+		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;
@@ -1034,7 +1058,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			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))