Subject: Re: [PATCH] builtin/worktree: support relative paths for worktrees

From: Junio C Hamano <[email-masked]>
Date: Fri, 20 Sep 2024 18:35:02 -0700

"Francesco Guastella via GitGitGadget" <[email-masked]>
writes:

> From: Francesco Guastella <[email-masked]>
>
> Add a new configuration option, `worktree.useRelativePaths`, which allows
> users to choose between storing relative or absolute paths in the files
> Git uses to maintain links between the main worktree and any additional
> ones.

I may be misremembering things, but wasn't the use of absolute paths
a concious design decision?  

When the source repository and an attached worktree know each other
with relative location, moving merely one of them would make it
impossible to locate the other.

On the other hand, if they know the other peer's absolute location,
at least the one that was moved would still be able to locate the
one that did not move, which means that it is possible to find from
the one that moved the other one that did not move, and teach the
latter where the new location of the moved one is.

The only case where it may be an improvement to have them refer to
each other with relative locations is when both of them move in
unison without breaking their relative location.

There may be problems other than the design choice I mentioned above
in the resulting code after applying this huge patch, but as it
stands, the patch does a bit too many things at once to be sensibly
reviewable.  I cannot comment much on the implementation (at least
in this message) in its current form.

For example, the refactoring of t/t2400 into t/lib-worktree might be
an excellent thing to do, but it looks like that the change to the
tests alone deserves to be split into at least a few patches (one to
refactor the test script without changing any functionality, and one
or more patches to add or improve test helpers, and another that
comes with code and behaviour change that add tests for the new
behaviour when configured, or something like that).

I might be tempted to come back to it later, but style violations in
the t/lib-worktree.sh were annoying enough to discourage me from
reviewing it (if it followed Documentation/CodingGuidelines, it
wouldn't have repelled my eyes).

Thanks.