builtin/worktree: support relative paths for worktrees

From: Francesco Guastella <[email protected]>

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.

By default, git worktree add will continue to use absolute paths, maintaining the existing behavior and therefore ensuring compatibility. However, when the worktree.useRelativePaths configuration is set to true, all newly added worktrees will use relative paths, offering greater flexibility, especially for repositories that are accessed from different operating systems.

Additionally, the git worktree move command has been enhanced to adhere to the current value of worktree.useRelativePaths. When moving a worktree, the path formats will be adjusted to match the desired configuration.

The git worktree repair command has also been updated to automatically adjust paths between relative and absolute formats based on the worktree.useRelativePaths setting.

---
builtin/worktree: support relative paths for worktrees

As of now, when creating or managing worktrees, paths are always stored in absolute form. While this behavior works well in many scenarios, it presents significant challenges when worktrees are accessed across different operating systems or when the repository is moved to a different location in the filesystem. Absolute paths hard-coded in the .git and gitdir files can break these links, causing issues that may require manual intervention to resolve.

To address this, I have introduced a new configuration option:
worktree.useRelativePaths. This option allows users to specify whether they prefer Git to store worktree paths in relative form rather than absolute. The new feature enhances Git’s flexibility, particularly in environments where repositories need to be portable across different systems or where directories are frequently relocated.

Key Changes: The new worktree.useRelativePaths option can be enabled by the user to store paths in relative form. When enabled, any new worktrees added using the git worktree add command will have their paths stored as relative paths in the necessary git files.

The git worktree move command has been updated to respect the current value of worktree.useRelativePaths. When a worktree is moved, Git will now automatically adjust the path format (relative or absolute) to match the user's configuration setting.

The git worktree repair command has been similarly enhanced. It will now automatically convert paths between relative and absolute forms based on the worktree.useRelativePaths setting, making it easier to maintain consistent links across different environments.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1783%2Fromeoxbm%2Frelative-paths-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1783/romeoxbm/relative-paths-v1
Pull-Request: https://github.com/git/git/pull/1783

 Documentation/git-worktree.txt |   18 +
 abspath.c                      |   38 +
 abspath.h                      |   44 +
 builtin/mv.c                   |    6 +-
 builtin/submodule--helper.c    |    2 +-
 builtin/worktree.c             |   51 +-
 config.c                       |   10 +
 config.h                       |   15 +
 dir.c                          |  106 +-
 dir.h                          |   75 +-
 submodule.c                    |   12 +-
 t/lib-worktree.sh              |  156 +++
 t/t2400-worktree-add.sh        | 1729 +++++++++++++++++---------------
 t/t2401-worktree-prune.sh      |   96 +-
 t/t2403-worktree-move.sh       |   10 +
 t/t2406-worktree-repair.sh     |  262 ++---
 t/test-lib.sh                  |   38 +-
 worktree.c                     |   78 +-
 18 files changed, 1682 insertions(+), 1064 deletions(-)
 create mode 100644 t/lib-worktree.sh

index 2a240f53ba7..c4f59229194 100644
@@ -105,6 +105,15 @@ passed to the command. In the event the repository has a remote and
 `--guess-remote` is used, but no remote or local branches exist, then the
 command fails with a warning reminding the user to fetch from their remote
 first (or override by using `-f/--force`).
++
+By default, the `git worktree add` command links the main worktree with the
+newly created worktree using absolute paths.
+However, this behavior may not be desirable when a repository needs
+to be accessed from different operating systems. To address this, Git can
+be configured to use relative paths instead. To enable this behavior, set the
+`worktree.useRelativePaths` configuration option to `true`. Once enabled,
+all newly added worktrees will be linked using relative paths.
+
 
 list::
 

@@ -128,6 +137,11 @@ Move a worktree to a new location. Note that the main worktree or linked
 worktrees containing submodules cannot be moved with this command. (The
 `git worktree repair` command, however, can reestablish the connection
 with linked worktrees if you move the main worktree manually.)
++
+When moving a worktree, the path formats will be adjusted to align with the
+current value of `worktree.useRelativePaths`. This means that if the worktree
+was created using absolute paths and the configuration is later set to `true`,
+the link will be reestablished using relative paths during the move.
 
 prune::
 

