Re: [PATCH v2 2/4] worktree: link worktrees with relative paths

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 6, 2024, 11:05 UTC
Message-ID: <CAPig+cT5BHuWMrjefP9BMocE7EFXgzRyHMFbUmEZv73F4g+XEQ@mail.gmail.com>
In-Reply-To: <20241006060017.171788-3-cdwhite3@pm.me>

On Sun, Oct 6, 2024 at 2:01 AM Caleb White <cdwhite3@pm.me> wrote:

Show 22 quoted lines

> This modifies Git’s handling of worktree linking to use relative
> paths instead of absolute paths. Previously, when creating a worktree,
> Git would store the absolute paths to both the main repository and the
> linked worktrees. These hardcoded absolute paths cause breakages such
> as when the repository is moved to a different directory or during
> containerized development where the absolute differs between systems.
>
> By switching to relative paths, we help ensure that worktree links are
> more resilient when the repository is moved. While links external to the
> repository may still break, Git still automatically handles many common
> scenarios, reducing the need for manual repair. This is particularly
> useful in containerized or portable development environments, where the
> absolute path to the repository can differ between systems. Developers
> no longer need to reinitialize or repair worktrees after relocating the
> repository, improving workflow efficiency and reducing breakages.
>
> For self-contained repositories (such as using a bare repository with
> worktrees), where both the repository and its worktrees are located
> within the same directory structure, using relative paths guarantees all
> links remain functional regardless of where the directory is located.
>
> Signed-off-by: Caleb White <cdwhite3@pm.me>

When you reroll, please extend the commit message to give a more detailed overview of how this patch actually changes the behavior both at a high level and at a low level. This is especially important since this patch is sufficiently long and involved that it's not easy to glean these details at-a-glance from the code changes themselves.

For instance, the cover letter talked about retaining correct behavior for both absolute and relative paths, but this commit message mentions only relative paths, so it's not obvious just from reading the commit message whether absolute paths are still supported or, if they are, what exactly that support looks like or how the user controls the choice between the two path types. This sort of information should be present in the commit message.

Similarly, if there is a chance that this change may break existing tooling and workflows, then the commit message should talk about those risks, as well, and what if any remediations are available. Also, if there are such risks, then Documentation/git-worktree.txt may need to be updated to warn users.

Regarding what you wrote above, there seems to be a good deal of redundancy between the first two paragraphs; combining the paragraphs and folding out the duplication might make the message more streamlined. I do like the discussion about containerized environments being used as (at least one) justification for employing relative paths, and think that may be a good lead-in for the commit message. Please see [1] for some helpful hints for composing a good commit message.

I'm not going to review the code itself right now since I haven't been able to apply the patches or play around with the functionality, but I wanted to get the above written up since I think this series is going to need to be rerolled anyhow.

[1]: https://lore.kernel.org/git/xmqqmsm6sc0q.fsf@gitster.g/