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

[Francesco Guastella <guastella.francesco@gmail.com>]

Thank you for your feedback on my patch.

> 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.

Regarding the design choice between absolute and relative paths,
I’m not certain whether using absolute paths was an intentional decision.
Your example highlights a valid advantage of absolute paths;
however, this approach is not without its limitations.
In my cover letter, I mentioned just a few issues related to absolute paths,
and my online research revealed many other problems that could be mitigated
by supporting relative paths.

To support this, many users have requested this functionality or have created
scripts as workarounds to convert paths in Git-generated files.
An example from my own experience is that worktrees created on Windows are
considered prunable by Git because paths starting with, for instance, ‘C:’
have no meaning in GNU/Linux environments.

To avoid breaking changes and maintain backward compatibility, I chose
to extend Git by adding support for relative paths rather than replacing
the original behavior.
This approach gives users the flexibility to choose what best suits their needs.

In my experience, having the option to use relative paths is essential for
effective worktree management.
For instance, not having this option has led me to avoid using worktrees
altogether (up to now, at least :-P).

If it’s helpful, I would be glad to enhance the documentation to clearly
outline the pros and cons of using relative paths, including
the valid example you provided.

> 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).

Regarding the extensive changes in the worktree add test suite,
I understand that this makes the review process challenging.
I consolidated many tests into a reusable function to ensure that
Git behaves correctly regardless of whether absolute or relative paths are chosen.
Each test there now also includes a call to the check_worktree_paths function
to verify the correctness of the generated paths.

> 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).

Lastly, I sincerely apologize for any coding guideline violations. I reviewed
the guidelines carefully and made every effort to adhere to them, so I regret any oversights.
I will make the necessary corrections as soon as I have the opportunity.

Thank you again for your time and consideration of my patch.