@@ -357,6 +371,10 @@ directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and
 (e.g. `/path/main/.git`). These settings are made in a `.git` file located at
 the top directory of the linked worktree.
 
+If `worktree.useRelativePaths` is enabled, the paths in this `.git` file
+will be stored in a relative format. Conversely, if it is disabled, the paths
+will be stored in an absolute format.
+
 Path resolution via `git rev-parse --git-path` uses either
 `$GIT_DIR` or `$GIT_COMMON_DIR` depending on the path. For example, in the
 linked worktree `git rev-parse --git-path HEAD` returns

index 1202cde23db..7a03becce99 100644
@@ -1,6 +1,9 @@
 #include "git-compat-util.h"
 #include "abspath.h"
+#include "environment.h"
+#include "path.h"
 #include "strbuf.h"
+#include "worktree.h"
 
 /*
  * Do not use this for inspecting *tracked* content.  When path is a

@@ -290,6 +293,41 @@ char *prefix_filename_except_for_dash(const char *pfx, const char *arg)
 	return prefix_filename(pfx, arg);
 }
 
+char *worktree_real_pathdup(const char *wt_path)
+{
+	struct strbuf abs_wt_path_sb = STRBUF_INIT;
+	char *retval = NULL;
+	char *git_dir_real_path = NULL;
+	char *repo_real_path = NULL;
+
+	if (!is_absolute_path(wt_path)) {
+		git_dir_real_path = real_pathdup(get_git_common_dir(), 1);
+		repo_real_path = strip_path_suffix(git_dir_real_path, ".git");
+		if (repo_real_path) {
+			strbuf_addf(&abs_wt_path_sb, "%s/%s", repo_real_path,
+				    wt_path);
+		} else {
+			strbuf_addf(&abs_wt_path_sb, "%s/%s", git_dir_real_path,
+				    wt_path);
+		}
+
+		strbuf_realpath_forgiving(&abs_wt_path_sb, abs_wt_path_sb.buf, 1);
+		retval = strbuf_detach(&abs_wt_path_sb, NULL);
+	} else {
+		retval = xstrdup(wt_path);
+	}
+
+	free(git_dir_real_path);
+	free(repo_real_path);
+	strbuf_release(&abs_wt_path_sb);
+	return retval;
+}
+
+char *worktree_real_pathdup_for_wt(struct worktree *wt)
+{
+	return worktree_real_pathdup(wt->path);
+}
+
 void strbuf_add_absolute_path(struct strbuf *sb, const char *path)
 {
 	if (!*path)

index 6c69033c5f5..8604525b85c 100644
@@ -490,9 +490,9 @@ remove_entry:
 			if (!update_path_in_gitmodules(src, dst))
 				gitmodules_modified = 1;
 			if (submodule_gitfiles[i] != SUBMODULE_WITH_GITDIR)
-				connect_work_tree_and_git_dir(dst,
-							      submodule_gitfiles[i],
-							      1);
+				connect_submodule_work_tree_and_git_dir(dst,
+									submodule_gitfiles[i],
+									1);
 		}
 
 		if (mode & (WORKING_DIRECTORY | SKIP_WORKTREE_DIR))

index 3e0b6c45c03..98c0c1f6722 100644
@@ -1806,7 +1806,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 		      "git dir"), sm_gitdir);
 	}
 
-	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
+	connect_submodule_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
 
 	p = git_pathdup_submodule(clone_data_path, "config");
 	if (!p)

index 56b5862e59d..5750d258ac9 100644
@@ -2731,6 +2731,16 @@ int repo_config_get_expiry_in_days(struct repository *r, const char *key,
 	return -1; /* thing exists but cannot be parsed */
 }
 
