From 77ab58c0919ea9d13251c3a1ba52b301d91e1912 Mon Sep 17 00:00:00 2001 From: John Cai Date: Fri, 18 Mar 2022 13:54:02 +0000 Subject: [PATCH 1/2] rebase: use test_commit helper in setup To prepare for the next commit that will test rebase with oids instead of branch names, update the rebase setup test to add a couple of tags we can use. This uses the test_commit helper so we can replace some lines that add a commit manually. Setting logAllRefUpdates is not necessary because it's on by default for repositories with a working tree. Helped-by: Phillip Wood Signed-off-by: John Cai Signed-off-by: Junio C Hamano --- t/t3400-rebase.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 71b1735e1d..0643d01525 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -18,10 +18,7 @@ GIT_AUTHOR_EMAIL=bogus@email@address export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL test_expect_success 'prepare repository with topic branches' ' - git config core.logAllRefUpdates true && - echo First >A && - git update-index --add A && - git commit -m "Add A." && + test_commit "Add A." A First First && git checkout -b force-3way && echo Dummy >Y && git update-index --add Y && @@ -32,9 +29,7 @@ test_expect_success 'prepare repository with topic branches' ' git mv A D/A && git commit -m "Move A." && git checkout -b my-topic-branch main && - echo Second >B && - git update-index --add B && - git commit -m "Add B." && + test_commit "Add B." B Second Second && git checkout -f main && echo Third >>A && git update-index A && From bdff97a3f6e66107abd136815c8fa1eda5c67aca Mon Sep 17 00:00:00 2001 From: John Cai Date: Fri, 18 Mar 2022 13:54:03 +0000 Subject: [PATCH 2/2] rebase: set REF_HEAD_DETACH in checkout_up_to_date() "git rebase A B" where B is not a commit should behave as if the HEAD got detached at B and then the detached HEAD got rebased on top of A. A bug however overwrites the current branch to point at B, when B is a descendant of A (i.e. the rebase ends up being a fast-forward). See [1] for the original bug report. The callstack from checkout_up_to_date() is the following: cmd_rebase() -> checkout_up_to_date() -> reset_head() -> update_refs() -> update_ref() When B is not a valid branch but an oid, rebase sets the head_name of rebase_options to NULL. This value gets passed down this call chain through the branch member of reset_head_opts also getting set to NULL all the way to update_refs(). Then update_refs() checks ropts.branch to decide whether or not to switch branches. If ropts.branch is NULL, it calls update_ref() to update HEAD. At this point however, from rebase's point of view, we want a detached HEAD. But, since checkout_up_to_date() does not set the RESET_HEAD_DETACH flag, the update_ref() call will deference HEAD and update the branch its pointing to. We want the HEAD detached at B instead. Fix this bug by adding the RESET_HEAD_DETACH flag in checkout_up_to_date if B is not a valid branch, so that once reset_head() calls update_refs(), it calls update_ref() with REF_NO_DEREF which updates HEAD directly intead of deferencing it and updating the branch that HEAD points to. Also add a test to ensure the correct behavior. [1] https://lore.kernel.org/git/YiokTm3GxIZQQUow@newk/ Reported-by: Michael McClimon Signed-off-by: John Cai Signed-off-by: Junio C Hamano --- builtin/rebase.c | 2 ++ t/t3400-rebase.sh | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/builtin/rebase.c b/builtin/rebase.c index b29ad2b65e..27fde7bf28 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -829,6 +829,8 @@ static int checkout_up_to_date(struct rebase_options *options) ropts.oid = &options->orig_head; ropts.branch = options->head_name; ropts.flags = RESET_HEAD_RUN_POST_CHECKOUT_HOOK; + if (!ropts.branch) + ropts.flags |= RESET_HEAD_DETACH; ropts.head_msg = buf.buf; if (reset_head(the_repository, &ropts) < 0) ret = error(_("could not switch to %s"), options->switch_to); diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index 0643d01525..d5a8ee39fc 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -394,6 +394,15 @@ test_expect_success 'switch to branch not checked out' ' git rebase main other ' +test_expect_success 'switch to non-branch detaches HEAD' ' + git checkout main && + old_main=$(git rev-parse HEAD) && + git rebase First Second^0 && + test_cmp_rev HEAD Second && + test_cmp_rev main $old_main && + test_must_fail git symbolic-ref HEAD +' + test_expect_success 'refuse to switch to branch checked out elsewhere' ' git checkout main && git worktree add wt &&