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

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

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jul 11, 2015, 00:05 UTC
Message-ID
<1436573146-3893-4-git-send-email-sunshine@sunshineco.com>
In-Reply-To
<1436573146-3893-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 be retrofitted to populate the new worktree via "git reset --hard" rather than "git checkout", and it will want to use die_if_checked_out() to perform this check as well. But, the reliance upon order of operations (creating .git/worktrees before checking if a branch is already checked out) is fragile, and, 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.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 builtin/checkout.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e75fb5e..fc8bd79 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -916,12 +916,6 @@ static void die_if_checked_out(struct branch_info *new)
 	DIR *dir;
 	struct dirent *d;
 
-	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
@@ -929,6 +923,12 @@ static void die_if_checked_out(struct branch_info *new)
 	 */
 	check_linked_checkout(new, NULL);
 
+	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+	if ((dir = opendir(path.buf)) == NULL) {
+		strbuf_release(&path);
+		return;
+	}
+
 	while ((d = readdir(dir)) != NULL) {
 		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
 			continue;
-- 
2.5.0.rc1.201.ga12d9f8
Previous: Eric SunshineNext: Eric Sunshine
Message 4 of 31 in “worktree: use "git reset --hard" to populate worktree”
  1. 00/16 worktree: use "git reset --hard" to populate worktreeEric Sunshine, Jul 11, 2015
  2. 01/16 checkout: avoid resolving HEAD unnecessarilyEric Sunshine, Jul 11, 2015
  3. 02/16 checkout: name check_linked_checkouts() more meaningfullyEric Sunshine, Jul 11, 2015
  4. 03/16 checkout: improve die_if_checked_out() robustnessEric Sunshine, Jul 11, 2015
  5. 04/16 checkout: die_if_checked_out: simplify strbuf managementEric Sunshine, Jul 11, 2015
  6. 05/16 checkout: generalize die_if_checked_out() branch name argumentEric Sunshine, Jul 11, 2015
  7. 06/16 branch: publish die_if_checked_out()Eric Sunshine, Jul 11, 2015
  8. 07/16 worktree: simplify new branch (-b/-B) option checkingEric Sunshine, Jul 11, 2015
  9. 08/16 worktree: introduce options containerEric Sunshine, Jul 11, 2015
  10. 09/16 worktree: make --detach mutually exclusive with -b/-BEric Sunshine, Jul 11, 2015
  11. 10/16 worktree: make branch creation distinct from worktree populationEric Sunshine, Jul 11, 2015
  12. Duy NguyenJul 12, 2015
  13. Eric SunshineJul 12, 2015
  14. Duy NguyenJul 12, 2015
  15. Eric SunshineJul 12, 2015
  16. Eric SunshineJul 12, 2015
  17. Eric SunshineJul 12, 2015
  18. Duy NguyenJul 12, 2015
  19. 11/16 worktree: add_worktree: construct worktree-population command locallyEric Sunshine, Jul 11, 2015
  20. 12/16 worktree: detect branch symref/detach and error conditions locallyEric Sunshine, Jul 11, 2015
  21. 13/16 worktree: make setup of new HEAD distinct from worktree populationEric Sunshine, Jul 11, 2015
  22. 14/16 worktree: avoid resolving HEAD unnecessarilyEric Sunshine, Jul 11, 2015
  23. 15/16 worktree: populate via "git reset --hard" rather than "git checkout"Eric Sunshine, Jul 11, 2015
  24. 16/16 checkout: drop intimate knowledge of new worktree initial populationEric Sunshine, Jul 11, 2015
  25. Junio C HamanoJul 13, 2015
  26. Michael J GruberJul 14, 2015
  27. Duy NguyenJul 14, 2015
  28. Eric SunshineJul 14, 2015
  29. Eric SunshineJul 15, 2015
  30. Duy NguyenJul 15, 2015
  31. Eric SunshineJul 12, 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.