mbox series

[v5,0/8] Allow relative worktree linking to be configured by the user

Message ID 20241125-wt_relative_options-v5-0-356d122ff3db@pm.me (mailing list archive)
Headers show
Series Allow relative worktree linking to be configured by the user | expand

Message

Caleb White Nov. 26, 2024, 1:51 a.m. UTC
This series introduces the `--[no-]relative-paths` CLI option for
`git worktree {add, move, repair}` commands, as well as the
`worktree.useRelativePaths` configuration setting. When enabled,
these options allow worktrees to be linked using relative paths,
enhancing portability across environments where absolute paths
may differ (e.g., containerized setups, shared network drives).
Git still creates absolute paths by default, but these options allow
users to opt-in to relative paths if desired.

Using the `--relative-paths` option with `worktree {move, repair}`
will convert absolute paths to relative ones, while `--no-relative-paths`
does the reverse. For cases where users want consistency in path handling,
the config option `worktree.useRelativePaths` provides a persistent setting.

A new extension, `relativeWorktrees`, is added to indicate that at least
one worktree in the repository has been linked with relative paths. This
extension is automatically set when a worktree is created or repaired
using the `--relative-paths` option, or when the
`worktree.useRelativePaths` config is set to `true`.

The `relativeWorktrees` extension ensures older Git versions do not
attempt to automatically prune worktrees with relative paths, as they
would not not recognize the paths as being valid.

Signed-off-by: Caleb White <cdwhite3@pm.me>
---
The base for this patch series is 090d24e9af.

Link to original patch series:
https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me

---
Changes in v5:
- Added docs to `--relative-paths` option.
- Added test coverage for `repair_worktrees()` and relative paths.
- Move `strbuf_reset` call in `infer_backlink()`.
- Cleaned up tests.
- Slight stylistic changes.
- Tweaked commit messages.
- Updated base to 090d24e9af.
- Link to v4: https://lore.kernel.org/r/20241031-wt_relative_options-v4-0-07a3dc0f02a3@pm.me
Changes in v4:
- Fixed failing test in ci
- Link to v3: https://lore.kernel.org/r/20241031-wt_relative_options-v3-0-3e44ccdf64e6@pm.me
Changes in v3:
- Split patches into smaller edits.
- Moved tests into the patches with the relevant code changes.
- Removed global `use_relative_paths` and instead pass parameter to functions.
- Changed `infer_backlink` return type from `int` to `ssize_t`.
- Updated `worktree.useRelativePaths` and `--relative-paths` descriptions.
- Reordered patches
- Link to v2: https://lore.kernel.org/r/20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me
Changes in v2:
- Fixed a bug where repositories with valid extensions would be downgraded
  to v0 during reinitialization, causing future operations to fail.
- Split patch [1/2] into three separate patches.
- Updated cover letter and commit messages.
- Updated documentation wording.
- Link to v1: https://lore.kernel.org/r/20241025-wt_relative_options-v1-0-c3005df76bf9@pm.me

---
Caleb White (8):
      setup: correctly reinitialize repository version
      worktree: add `relativeWorktrees` extension
      worktree: refactor infer_backlink return
      worktree: add `write_worktree_linking_files()` function
      worktree: add relative cli/config options to `add` command
      worktree: add relative cli/config options to `move` command
      worktree: add relative cli/config options to `repair` command
      worktree: refactor `repair_worktree_after_gitdir_move()`

 Documentation/config/extensions.txt |   6 ++
 Documentation/config/worktree.txt   |  10 +++
 Documentation/git-worktree.txt      |   8 ++
 builtin/worktree.c                  |  29 ++++---
 repository.c                        |   1 +
 repository.h                        |   1 +
 setup.c                             |  39 ++++++---
 setup.h                             |   1 +
 t/t0001-init.sh                     |  22 ++++-
 t/t2400-worktree-add.sh             |  45 +++++++++++
 t/t2401-worktree-prune.sh           |   3 +-
 t/t2402-worktree-list.sh            |  22 +++++
 t/t2403-worktree-move.sh            |  25 ++++++
 t/t2406-worktree-repair.sh          |  39 +++++++++
 t/t2408-worktree-relative.sh        |  39 ---------
 t/t5504-fetch-receive-strict.sh     |   6 +-
 worktree.c                          | 157 ++++++++++++++++++++----------------
 worktree.h                          |  22 ++++-
 18 files changed, 333 insertions(+), 142 deletions(-)
---
base-commit: 090d24e9af6e9f59c3f7bee97c42bb1ae3c7f559
change-id: 20241025-wt_relative_options-afa41987bc32

Best regards,

Comments

Junio C Hamano Nov. 26, 2024, 6:18 a.m. UTC | #1
Caleb White <cdwhite3@pm.me> writes:

> Changes in v5:
> - Added docs to `--relative-paths` option.

You already had doc on this, but the default was not described at
all.

 --[no-]relative-paths::
+       Link worktrees using relative paths or absolute paths (default).

> - Added test coverage for `repair_worktrees()` and relative paths.
> - Move `strbuf_reset` call in `infer_backlink()`.

This was more like "revert the change in v4 that moved it
unnecessarily", no?

> - Cleaned up tests.

Yup, there truely a lot of test changes between v4 and v5.  Many
tests now use existing test helpers, which is good.


> - Slight stylistic changes.

I saw many changes like these (the diff is between v4 and v5)

 static void repair_gitfile(struct worktree *wt,
-                          worktree_repair_fn fn,
-                          void *cb_data,
+                          worktree_repair_fn fn, void *cb_data,
                           int use_relative_paths)

which looked good (the original had fn and cb_data defined on the
same line).

> - Tweaked commit messages.

Updates to the proposed log message for `repair` step [7/8] did not
really "clarify", other than helping readers to see how messy things
are.  It said:

    +    To simplify things, both linking files are written when one of the files
    +    needs to be repaired. In some cases, this fixes the other file before it
    +    is checked, in other cases this results in a correct file being written
    +    with the same contents.

which may describe what the code happens to do correctly, but does
not quite help building the confidence in what it does is correct.

Suppose that the directory X has a repository, and the repository
thinks that the directory W is its worktree.  But the worktree at
the directory W thinks that its repository is not X but Y, and there
indeed is a repository at the directory Y.  That repository thinks W
belongs to it.

If we examine X first, would we end up updating W to point at X
(because X thinks W is its worktree)?

Or do we make W to point at Y (because Y thinks W is its, and W
thinks it is Y's)"?

Either way, I think the comment is trying to say that, if we decide
to make X and W belong to each other, we'd overwrite links from X to
W and also W to X, even though the link from X was already pointing
at W and the minimum fix we needed to make was to update the link
from W to point at X.  Overwriting a link from X to W with a new
link from X to W is a no-op, so it does not seem to help greatly,
since `repair` is not at all performance critical.  The correctness
is a lot more important.


> - Updated base to 090d24e9af.

This made it harder than necessary to compare the two iterations, by
the way.


Thanks.