undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

checkout: improve die_if_checked_out() robustness

Browse Source 
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Eric Sunshine 2015-07-17 18:59:58 -04:00 committed by Junio C Hamano
parent e13d37094e
commit aaad2c948f
1 changed files with 7 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
builtin/checkout.c
View File

@ -880,6 +880,11 @@ static void check_linked_checkout(struct branch_info *new, const char *id)
struct strbuf gitdir = STRBUF_INIT; struct strbuf gitdir = STRBUF_INIT;
const char *start, *end; 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) if (id)
strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id); strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
else else
@ -916,19 +921,14 @@ static void die_if_checked_out(struct branch_info *new)
DIR *dir; DIR *dir;
struct dirent *d; struct dirent *d;
check_linked_checkout(new, NULL);
strbuf_addf(&path, "%s/worktrees", get_git_common_dir()); strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
if ((dir = opendir(path.buf)) == NULL) { if ((dir = opendir(path.buf)) == NULL) {
strbuf_release(&path); strbuf_release(&path);
return; 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) { while ((d = readdir(dir)) != NULL) {
if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, "..")) if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
continue; continue;