From d2172ef02dee468b85bc4567332cbf58cd6b4b0a Mon Sep 17 00:00:00 2001 From: Ben Wijen Date: Fri, 30 Aug 2019 17:16:05 +0200 Subject: [PATCH 1/2] builtin/rebase.c: make sure the active branch isn't moved when autostashing Consider the following scenario: git checkout not-the-master work work work git rebase --autostash upstream master Here 'rebase --autostash ' incorrectly moves the active branch (not-the-master) to master (before the rebase). The expected behavior: (58794775:/git-rebase.sh:526) AUTOSTASH=$(git stash create autostash) git reset --hard git checkout master git rebase upstream git stash apply $AUTOSTASH The actual behavior: (6defce2b:/builtin/rebase.c:1062) AUTOSTASH=$(git stash create autostash) git reset --hard master git checkout master git rebase upstream git stash apply $AUTOSTASH This commit reinstates the 'legacy script' behavior as introduced with 58794775: rebase: implement --[no-]autostash and rebase.autostash Signed-off-by: Ben Wijen Signed-off-by: Junio C Hamano --- builtin/rebase.c | 9 ++++++--- t/t3420-rebase-autostash.sh | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 670096c065..0a2f9273ee 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1968,9 +1968,12 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) state_dir_path("autostash", &options); struct child_process stash = CHILD_PROCESS_INIT; struct object_id oid; - struct commit *head = - lookup_commit_reference(the_repository, - &options.orig_head); + struct object_id head_oid; + struct commit *head; + + if (get_oid("HEAD", &head_oid)) + die(_("could not determine HEAD revision")); + head = lookup_commit_reference(the_repository, &head_oid); argv_array_pushl(&stash.args, "stash", "create", "autostash", NULL); diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index b8f4d03467..1131e0016a 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -306,4 +306,12 @@ test_expect_success 'branch is left alone when possible' ' test unchanged-branch = "$(git rev-parse --abbrev-ref HEAD)" ' +test_expect_success 'never change active branch' ' + git checkout -b not-the-feature-branch unrelated-onto-branch && + test_when_finished "git reset --hard && git checkout master" && + echo changed >file0 && + git rebase --autostash not-the-feature-branch feature-branch && + test_cmp_rev not-the-feature-branch unrelated-onto-branch +' + test_done From bf1e28e0ad9b1d0d04203ebc43b9008de1969503 Mon Sep 17 00:00:00 2001 From: Ben Wijen Date: Fri, 30 Aug 2019 17:16:06 +0200 Subject: [PATCH 2/2] builtin/rebase.c: Remove pointless message When doing 'git rebase --autostash ' with a dirty worktree a 'HEAD is now at ...' message is emitted, which is pointless as it refers to the old active branch which isn't actually moved. This commit removes the 'HEAD is now at...' message. Signed-off-by: Ben Wijen Signed-off-by: Junio C Hamano --- builtin/rebase.c | 16 +--------------- t/t3420-rebase-autostash.sh | 4 ---- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/builtin/rebase.c b/builtin/rebase.c index 0a2f9273ee..118205e481 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1968,12 +1968,6 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) state_dir_path("autostash", &options); struct child_process stash = CHILD_PROCESS_INIT; struct object_id oid; - struct object_id head_oid; - struct commit *head; - - if (get_oid("HEAD", &head_oid)) - die(_("could not determine HEAD revision")); - head = lookup_commit_reference(the_repository, &head_oid); argv_array_pushl(&stash.args, "stash", "create", "autostash", NULL); @@ -1994,17 +1988,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) options.state_dir); write_file(autostash, "%s", oid_to_hex(&oid)); printf(_("Created autostash: %s\n"), buf.buf); - if (reset_head(&head->object.oid, "reset --hard", + if (reset_head(NULL, "reset --hard", NULL, RESET_HEAD_HARD, NULL, NULL) < 0) die(_("could not reset --hard")); - printf(_("HEAD is now at %s"), - find_unique_abbrev(&head->object.oid, - DEFAULT_ABBREV)); - strbuf_reset(&buf); - pp_commit_easy(CMIT_FMT_ONELINE, head, &buf); - if (buf.len > 0) - printf(" %s", buf.buf); - putchar('\n'); if (discard_index(the_repository->index) < 0 || repo_read_index(the_repository) < 0) diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index 1131e0016a..5f7e73cf83 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -37,7 +37,6 @@ test_expect_success setup ' create_expected_success_am () { cat >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) - HEAD is now at $(git rev-parse --short feature-branch) third commit First, rewinding head to replay your work on top of it... Applying: second commit Applying: third commit @@ -48,7 +47,6 @@ create_expected_success_am () { create_expected_success_interactive () { q_to_cr >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) - HEAD is now at $(git rev-parse --short feature-branch) third commit Applied autostash. Successfully rebased and updated refs/heads/rebased-feature-branch. EOF @@ -57,7 +55,6 @@ create_expected_success_interactive () { create_expected_failure_am () { cat >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) - HEAD is now at $(git rev-parse --short feature-branch) third commit First, rewinding head to replay your work on top of it... Applying: second commit Applying: third commit @@ -70,7 +67,6 @@ create_expected_failure_am () { create_expected_failure_interactive () { cat >expected <<-EOF $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual) - HEAD is now at $(git rev-parse --short feature-branch) third commit Applying autostash resulted in conflicts. Your changes are safe in the stash. You can run "git stash pop" or "git stash drop" at any time.