+int repo_config_get_worktree_use_relative_paths(struct repository *r)
+{
+	int val;
+
+	if (!repo_config_get_maybe_bool(r, "worktree.userelativepaths", &val))
+		return val;
+
+	return 0; /* by default, absolute paths are used */
+}
+
 int repo_config_get_split_index(struct repository *r)
 {
 	int val;

index d0497157c52..77e1c3812fb 100644
@@ -678,6 +678,21 @@ int repo_config_get_expiry(struct repository *r, const char *key, char **output)
 int repo_config_get_expiry_in_days(struct repository *r, const char *key,
 				   timestamp_t *, timestamp_t now);
 
+/**
+ * Retrieves the configuration that determines if worktrees should use relative paths.
+ *
+ * This function checks the "worktree.userelativepaths" configuration.
+ * If the configuration was found and is a valid boolean value, the function returns that value.
+ * If the configuration was not found or invalid, the function returns 0,
+ * indicating that by default, absolute paths are used for worktrees.
+ *
+ * Return:
+ * - 1: If "worktree.userelativepaths" is set to true (use relative paths).
+ * - 0: If "worktree.userelativepaths" is set to false or is not set
+ *      (use absolute paths by default).
+ */
+int repo_config_get_worktree_use_relative_paths(struct repository *r);
+
 /**
  * First prints the error message specified by the caller in `err` and then
  * dies printing the line number and the file name of the highest priority

index 97516b0fec1..1b3c6bde4a9 100644
@@ -1672,7 +1672,7 @@ get_fetch_task_from_changed(struct submodule_parallel_fetch *spf,
 		 * core.worktree when they are populated/unpopulated by
 		 * "git checkout" (and similar commands, see
 		 * submodule_move_head() and
-		 * connect_work_tree_and_git_dir()), but if the
+		 * connect_submodule_work_tree_and_git_dir()), but if the
 		 * submodule is unpopulated in another way (e.g. "git
 		 * rm", "rm -r"), core.worktree will still be set even
 		 * though the directory doesn't exist, and the child

@@ -2147,7 +2147,7 @@ int submodule_move_head(const char *path, const char *super_prefix,
 	if (flags & SUBMODULE_MOVE_HEAD_FORCE)
 		/*
 		 * Pass non NULL pointer to is_submodule_populated_gently
-		 * to prevent die()-ing. We'll use connect_work_tree_and_git_dir
+		 * to prevent die()-ing. We'll use connect_submodule_work_tree_and_git_dir
 		 * to fixup the submodule in the force case later.
 		 */
 		error_code_ptr = &error_code;

@@ -2194,7 +2194,8 @@ int submodule_move_head(const char *path, const char *super_prefix,
 				die(_("refusing to create/use '%s' in another "
 				      "submodule's git dir"),
 				    gitdir.buf);
-			connect_work_tree_and_git_dir(path, gitdir.buf, 0);
+			connect_submodule_work_tree_and_git_dir(path,
+								gitdir.buf, 0);
 			strbuf_release(&gitdir);
 
 			/* make sure the index is clean as well */

@@ -2205,7 +2206,8 @@ int submodule_move_head(const char *path, const char *super_prefix,
 			struct strbuf gitdir = STRBUF_INIT;
 			submodule_name_to_gitdir(&gitdir, the_repository,
 						 sub->name);
-			connect_work_tree_and_git_dir(path, gitdir.buf, 1);
+			connect_submodule_work_tree_and_git_dir(path,
+								gitdir.buf, 1);
 			strbuf_release(&gitdir);
 		}
 	}

@@ -2457,7 +2459,7 @@ void absorb_git_dir_into_superproject(const char *path,
 		if (!sub)
 			die(_("could not lookup name for submodule '%s'"), path);
 		submodule_name_to_gitdir(&sub_gitdir, the_repository, sub->name);
-		connect_work_tree_and_git_dir(path, sub_gitdir.buf, 0);
+		connect_submodule_work_tree_and_git_dir(path, sub_gitdir.buf, 0);
 		strbuf_release(&sub_gitdir);
 	} else {
 		/* Is it already absorbed into the superprojects git dir? */

index 901342ea09b..90fab3800bc 100755
@@ -4,6 +4,7 @@ test_description='test git worktree move, remove, lock and unlock'
 
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-worktree.sh
 
 test_expect_success 'setup' '
 	test_commit init &&

@@ -247,4 +248,13 @@ test_expect_success 'not remove a repo with initialized submodule' '
 	)
 '
 
+test_expect_success 'move worktree and verify path format' '
+	test_config worktree.userelativepaths false &&
+	git worktree add fred &&
+	check_worktree_paths false fred &&
+	test_config worktree.userelativepaths true &&
+	git worktree move fred barney &&
+	check_worktree_paths true barney
+'
+
 test_done

index 278d1215f15..c1fe8df2c39 100644
@@ -1627,26 +1627,30 @@ remove_trash_directory () {
 	! test -d "$dir"
 }
 
-# Test repository
-remove_trash_directory "$TRASH_DIRECTORY" || {
-	BAIL_OUT 'cannot prepare test area'
-}
+setup_test_repo() {
+	# Remove the trash directory if it exists
+	remove_trash_directory "$TRASH_DIRECTORY" || {
+		BAIL_OUT 'cannot prepare test area'
+	}
 
-remove_trash=t
[… more lines in the raw message]

base-commit: 6531f31ef3bead57a3255fa08efa6e7553c5a9a7

gitgitgadget

Part of the diff is not shown here. View the raw message.

"Francesco Guastella via GitGitGadget" <[email protected]>

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.

Thank you for your feedback on my patch.

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.

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.

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.

Using relative rather than absolute paths has been discussed previously[1], and the general feeling was that relative paths would probably be beneficial (despite some obvious downsides mentioned by Junio in his response[2]). That earlier discussion petered out without any changes being made partly because some stated and some implied questions lacked answers.

A significant concern was whether any such change could be made without breaking existing third-party tooling, non-canonical Git implementations, and even older versions of Git itself. Your choice of controlling the behavior via a configuration option somewhat sidesteps the issue by giving the user the choice of tolerating or not tolerating such potential breakage.

Sidestepping like that may be the best we can hope for, though it would also be nice if the choice could be automated and hidden from the user as an implementation detail. One idea which was floated was for Git to store both the absolute and relative paths, thus leveraging the benefits of both path types. But that, too, has the potential downside of breaking existing tooling and other Git implementations, so it requires careful consideration. At any rate, it would be good to hear your thoughts on the idea since it might somehow fit into the overall scheme laid out by your work, or your work may gain additional direction by taking that idea into consideration.

Anyhow, below is a set of questions and observations that I jotted down as I lightly scanned the patch and played around with "git worktree" after applying the patch. Many of the items may seem to sound negative, but do not interpret that as rejection of the idea of using relative paths; as stated above earlier discussion looked upon relative paths as potentially beneficial.

It's possible that the actual code changes in the patch answer some of the questions I ask below, but I simply don't have the mental bandwidth to digest and reason about a 3,500 line patch in order to figure out the answers myself. As such, any answers and responses you can provide directly will be appreciated.

Here are my notes...

Since it is likely that third-party tools and non-canonical Git implementations (and even older versions of Git itself) will break with relative paths, people have created tools which let them switch between the two on-demand. This way, they can benefit from relative paths but still interoperate (albeit in a painfully manual fashion) with those tools or implementations which would otherwise break. Does this implementation provide such a tool? At first glance, based upon your above commentary, it sounded as if "git worktree repair" would serve a similar purpose after a user manually changes the worktree.useRelativePaths setting, but that doesn't seem to be the case in practice.

That leads to the next question: Should there be a dedicated git-worktree subcommand which both changes the worktree.useRelativePaths setting and converts all the stored paths to reflect the new setting?

Your above commentary says that "git worktree repair" converts between absolute and relative, but it's not clear whether it does this for *all* worktrees or only worktrees which it fixes. Moreover, does it convert both <repo>/worktrees/<id>/gitdir and <worktree>/.git in unison or does it only convert the path in a file it actually repairs? Based upon testing, I'm guessing it only does the absolute/relative conversion *if* it actually repairs a file, and *only* converts the repaired file. As such, the links between <repo> and <worktree> can end up a mix of absolute and relative.

According to your above commentary, "git worktree repair" converts between absolute and relative but the documentation for "repair" in Documentation/git-worktree.txt has not been updated to reflect this (unlike the documentation for "git worktree move" which now does conversion and is documented as doing so).

The new worktree.useRelativePaths configuration should be documented in Documentation/config/worktree.txt.

The "worktrees/<id>/gitdir::" entry in Documentation/gitrepository-layout.txt says that the contained path is absolute. This needs to be updated to mention relative paths and worktree.useRelativePaths.

The documentation should discuss and stress the potential dangers of using relative paths so that people know what they are getting themselves into. In particular, it should mention the possibility of relative paths breaking third-party tooling, non-canonical Git implementations, and older versions of Git itself.

Sometimes patch authors don't bother documenting newly-added "public" API functions (possibly because a lot of existing API functions lack documentation), so it was nice to see this patch being thorough about documenting new API functions.

In my testing, several "git worktree" subcommands failed or misbehaved badly when invoked from within a linked worktree. This surprised me since my light scan over the patch seemed to indicate that you had taken such cases into consideration, so perhaps I'm doing something wrong(?). For my tests, I had set up a repository and worktrees like this:

% git init a
% cd a
% git config set worktree.useRelativePaths true
% echo content >file
% git add file
% git commit file -m msg
% git worktree add ../b
% git worktree add c

"git worktree" subcommands are supposed to be agnostic in the sense that you should be able to invoke them from within any worktree successfully, however, with your patch applied, this seems to no longer be the case. For instance:

% cd c
% git worktree lock --reason because ../../b
fatal: '../../b' is not a working tree

Similarly, when run from within a linked worktree, "git worktree prune" now thinks that *all* worktrees should be pruned:

  # still in "c"
  % git worktree prune -n -v
  Removing worktrees/c: gitdir file points to non-existent location
  Removing worktrees/b: gitdir file points to non-existent location

When run from within a linked worktree, "git worktree list" shows paths relative to the main worktree (or bare repository):

  # still in "c"
  % git worktree list
  /.../a  935238d [main]
  ../b  935238d [b] prunable
  c  935238d [c] prunable

This makes the direct output useless for any sort of navigation and especially so in --porcelain mode for scripting purposes. To fix this, "list" should either show the absolute paths (even if they are maintained as relative internally) or the paths need to be recomputed and shown as relative from the *current* worktree, not from the main worktree.

Assuming I'm not doing something wrong in my testing, then do the above breakages indicate some gaping holes in the test suite? If so, then we probably need a bunch of new tests to ensure that the above behaviors don't break when relative paths are in use.

You clearly put a lot of work and care into this patch, and (as noted above) the idea of using relative paths has previously been considered in a reasonably positive light, so it is unlikely that reviewers want to lightly dismiss your patch, but at 3,500 lines, the patch is unreviewable. No reviewer is going to have the mental bandwidth to be able to remember or reason about *every* change made by this monolithic patch. As the author of the patch, having developed it incrementally over the course of days or weeks, you have a good overall understanding of all the changes in the patch and the evolution of the code from its present state in the project to the state in your fork, but reviewer time is a limited resource on this project, and it is almost certainly impossible for any reviewer to be able to properly digest this all in a single patch.

As such, aside from answering the above questions, the way to move forward is to split these changes out into many small, independent, well-isolated pieces, each in its own patch, and to arrange the order of patches so that they hand-hold and lead reviewers through the evolution from the current implementation to the end-state. Each patch should be one step in the journey toward the end-goal, and each patch should build upon the previous patch, requiring reviewers to remember only one or two important items from the current patch in order to understand the subsequent patch. For a topic with so many changes, this will likely require quite a few patches; it may even make sense to split it up into several series of patches, with each series submitted separately after reviewers have had time to digest the preceding series.

Organizing a patch series like this places a lot of extra work on your shoulders (beyond the work you already invested making the changes to the code itself), but providing such hand-holding and baby-steps is the only way reviewers will be able to grasp all the changes. A well-organized patch series is also important for future readers of the project history when they need to understand why the current implementation is the way it is.

[1]: The discussion begins at the "Also Eric" paragraph of this email and continues in emails following it:
https://lore.kernel.org/git/CACsJy8CXEKG+WNdSPOWF7JDzPXidSRWZZ5zkdMW3N3Dg8SGW_Q@mail.gmail.com/

[2]: https://lore.kernel.org/git/[email protected]/