Hi Caleb
Thanks for working on this.
On 28/10/2024 19:09, Caleb White wrote:
This patch introduces the `--[no-]relative-paths` CLI option for
"This patch" is a bit redundant, I'd say "Introduce a
`--[no-]-relative-paths ..`
`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.
This sounds good, I'm not sure the patch actually implements anything
other than the option parsing though. I think it would make sense to add
these options at the end of the patch series once the implementation has
been changed to support them. I'd start with patches 4 and 5 to add the
new extension setting first, then refactor worktree.c to handle creating
worktrees with relative or absolute paths and set the extension if
appropriate, then add the --relative-path option to "git worktree"
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.
In response to reviewer feedback from the initial patch series[1],
this revision includes slight refactoring for improved
maintainability and clarity in the code base.
Please don't mix cleanups with other code changes as it makes it hard to
check that the cleanups don't change the behavior.
[1]: https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@xxxxx
Suggested-by: Junio C Hamano <gitster@xxxxxxxxx>
Signed-off-by: Caleb White <cdwhite3@xxxxx>
---
Documentation/config/worktree.txt | 5 +++++
Documentation/git-worktree.txt | 12 ++++++++++++
builtin/worktree.c | 9 +++++++++
t/t2408-worktree-relative.sh | 39 ---------------------------------------
worktree.c | 17 ++++++++++-------
worktree.h | 2 ++
6 files changed, 38 insertions(+), 46 deletions(-)
diff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt
index 048e349482df6c892055720eb53cdcd6c327b6ed..44b783c2774dc5ff65e3fa232b0c25cd5254876b 100644
--- a/Documentation/config/worktree.txt
+++ b/Documentation/config/worktree.txt
@@ -7,3 +7,8 @@ worktree.guessRemote::
such a branch exists, it is checked out and set as "upstream"
for the new branch. If no such match can be found, it falls
back to creating a new branch from the current HEAD.
+worktree.useRelativePaths::
+ If set to `true`, worktrees will be linked to the repository using
+ relative paths rather than using absolute paths. This is particularly
+ useful for setups where the repository and worktrees may be moved between
+ different locations or environments.
I think it would be helpful to spell out the implications of this to the
user - namely that you cannot use older versions of git on a repository
with worktrees using relative paths and it may break third-party
software as well.
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 70437c815f13852bd2eb862176b8b933e6de0acf..975dc3c46d480480457ec4857988a6b8bc67b647 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -216,6 +216,18 @@ To remove a locked worktree, specify `--force` twice.
This can also be set up as the default behaviour by using the
`worktree.guessRemote` config option.
+--[no-]relative-paths::
+ Worktrees will be linked to the repository using relative paths
+ rather than using absolute paths. This is particularly useful for setups
+ where the repository and worktrees may be moved between different
+ locations or environments.
Again we should spell out the implications of using relative paths.
+With `repair`, the linking files will be updated if there's an absolute/relative
+mismatch, even if the links are correct.
++
+This can also be set up as the default behaviour by using the
+`worktree.useRelativePaths` config option.
+
--[no-]track::
When creating a new branch, if `<commit-ish>` is a branch,
mark it as "upstream" from the new branch. This is the
diff --git a/builtin/worktree.c b/builtin/worktree.c
index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,
if (!strcmp(var, "worktree.guessremote")) {
guess_remote = git_config_bool(var, value);
return 0;
+ } else if (!strcmp(var, "worktree.userelativepaths")) {
+ use_relative_paths = git_config_bool(var, value);
As we're trying to remove global variables from libgit.a as part of the
libification effort I'd be much happier if "use_relative_paths" was
declared as a "static int" in this file and then passed down to the
functions that need it rather than declaring it as a global in "worktree.c".
diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh
deleted file mode 100755
There's no explanation for this change in the commit message
Best Wishes
Phillip
index a3136db7e28cb20926ff44211e246ce625a6e51a..0000000000000000000000000000000000000000
--- a/t/t2408-worktree-relative.sh
+++ /dev/null
@@ -1,39 +0,0 @@
-#!/bin/sh
-
-test_description='test worktrees linked with relative paths'
-
-TEST_PASSES_SANITIZE_LEAK=true
-. ./test-lib.sh
-
-test_expect_success 'links worktrees with relative paths' '
- test_when_finished rm -rf repo &&
- git init repo &&
- (
- cd repo &&
- test_commit initial &&
- git worktree add wt1 &&
- echo "../../../wt1/.git" >expected_gitdir &&
- cat .git/worktrees/wt1/gitdir >actual_gitdir &&
- echo "gitdir: ../.git/worktrees/wt1" >expected_git &&
- cat wt1/.git >actual_git &&
- test_cmp expected_gitdir actual_gitdir &&
- test_cmp expected_git actual_git
- )
-'
-
-test_expect_success 'move repo without breaking relative internal links' '
- test_when_finished rm -rf repo moved &&
- git init repo &&
- (
- cd repo &&
- test_commit initial &&
- git worktree add wt1 &&
- cd .. &&
- mv repo moved &&
- cd moved/wt1 &&
- git status >out 2>err &&
- test_must_be_empty err
- )
-'
-
-test_done
diff --git a/worktree.c b/worktree.c
index 77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7..de5c5e53a5f2a758ddf470b5d6a9ad6c66247181 100644
--- a/worktree.c
+++ b/worktree.c
@@ -14,6 +14,8 @@
#include "wt-status.h"
#include "config.h"
+int use_relative_paths = 0;
+
void free_worktree(struct worktree *worktree)
{
if (!worktree)
@@ -111,9 +113,9 @@ struct worktree *get_linked_worktree(const char *id,
strbuf_strip_suffix(&worktree_path, "/.git");
if (!is_absolute_path(worktree_path.buf)) {
- strbuf_strip_suffix(&path, "gitdir");
- strbuf_addbuf(&path, &worktree_path);
- strbuf_realpath_forgiving(&worktree_path, path.buf, 0);
+ strbuf_strip_suffix(&path, "gitdir");
+ strbuf_addbuf(&path, &worktree_path);
+ strbuf_realpath_forgiving(&worktree_path, path.buf, 0);
}
CALLOC_ARRAY(worktree, 1);
@@ -725,12 +727,15 @@ static int is_main_worktree_path(const char *path)
* won't know which <repo>/worktrees/<id>/gitdir to repair. However, we may
* be able to infer the gitdir by manually reading /path/to/worktree/.git,
* extracting the <id>, and checking if <repo>/worktrees/<id> exists.
+ *
+ * Returns -1 on failure and strbuf.len on success.
*/
static int infer_backlink(const char *gitfile, struct strbuf *inferred)
{
struct strbuf actual = STRBUF_INIT;
const char *id;
+ strbuf_reset(inferred);
if (strbuf_read_file(&actual, gitfile, 0) < 0)
goto error;
if (!starts_with(actual.buf, "gitdir:"))
@@ -741,18 +746,16 @@ static int infer_backlink(const char *gitfile, struct strbuf *inferred)
id++; /* advance past '/' to point at <id> */
if (!*id)
goto error;
- strbuf_reset(inferred);
strbuf_git_common_path(inferred, the_repository, "worktrees/%s", id);
if (!is_directory(inferred->buf))
goto error;
strbuf_release(&actual);
- return 1;
-
+ return inferred->len;
error:
strbuf_release(&actual);
strbuf_reset(inferred); /* clear invalid path */
- return 0;
+ return -1;
}
/*
diff --git a/worktree.h b/worktree.h
index e96118621638667d87c5d7e0452ed10bd1ddf606..37e65d508ed23d3e7a29850bb938285072a3aaa6 100644
--- a/worktree.h
+++ b/worktree.h
@@ -5,6 +5,8 @@
struct strbuf;
+extern int use_relative_paths;
+
struct worktree {
/* The repository this worktree belongs to. */
struct repository *repo;
