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
This commit is contained in:
Junio C Hamano 2020-12-03 00:18:06 -08:00
commit c692e1b673
2 changed files with 90 additions and 14 deletions

View file

@ -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))

View file

@ -101,7 +101,12 @@ test_expect_success " --[no-]recurse-submodule and submodule.recurse" '
test_path_is_file super/sub/merge_strategy_4.t
'
test_expect_success 'recursive rebasing pull' '
test_expect_success 'pull --rebase --recurse-submodules (remote superproject submodule changes, local submodule changes)' '
# This tests the following scenario :
# - local submodule has new commits
# - local superproject does not have new commits
# - upstream superproject has new commits that change the submodule pointer
# change upstream
test_commit -C child rebase_strategy &&
git -C parent submodule update --remote &&
@ -116,7 +121,10 @@ test_expect_success 'recursive rebasing pull' '
test_path_is_file super/sub/local_stuff.t
'
test_expect_success 'pull rebase recursing fails with conflicts' '
test_expect_success 'pull --rebase --recurse-submodules fails if both sides record submodule changes' '
# This tests the following scenario :
# - local superproject has new commits that change the submodule pointer
# - upstream superproject has new commits that change the submodule pointer
# local changes in submodule recorded in superproject:
test_commit -C super/sub local_stuff_2 &&
@ -136,6 +144,50 @@ test_expect_success 'pull rebase recursing fails with conflicts' '
test_i18ngrep "locally recorded submodule modifications" err
'
test_expect_success 'pull --rebase --recurse-submodules (no submodule changes, no fork-point)' '
# This tests the following scenario :
# - local submodule does not have new commits
# - local superproject has new commits that *do not* change the submodule pointer
# - upstream superproject has new commits that *do not* change the submodule pointer
# - local superproject branch has no fork-point with its remote-tracking counter-part
# create upstream superproject
test_create_repo submodule &&
test_commit -C submodule first_in_sub &&
test_create_repo superprojet &&
test_commit -C superprojet first_in_super &&
git -C superprojet submodule add ../submodule &&
git -C superprojet commit -m "add submodule" &&
test_commit -C superprojet third_in_super &&
# clone superproject
git clone --recurse-submodules superprojet superclone &&
# add commits upstream
test_commit -C superprojet fourth_in_super &&
# create topic branch in clone, not based on any remote-tracking branch
git -C superclone checkout -b feat HEAD~1 &&
test_commit -C superclone first_on_feat &&
git -C superclone pull --rebase --recurse-submodules origin master
'
# NOTE:
#
# This test is particular because there is only a single commit in the upstream superproject
# 'parent' (which adds the submodule 'a-submodule'). The clone of the superproject
# ('child') hard-resets its branch to a new root commit with the same tree as the one
# from the upstream superproject, so that its branch has no merge-base with its
# remote-tracking counterpart, and then calls 'git pull --recurse-submodules --rebase'.
# The result is that the local branch is reset to the remote-tracking branch (as it was
# originally before the hard-reset).
# The only commit in the range generated by 'submodule.c::submodule_touches_in_range' and
# passed to 'submodule.c::collect_changed_submodules' is the new (regenerated) initial commit,
# which adds the submodule.
# However, 'submodule_touches_in_range' does not error (even though this commit adds the submodule)
# because 'combine-diff.c::diff_tree_combined' returns early, as the initial commit has no parents.
test_expect_success 'branch has no merge base with remote-tracking counterpart' '
rm -rf parent child &&