git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: Duy Nguyen <pclouds@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Mark Levedahl <mlevedahl@gmail.com>,
	Mikael Magnusson <mikachu@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH 03/16] checkout: improve die_if_checked_out() robustness
Date: Fri, 10 Jul 2015 20:05:33 -0400	[thread overview]
Message-ID: <1436573146-3893-4-git-send-email-sunshine@sunshineco.com> (raw)
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

  parent reply	other threads:[~2015-07-11  7:20 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-11  0:05 [PATCH 00/16] worktree: use "git reset --hard" to populate worktree Eric Sunshine
2015-07-11  0:05 ` [PATCH 01/16] checkout: avoid resolving HEAD unnecessarily Eric Sunshine
2015-07-11  0:05 ` [PATCH 02/16] checkout: name check_linked_checkouts() more meaningfully Eric Sunshine
2015-07-11  0:05 ` Eric Sunshine [this message]
2015-07-11  0:05 ` [PATCH 04/16] checkout: die_if_checked_out: simplify strbuf management Eric Sunshine
2015-07-11  0:05 ` [PATCH 05/16] checkout: generalize die_if_checked_out() branch name argument Eric Sunshine
2015-07-11  0:05 ` [PATCH 06/16] branch: publish die_if_checked_out() Eric Sunshine
2015-07-11  0:05 ` [PATCH 07/16] worktree: simplify new branch (-b/-B) option checking Eric Sunshine
2015-07-11  0:05 ` [PATCH 08/16] worktree: introduce options container Eric Sunshine
2015-07-11  0:05 ` [PATCH 09/16] worktree: make --detach mutually exclusive with -b/-B Eric Sunshine
2015-07-11  0:05 ` [PATCH 10/16] worktree: make branch creation distinct from worktree population Eric Sunshine
2015-07-12  1:20   ` Duy Nguyen
2015-07-12  2:36     ` Eric Sunshine
2015-07-12  2:48       ` Duy Nguyen
2015-07-12  3:10         ` Eric Sunshine
2015-07-12  3:14           ` Eric Sunshine
2015-07-12  3:27             ` Eric Sunshine
2015-07-12 10:03               ` Duy Nguyen
     [not found]               ` <CACsJy8BTTdWrCZNz=y686pgju5X8-2mPrNNQ-+z4ByeKD6O5Uw@mail.gmail.com>
2015-07-12 19:20                 ` Eric Sunshine
2015-07-11  0:05 ` [PATCH 11/16] worktree: add_worktree: construct worktree-population command locally Eric Sunshine
2015-07-11  0:05 ` [PATCH 12/16] worktree: detect branch symref/detach and error conditions locally Eric Sunshine
2015-07-11  0:05 ` [PATCH 13/16] worktree: make setup of new HEAD distinct from worktree population Eric Sunshine
2015-07-11  0:05 ` [PATCH 14/16] worktree: avoid resolving HEAD unnecessarily Eric Sunshine
2015-07-11  0:05 ` [PATCH 15/16] worktree: populate via "git reset --hard" rather than "git checkout" Eric Sunshine
2015-07-11  0:05 ` [PATCH 16/16] checkout: drop intimate knowledge of new worktree initial population Eric Sunshine
2015-07-13 18:36 ` [PATCH 00/16] worktree: use "git reset --hard" to populate worktree Junio C Hamano
2015-07-14  9:53   ` Michael J Gruber
2015-07-14 10:09     ` Duy Nguyen
2015-07-14 16:40     ` Eric Sunshine
2015-07-15  6:48   ` Eric Sunshine
2015-07-15  9:59     ` Duy Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1436573146-3893-4-git-send-email-sunshine@sunshineco.com \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mikachu@gmail.com \
    --cc=mlevedahl@gmail.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).