[PATCH v3 0/3] worktree: stop using "the_repository" in is_current_worktree()

[Phillip Wood <phillip.wood123@gmail.com>]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

This is a follow up to pw/no-more-NULL-means-current-worktree that removes
"the_repository" from is_current_worktree() and get_worktree_git_dir().
The first patch removes the use of "the_repository" when determining
if a worktree is current. Patches 2 & 3 require a non-NULL worktree
when calling get_worktree_git_dir() to remove the last use of
"the_repository" in that function.

Changes since V2

 - Patch 1: expanded commit message and added a comment to is_current
            member of struct worktree. (thanks to Patrick)

Changes since V1

 - Patch 2: fixed indentation (thanks to Junio)
 - Patch 3: removed stale comment (thanks to Junio)

Base-Commit: 7f19e4e1b6a3ad259e2ed66033e01e03b8b74c5e
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fworktree-is-current-use-repo%2Fv3
View-Changes-At: https://github.com/phillipwood/git/compare/7f19e4e1b...c33290280
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/worktree-is-current-use-repo/v3


Phillip Wood (3):
  worktree: remove "the_repository" from is_current_worktree()
  worktree add: stop reading ".git/HEAD"
  worktree: reject NULL worktree in get_worktree_git_dir()

 builtin/worktree.c      | 21 ++-------------------
 t/t2400-worktree-add.sh | 28 ++++++++++++----------------
 worktree.c              | 10 +++++-----
 worktree.h              |  3 +--
 4 files changed, 20 insertions(+), 42 deletions(-)

Range-diff against v2:
1:  075700a2256 ! 1:  5357c0dd53e worktree: remove "the_repository" from is_current_worktree()
    @@ Metadata
      ## Commit message ##
         worktree: remove "the_repository" from is_current_worktree()
     
    -    is_current_worktree() compares the gitdir of the worktree to the gitdir
    -    of "the_repository" and returns true when they match. To get the gitdir
    -    of the worktree it calls get_workree_git_dir() which also depends on
    -    "the_repository". This has the effect that even if "wt->path" matches
    +    The "is_current" member of struct worktree was added in 750e8a60d69
    +    (worktree.c: mark current worktree, 2016-04-22) and was used in
    +    8d9fdd7087d (worktree.c: check whether branch is rebased in another
    +    worktree, 2016-04-22) to optionally skip the current worktree when
    +    seeing if a branch is already checked out in die_if_checked_out().
    +
    +    To determine if a worktree is "current" is_current_worktree() compares
    +    the gitdir of the worktree to the gitdir of "the_repository"
    +    and returns true when they match. To get the gitdir of the
    +    worktree it calls get_workree_git_dir() which also depends on
    +    "the_repository". This means that even if "wt->path" matches
         "wt->repo->worktree" is_current_worktree(wt) will return false when
    -    "wt->repo" is not "the_repository" which is confusing.
    +    "wt->repo" is not "the_repository". Consequently die_if_checked_out()
    +    will fail to skip such a worktree when checking if a branch is already
    +    checked out and may die errounously. Fix this by using the worktree's
    +    repository instance instead of "the_repository" when comparing gitdirs.
     
         The use of "the_repository" in is_current_wortree() comes from
         replacing get_git_dir() with repo_get_git_dir() in 246deeac951
         (environment: make `get_git_dir()` accept a repository, 2024-09-12). In
         get_worktree_git_dir() it comes from replacing git_common_path() with
         repo_common_path() in 07242c2a5af (path: drop `git_common_path()`
    -    in favor of `repo_common_path()`, 2025-02-07). In both cases we have
    -    a repository instance available so use that instead. This means
    -    that a worktree "wt" is always considered current when "wt->path"
    -    matches "wt->repo->worktree" and so the worktree returned by
    -    get_worktree_from_repository() is always considered current.
    +    in favor of `repo_common_path()`, 2025-02-07). In both cases the
    +    replacements appear to have been mechanical.
     
         Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     
    @@ worktree.c: char *get_worktree_git_dir(const struct worktree *wt)
      }
      
      static struct worktree *find_worktree_by_suffix(struct worktree **list,
    +
    + ## worktree.h ##
    +@@ worktree.h: struct worktree {
    + 	struct object_id head_oid;
    + 	int is_detached;
    + 	int is_bare;
    +-	int is_current;
    ++	int is_current;		/* does `path` match `repo->worktree` */
    + 	int lock_reason_valid; /* private */
    + 	int prune_reason_valid; /* private */
    + };
2:  c3c5767725d = 2:  4d50e6bcb2e worktree add: stop reading ".git/HEAD"
3:  75eecc8492e = 3:  c3329028010 worktree: reject NULL worktree in get_worktree_git_dir()
-- 
2.52.0.362.g884e03848a9.dirty