From 62573a57f0919f4d1129962b09462946e69025ff Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Jul 2020 19:32:11 -0400 Subject: [PATCH 1/4] worktree: drop pointless strbuf_release() The content of this strbuf is unconditionally detached several lines before the strbuf_release() and the strbuf is never touched again after that point. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- worktree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/worktree.c b/worktree.c index cba2e54598..c0df5e2c79 100644 --- a/worktree.c +++ b/worktree.c @@ -66,8 +66,6 @@ static struct worktree *get_main_worktree(void) worktree->is_bare = (is_bare_repository_cfg == 1) || is_bare_repository(); add_head_info(worktree); - - strbuf_release(&worktree_path); return worktree; } From 246756f775bdd3055da969a16d9e9c74e76a3e06 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Jul 2020 19:32:12 -0400 Subject: [PATCH 2/4] worktree: drop unused code from get_linked_worktree() This code has been unused since fa099d2322 (worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe(), 2017-04-24), so drop it. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- worktree.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/worktree.c b/worktree.c index c0df5e2c79..fa8366a3ca 100644 --- a/worktree.c +++ b/worktree.c @@ -90,9 +90,6 @@ static struct worktree *get_linked_worktree(const char *id) strbuf_strip_suffix(&worktree_path, "/."); } - strbuf_reset(&path); - strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id); - worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL); worktree->id = xstrdup(id); From 1c4854ec7333f412cd60a5b673bb4f4e43319391 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Jul 2020 19:32:13 -0400 Subject: [PATCH 3/4] worktree: drop bogus and unnecessary path munging The content of .git/worktrees//gitdir must be a path of the form "/path/to/worktree/.git". Any other content would be indicative of a corrupt "gitdir" file. To determine the path of the worktree itself one merely strips the "/.git" suffix, and this is indeed how the worktree path was determined from inception. However, 5193490442 (worktree: add a function to get worktree details, 2015-10-08) extended the path manipulation in a mysterious way. If it is unable to strip "/.git" from the path, then it instead reports the current working directory as the linked worktree's path: if (!strbuf_strip_suffix(&worktree_path, "/.git")) { strbuf_reset(&worktree_path); strbuf_add_absolute_path(&worktree_path, "."); strbuf_strip_suffix(&worktree_path, "/."); } This logic is clearly bogus; it can never be generally correct behavior. It materialized out of thin air in 5193490442 with neither explanation nor tests to illustrate a case in which it would be desirable. It's possible that this logic was introduced to somehow deal with a corrupt "gitdir" file, so that it returns _some_ sort of meaningful value, but returning the current working directory is not helpful. In fact, it is quite misleading (except in the one specific case when the current directory is the worktree whose "gitdir" entry is corrupt). Moreover, reporting the corrupt value to the user, rather than fibbing about it and hiding it outright, is more helpful since it may aid in diagnosing the problem. Therefore, drop this bogus path munging and restore the logic to the original behavior of merely stripping "/.git". Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- worktree.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/worktree.c b/worktree.c index fa8366a3ca..355824bf87 100644 --- a/worktree.c +++ b/worktree.c @@ -82,13 +82,8 @@ static struct worktree *get_linked_worktree(const char *id) if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0) /* invalid gitdir file */ goto done; - strbuf_rtrim(&worktree_path); - if (!strbuf_strip_suffix(&worktree_path, "/.git")) { - strbuf_reset(&worktree_path); - strbuf_add_absolute_path(&worktree_path, "."); - strbuf_strip_suffix(&worktree_path, "/."); - } + strbuf_strip_suffix(&worktree_path, "/.git"); worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL); From 918d8ff78099004c561e0da90fa04cd629bb3b0e Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Fri, 31 Jul 2020 19:32:14 -0400 Subject: [PATCH 4/4] worktree: retire special-case normalization of main worktree path In order for "git-worktree list" to present consistent results, get_main_worktree() performs manual normalization on the repository path (returned by get_common_dir()) after passing it through strbuf_add_absolute_path(). In particular, it cleans up the path for three distinct cases when the current working directory is (1) the main worktree, (2) the .git/ subdirectory, or (3) a bare repository. The need for such special-cases is a direct consequence of employing strbuf_add_absolute_path() which, for the sake of efficiency, doesn't bother normalizing the path (such as folding out redundant path components) after making it absolute. Lack of normalization is not typically a problem since redundant path elements make no difference when working with paths at the filesystem level. However, when preparing paths for presentation, possible redundant path components make it difficult to ensure consistency. Eliminate the need for these special cases by instead making the path absolute via strbuf_add_real_path() which normalizes the path for us. Once normalized, the only case we need to handle manually is converting it to the path of the main worktree by stripping the "/.git" suffix. This stripping of the "/.git" suffix is a regular idiom in worktree-related code; for instance, it is employed by get_linked_worktree(), as well. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- worktree.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/worktree.c b/worktree.c index 355824bf87..62217b4a6b 100644 --- a/worktree.c +++ b/worktree.c @@ -49,10 +49,8 @@ static struct worktree *get_main_worktree(void) struct worktree *worktree = NULL; struct strbuf worktree_path = STRBUF_INIT; - strbuf_add_absolute_path(&worktree_path, get_git_common_dir()); - if (!strbuf_strip_suffix(&worktree_path, "/.git/.") && /* in .git */ - !strbuf_strip_suffix(&worktree_path, "/.git")) /* in worktree */ - strbuf_strip_suffix(&worktree_path, "/."); /* in bare repo */ + strbuf_add_real_path(&worktree_path, get_git_common_dir()); + strbuf_strip_suffix(&worktree_path, "/.git"); worktree = xcalloc(1, sizeof(*worktree)); worktree->path = strbuf_detach(&worktree_path, NULL);