From: Yoichi Nakayama Date: Thu, 27 Aug 2026 14:38:15 GMT Subject: Re: [PATCH v3] worktree repair: detect relative path in .git file correctly Message-ID: In-Reply-To: On Sat, Aug 22, 2026 at 7:21 AM Junio C Hamano wrote: > > Junio C Hamano writes: > > > Among these three, the last one obviously belongs here. Leaving the > > relative path relative was the reason why we wanted to add > > read_gitfile_raw() in the first place. > > > > But moving the other two to here is a bit iffy. The worktree repair > > job used to call read_gitfile_gently(), which means it used to > > depend on what the first two did for it, namely, to make the > > relative path after "gitdir:" from the .git file relative to the > > current process to make it usable, and to ensure that the directory > > pointed at by .git is indeed a git directory. Is it correct to drop > > these from the caller, which now calls read_gitfile_raw() instead? > > > > IOW, I am not sure if the two functions are split correctly. I > > expected that the only two things read_gitfile_gently() would do > > after read_gitfile_raw() are (1) upon error, jump to cleanup_return, > > and (2) otherwise call strbuf_realpath(). > > Actually, I take half of that back. If we pretend the leading part > of the "path", which could be absolute, the result will lose the > relative-ness of the original. Keeping the tweaking of the relative > path in read_gitfile_gently() is reasonable. As is_git_directory() > needs to be called on a usable path, if the relative path tweaking > cannot be done inside read_gitfile_raw(), it cannot check if the > directory is is_git_directory(), either. > > So, the change to setup.c is fine as is. I didn't look at the > changes to worktree.c, though. If we were to keep the call to `is_git_directory()` inside `read_gitfile_raw()`, it is necessary to calculate the absolute path of the candidate. While it is possible to calculate the path in `read_gitfile_raw()`, call `is_git_directory()`, and then discard the calculated path, I felt it was wasteful to calculate the absolute path twice when `read_gitfile_raw()` is called from `read_gitfile_gently()`. From another perspective, while the function name `read_gitfile_*()` suggests its role is simply to read the `.git` file, I felt that verifying whether the resulting path is a valid git directory went beyond that scope. I understand the desire to minimize the functional differences between `read_gitfile_raw()` and `read_gitfile_gently()`, but for the reasons mentioned above, I have moved the check performed by `is_git_directory()` to the caller of `read_gitfile_raw()` within worktree.c. I have moved the `is_git_directory()` call to worktree.c so as not to alter the behavior when a `.git` file points to a location other than any git directory, but I didn't mention it in the commit message. Are you concerned about the lack of explanation in the commit message, or about the functional differences between `read_gitfile_raw()` and `read_gitfile_gently()`? Thanks, -- Yoichi NAKAYAMA