git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 03/22] checkout: improve die_if_checked_out() robustness

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jul 17, 2015, 22:59 UTC
Message-ID
<1437174017-81687-4-git-send-email-sunshine@sunshineco.com>
In-Reply-To
<1437174017-81687-1-git-send-email-sunshine@sunshineco.com>

die_if_checked_out() is intended to check if the branch about to be checked out is already checked out either in the main worktree or in a linked worktree. However, if .git/worktrees directory does not exist, then it never bothers checking the main worktree, even though the specified branch might indeed be checked out there, which is fragile behavior.

This hasn't been a problem in practice since the current implementation of "git worktree add" (and, earlier, "git checkout --to") always creates .git/worktrees before die_if_checked_out() is called by the child "git checkout" invocation which populates the new worktree.

However, git-worktree will eventually want to call die_if_checked_out() itself rather than only doing so indirectly as a side-effect of invoking git-checkout, and reliance upon order of operations (creating .git/worktrees before checking if a branch is already checked out) is fragile. As a general function, callers should not be expected to abide by this undocumented and unwarranted restriction. Therefore, make die_if_checked_out() more robust by checking the main worktree whether .git/worktrees exists or not.

While here, also move a comment explaining why die_if_checked_out()'s helper parses HEAD manually. Such information resides more naturally with the helper itself rather than at its first point of call.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
No changes since v2.
 builtin/checkout.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e75fb5e..1992c41 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -880,6 +880,11 @@ static void check_linked_checkout(struct branch_info *new, const char *id)
 	struct strbuf gitdir = STRBUF_INIT;
 	const char *start, *end;
 
+	/*
+	 * $GIT_COMMON_DIR/HEAD is practically outside
+	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
+	 * uses git_path). Parse the ref ourselves.
+	 */
 	if (id)
 		strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
 	else
@@ -916,19 +921,14 @@ static void die_if_checked_out(struct branch_info *new)
 	DIR *dir;
 	struct dirent *d;
 
+	check_linked_checkout(new, NULL);
+
 	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
 	if ((dir = opendir(path.buf)) == NULL) {
 		strbuf_release(&path);
 		return;
 	}
 
-	/*
-	 * $GIT_COMMON_DIR/HEAD is practically outside
-	 * $GIT_DIR so resolve_ref_unsafe() won't work (it
-	 * uses git_path). Parse the ref ourselves.
-	 */
-	check_linked_checkout(new, NULL);
-
 	while ((d = readdir(dir)) != NULL) {
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;

-- 
2.5.0.rc2.378.g0af52e8
Previous: Eric SunshineNext: Eric Sunshine
Message 4 of 24 in “rid git-checkout of too-intimate knowledge of new worktree”
  1. 00/22 rid git-checkout of too-intimate knowledge of new worktreeEric Sunshine, Jul 17, 2015
  2. 01/22 checkout: avoid resolving HEAD unnecessarilyEric Sunshine, Jul 17, 2015
  3. 02/22 checkout: name check_linked_checkouts() more meaningfullyEric Sunshine, Jul 17, 2015
  4. 03/22 checkout: improve die_if_checked_out() robustnessEric Sunshine, Jul 17, 2015
  5. 04/22 checkout: die_if_checked_out: simplify strbuf managementEric Sunshine, Jul 17, 2015
  6. 05/22 checkout: generalize die_if_checked_out() branch name argumentEric Sunshine, Jul 17, 2015
  7. 06/22 checkout: check_linked_checkout: improve "already checked out" aestheticEric Sunshine, Jul 17, 2015
  8. 07/22 checkout: check_linked_checkout: simplify symref parsingEric Sunshine, Jul 17, 2015
  9. 08/22 checkout: teach check_linked_checkout() about symbolic link HEADEric Sunshine, Jul 17, 2015
  10. 09/22 branch: publish die_if_checked_out()Eric Sunshine, Jul 17, 2015
  11. 10/22 worktree: improve worktree setup messageEric Sunshine, Jul 17, 2015
  12. 11/22 worktree: simplify new branch (-b/-B) option checkingEric Sunshine, Jul 17, 2015
  13. 12/22 worktree: introduce options containerEric Sunshine, Jul 17, 2015
  14. 13/22 worktree: make --detach mutually exclusive with -b/-BEric Sunshine, Jul 17, 2015
  15. 14/22 worktree: add: suppress auto-vivication with --detach and no <branch>Eric Sunshine, Jul 17, 2015
  16. Eric SunshineJul 17, 2015
  17. 15/22 worktree: make branch creation distinct from worktree populationEric Sunshine, Jul 17, 2015
  18. 16/22 worktree: elucidate environment variables intended for child processesEric Sunshine, Jul 17, 2015
  19. 17/22 worktree: add_worktree: construct worktree-population command locallyEric Sunshine, Jul 17, 2015
  20. 18/22 worktree: detect branch-name/detached and error conditions locallyEric Sunshine, Jul 17, 2015
  21. 19/22 worktree: make setup of new HEAD distinct from worktree populationEric Sunshine, Jul 17, 2015
  22. 20/22 worktree: avoid resolving HEAD unnecessarilyEric Sunshine, Jul 17, 2015
  23. 21/22 worktree: populate via "git reset --hard" rather than "git checkout"Eric Sunshine, Jul 17, 2015
  24. 22/22 checkout: drop intimate knowledge of newly created worktreeEric Sunshine, Jul 17, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.