Re: [PATCH] worktree repair: detect relative path in .git file correctly

On Tue, Aug 18, 2026 at 2:21 AM Junio C Hamano <[email protected]> wrote:
>
> "Yoichi NAKAYAMA via GitGitGadget" <[email protected]> writes:
>
> > From: Yoichi NAKAYAMA <[email protected]>
> >
> > Since read_gitfile_gently() always returns an absolute path, the
> > conversion from a relative path to an absolute path was not
> > functioning and dead code existed.
>
> This is ugly.  What problem is this really fixing?  What "conversion
> from a relative path to an absolute path" does the above refer to?
> What "dead code"?  Where in what file and what function?  Why does
> the caller even care if it is absolute or relative?  Shouldn't they
> work equally well as long as they point at the right location?
>
> The proposed log message hides so many details to evaluate the claim
> that this is a good change, and raises many unanswered questions.

I'm sorry, the commit message lacked an explanation.
Let me explain the details of the issue I want to resolve.

When we create a worktree using default settings or with
`worktree.useRelativePaths=false`,
the cross references between the worktree and the repository
(specifically `worktree/id/gitdir`
in the main repository and the `.git` file in the worktree) are
recorded using absolute paths.

% mkdir repo
% cd repo
repo % git init
Initialized empty Git repository in /private/tmp/repo/.git/
repo % git commit --allow-empty -m init
[master (root-commit) bb4f6a1] init
repo % git config worktree.useRelativePaths
repo % git worktree add ../foo --detach
Preparing worktree (detached HEAD bb4f6a1)
HEAD is now at bb4f6a1 init
repo % cat .git/worktrees/foo/gitdir
/private/tmp/foo/.git
repo % cat ../foo/.git
gitdir: /private/tmp/repo/.git/worktrees/foo

In this situation, if we change the setting to
`worktree.useRelativePaths=true` and run
 `git worktree repair` within the main worktree, the cross references
are converted to
relative paths (this is an expected behavior).

repo % git config worktree.useRelativePaths true
repo % git worktree repair
repair: .git file absolute/relative path mismatch: /private/tmp/foo
repo % cat .git/worktrees/foo/gitdir
../../../../foo/.git
repo % cat ../foo/.git
gitdir: ../repo/.git/worktrees/foo

On the other hand, given a state where cross references are recorded
using relative paths,
one would expect (by symmetry) that changing
`worktree.useRelativePath` from `true` to `false`
and running `git worktree repair` would convert the cross references
to absolute paths. However,
no "absolute/relative path mismatch" is detected, and the cross
references remain as relative paths.
This is the problem I wanted to fix.

repo % cat .git/worktrees/foo/gitdir
../../../../foo/.git
repo % cat ../foo/.git
gitdir: ../repo/.git/worktrees/foo
repo % git config worktree.useRelativePaths false
repo % git worktree repair
repo % cat .git/worktrees/foo/gitdir
../../../../foo/.git
repo % cat ../foo/.git
gitdir: ../repo/.git/worktrees/foo

The issue has been present since the initial implementation:
717af916cd (worktree: link worktrees with relative paths, 2024-10-07)
Although `dotgit_contents` (retrieved via `read_gitfile_gently()`) is
always an absolute path,
the implementations of `repair_gitfile()` and
`repair_worktree_at_path()` treat it as if the
actual contents of the `.git` file had been returned.

I have confirmed that the above issue can be reproduced even in the
v2.48.0 tag, which was
the first release to include that change.

> Yes, read_gitfile_gently() always turns the gitfile it reads into an
> absolute form.  Is there a caller A that wants the underlying
> relative form, and if so why?  Is it to compare with some other path
> that is relative?  How did the code B obtained the other path to be
> compared that is relative?  If that code B used the helper that is
> different from read_gitfile_gently() to obtain the other path that
> is relative, perhaps the caller A can be changed to call it instead
> of calling read_gitfile_gently() and the fix can be done without
> churning so many existing call sites?
>
> Stepping back a bit, why does "repair" even care if it is relative?
> Is it considered a semi-error when a gitfile records its target as a
> relative path?  If so, I wonder if a cleaner way may be to add a new
> READ_GITFILE_ERR_RELATIVE_PATH constant that is treated as non-fatal
> error by the read_gitfile_error_die() function?  If that approach
> works, that may be the cleanest, as I suspect that "was it recorded
> as an absolute path?" will not stay to be the only special case in
> niche applications like "repair", but we need to audit callers of
> the _gently() function and make sure they do not barf with the new
> return code.
>
> If not, perhaps introduce a separate function that returns the path
> it read without any conversion, i.e.,
>
>     char *read_raw_gitfile(const char *path);
>
> that "repair" thing can use, and have it do the relateve-to-absolute
> converaion itself, perhaps?  That function would be created by moving
> most of the code from read_gitfile_gently() and read_gitfile_gently()
> would become a very thin wrapper around that function.  Wouldn't that
> be the least invasive and cleanest solution, if it works?

You're right; changing the signature of `read_gitfile_gently` for a niche use
case like `worktree repair` isn't a good idea. I'll revise the
approach to introduce
something like the `read_raw_gitfile()` you suggested.

Thanks,
-- 
Yoichi NAKAYAMA