[PATCH v3 08/23] checkout: fix bug with --to and relative HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Given "git checkout --to <path> HEAD~1", the new worktree's HEAD should
begin life at the current branch's HEAD~1, however, it actually ends up
at HEAD~2. This happens because:

    1. git-checkout resolves HEAD~1

    2. to satisfy is_git_directory(), prepare_linked_worktree() creates
       a HEAD for the new worktree with the value of the resolved HEAD~1

    3. git-checkout re-invokes itself with the same arguments within the
       new worktree to populate the worktree

    4. the sub git-checkout resolves HEAD~1 relative to its own HEAD,
       which is the resolved HEAD~1 from the original invocation,
       resulting unexpectedly and incorrectly in HEAD~2 (relative to the
       original)

Fix this by unconditionally assigning the current worktree's HEAD as the
value of the new worktree's HEAD.

As a side-effect, this change also eliminates a dependence within
prepare_linked_checkout() upon 'struct branch_info'. The plan is to
eventually relocate "git checkout --to" functionality to "git worktree
add", and worktree.c won't have knowledge of 'struct branch_info', so
removal of this dependency is a step toward that goal.

Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
---
 builtin/checkout.c     | 16 ++++++++++++----
 t/t2025-checkout-to.sh | 10 ++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2079aa4..5ada22a 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -863,6 +863,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,
 	struct stat st;
 	struct child_process cp;
 	int counter = 0, len, ret;
+	unsigned char rev[20];
 
 	if (!new->commit)
 		die(_("no branch specified"));
@@ -920,13 +921,20 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,
 		   real_path(get_git_common_dir()), name);
 	/*
 	 * This is to keep resolve_ref() happy. We need a valid HEAD
-	 * or is_git_directory() will reject the directory. Any valid
-	 * value would do because this value will be ignored and
-	 * replaced at the next (real) checkout.
+	 * or is_git_directory() will reject the directory. Moreover, HEAD
+	 * in the new worktree must resolve to the same value as HEAD in
+	 * the current tree since the command invoked to populate the new
+	 * worktree will be handed the branch/ref specified by the user.
+	 * For instance, if the user asks for the new worktree to be based
+	 * at HEAD~5, then the resolved HEAD~5 in the new worktree must
+	 * match the resolved HEAD~5 in the current tree in order to match
+	 * the user's expectation.
 	 */
+	if (!resolve_ref_unsafe("HEAD", 0, rev, NULL))
+		die(_("unable to resolve HEAD"));
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/HEAD", sb_repo.buf);
-	write_file(sb.buf, 1, "%s\n", sha1_to_hex(new->commit->object.sha1));
+	write_file(sb.buf, 1, "%s\n", sha1_to_hex(rev));
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
 	write_file(sb.buf, 1, "../..\n");
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index a8d9336..0fd731b 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -134,4 +134,14 @@ test_expect_success 'checkout with grafts' '
 	test_cmp expected actual
 '
 
+test_expect_success 'checkout --to from relative HEAD' '
+	test_commit a &&
+	test_commit b &&
+	test_commit c &&
+	git rev-parse HEAD~1 >expected &&
+	git checkout --to relhead HEAD~1 &&
+	git -C relhead rev-parse HEAD >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
2.5.0.rc1.197.g417e668

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]