{"thread":{"id":"62421","subject":"[PATCH v2 0/5] Allow relative worktree linking to be configured by the user","startedAt":"2024-10-28T19:09:36Z","lastAt":"2024-10-30T20:36:17Z","messageCount":26,"participants":["Caleb White","Phillip Wood","phillip.wood123@gmail.com","Taylor Blau"],"isPatch":true,"patchVersion":2,"patchTotal":5},"messages":[{"id":"506245","messageId":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":null,"subject":"[PATCH v2 0/5] Allow relative worktree linking to be configured by the user","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:09:30Z","receivedAt":"2024-10-28T19:09:36Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"This patch introduces the `--[no-]relative-paths` CLI option for\n`git worktree {add, move, repair}` commands, as well as the\n`worktree.useRelativePaths` configuration setting. When enabled,\nthese options allow worktrees to be linked using relative paths,\nenhancing portability across environments where absolute paths\nmay differ (e.g., containerized setups, shared network drives).\nGit still creates absolute paths by default, but these options allow\nusers to opt-in to relative paths if desired.\n\nUsing the `--relative-paths` option with `worktree {move, repair}`\nwill convert absolute paths to relative ones, while `--no-relative-paths`\ndoes the reverse. For cases where users want consistency in path handling,\nthe config option `worktree.useRelativePaths` provides a persistent setting.\n\nA new extension, `relativeWorktrees`, is added to indicate that at least\none worktree in the repository has been linked with relative paths. This\nextension is automatically set when a worktree is created or repaired\nusing the `--relative-paths` option, or when the\n`worktree.useRelativePaths` config is set to `true`.\n\nThe `relativeWorktrees` extension ensures older Git versions do not\nattempt to automatically prune worktrees with relative paths, as they\nwould not not recognize the paths as being valid.\n\nSigned-off-by: Caleb White \n---\nThe base for this patch series is obtained by applying the following\npatch onto 6a11438f43:\n- cw/config-extensions topic (doc: consolidate extensions in git-config\n documentation, 2024-10-22, <20241021-cleanup-extension-docs-v1-1-ab02cece3132@pm.me>)\n\nLink to original patch series:\nhttps://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me\n\n---\nChanges in v2:\n- Fixed a bug where repositories with valid extensions would be downgraded\n to v0 during reinitialization, causing future operations to fail.\n- Split patch [1/2] into three separate patches.\n- Updated cover letter and commit messages.\n- Updated documentation wording.\n- Link to v1: https://lore.kernel.org/r/20241025-wt_relative_options-v1-0-c3005df76bf9@pm.me\n\n---\nCaleb White (5):\n worktree: add CLI/config options for relative path linking\n worktree: add `write_worktree_linking_files` function\n worktree: add tests for worktrees with relative paths\n setup: correctly reinitialize repository version\n worktree: add `relativeWorktrees` extension\n\n Documentation/config/extensions.txt | 6 ++\n Documentation/config/worktree.txt | 5 ++\n Documentation/git-worktree.txt | 12 +++\n builtin/worktree.c | 20 ++---\n repository.c | 1 +\n repository.h | 1 +\n setup.c | 39 +++++++---\n setup.h | 1 +\n t/t0001-init.sh | 17 ++++-\n t/t2400-worktree-add.sh | 54 ++++++++++++++\n t/t2401-worktree-prune.sh | 3 +-\n t/t2402-worktree-list.sh | 22 ++++++\n t/t2403-worktree-move.sh | 22 ++++++\n t/t2406-worktree-repair.sh | 26 +++++++\n t/t2408-worktree-relative.sh | 39 ----------\n worktree.c | 143 ++++++++++++++++++++----------------\n worktree.h | 14 ++++\n 17 files changed, 298 insertions(+), 127 deletions(-)\n---\nbase-commit: 6a11438f43469f3815f2f0fc997bd45792ff04c0\nchange-id: 20241025-wt_relative_options-afa41987bc32\nprerequisite-change-id: 20241020-cleanup-extension-docs-f365868711bf:v1\nprerequisite-patch-id: 60a443b24e92938b9b6f4a016a7bab87e13bf3ea\n\nBest regards,\n-- \nCaleb White \n\n\n"},{"id":"506246","messageId":"20241028-wt_relative_options-v2-1-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"[PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:09:37Z","receivedAt":"2024-10-28T19:09:43Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"This patch introduces the `--[no-]relative-paths` CLI option for\n`git worktree {add, move, repair}` commands, as well as the\n`worktree.useRelativePaths` configuration setting. When enabled,\nthese options allow worktrees to be linked using relative paths,\nenhancing portability across environments where absolute paths\nmay differ (e.g., containerized setups, shared network drives).\nGit still creates absolute paths by default, but these options allow\nusers to opt-in to relative paths if desired.\n\nUsing the `--relative-paths` option with `worktree {move, repair}`\nwill convert absolute paths to relative ones, while `--no-relative-paths`\ndoes the reverse. For cases where users want consistency in path\nhandling, the config option `worktree.useRelativePaths` provides\na persistent setting.\n\nIn response to reviewer feedback from the initial patch series[1],\nthis revision includes slight refactoring for improved\nmaintainability and clarity in the code base.\n\n[1]: https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Caleb White \n---\n Documentation/config/worktree.txt | 5 +++++\n Documentation/git-worktree.txt | 12 ++++++++++++\n builtin/worktree.c | 9 +++++++++\n t/t2408-worktree-relative.sh | 39 ---------------------------------------\n worktree.c | 17 ++++++++++-------\n worktree.h | 2 ++\n 6 files changed, 38 insertions(+), 46 deletions(-)\n\ndiff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\nindex 048e349482df6c892055720eb53cdcd6c327b6ed..44b783c2774dc5ff65e3fa232b0c25cd5254876b 100644\n--- a/Documentation/config/worktree.txt\n+++ b/Documentation/config/worktree.txt\n@@ -7,3 +7,8 @@ worktree.guessRemote::\n \tsuch a branch exists, it is checked out and set as \"upstream\"\n \tfor the new branch. If no such match can be found, it falls\n \tback to creating a new branch from the current HEAD.\n+worktree.useRelativePaths::\n+\tIf set to `true`, worktrees will be linked to the repository using\n+\trelative paths rather than using absolute paths. This is particularly\n+\tuseful for setups where the repository and worktrees may be moved between\n+\tdifferent locations or environments.\ndiff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\nindex 70437c815f13852bd2eb862176b8b933e6de0acf..975dc3c46d480480457ec4857988a6b8bc67b647 100644\n--- a/Documentation/git-worktree.txt\n+++ b/Documentation/git-worktree.txt\n@@ -216,6 +216,18 @@ To remove a locked worktree, specify `--force` twice.\n This can also be set up as the default behaviour by using the\n `worktree.guessRemote` config option.\n \n+--[no-]relative-paths::\n+\tWorktrees will be linked to the repository using relative paths\n+\trather than using absolute paths. This is particularly useful for setups\n+\twhere the repository and worktrees may be moved between different\n+\tlocations or environments.\n++\n+With `repair`, the linking files will be updated if there's an absolute/relative\n+mismatch, even if the links are correct.\n++\n+This can also be set up as the default behaviour by using the\n+`worktree.useRelativePaths` config option.\n+\n --[no-]track::\n \tWhen creating a new branch, if `` is a branch,\n \tmark it as \"upstream\" from the new branch. This is the\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n \tif (!strcmp(var, \"worktree.guessremote\")) {\n \t\tguess_remote = git_config_bool(var, value);\n \t\treturn 0;\n+\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n+\t\tuse_relative_paths = git_config_bool(var, value);\n+\t\treturn 0;\n \t}\n \n \treturn git_default_config(var, value, ctx, cb);\n@@ -796,6 +799,8 @@ static int add(int ac, const char **av, const char *prefix)\n \t\t\t PARSE_OPT_NOARG | PARSE_OPT_OPTARG),\n \t\tOPT_BOOL(0, \"guess-remote\", &guess_remote,\n \t\t\t N_(\"try to match the new branch name with a remote-tracking branch\")),\n+\t\tOPT_BOOL(0, \"relative-paths\", &use_relative_paths,\n+\t\t\t N_(\"use relative paths for worktrees\")),\n \t\tOPT_END()\n \t};\n \tint ret;\n@@ -1189,6 +1194,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)\n \t\tOPT__FORCE(&force,\n \t\t\t N_(\"force move even if worktree is dirty or locked\"),\n \t\t\t PARSE_OPT_NOCOMPLETE),\n+\t\tOPT_BOOL(0, \"relative-paths\", &use_relative_paths,\n+\t\t\t N_(\"use relative paths for worktrees\")),\n \t\tOPT_END()\n \t};\n \tstruct worktree **worktrees, *wt;\n@@ -1382,6 +1389,8 @@ static int repair(int ac, const char **av, const char *prefix)\n \tconst char **p;\n \tconst char *self[] = { \".\", NULL };\n \tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"relative-paths\", &use_relative_paths,\n+\t\t\t N_(\"use relative paths for worktrees\")),\n \t\tOPT_END()\n \t};\n \tint rc = 0;\ndiff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh\ndeleted file mode 100755\nindex a3136db7e28cb20926ff44211e246ce625a6e51a..0000000000000000000000000000000000000000\n--- a/t/t2408-worktree-relative.sh\n+++ /dev/null\n@@ -1,39 +0,0 @@\n-#!/bin/sh\n-\n-test_description='test worktrees linked with relative paths'\n-\n-TEST_PASSES_SANITIZE_LEAK=true\n-. ./test-lib.sh\n-\n-test_expect_success 'links worktrees with relative paths' '\n-\ttest_when_finished rm -rf repo &&\n-\tgit init repo &&\n-\t(\n-\t\tcd repo &&\n-\t\ttest_commit initial &&\n-\t\tgit worktree add wt1 &&\n-\t\techo \"../../../wt1/.git\" >expected_gitdir &&\n-\t\tcat .git/worktrees/wt1/gitdir >actual_gitdir &&\n-\t\techo \"gitdir: ../.git/worktrees/wt1\" >expected_git &&\n-\t\tcat wt1/.git >actual_git &&\n-\t\ttest_cmp expected_gitdir actual_gitdir &&\n-\t\ttest_cmp expected_git actual_git\n-\t)\n-'\n-\n-test_expect_success 'move repo without breaking relative internal links' '\n-\ttest_when_finished rm -rf repo moved &&\n-\tgit init repo &&\n-\t(\n-\t\tcd repo &&\n-\t\ttest_commit initial &&\n-\t\tgit worktree add wt1 &&\n-\t\tcd .. &&\n-\t\tmv repo moved &&\n-\t\tcd moved/wt1 &&\n-\t\tgit status >out 2>err &&\n-\t\ttest_must_be_empty err\n-\t)\n-'\n-\n-test_done\ndiff --git a/worktree.c b/worktree.c\nindex 77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7..de5c5e53a5f2a758ddf470b5d6a9ad6c66247181 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -14,6 +14,8 @@\n #include \"wt-status.h\"\n #include \"config.h\"\n \n+int use_relative_paths = 0;\n+\n void free_worktree(struct worktree *worktree)\n {\n \tif (!worktree)\n@@ -111,9 +113,9 @@ struct worktree *get_linked_worktree(const char *id,\n \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n \n \tif (!is_absolute_path(worktree_path.buf)) {\n-\t strbuf_strip_suffix(&path, \"gitdir\");\n-\t strbuf_addbuf(&path, &worktree_path);\n-\t strbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n+\t\tstrbuf_strip_suffix(&path, \"gitdir\");\n+\t\tstrbuf_addbuf(&path, &worktree_path);\n+\t\tstrbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n \t}\n \n \tCALLOC_ARRAY(worktree, 1);\n@@ -725,12 +727,15 @@ static int is_main_worktree_path(const char *path)\n * won't know which /worktrees//gitdir to repair. However, we may\n * be able to infer the gitdir by manually reading /path/to/worktree/.git,\n * extracting the , and checking if /worktrees/ exists.\n+ *\n+ * Returns -1 on failure and strbuf.len on success.\n */\n static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n {\n \tstruct strbuf actual = STRBUF_INIT;\n \tconst char *id;\n \n+\tstrbuf_reset(inferred);\n \tif (strbuf_read_file(&actual, gitfile, 0) < 0)\n \t\tgoto error;\n \tif (!starts_with(actual.buf, \"gitdir:\"))\n@@ -741,18 +746,16 @@ static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n \tid++; /* advance past '/' to point at */\n \tif (!*id)\n \t\tgoto error;\n-\tstrbuf_reset(inferred);\n \tstrbuf_git_common_path(inferred, the_repository, \"worktrees/%s\", id);\n \tif (!is_directory(inferred->buf))\n \t\tgoto error;\n \n \tstrbuf_release(&actual);\n-\treturn 1;\n-\n+\treturn inferred->len;\n error:\n \tstrbuf_release(&actual);\n \tstrbuf_reset(inferred); /* clear invalid path */\n-\treturn 0;\n+\treturn -1;\n }\n \n /*\ndiff --git a/worktree.h b/worktree.h\nindex e96118621638667d87c5d7e0452ed10bd1ddf606..37e65d508ed23d3e7a29850bb938285072a3aaa6 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -5,6 +5,8 @@\n \n struct strbuf;\n \n+extern int use_relative_paths;\n+\n struct worktree {\n \t/* The repository this worktree belongs to. */\n \tstruct repository *repo;\n\n-- \n2.47.0\n\n\n"},{"id":"506247","messageId":"20241028-wt_relative_options-v2-2-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"[PATCH v2 2/5] worktree: add `write_worktree_linking_files` function","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:09:44Z","receivedAt":"2024-10-28T19:09:52Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"A new helper function, `write_worktree_linking_files()`, centralizes\nthe logic for computing and writing either relative or absolute\npaths, based on the provided configuration. This function accepts\n`strbuf` pointers to both the worktree’s `.git` link and the\nrepository’s `gitdir`, and then writes the appropriate path to each.\n\nThis also teachs `git worktree repair` to fix the linking files if\nthere is an absolute/relative paths but the links are correct. E.g.,\n`git worktree repair` can be used to convert a valid worktree between\nabsolute and relative paths.\n\nSigned-off-by: Caleb White \n---\n builtin/worktree.c | 11 +----\n worktree.c | 118 +++++++++++++++++++++++++++--------------------------\n worktree.h | 12 ++++++\n 3 files changed, 74 insertions(+), 67 deletions(-)\n\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex c1130be5890c905c0b648782a834eb8dfcd79ba5..bb06830d6fe82aa97833c6e87f034115dfaa23bd 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -417,8 +417,7 @@ static int add_worktree(const char *path, const char *refname,\n \t\t\tconst struct add_opts *opts)\n {\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT, sb_tmp = STRBUF_INIT;\n-\tstruct strbuf sb_path_realpath = STRBUF_INIT, sb_repo_realpath = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n \tconst char *name;\n \tstruct strvec child_env = STRVEC_INIT;\n \tunsigned int counter = 0;\n@@ -494,10 +493,7 @@ static int add_worktree(const char *path, const char *refname,\n \n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n-\tstrbuf_realpath(&sb_path_realpath, path, 1);\n-\tstrbuf_realpath(&sb_repo_realpath, sb_repo.buf, 1);\n-\twrite_file(sb.buf, \"%s/.git\", relative_path(sb_path_realpath.buf, sb_repo_realpath.buf, &sb_tmp));\n-\twrite_file(sb_git.buf, \"gitdir: %s\", relative_path(sb_repo_realpath.buf, sb_path_realpath.buf, &sb_tmp));\n+\twrite_worktree_linking_files(sb_git, sb);\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n \twrite_file(sb.buf, \"../..\");\n@@ -581,12 +577,9 @@ static int add_worktree(const char *path, const char *refname,\n \n \tstrvec_clear(&child_env);\n \tstrbuf_release(&sb);\n-\tstrbuf_release(&sb_tmp);\n \tstrbuf_release(&symref);\n \tstrbuf_release(&sb_repo);\n-\tstrbuf_release(&sb_repo_realpath);\n \tstrbuf_release(&sb_git);\n-\tstrbuf_release(&sb_path_realpath);\n \tstrbuf_release(&sb_name);\n \tfree_worktree(wt);\n \treturn ret;\ndiff --git a/worktree.c b/worktree.c\nindex de5c5e53a5f2a758ddf470b5d6a9ad6c66247181..f4cee73d7a1edecafdff30b6d5e2d9dd1365b93e 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -381,29 +381,24 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n void update_worktree_location(struct worktree *wt, const char *path_)\n {\n \tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf repo = STRBUF_INIT;\n-\tstruct strbuf file = STRBUF_INIT;\n-\tstruct strbuf tmp = STRBUF_INIT;\n+\tstruct strbuf dotgit = STRBUF_INIT;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n \n \tif (is_main_worktree(wt))\n \t\tBUG(\"can't relocate main worktree\");\n \n-\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n+\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n \tstrbuf_realpath(&path, path_, 1);\n+\tstrbuf_addf(&dotgit, \"%s/.git\", path.buf);\n \tif (fspathcmp(wt->path, path.buf)) {\n-\t\tstrbuf_addf(&file, \"%s/gitdir\", repo.buf);\n-\t\twrite_file(file.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n-\t\tstrbuf_reset(&file);\n-\t\tstrbuf_addf(&file, \"%s/.git\", path.buf);\n-\t\twrite_file(file.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n+\t\twrite_worktree_linking_files(dotgit, gitdir);\n \n \t\tfree(wt->path);\n \t\twt->path = strbuf_detach(&path, NULL);\n \t}\n \tstrbuf_release(&path);\n-\tstrbuf_release(&repo);\n-\tstrbuf_release(&file);\n-\tstrbuf_release(&tmp);\n+\tstrbuf_release(&dotgit);\n+\tstrbuf_release(&gitdir);\n }\n \n int is_worktree_being_rebased(const struct worktree *wt,\n@@ -582,9 +577,9 @@ static void repair_gitfile(struct worktree *wt,\n \t\t\t worktree_repair_fn fn, void *cb_data)\n {\n \tstruct strbuf dotgit = STRBUF_INIT;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n \tstruct strbuf repo = STRBUF_INIT;\n \tstruct strbuf backlink = STRBUF_INIT;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \tchar *dotgit_contents = NULL;\n \tconst char *repair = NULL;\n \tint err;\n@@ -600,6 +595,7 @@ static void repair_gitfile(struct worktree *wt,\n \n \tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n \tstrbuf_addf(&dotgit, \"%s/.git\", wt->path);\n+\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n \tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n \n \tif (dotgit_contents) {\n@@ -617,18 +613,20 @@ static void repair_gitfile(struct worktree *wt,\n \t\trepair = _(\".git file broken\");\n \telse if (fspathcmp(backlink.buf, repo.buf))\n \t\trepair = _(\".git file incorrect\");\n+\telse if (use_relative_paths == is_absolute_path(dotgit_contents))\n+\t\trepair = _(\".git file absolute/relative path mismatch\");\n \n \tif (repair) {\n \t\tfn(0, wt->path, repair, cb_data);\n-\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, wt->path, &tmp));\n+\t\twrite_worktree_linking_files(dotgit, gitdir);\n \t}\n \n done:\n \tfree(dotgit_contents);\n \tstrbuf_release(&repo);\n \tstrbuf_release(&dotgit);\n+\tstrbuf_release(&gitdir);\n \tstrbuf_release(&backlink);\n-\tstrbuf_release(&tmp);\n }\n \n static void repair_noop(int iserr UNUSED,\n@@ -653,45 +651,30 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data)\n \n void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path)\n {\n-\tstruct strbuf path = STRBUF_INIT;\n-\tstruct strbuf repo = STRBUF_INIT;\n \tstruct strbuf gitdir = STRBUF_INIT;\n \tstruct strbuf dotgit = STRBUF_INIT;\n-\tstruct strbuf olddotgit = STRBUF_INIT;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \n \tif (is_main_worktree(wt))\n \t\tgoto done;\n \n-\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n-\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n+\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n \n-\tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n+\tif (strbuf_read_file(&dotgit, gitdir.buf, 0) < 0)\n \t\tgoto done;\n \n-\tstrbuf_rtrim(&olddotgit);\n-\tif (is_absolute_path(olddotgit.buf)) {\n-\t\tstrbuf_addbuf(&dotgit, &olddotgit);\n-\t} else {\n-\t\tstrbuf_addf(&dotgit, \"%s/worktrees/%s/%s\", old_path, wt->id, olddotgit.buf);\n+\tstrbuf_rtrim(&dotgit);\n+\tif (!is_absolute_path(dotgit.buf)) {\n+\t\tstrbuf_insertf(&dotgit, 0, \"%s/worktrees/%s/\", old_path, wt->id);\n \t\tstrbuf_realpath_forgiving(&dotgit, dotgit.buf, 0);\n \t}\n \n \tif (!file_exists(dotgit.buf))\n \t\tgoto done;\n \n-\tstrbuf_addbuf(&path, &dotgit);\n-\tstrbuf_strip_suffix(&path, \"/.git\");\n-\n-\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n-\twrite_file(gitdir.buf, \"%s\", relative_path(dotgit.buf, repo.buf, &tmp));\n+\twrite_worktree_linking_files(dotgit, gitdir);\n done:\n-\tstrbuf_release(&path);\n-\tstrbuf_release(&repo);\n \tstrbuf_release(&gitdir);\n \tstrbuf_release(&dotgit);\n-\tstrbuf_release(&olddotgit);\n-\tstrbuf_release(&tmp);\n }\n \n void repair_worktrees_after_gitdir_move(const char *old_path)\n@@ -766,13 +749,10 @@ void repair_worktree_at_path(const char *path,\n \t\t\t worktree_repair_fn fn, void *cb_data)\n {\n \tstruct strbuf dotgit = STRBUF_INIT;\n-\tstruct strbuf realdotgit = STRBUF_INIT;\n \tstruct strbuf backlink = STRBUF_INIT;\n \tstruct strbuf inferred_backlink = STRBUF_INIT;\n \tstruct strbuf gitdir = STRBUF_INIT;\n \tstruct strbuf olddotgit = STRBUF_INIT;\n-\tstruct strbuf realolddotgit = STRBUF_INIT;\n-\tstruct strbuf tmp = STRBUF_INIT;\n \tchar *dotgit_contents = NULL;\n \tconst char *repair = NULL;\n \tint err;\n@@ -784,25 +764,25 @@ void repair_worktree_at_path(const char *path,\n \t\tgoto done;\n \n \tstrbuf_addf(&dotgit, \"%s/.git\", path);\n-\tif (!strbuf_realpath(&realdotgit, dotgit.buf, 0)) {\n+\tif (!strbuf_realpath(&dotgit, dotgit.buf, 0)) {\n \t\tfn(1, path, _(\"not a valid path\"), cb_data);\n \t\tgoto done;\n \t}\n \n-\tinfer_backlink(realdotgit.buf, &inferred_backlink);\n+\tinfer_backlink(dotgit.buf, &inferred_backlink);\n \tstrbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);\n-\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err));\n+\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n \tif (dotgit_contents) {\n \t\tif (is_absolute_path(dotgit_contents)) {\n \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n \t\t} else {\n-\t\t\tstrbuf_addbuf(&backlink, &realdotgit);\n+\t\t\tstrbuf_addbuf(&backlink, &dotgit);\n \t\t\tstrbuf_strip_suffix(&backlink, \".git\");\n \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n \t\t}\n \t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n+\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n \t\tgoto done;\n \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n \t\tif (inferred_backlink.len) {\n@@ -815,11 +795,11 @@ void repair_worktree_at_path(const char *path,\n \t\t\t */\n \t\t\tstrbuf_swap(&backlink, &inferred_backlink);\n \t\t} else {\n-\t\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n+\t\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n \t\t\tgoto done;\n \t\t}\n \t} else {\n-\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n+\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n \t\tgoto done;\n \t}\n \n@@ -841,39 +821,35 @@ void repair_worktree_at_path(const char *path,\n \t * in the \"copy\" repository. In this case, point the \"copy\" worktree's\n \t * .git file at the \"copy\" repository.\n \t */\n-\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf)) {\n+\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))\n \t\tstrbuf_swap(&backlink, &inferred_backlink);\n-\t}\n \n \tstrbuf_addf(&gitdir, \"%s/gitdir\", backlink.buf);\n \tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n \t\trepair = _(\"gitdir unreadable\");\n+\telse if (use_relative_paths == is_absolute_path(olddotgit.buf))\n+\t\trepair = _(\"gitdir absolute/relative path mismatch\");\n \telse {\n \t\tstrbuf_rtrim(&olddotgit);\n-\t\tif (is_absolute_path(olddotgit.buf)) {\n-\t\t\tstrbuf_addbuf(&realolddotgit, &olddotgit);\n-\t\t} else {\n-\t\t\tstrbuf_addf(&realolddotgit, \"%s/%s\", backlink.buf, olddotgit.buf);\n-\t\t\tstrbuf_realpath_forgiving(&realolddotgit, realolddotgit.buf, 0);\n+\t\tif (!is_absolute_path(olddotgit.buf)) {\n+\t\t\tstrbuf_insertf(&olddotgit, 0, \"%s/\", backlink.buf);\n+\t\t\tstrbuf_realpath_forgiving(&olddotgit, olddotgit.buf, 0);\n \t\t}\n-\t\tif (fspathcmp(realolddotgit.buf, realdotgit.buf))\n+\t\tif (fspathcmp(olddotgit.buf, dotgit.buf))\n \t\t\trepair = _(\"gitdir incorrect\");\n \t}\n \n \tif (repair) {\n \t\tfn(0, gitdir.buf, repair, cb_data);\n-\t\twrite_file(gitdir.buf, \"%s\", relative_path(realdotgit.buf, backlink.buf, &tmp));\n+\t\twrite_worktree_linking_files(dotgit, gitdir);\n \t}\n done:\n \tfree(dotgit_contents);\n \tstrbuf_release(&olddotgit);\n-\tstrbuf_release(&realolddotgit);\n \tstrbuf_release(&backlink);\n \tstrbuf_release(&inferred_backlink);\n \tstrbuf_release(&gitdir);\n-\tstrbuf_release(&realdotgit);\n \tstrbuf_release(&dotgit);\n-\tstrbuf_release(&tmp);\n }\n \n int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, timestamp_t expire)\n@@ -1034,3 +1010,29 @@ int init_worktree_config(struct repository *r)\n \tfree(main_worktree_file);\n \treturn res;\n }\n+\n+void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf repo = STRBUF_INIT;\n+\tstruct strbuf tmp = STRBUF_INIT;\n+\n+\tstrbuf_addbuf(&path, &dotgit);\n+\tstrbuf_strip_suffix(&path, \"/.git\");\n+\tstrbuf_realpath(&path, path.buf, 1);\n+\tstrbuf_addbuf(&repo, &gitdir);\n+\tstrbuf_strip_suffix(&repo, \"/gitdir\");\n+\tstrbuf_realpath(&repo, repo.buf, 1);\n+\n+\tif (use_relative_paths) {\n+\t\twrite_file(gitdir.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n+\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n+\t} else {\n+\t\twrite_file(gitdir.buf, \"%s/.git\", path.buf);\n+\t\twrite_file(dotgit.buf, \"gitdir: %s\", repo.buf);\n+\t}\n+\n+\tstrbuf_release(&path);\n+\tstrbuf_release(&repo);\n+\tstrbuf_release(&tmp);\n+}\ndiff --git a/worktree.h b/worktree.h\nindex 37e65d508ed23d3e7a29850bb938285072a3aaa6..5929089891c97318a8f5329f7938264c717050d5 100644\n--- a/worktree.h\n+++ b/worktree.h\n@@ -217,4 +217,16 @@ void strbuf_worktree_ref(const struct worktree *wt,\n */\n int init_worktree_config(struct repository *r);\n \n+/**\n+ * Write the .git file and gitdir file that links the worktree to the repository.\n+ *\n+ * The `dotgit` parameter is the path to the worktree's .git file, and `gitdir`\n+ * is the path to the repository's `gitdir` file.\n+ *\n+ * Example\n+ * dotgit: \"/path/to/foo/.git\"\n+ * gitdir: \"/path/to/repo/worktrees/foo/gitdir\"\n+ */\n+void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir);\n+\n #endif\n\n-- \n2.47.0\n\n\n"},{"id":"506248","messageId":"20241028-wt_relative_options-v2-3-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"[PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:09:52Z","receivedAt":"2024-10-28T19:10:00Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"This patch expands the test coverage by adding cases that specifically\nhandle relative paths. These tests verify correct behavior in a variety\nof operations, including: adding, listing, pruning, moving, and\nrepairing worktrees with relative paths configured.\n\nThis also adds a test case for reinitializing a repository that has\nrelative worktrees.\n\nSigned-off-by: Caleb White \n---\n t/t0001-init.sh | 17 +++++++++++++----\n t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n t/t2401-worktree-prune.sh | 3 ++-\n t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++\n t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++\n t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++\n 6 files changed, 126 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -435,6 +435,7 @@ sep_git_dir_worktree () {\n \ttest_when_finished \"rm -rf mainwt linkwt seprepo\" &&\n \tgit init mainwt &&\n \ttest_commit -C mainwt gumby &&\n+\tgit -C mainwt config worktree.useRelativePaths \"$([ \"$2\" = \"relative\" ] && echo true || echo false)\" &&\n \tgit -C mainwt worktree add --detach ../linkwt &&\n \tgit -C \"$1\" init --separate-git-dir ../seprepo &&\n \tgit -C mainwt rev-parse --git-common-dir >expect &&\n@@ -442,12 +443,20 @@ sep_git_dir_worktree () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success 're-init to move gitdir with linked worktrees' '\n-\tsep_git_dir_worktree mainwt\n+test_expect_success 're-init to move gitdir with linked worktrees (absolute)' '\n+\tsep_git_dir_worktree mainwt absolute\n '\n \n-test_expect_success 're-init to move gitdir within linked worktree' '\n-\tsep_git_dir_worktree linkwt\n+test_expect_success 're-init to move gitdir within linked worktree (absolute)' '\n+\tsep_git_dir_worktree linkwt absolute\n+'\n+\n+test_expect_success 're-init to move gitdir with linked worktrees (relative)' '\n+\tsep_git_dir_worktree mainwt relative\n+'\n+\n+test_expect_success 're-init to move gitdir within linked worktree (relative)' '\n+\tsep_git_dir_worktree linkwt relative\n '\n \n test_expect_success MINGW '.git hidden' '\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex cfc4aeb1798c6d023909cec771e5b74e983af5ea..630c13230b3cc762ce8d943e22be8891aa2b1871 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -1207,4 +1207,45 @@ test_expect_success '\"add\" with initialized submodule, with submodule.recurse se\n \tgit -C project-clone -c submodule.recurse worktree add ../project-5\n '\n \n+test_expect_success 'can create worktrees with relative paths' '\n+\ttest_when_finished \"git worktree remove relative\" &&\n+\tgit config worktree.useRelativePaths false &&\n+\tgit worktree add --relative-paths ./relative &&\n+\tcat relative/.git >actual &&\n+\techo \"gitdir: ../.git/worktrees/relative\" >expect &&\n+\ttest_cmp expect actual &&\n+\tcat .git/worktrees/relative/gitdir >actual &&\n+\techo \"../../../relative/.git\" >expect &&\n+\ttest_cmp expect actual\n+\n+'\n+\n+test_expect_success 'can create worktrees with absolute paths' '\n+\tgit config worktree.useRelativePaths true &&\n+\tgit worktree add ./relative &&\n+\tcat relative/.git >actual &&\n+\techo \"gitdir: ../.git/worktrees/relative\" >expect &&\n+\ttest_cmp expect actual &&\n+\tgit worktree add --no-relative-paths ./absolute &&\n+\tcat absolute/.git >actual &&\n+\techo \"gitdir: $(pwd)/.git/worktrees/absolute\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'move repo without breaking relative internal links' '\n+\ttest_when_finished rm -rf repo moved &&\n+\tgit init repo &&\n+\t(\n+\t\tcd repo &&\n+\t\tgit config worktree.useRelativePaths true &&\n+\t\ttest_commit initial &&\n+\t\tgit worktree add wt1 &&\n+\t\tcd .. &&\n+\t\tmv repo moved &&\n+\t\tcd moved/wt1 &&\n+\t\tgit status >out 2>err &&\n+\t\ttest_must_be_empty err\n+\t)\n+'\n+\n test_done\ndiff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh\nindex 976d048e3efc74be9cd909ce76d552b3944d2e10..5eb52b9abbf29514dc082c260ebb7a5e8e63aae0 100755\n--- a/t/t2401-worktree-prune.sh\n+++ b/t/t2401-worktree-prune.sh\n@@ -120,11 +120,12 @@ test_expect_success 'prune duplicate (main/linked)' '\n \t! test -d .git/worktrees/wt\n '\n \n-test_expect_success 'not prune proper worktrees when run inside linked worktree' '\n+test_expect_success 'not prune proper worktrees inside linked worktree with relative paths' '\n \ttest_when_finished rm -rf repo wt_ext &&\n \tgit init repo &&\n \t(\n \t cd repo &&\n+\t git config worktree.useRelativePaths true &&\n \t echo content >file &&\n \t git add file &&\n \t git commit -m msg &&\ndiff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh\nindex 33ea9cb21ba07c9563530b54da06753eaa993fe2..780daa6cd6351f8fa9434619cc212aade8f01420 100755\n--- a/t/t2402-worktree-list.sh\n+++ b/t/t2402-worktree-list.sh\n@@ -261,6 +261,7 @@ test_expect_success 'broken main worktree still at the top' '\n '\n \n test_expect_success 'linked worktrees are sorted' '\n+\ttest_when_finished \"rm -rf sorted\" &&\n \tmkdir sorted &&\n \tgit init sorted/main &&\n \t(\n@@ -280,6 +281,27 @@ test_expect_success 'linked worktrees are sorted' '\n \ttest_cmp expected sorted/main/actual\n '\n \n+test_expect_success 'linked worktrees with relative paths are shown with absolute paths' '\n+\ttest_when_finished \"rm -rf sorted\" &&\n+\tmkdir sorted &&\n+\tgit init sorted/main &&\n+\t(\n+\t\tcd sorted/main &&\n+\t\ttest_tick &&\n+\t\ttest_commit new &&\n+\t\tgit worktree add --relative-paths ../first &&\n+\t\tgit worktree add ../second &&\n+\t\tgit worktree list --porcelain >out &&\n+\t\tgrep ^worktree out >actual\n+\t) &&\n+\tcat >expected <<-EOF &&\n+\tworktree $(pwd)/sorted/main\n+\tworktree $(pwd)/sorted/first\n+\tworktree $(pwd)/sorted/second\n+\tEOF\n+\ttest_cmp expected sorted/main/actual\n+'\n+\n test_expect_success 'worktree path when called in .git directory' '\n \tgit worktree list >list1 &&\n \tgit -C .git worktree list >list2 &&\ndiff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh\nindex 901342ea09b51a8e832f1109fbb737df84283aa2..6ce9ed3f1e6b3f73d2a290e770233eec30221fe5 100755\n--- a/t/t2403-worktree-move.sh\n+++ b/t/t2403-worktree-move.sh\n@@ -247,4 +247,26 @@ test_expect_success 'not remove a repo with initialized submodule' '\n \t)\n '\n \n+test_expect_success 'move worktree with absolute path to relative path' '\n+\tgit config worktree.useRelativePaths false &&\n+\tgit worktree add ./absolute &&\n+\tgit worktree move --relative-paths absolute relative &&\n+\tcat relative/.git >actual &&\n+\techo \"gitdir: ../.git/worktrees/absolute\" >expect &&\n+\ttest_cmp expect actual &&\n+\tgit config worktree.useRelativePaths true &&\n+\tgit worktree move relative relative2 &&\n+\tcat relative2/.git >actual &&\n+\techo \"gitdir: ../.git/worktrees/absolute\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'move worktree with relative path to absolute path' '\n+\tgit config worktree.useRelativePaths true &&\n+\tgit worktree move --no-relative-paths relative2 absolute &&\n+\tcat absolute/.git >actual &&\n+\techo \"gitdir: $(pwd)/.git/worktrees/absolute\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh\nindex 7686e60f6ad186519b275f11a5e14064c905b207..84451e903b2ef3c645c0311faf055c846588baf6 100755\n--- a/t/t2406-worktree-repair.sh\n+++ b/t/t2406-worktree-repair.sh\n@@ -216,4 +216,30 @@ test_expect_success 'repair copied main and linked worktrees' '\n \ttest_cmp dup/linked.expect dup/linked/.git\n '\n \n+test_expect_success 'repair absolute worktree to use relative paths' '\n+\ttest_when_finished \"rm -rf main side sidemoved\" &&\n+\ttest_create_repo main &&\n+\ttest_commit -C main init &&\n+\tgit -C main worktree add --detach ../side &&\n+\techo \"../../../../sidemoved/.git\" >expect-gitdir &&\n+\techo \"gitdir: ../main/.git/worktrees/side\" >expect-gitfile &&\n+\tmv side sidemoved &&\n+\tgit -C main worktree repair --relative-paths ../sidemoved &&\n+\ttest_cmp expect-gitdir main/.git/worktrees/side/gitdir &&\n+\ttest_cmp expect-gitfile sidemoved/.git\n+'\n+\n+test_expect_success 'repair relative worktree to use absolute paths' '\n+\ttest_when_finished \"rm -rf main side sidemoved\" &&\n+\ttest_create_repo main &&\n+\ttest_commit -C main init &&\n+\tgit -C main worktree add --relative-paths --detach ../side &&\n+\techo \"$(pwd)/sidemoved/.git\" >expect-gitdir &&\n+\techo \"gitdir: $(pwd)/main/.git/worktrees/side\" >expect-gitfile &&\n+\tmv side sidemoved &&\n+\tgit -C main worktree repair ../sidemoved &&\n+\ttest_cmp expect-gitdir main/.git/worktrees/side/gitdir &&\n+\ttest_cmp expect-gitfile sidemoved/.git\n+'\n+\n test_done\n\n-- \n2.47.0\n\n\n"},{"id":"506249","messageId":"20241028-wt_relative_options-v2-4-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"[PATCH v2 4/5] setup: correctly reinitialize repository version","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:10:01Z","receivedAt":"2024-10-28T19:10:06Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"When reinitializing a repository, Git does not account for extensions\nother than `objectformat` and `refstorage` when determining the\nrepository version. This can lead to a repository being downgraded to\nversion 0 if extensions are set, causing Git future operations to fail.\n\nThis patch teaches Git to check if other extensions are defined in the\nconfig to ensure that the repository version is set correctly.\n\nSigned-off-by: Caleb White \n---\n setup.c | 32 +++++++++++++++++++++++---------\n 1 file changed, 23 insertions(+), 9 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 7b648de0279116b381eea46800ad130606926103..1e5c2eacb19eb6b230d7c9954f66fc7ae0b05631 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -2204,8 +2204,8 @@ void initialize_repository_version(int hash_algo,\n \t\t\t\t enum ref_storage_format ref_storage_format,\n \t\t\t\t int reinit)\n {\n-\tchar repo_version_string[10];\n-\tint repo_version = GIT_REPO_VERSION;\n+\tstruct strbuf repo_version = STRBUF_INIT;\n+\tint target_version = GIT_REPO_VERSION;\n \n \t/*\n \t * Note that we initialize the repository version to 1 when the ref\n@@ -2216,12 +2216,7 @@ void initialize_repository_version(int hash_algo,\n \t */\n \tif (hash_algo != GIT_HASH_SHA1 ||\n \t ref_storage_format != REF_STORAGE_FORMAT_FILES)\n-\t\trepo_version = GIT_REPO_VERSION_READ;\n-\n-\t/* This forces creation of new config file */\n-\txsnprintf(repo_version_string, sizeof(repo_version_string),\n-\t\t \"%d\", repo_version);\n-\tgit_config_set(\"core.repositoryformatversion\", repo_version_string);\n+\t\ttarget_version = GIT_REPO_VERSION_READ;\n \n \tif (hash_algo != GIT_HASH_SHA1 && hash_algo != GIT_HASH_UNKNOWN)\n \t\tgit_config_set(\"extensions.objectformat\",\n@@ -2234,6 +2229,25 @@ void initialize_repository_version(int hash_algo,\n \t\t\t ref_storage_format_to_name(ref_storage_format));\n \telse if (reinit)\n \t\tgit_config_set_gently(\"extensions.refstorage\", NULL);\n+\n+\tif (reinit) {\n+\t\tstruct strbuf config = STRBUF_INIT;\n+\t\tstruct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;\n+\n+\t\tstrbuf_git_common_path(&config, the_repository, \"config\");\n+\t\tread_repository_format(&repo_fmt, config.buf);\n+\n+\t\tif (repo_fmt.v1_only_extensions.nr)\n+\t\t\ttarget_version = GIT_REPO_VERSION_READ;\n+\n+\t\tstrbuf_release(&config);\n+\t\tclear_repository_format(&repo_fmt);\n+\t}\n+\n+\tstrbuf_addf(&repo_version, \"%d\", target_version);\n+\tgit_config_set(\"core.repositoryformatversion\", repo_version.buf);\n+\n+\tstrbuf_release(&repo_version);\n }\n \n static int is_reinit(void)\n@@ -2333,7 +2347,7 @@ static int create_default_files(const char *template_path,\n \t\tadjust_shared_perm(repo_get_git_dir(the_repository));\n \t}\n \n-\tinitialize_repository_version(fmt->hash_algo, fmt->ref_storage_format, 0);\n+\tinitialize_repository_version(fmt->hash_algo, fmt->ref_storage_format, reinit);\n \n \t/* Check filemode trustability */\n \tpath = git_path_buf(&buf, \"config\");\n\n-- \n2.47.0\n\n\n"},{"id":"506250","messageId":"20241028-wt_relative_options-v2-5-33a5021bd7bb@pm.me","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"[PATCH v2 5/5] worktree: add `relativeWorktrees` extension","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-28T19:10:09Z","receivedAt":"2024-10-28T19:10:18Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"A new extension, `relativeWorktrees`, is added to indicate that at least\none worktree in the repository has been linked with relative paths. This\nextension is automatically set when a worktree is created or repaired\nusing the `--relative-paths` option, or when the\n`worktree.useRelativePaths` config is set to `true`.\n\nThe `relativeWorktrees` extension ensures older Git versions do not\nattempt to automatically prune worktrees with relative paths, as they\nwould not not recognize the paths as being valid.\n\nSuggested-by: Junio C Hamano \nSigned-off-by: Caleb White \n---\n Documentation/config/extensions.txt | 6 ++++++\n repository.c | 1 +\n repository.h | 1 +\n setup.c | 7 +++++++\n setup.h | 1 +\n t/t2400-worktree-add.sh | 13 +++++++++++++\n worktree.c | 8 ++++++++\n 7 files changed, 37 insertions(+)\n\ndiff --git a/Documentation/config/extensions.txt b/Documentation/config/extensions.txt\nindex 5dc569d1c9c77c15e32441493289f9c9dd5e7f0b..5cb4721a0e0ae1ed64f90492c0dc18b96473cb33 100644\n--- a/Documentation/config/extensions.txt\n+++ b/Documentation/config/extensions.txt\n@@ -63,6 +63,12 @@ Note that this setting should only be set by linkgit:git-init[1] or\n linkgit:git-clone[1]. Trying to change it after initialization will not\n work and will produce hard-to-diagnose issues.\n \n+relativeWorktrees::\n+\tIf enabled, indicates at least one worktree has been linked with\n+\trelative paths. Automatically set if a worktree has been created or\n+\trepaired with either the `--relative-paths` option or with the\n+\t`worktree.useRelativePaths` config set to `true`.\n+\n worktreeConfig::\n \tIf enabled, then worktrees will load config settings from the\n \t`$GIT_DIR/config.worktree` file in addition to the\ndiff --git a/repository.c b/repository.c\nindex f988b8ae68a6a29792e7f2c980a02bd0e388a3b9..1a6a62bbd03a5dc4fdade3eb45ea2696968abc23 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -283,6 +283,7 @@ int repo_init(struct repository *repo,\n \trepo_set_compat_hash_algo(repo, format.compat_hash_algo);\n \trepo_set_ref_storage_format(repo, format.ref_storage_format);\n \trepo->repository_format_worktree_config = format.worktree_config;\n+\trepo->repository_format_relative_worktrees = format.relative_worktrees;\n \n \t/* take ownership of format.partial_clone */\n \trepo->repository_format_partial_clone = format.partial_clone;\ndiff --git a/repository.h b/repository.h\nindex 24a66a496a6ff516ce06d47b7329b3d36eb701ca..c4c92b2ab9c9e3b425dc2974636e33d1f4089c69 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -150,6 +150,7 @@ struct repository {\n \n \t/* Configurations */\n \tint repository_format_worktree_config;\n+\tint repository_format_relative_worktrees;\n \n \t/* Indicate if a repository has a different 'commondir' from 'gitdir' */\n \tunsigned different_commondir:1;\ndiff --git a/setup.c b/setup.c\nindex 1e5c2eacb19eb6b230d7c9954f66fc7ae0b05631..39ff48d9dc5d67b16159c6cca66ff2663bbba6cf 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -683,6 +683,9 @@ static enum extension_result handle_extension(const char *var,\n \t\t\t\t \"extensions.refstorage\", value);\n \t\tdata->ref_storage_format = format;\n \t\treturn EXTENSION_OK;\n+\t} else if (!strcmp(ext, \"relativeworktrees\")) {\n+\t\tdata->relative_worktrees = git_config_bool(var, value);\n+\t\treturn EXTENSION_OK;\n \t}\n \treturn EXTENSION_UNKNOWN;\n }\n@@ -1854,6 +1857,8 @@ const char *setup_git_directory_gently(int *nongit_ok)\n \t\t\t\t\t\t repo_fmt.ref_storage_format);\n \t\t\tthe_repository->repository_format_worktree_config =\n \t\t\t\trepo_fmt.worktree_config;\n+\t\t\tthe_repository->repository_format_relative_worktrees =\n+\t\t\t\trepo_fmt.relative_worktrees;\n \t\t\t/* take ownership of repo_fmt.partial_clone */\n \t\t\tthe_repository->repository_format_partial_clone =\n \t\t\t\trepo_fmt.partial_clone;\n@@ -1950,6 +1955,8 @@ void check_repository_format(struct repository_format *fmt)\n \t\t\t\t fmt->ref_storage_format);\n \tthe_repository->repository_format_worktree_config =\n \t\tfmt->worktree_config;\n+\tthe_repository->repository_format_relative_worktrees =\n+\t\tfmt->relative_worktrees;\n \tthe_repository->repository_format_partial_clone =\n \t\txstrdup_or_null(fmt->partial_clone);\n \tclear_repository_format(&repo_fmt);\ndiff --git a/setup.h b/setup.h\nindex e496ab3e4de580c2d9f95a7ea0eaf90e0d41b070..18dc3b73686ce28fac2fe04282ce95f8bf3e6b74 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -129,6 +129,7 @@ struct repository_format {\n \tint precious_objects;\n \tchar *partial_clone; /* value of extensions.partialclone */\n \tint worktree_config;\n+\tint relative_worktrees;\n \tint is_bare;\n \tint hash_algo;\n \tint compat_hash_algo;\ndiff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\nindex 630c13230b3cc762ce8d943e22be8891aa2b1871..d36d8a4db0e924877787697579544f10f92dc0cf 100755\n--- a/t/t2400-worktree-add.sh\n+++ b/t/t2400-worktree-add.sh\n@@ -1248,4 +1248,17 @@ test_expect_success 'move repo without breaking relative internal links' '\n \t)\n '\n \n+test_expect_success 'relative worktree sets extension config' '\n+\ttest_when_finished \"rm -rf repo\" &&\n+\tgit init repo &&\n+\tgit -C repo commit --allow-empty -m base &&\n+\tgit -C repo worktree add --relative-paths ./foo &&\n+\tgit -C repo config get core.repositoryformatversion >actual &&\n+\techo 1 >expected &&\n+\ttest_cmp expected actual &&\n+\tgit -C repo config get extensions.relativeworktrees >actual &&\n+\techo true >expected &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\ndiff --git a/worktree.c b/worktree.c\nindex f4cee73d7a1edecafdff30b6d5e2d9dd1365b93e..0ae4a5f796f10c017ef90413e969e1622b54a897 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -1024,6 +1024,14 @@ void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir)\n \tstrbuf_strip_suffix(&repo, \"/gitdir\");\n \tstrbuf_realpath(&repo, repo.buf, 1);\n \n+\tif (use_relative_paths && !the_repository->repository_format_relative_worktrees) {\n+\t\tif (upgrade_repository_format(1) < 0)\n+\t\t\tdie(_(\"unable to upgrade repository format to support relative worktrees\"));\n+\t\tif (git_config_set_gently(\"extensions.relativeWorktrees\", \"true\"))\n+\t\t\tdie(_(\"unable to set extensions.relativeWorktrees setting\"));\n+\t\tthe_repository->repository_format_relative_worktrees = 1;\n+\t}\n+\n \tif (use_relative_paths) {\n \t\twrite_file(gitdir.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n \t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n\n-- \n2.47.0\n\n\n"},{"id":"506282","messageId":"0bea06b8-11d0-497f-88e1-153cb61eb06d@gmail.com","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-1-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-29T14:52:28Z","receivedAt":"2024-10-29T14:52:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Caleb\n\nThanks for working on this.\n\nOn 28/10/2024 19:09, Caleb White wrote:\n> This patch introduces the `--[no-]relative-paths` CLI option for\n\n\"This patch\" is a bit redundant, I'd say \"Introduce a \n`--[no-]-relative-paths ..`\n\n> `git worktree {add, move, repair}` commands, as well as the\n> `worktree.useRelativePaths` configuration setting. When enabled,\n> these options allow worktrees to be linked using relative paths,\n> enhancing portability across environments where absolute paths\n> may differ (e.g., containerized setups, shared network drives).\n> Git still creates absolute paths by default, but these options allow\n> users to opt-in to relative paths if desired.\n\nThis sounds good, I'm not sure the patch actually implements anything \nother than the option parsing though. I think it would make sense to add \nthese options at the end of the patch series once the implementation has \nbeen changed to support them. I'd start with patches 4 and 5 to add the \nnew extension setting first, then refactor worktree.c to handle creating \nworktrees with relative or absolute paths and set the extension if \nappropriate, then add the --relative-path option to \"git worktree\"\n\n> Using the `--relative-paths` option with `worktree {move, repair}`\n> will convert absolute paths to relative ones, while `--no-relative-paths`\n> does the reverse. For cases where users want consistency in path\n> handling, the config option `worktree.useRelativePaths` provides\n> a persistent setting.\n> \n> In response to reviewer feedback from the initial patch series[1],\n> this revision includes slight refactoring for improved\n> maintainability and clarity in the code base.\n\nPlease don't mix cleanups with other code changes as it makes it hard to \ncheck that the cleanups don't change the behavior.\n\n> [1]: https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me\n> \n> Suggested-by: Junio C Hamano \n> Signed-off-by: Caleb White \n> ---\n> Documentation/config/worktree.txt | 5 +++++\n> Documentation/git-worktree.txt | 12 ++++++++++++\n> builtin/worktree.c | 9 +++++++++\n> t/t2408-worktree-relative.sh | 39 ---------------------------------------\n> worktree.c | 17 ++++++++++-------\n> worktree.h | 2 ++\n> 6 files changed, 38 insertions(+), 46 deletions(-)\n> \n> diff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\n> index 048e349482df6c892055720eb53cdcd6c327b6ed..44b783c2774dc5ff65e3fa232b0c25cd5254876b 100644\n> --- a/Documentation/config/worktree.txt\n> +++ b/Documentation/config/worktree.txt\n> @@ -7,3 +7,8 @@ worktree.guessRemote::\n> \tsuch a branch exists, it is checked out and set as \"upstream\"\n> \tfor the new branch. If no such match can be found, it falls\n> \tback to creating a new branch from the current HEAD.\n> +worktree.useRelativePaths::\n> +\tIf set to `true`, worktrees will be linked to the repository using\n> +\trelative paths rather than using absolute paths. This is particularly\n> +\tuseful for setups where the repository and worktrees may be moved between\n> +\tdifferent locations or environments.\n\nI think it would be helpful to spell out the implications of this to the \nuser - namely that you cannot use older versions of git on a repository \nwith worktrees using relative paths and it may break third-party \nsoftware as well.\n\n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index 70437c815f13852bd2eb862176b8b933e6de0acf..975dc3c46d480480457ec4857988a6b8bc67b647 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -216,6 +216,18 @@ To remove a locked worktree, specify `--force` twice.\n> This can also be set up as the default behaviour by using the\n> `worktree.guessRemote` config option.\n> \n> +--[no-]relative-paths::\n> +\tWorktrees will be linked to the repository using relative paths\n> +\trather than using absolute paths. This is particularly useful for setups\n> +\twhere the repository and worktrees may be moved between different\n> +\tlocations or environments.\n\nAgain we should spell out the implications of using relative paths.\n\n> +With `repair`, the linking files will be updated if there's an absolute/relative\n> +mismatch, even if the links are correct.\n> ++\n> +This can also be set up as the default behaviour by using the\n> +`worktree.useRelativePaths` config option.\n> +\n> --[no-]track::\n> \tWhen creating a new branch, if `` is a branch,\n> \tmark it as \"upstream\" from the new branch. This is the\n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n> \tif (!strcmp(var, \"worktree.guessremote\")) {\n> \t\tguess_remote = git_config_bool(var, value);\n> \t\treturn 0;\n> +\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n> +\t\tuse_relative_paths = git_config_bool(var, value);\n\nAs we're trying to remove global variables from libgit.a as part of the \nlibification effort I'd be much happier if \"use_relative_paths\" was \ndeclared as a \"static int\" in this file and then passed down to the \nfunctions that need it rather than declaring it as a global in \"worktree.c\".\n\n> diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh\n> deleted file mode 100755\n\nThere's no explanation for this change in the commit message\n\nBest Wishes\n\nPhillip\n\n> index a3136db7e28cb20926ff44211e246ce625a6e51a..0000000000000000000000000000000000000000\n> --- a/t/t2408-worktree-relative.sh\n> +++ /dev/null\n> @@ -1,39 +0,0 @@\n> -#!/bin/sh\n> -\n> -test_description='test worktrees linked with relative paths'\n> -\n> -TEST_PASSES_SANITIZE_LEAK=true\n> -. ./test-lib.sh\n> -\n> -test_expect_success 'links worktrees with relative paths' '\n> -\ttest_when_finished rm -rf repo &&\n> -\tgit init repo &&\n> -\t(\n> -\t\tcd repo &&\n> -\t\ttest_commit initial &&\n> -\t\tgit worktree add wt1 &&\n> -\t\techo \"../../../wt1/.git\" >expected_gitdir &&\n> -\t\tcat .git/worktrees/wt1/gitdir >actual_gitdir &&\n> -\t\techo \"gitdir: ../.git/worktrees/wt1\" >expected_git &&\n> -\t\tcat wt1/.git >actual_git &&\n> -\t\ttest_cmp expected_gitdir actual_gitdir &&\n> -\t\ttest_cmp expected_git actual_git\n> -\t)\n> -'\n> -\n> -test_expect_success 'move repo without breaking relative internal links' '\n> -\ttest_when_finished rm -rf repo moved &&\n> -\tgit init repo &&\n> -\t(\n> -\t\tcd repo &&\n> -\t\ttest_commit initial &&\n> -\t\tgit worktree add wt1 &&\n> -\t\tcd .. &&\n> -\t\tmv repo moved &&\n> -\t\tcd moved/wt1 &&\n> -\t\tgit status >out 2>err &&\n> -\t\ttest_must_be_empty err\n> -\t)\n> -'\n> -\n> -test_done\n> diff --git a/worktree.c b/worktree.c\n> index 77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7..de5c5e53a5f2a758ddf470b5d6a9ad6c66247181 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -14,6 +14,8 @@\n> #include \"wt-status.h\"\n> #include \"config.h\"\n> \n> +int use_relative_paths = 0;\n> +\n> void free_worktree(struct worktree *worktree)\n> {\n> \tif (!worktree)\n> @@ -111,9 +113,9 @@ struct worktree *get_linked_worktree(const char *id,\n> \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n> \n> \tif (!is_absolute_path(worktree_path.buf)) {\n> -\t strbuf_strip_suffix(&path, \"gitdir\");\n> -\t strbuf_addbuf(&path, &worktree_path);\n> -\t strbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n> +\t\tstrbuf_strip_suffix(&path, \"gitdir\");\n> +\t\tstrbuf_addbuf(&path, &worktree_path);\n> +\t\tstrbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n> \t}\n> \n> \tCALLOC_ARRAY(worktree, 1);\n> @@ -725,12 +727,15 @@ static int is_main_worktree_path(const char *path)\n> * won't know which /worktrees//gitdir to repair. However, we may\n> * be able to infer the gitdir by manually reading /path/to/worktree/.git,\n> * extracting the , and checking if /worktrees/ exists.\n> + *\n> + * Returns -1 on failure and strbuf.len on success.\n> */\n> static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n> {\n> \tstruct strbuf actual = STRBUF_INIT;\n> \tconst char *id;\n> \n> +\tstrbuf_reset(inferred);\n> \tif (strbuf_read_file(&actual, gitfile, 0) < 0)\n> \t\tgoto error;\n> \tif (!starts_with(actual.buf, \"gitdir:\"))\n> @@ -741,18 +746,16 @@ static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n> \tid++; /* advance past '/' to point at */\n> \tif (!*id)\n> \t\tgoto error;\n> -\tstrbuf_reset(inferred);\n> \tstrbuf_git_common_path(inferred, the_repository, \"worktrees/%s\", id);\n> \tif (!is_directory(inferred->buf))\n> \t\tgoto error;\n> \n> \tstrbuf_release(&actual);\n> -\treturn 1;\n> -\n> +\treturn inferred->len;\n> error:\n> \tstrbuf_release(&actual);\n> \tstrbuf_reset(inferred); /* clear invalid path */\n> -\treturn 0;\n> +\treturn -1;\n> }\n> \n> /*\n> diff --git a/worktree.h b/worktree.h\n> index e96118621638667d87c5d7e0452ed10bd1ddf606..37e65d508ed23d3e7a29850bb938285072a3aaa6 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -5,6 +5,8 @@\n> \n> struct strbuf;\n> \n> +extern int use_relative_paths;\n> +\n> struct worktree {\n> \t/* The repository this worktree belongs to. */\n> \tstruct repository *repo;\n> \n\n"},{"id":"506283","messageId":"4a316a5f-9360-4f24-843f-bcbb5b3378c4@gmail.com","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-2-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 2/5] worktree: add `write_worktree_linking_files` function","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-29T14:52:36Z","receivedAt":"2024-10-29T14:52:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Caleb\n\nOn 28/10/2024 19:09, Caleb White wrote:\n> A new helper function, `write_worktree_linking_files()`, centralizes\n> the logic for computing and writing either relative or absolute\n> paths, based on the provided configuration. This function accepts\n> `strbuf` pointers to both the worktree’s `.git` link and the\n> repository’s `gitdir`, and then writes the appropriate path to each.\n\nThat sounds like a useful change. I think it would be better to pass an \nextra parameter \"use_relative_paths\" rather than relying on a global \nvaribale in worktree.c. Thank you for adding some documentaion for the \nnew function.\n\n> This also teachs `git worktree repair` to fix the linking files if\n> there is an absolute/relative paths but the links are correct. E.g.,\n> `git worktree repair` can be used to convert a valid worktree between\n> absolute and relative paths.\n\nThis might be better as a separate step so that reviewers can \nconcentrate on the correctness of write_werktree_linking_files() when \nreviewing this patch.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Caleb White \n> ---\n> builtin/worktree.c | 11 +----\n> worktree.c | 118 +++++++++++++++++++++++++++--------------------------\n> worktree.h | 12 ++++++\n> 3 files changed, 74 insertions(+), 67 deletions(-)\n> \n> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> index c1130be5890c905c0b648782a834eb8dfcd79ba5..bb06830d6fe82aa97833c6e87f034115dfaa23bd 100644\n> --- a/builtin/worktree.c\n> +++ b/builtin/worktree.c\n> @@ -417,8 +417,7 @@ static int add_worktree(const char *path, const char *refname,\n> \t\t\tconst struct add_opts *opts)\n> {\n> \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n> -\tstruct strbuf sb = STRBUF_INIT, sb_tmp = STRBUF_INIT;\n> -\tstruct strbuf sb_path_realpath = STRBUF_INIT, sb_repo_realpath = STRBUF_INIT;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> \tconst char *name;\n> \tstruct strvec child_env = STRVEC_INIT;\n> \tunsigned int counter = 0;\n> @@ -494,10 +493,7 @@ static int add_worktree(const char *path, const char *refname,\n> \n> \tstrbuf_reset(&sb);\n> \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n> -\tstrbuf_realpath(&sb_path_realpath, path, 1);\n> -\tstrbuf_realpath(&sb_repo_realpath, sb_repo.buf, 1);\n> -\twrite_file(sb.buf, \"%s/.git\", relative_path(sb_path_realpath.buf, sb_repo_realpath.buf, &sb_tmp));\n> -\twrite_file(sb_git.buf, \"gitdir: %s\", relative_path(sb_repo_realpath.buf, sb_path_realpath.buf, &sb_tmp));\n> +\twrite_worktree_linking_files(sb_git, sb);\n> \tstrbuf_reset(&sb);\n> \tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n> \twrite_file(sb.buf, \"../..\");\n> @@ -581,12 +577,9 @@ static int add_worktree(const char *path, const char *refname,\n> \n> \tstrvec_clear(&child_env);\n> \tstrbuf_release(&sb);\n> -\tstrbuf_release(&sb_tmp);\n> \tstrbuf_release(&symref);\n> \tstrbuf_release(&sb_repo);\n> -\tstrbuf_release(&sb_repo_realpath);\n> \tstrbuf_release(&sb_git);\n> -\tstrbuf_release(&sb_path_realpath);\n> \tstrbuf_release(&sb_name);\n> \tfree_worktree(wt);\n> \treturn ret;\n> diff --git a/worktree.c b/worktree.c\n> index de5c5e53a5f2a758ddf470b5d6a9ad6c66247181..f4cee73d7a1edecafdff30b6d5e2d9dd1365b93e 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -381,29 +381,24 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n> void update_worktree_location(struct worktree *wt, const char *path_)\n> {\n> \tstruct strbuf path = STRBUF_INIT;\n> -\tstruct strbuf repo = STRBUF_INIT;\n> -\tstruct strbuf file = STRBUF_INIT;\n> -\tstruct strbuf tmp = STRBUF_INIT;\n> +\tstruct strbuf dotgit = STRBUF_INIT;\n> +\tstruct strbuf gitdir = STRBUF_INIT;\n> \n> \tif (is_main_worktree(wt))\n> \t\tBUG(\"can't relocate main worktree\");\n> \n> -\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n> +\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n> \tstrbuf_realpath(&path, path_, 1);\n> +\tstrbuf_addf(&dotgit, \"%s/.git\", path.buf);\n> \tif (fspathcmp(wt->path, path.buf)) {\n> -\t\tstrbuf_addf(&file, \"%s/gitdir\", repo.buf);\n> -\t\twrite_file(file.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n> -\t\tstrbuf_reset(&file);\n> -\t\tstrbuf_addf(&file, \"%s/.git\", path.buf);\n> -\t\twrite_file(file.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n> \n> \t\tfree(wt->path);\n> \t\twt->path = strbuf_detach(&path, NULL);\n> \t}\n> \tstrbuf_release(&path);\n> -\tstrbuf_release(&repo);\n> -\tstrbuf_release(&file);\n> -\tstrbuf_release(&tmp);\n> +\tstrbuf_release(&dotgit);\n> +\tstrbuf_release(&gitdir);\n> }\n> \n> int is_worktree_being_rebased(const struct worktree *wt,\n> @@ -582,9 +577,9 @@ static void repair_gitfile(struct worktree *wt,\n> \t\t\t worktree_repair_fn fn, void *cb_data)\n> {\n> \tstruct strbuf dotgit = STRBUF_INIT;\n> +\tstruct strbuf gitdir = STRBUF_INIT;\n> \tstruct strbuf repo = STRBUF_INIT;\n> \tstruct strbuf backlink = STRBUF_INIT;\n> -\tstruct strbuf tmp = STRBUF_INIT;\n> \tchar *dotgit_contents = NULL;\n> \tconst char *repair = NULL;\n> \tint err;\n> @@ -600,6 +595,7 @@ static void repair_gitfile(struct worktree *wt,\n> \n> \tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n> \tstrbuf_addf(&dotgit, \"%s/.git\", wt->path);\n> +\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n> \tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n> \n> \tif (dotgit_contents) {\n> @@ -617,18 +613,20 @@ static void repair_gitfile(struct worktree *wt,\n> \t\trepair = _(\".git file broken\");\n> \telse if (fspathcmp(backlink.buf, repo.buf))\n> \t\trepair = _(\".git file incorrect\");\n> +\telse if (use_relative_paths == is_absolute_path(dotgit_contents))\n> +\t\trepair = _(\".git file absolute/relative path mismatch\");\n> \n> \tif (repair) {\n> \t\tfn(0, wt->path, repair, cb_data);\n> -\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, wt->path, &tmp));\n> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n> \t}\n> \n> done:\n> \tfree(dotgit_contents);\n> \tstrbuf_release(&repo);\n> \tstrbuf_release(&dotgit);\n> +\tstrbuf_release(&gitdir);\n> \tstrbuf_release(&backlink);\n> -\tstrbuf_release(&tmp);\n> }\n> \n> static void repair_noop(int iserr UNUSED,\n> @@ -653,45 +651,30 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data)\n> \n> void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path)\n> {\n> -\tstruct strbuf path = STRBUF_INIT;\n> -\tstruct strbuf repo = STRBUF_INIT;\n> \tstruct strbuf gitdir = STRBUF_INIT;\n> \tstruct strbuf dotgit = STRBUF_INIT;\n> -\tstruct strbuf olddotgit = STRBUF_INIT;\n> -\tstruct strbuf tmp = STRBUF_INIT;\n> \n> \tif (is_main_worktree(wt))\n> \t\tgoto done;\n> \n> -\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n> -\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n> +\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n> \n> -\tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n> +\tif (strbuf_read_file(&dotgit, gitdir.buf, 0) < 0)\n> \t\tgoto done;\n> \n> -\tstrbuf_rtrim(&olddotgit);\n> -\tif (is_absolute_path(olddotgit.buf)) {\n> -\t\tstrbuf_addbuf(&dotgit, &olddotgit);\n> -\t} else {\n> -\t\tstrbuf_addf(&dotgit, \"%s/worktrees/%s/%s\", old_path, wt->id, olddotgit.buf);\n> +\tstrbuf_rtrim(&dotgit);\n> +\tif (!is_absolute_path(dotgit.buf)) {\n> +\t\tstrbuf_insertf(&dotgit, 0, \"%s/worktrees/%s/\", old_path, wt->id);\n> \t\tstrbuf_realpath_forgiving(&dotgit, dotgit.buf, 0);\n> \t}\n> \n> \tif (!file_exists(dotgit.buf))\n> \t\tgoto done;\n> \n> -\tstrbuf_addbuf(&path, &dotgit);\n> -\tstrbuf_strip_suffix(&path, \"/.git\");\n> -\n> -\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n> -\twrite_file(gitdir.buf, \"%s\", relative_path(dotgit.buf, repo.buf, &tmp));\n> +\twrite_worktree_linking_files(dotgit, gitdir);\n> done:\n> -\tstrbuf_release(&path);\n> -\tstrbuf_release(&repo);\n> \tstrbuf_release(&gitdir);\n> \tstrbuf_release(&dotgit);\n> -\tstrbuf_release(&olddotgit);\n> -\tstrbuf_release(&tmp);\n> }\n> \n> void repair_worktrees_after_gitdir_move(const char *old_path)\n> @@ -766,13 +749,10 @@ void repair_worktree_at_path(const char *path,\n> \t\t\t worktree_repair_fn fn, void *cb_data)\n> {\n> \tstruct strbuf dotgit = STRBUF_INIT;\n> -\tstruct strbuf realdotgit = STRBUF_INIT;\n> \tstruct strbuf backlink = STRBUF_INIT;\n> \tstruct strbuf inferred_backlink = STRBUF_INIT;\n> \tstruct strbuf gitdir = STRBUF_INIT;\n> \tstruct strbuf olddotgit = STRBUF_INIT;\n> -\tstruct strbuf realolddotgit = STRBUF_INIT;\n> -\tstruct strbuf tmp = STRBUF_INIT;\n> \tchar *dotgit_contents = NULL;\n> \tconst char *repair = NULL;\n> \tint err;\n> @@ -784,25 +764,25 @@ void repair_worktree_at_path(const char *path,\n> \t\tgoto done;\n> \n> \tstrbuf_addf(&dotgit, \"%s/.git\", path);\n> -\tif (!strbuf_realpath(&realdotgit, dotgit.buf, 0)) {\n> +\tif (!strbuf_realpath(&dotgit, dotgit.buf, 0)) {\n> \t\tfn(1, path, _(\"not a valid path\"), cb_data);\n> \t\tgoto done;\n> \t}\n> \n> -\tinfer_backlink(realdotgit.buf, &inferred_backlink);\n> +\tinfer_backlink(dotgit.buf, &inferred_backlink);\n> \tstrbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);\n> -\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err));\n> +\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n> \tif (dotgit_contents) {\n> \t\tif (is_absolute_path(dotgit_contents)) {\n> \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n> \t\t} else {\n> -\t\t\tstrbuf_addbuf(&backlink, &realdotgit);\n> +\t\t\tstrbuf_addbuf(&backlink, &dotgit);\n> \t\t\tstrbuf_strip_suffix(&backlink, \".git\");\n> \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n> \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n> \t\t}\n> \t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n> -\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n> +\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n> \t\tgoto done;\n> \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n> \t\tif (inferred_backlink.len) {\n> @@ -815,11 +795,11 @@ void repair_worktree_at_path(const char *path,\n> \t\t\t */\n> \t\t\tstrbuf_swap(&backlink, &inferred_backlink);\n> \t\t} else {\n> -\t\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n> +\t\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n> \t\t\tgoto done;\n> \t\t}\n> \t} else {\n> -\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n> +\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n> \t\tgoto done;\n> \t}\n> \n> @@ -841,39 +821,35 @@ void repair_worktree_at_path(const char *path,\n> \t * in the \"copy\" repository. In this case, point the \"copy\" worktree's\n> \t * .git file at the \"copy\" repository.\n> \t */\n> -\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf)) {\n> +\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))\n> \t\tstrbuf_swap(&backlink, &inferred_backlink);\n> -\t}\n> \n> \tstrbuf_addf(&gitdir, \"%s/gitdir\", backlink.buf);\n> \tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n> \t\trepair = _(\"gitdir unreadable\");\n> +\telse if (use_relative_paths == is_absolute_path(olddotgit.buf))\n> +\t\trepair = _(\"gitdir absolute/relative path mismatch\");\n> \telse {\n> \t\tstrbuf_rtrim(&olddotgit);\n> -\t\tif (is_absolute_path(olddotgit.buf)) {\n> -\t\t\tstrbuf_addbuf(&realolddotgit, &olddotgit);\n> -\t\t} else {\n> -\t\t\tstrbuf_addf(&realolddotgit, \"%s/%s\", backlink.buf, olddotgit.buf);\n> -\t\t\tstrbuf_realpath_forgiving(&realolddotgit, realolddotgit.buf, 0);\n> +\t\tif (!is_absolute_path(olddotgit.buf)) {\n> +\t\t\tstrbuf_insertf(&olddotgit, 0, \"%s/\", backlink.buf);\n> +\t\t\tstrbuf_realpath_forgiving(&olddotgit, olddotgit.buf, 0);\n> \t\t}\n> -\t\tif (fspathcmp(realolddotgit.buf, realdotgit.buf))\n> +\t\tif (fspathcmp(olddotgit.buf, dotgit.buf))\n> \t\t\trepair = _(\"gitdir incorrect\");\n> \t}\n> \n> \tif (repair) {\n> \t\tfn(0, gitdir.buf, repair, cb_data);\n> -\t\twrite_file(gitdir.buf, \"%s\", relative_path(realdotgit.buf, backlink.buf, &tmp));\n> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n> \t}\n> done:\n> \tfree(dotgit_contents);\n> \tstrbuf_release(&olddotgit);\n> -\tstrbuf_release(&realolddotgit);\n> \tstrbuf_release(&backlink);\n> \tstrbuf_release(&inferred_backlink);\n> \tstrbuf_release(&gitdir);\n> -\tstrbuf_release(&realdotgit);\n> \tstrbuf_release(&dotgit);\n> -\tstrbuf_release(&tmp);\n> }\n> \n> int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, timestamp_t expire)\n> @@ -1034,3 +1010,29 @@ int init_worktree_config(struct repository *r)\n> \tfree(main_worktree_file);\n> \treturn res;\n> }\n> +\n> +void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir)\n> +{\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tstruct strbuf repo = STRBUF_INIT;\n> +\tstruct strbuf tmp = STRBUF_INIT;\n> +\n> +\tstrbuf_addbuf(&path, &dotgit);\n> +\tstrbuf_strip_suffix(&path, \"/.git\");\n> +\tstrbuf_realpath(&path, path.buf, 1);\n> +\tstrbuf_addbuf(&repo, &gitdir);\n> +\tstrbuf_strip_suffix(&repo, \"/gitdir\");\n> +\tstrbuf_realpath(&repo, repo.buf, 1);\n> +\n> +\tif (use_relative_paths) {\n> +\t\twrite_file(gitdir.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n> +\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n> +\t} else {\n> +\t\twrite_file(gitdir.buf, \"%s/.git\", path.buf);\n> +\t\twrite_file(dotgit.buf, \"gitdir: %s\", repo.buf);\n> +\t}\n> +\n> +\tstrbuf_release(&path);\n> +\tstrbuf_release(&repo);\n> +\tstrbuf_release(&tmp);\n> +}\n> diff --git a/worktree.h b/worktree.h\n> index 37e65d508ed23d3e7a29850bb938285072a3aaa6..5929089891c97318a8f5329f7938264c717050d5 100644\n> --- a/worktree.h\n> +++ b/worktree.h\n> @@ -217,4 +217,16 @@ void strbuf_worktree_ref(const struct worktree *wt,\n> */\n> int init_worktree_config(struct repository *r);\n> \n> +/**\n> + * Write the .git file and gitdir file that links the worktree to the repository.\n> + *\n> + * The `dotgit` parameter is the path to the worktree's .git file, and `gitdir`\n> + * is the path to the repository's `gitdir` file.\n> + *\n> + * Example\n> + * dotgit: \"/path/to/foo/.git\"\n> + * gitdir: \"/path/to/repo/worktrees/foo/gitdir\"\n> + */\n> +void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir);\n> +\n> #endif\n> \n\n"},{"id":"506284","messageId":"985646c7-8a16-4014-a379-644408157934@gmail.com","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-3-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-29T14:52:42Z","receivedAt":"2024-10-29T14:52:46Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Caleb\n\nOn 28/10/2024 19:09, Caleb White wrote:\n> This patch expands the test coverage by adding cases that specifically\n> handle relative paths. These tests verify correct behavior in a variety\n> of operations, including: adding, listing, pruning, moving, and\n> repairing worktrees with relative paths configured.\n> \n> This also adds a test case for reinitializing a repository that has\n> relative worktrees.\n\nIt's nice to see new tests being added. If they were added with the code \nchanges they test that would help reader understand the changes being \nmade I think.\n\nBest Wishes\n\nPhillip\n\n> Signed-off-by: Caleb White \n> ---\n> t/t0001-init.sh | 17 +++++++++++++----\n> t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n> t/t2401-worktree-prune.sh | 3 ++-\n> t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++\n> t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++\n> t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++\n> 6 files changed, 126 insertions(+), 5 deletions(-)\n> \n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -435,6 +435,7 @@ sep_git_dir_worktree () {\n> \ttest_when_finished \"rm -rf mainwt linkwt seprepo\" &&\n> \tgit init mainwt &&\n> \ttest_commit -C mainwt gumby &&\n> +\tgit -C mainwt config worktree.useRelativePaths \"$([ \"$2\" = \"relative\" ] && echo true || echo false)\" &&\n> \tgit -C mainwt worktree add --detach ../linkwt &&\n> \tgit -C \"$1\" init --separate-git-dir ../seprepo &&\n> \tgit -C mainwt rev-parse --git-common-dir >expect &&\n> @@ -442,12 +443,20 @@ sep_git_dir_worktree () {\n> \ttest_cmp expect actual\n> }\n> \n> -test_expect_success 're-init to move gitdir with linked worktrees' '\n> -\tsep_git_dir_worktree mainwt\n> +test_expect_success 're-init to move gitdir with linked worktrees (absolute)' '\n> +\tsep_git_dir_worktree mainwt absolute\n> '\n> \n> -test_expect_success 're-init to move gitdir within linked worktree' '\n> -\tsep_git_dir_worktree linkwt\n> +test_expect_success 're-init to move gitdir within linked worktree (absolute)' '\n> +\tsep_git_dir_worktree linkwt absolute\n> +'\n> +\n> +test_expect_success 're-init to move gitdir with linked worktrees (relative)' '\n> +\tsep_git_dir_worktree mainwt relative\n> +'\n> +\n> +test_expect_success 're-init to move gitdir within linked worktree (relative)' '\n> +\tsep_git_dir_worktree linkwt relative\n> '\n> \n> test_expect_success MINGW '.git hidden' '\n> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh\n> index cfc4aeb1798c6d023909cec771e5b74e983af5ea..630c13230b3cc762ce8d943e22be8891aa2b1871 100755\n> --- a/t/t2400-worktree-add.sh\n> +++ b/t/t2400-worktree-add.sh\n> @@ -1207,4 +1207,45 @@ test_expect_success '\"add\" with initialized submodule, with submodule.recurse se\n> \tgit -C project-clone -c submodule.recurse worktree add ../project-5\n> '\n> \n> +test_expect_success 'can create worktrees with relative paths' '\n> +\ttest_when_finished \"git worktree remove relative\" &&\n> +\tgit config worktree.useRelativePaths false &&\n> +\tgit worktree add --relative-paths ./relative &&\n> +\tcat relative/.git >actual &&\n> +\techo \"gitdir: ../.git/worktrees/relative\" >expect &&\n> +\ttest_cmp expect actual &&\n> +\tcat .git/worktrees/relative/gitdir >actual &&\n> +\techo \"../../../relative/.git\" >expect &&\n> +\ttest_cmp expect actual\n> +\n> +'\n> +\n> +test_expect_success 'can create worktrees with absolute paths' '\n> +\tgit config worktree.useRelativePaths true &&\n> +\tgit worktree add ./relative &&\n> +\tcat relative/.git >actual &&\n> +\techo \"gitdir: ../.git/worktrees/relative\" >expect &&\n> +\ttest_cmp expect actual &&\n> +\tgit worktree add --no-relative-paths ./absolute &&\n> +\tcat absolute/.git >actual &&\n> +\techo \"gitdir: $(pwd)/.git/worktrees/absolute\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'move repo without breaking relative internal links' '\n> +\ttest_when_finished rm -rf repo moved &&\n> +\tgit init repo &&\n> +\t(\n> +\t\tcd repo &&\n> +\t\tgit config worktree.useRelativePaths true &&\n> +\t\ttest_commit initial &&\n> +\t\tgit worktree add wt1 &&\n> +\t\tcd .. &&\n> +\t\tmv repo moved &&\n> +\t\tcd moved/wt1 &&\n> +\t\tgit status >out 2>err &&\n> +\t\ttest_must_be_empty err\n> +\t)\n> +'\n> +\n> test_done\n> diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh\n> index 976d048e3efc74be9cd909ce76d552b3944d2e10..5eb52b9abbf29514dc082c260ebb7a5e8e63aae0 100755\n> --- a/t/t2401-worktree-prune.sh\n> +++ b/t/t2401-worktree-prune.sh\n> @@ -120,11 +120,12 @@ test_expect_success 'prune duplicate (main/linked)' '\n> \t! test -d .git/worktrees/wt\n> '\n> \n> -test_expect_success 'not prune proper worktrees when run inside linked worktree' '\n> +test_expect_success 'not prune proper worktrees inside linked worktree with relative paths' '\n> \ttest_when_finished rm -rf repo wt_ext &&\n> \tgit init repo &&\n> \t(\n> \t cd repo &&\n> +\t git config worktree.useRelativePaths true &&\n> \t echo content >file &&\n> \t git add file &&\n> \t git commit -m msg &&\n> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh\n> index 33ea9cb21ba07c9563530b54da06753eaa993fe2..780daa6cd6351f8fa9434619cc212aade8f01420 100755\n> --- a/t/t2402-worktree-list.sh\n> +++ b/t/t2402-worktree-list.sh\n> @@ -261,6 +261,7 @@ test_expect_success 'broken main worktree still at the top' '\n> '\n> \n> test_expect_success 'linked worktrees are sorted' '\n> +\ttest_when_finished \"rm -rf sorted\" &&\n> \tmkdir sorted &&\n> \tgit init sorted/main &&\n> \t(\n> @@ -280,6 +281,27 @@ test_expect_success 'linked worktrees are sorted' '\n> \ttest_cmp expected sorted/main/actual\n> '\n> \n> +test_expect_success 'linked worktrees with relative paths are shown with absolute paths' '\n> +\ttest_when_finished \"rm -rf sorted\" &&\n> +\tmkdir sorted &&\n> +\tgit init sorted/main &&\n> +\t(\n> +\t\tcd sorted/main &&\n> +\t\ttest_tick &&\n> +\t\ttest_commit new &&\n> +\t\tgit worktree add --relative-paths ../first &&\n> +\t\tgit worktree add ../second &&\n> +\t\tgit worktree list --porcelain >out &&\n> +\t\tgrep ^worktree out >actual\n> +\t) &&\n> +\tcat >expected <<-EOF &&\n> +\tworktree $(pwd)/sorted/main\n> +\tworktree $(pwd)/sorted/first\n> +\tworktree $(pwd)/sorted/second\n> +\tEOF\n> +\ttest_cmp expected sorted/main/actual\n> +'\n> +\n> test_expect_success 'worktree path when called in .git directory' '\n> \tgit worktree list >list1 &&\n> \tgit -C .git worktree list >list2 &&\n> diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh\n> index 901342ea09b51a8e832f1109fbb737df84283aa2..6ce9ed3f1e6b3f73d2a290e770233eec30221fe5 100755\n> --- a/t/t2403-worktree-move.sh\n> +++ b/t/t2403-worktree-move.sh\n> @@ -247,4 +247,26 @@ test_expect_success 'not remove a repo with initialized submodule' '\n> \t)\n> '\n> \n> +test_expect_success 'move worktree with absolute path to relative path' '\n> +\tgit config worktree.useRelativePaths false &&\n> +\tgit worktree add ./absolute &&\n> +\tgit worktree move --relative-paths absolute relative &&\n> +\tcat relative/.git >actual &&\n> +\techo \"gitdir: ../.git/worktrees/absolute\" >expect &&\n> +\ttest_cmp expect actual &&\n> +\tgit config worktree.useRelativePaths true &&\n> +\tgit worktree move relative relative2 &&\n> +\tcat relative2/.git >actual &&\n> +\techo \"gitdir: ../.git/worktrees/absolute\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'move worktree with relative path to absolute path' '\n> +\tgit config worktree.useRelativePaths true &&\n> +\tgit worktree move --no-relative-paths relative2 absolute &&\n> +\tcat absolute/.git >actual &&\n> +\techo \"gitdir: $(pwd)/.git/worktrees/absolute\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n> diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh\n> index 7686e60f6ad186519b275f11a5e14064c905b207..84451e903b2ef3c645c0311faf055c846588baf6 100755\n> --- a/t/t2406-worktree-repair.sh\n> +++ b/t/t2406-worktree-repair.sh\n> @@ -216,4 +216,30 @@ test_expect_success 'repair copied main and linked worktrees' '\n> \ttest_cmp dup/linked.expect dup/linked/.git\n> '\n> \n> +test_expect_success 'repair absolute worktree to use relative paths' '\n> +\ttest_when_finished \"rm -rf main side sidemoved\" &&\n> +\ttest_create_repo main &&\n> +\ttest_commit -C main init &&\n> +\tgit -C main worktree add --detach ../side &&\n> +\techo \"../../../../sidemoved/.git\" >expect-gitdir &&\n> +\techo \"gitdir: ../main/.git/worktrees/side\" >expect-gitfile &&\n> +\tmv side sidemoved &&\n> +\tgit -C main worktree repair --relative-paths ../sidemoved &&\n> +\ttest_cmp expect-gitdir main/.git/worktrees/side/gitdir &&\n> +\ttest_cmp expect-gitfile sidemoved/.git\n> +'\n> +\n> +test_expect_success 'repair relative worktree to use absolute paths' '\n> +\ttest_when_finished \"rm -rf main side sidemoved\" &&\n> +\ttest_create_repo main &&\n> +\ttest_commit -C main init &&\n> +\tgit -C main worktree add --relative-paths --detach ../side &&\n> +\techo \"$(pwd)/sidemoved/.git\" >expect-gitdir &&\n> +\techo \"gitdir: $(pwd)/main/.git/worktrees/side\" >expect-gitfile &&\n> +\tmv side sidemoved &&\n> +\tgit -C main worktree repair ../sidemoved &&\n> +\ttest_cmp expect-gitdir main/.git/worktrees/side/gitdir &&\n> +\ttest_cmp expect-gitfile sidemoved/.git\n> +'\n> +\n> test_done\n> \n\n"},{"id":"506285","messageId":"2ce3998f-ea16-4fd4-9756-c5849d834b1e@gmail.com","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-0-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 0/5] Allow relative worktree linking to be configured by the user","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-29T14:55:38Z","receivedAt":"2024-10-29T14:55:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Caleb\n\nThanks for working on this. I've left so high-level comments on the \nfirst three patches. As I said in my comments on patch 1 I think it \nwould be helpful to re-order the changes so that the extension is \nimplementation first and then adjust the implementation to support both \nrelative and absolute paths before adding \"--no-relative-paths\" to \"git \nworktree add/move/repair\". I'd also like to avoid adding a new global \nvariable\n\nBest Wishes\n\nPhillip\n\nOn 28/10/2024 19:09, Caleb White wrote:\n> This patch introduces the `--[no-]relative-paths` CLI option for\n> `git worktree {add, move, repair}` commands, as well as the\n> `worktree.useRelativePaths` configuration setting. When enabled,\n> these options allow worktrees to be linked using relative paths,\n> enhancing portability across environments where absolute paths\n> may differ (e.g., containerized setups, shared network drives).\n> Git still creates absolute paths by default, but these options allow\n> users to opt-in to relative paths if desired.\n> \n> Using the `--relative-paths` option with `worktree {move, repair}`\n> will convert absolute paths to relative ones, while `--no-relative-paths`\n> does the reverse. For cases where users want consistency in path handling,\n> the config option `worktree.useRelativePaths` provides a persistent setting.\n> \n> A new extension, `relativeWorktrees`, is added to indicate that at least\n> one worktree in the repository has been linked with relative paths. This\n> extension is automatically set when a worktree is created or repaired\n> using the `--relative-paths` option, or when the\n> `worktree.useRelativePaths` config is set to `true`.\n> \n> The `relativeWorktrees` extension ensures older Git versions do not\n> attempt to automatically prune worktrees with relative paths, as they\n> would not not recognize the paths as being valid.\n> \n> Signed-off-by: Caleb White \n> ---\n> The base for this patch series is obtained by applying the following\n> patch onto 6a11438f43:\n> - cw/config-extensions topic (doc: consolidate extensions in git-config\n> documentation, 2024-10-22, <20241021-cleanup-extension-docs-v1-1-ab02cece3132@pm.me>)\n> \n> Link to original patch series:\n> https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@pm.me\n> \n> ---\n> Changes in v2:\n> - Fixed a bug where repositories with valid extensions would be downgraded\n> to v0 during reinitialization, causing future operations to fail.\n> - Split patch [1/2] into three separate patches.\n> - Updated cover letter and commit messages.\n> - Updated documentation wording.\n> - Link to v1: https://lore.kernel.org/r/20241025-wt_relative_options-v1-0-c3005df76bf9@pm.me\n> \n> ---\n> Caleb White (5):\n> worktree: add CLI/config options for relative path linking\n> worktree: add `write_worktree_linking_files` function\n> worktree: add tests for worktrees with relative paths\n> setup: correctly reinitialize repository version\n> worktree: add `relativeWorktrees` extension\n> \n> Documentation/config/extensions.txt | 6 ++\n> Documentation/config/worktree.txt | 5 ++\n> Documentation/git-worktree.txt | 12 +++\n> builtin/worktree.c | 20 ++---\n> repository.c | 1 +\n> repository.h | 1 +\n> setup.c | 39 +++++++---\n> setup.h | 1 +\n> t/t0001-init.sh | 17 ++++-\n> t/t2400-worktree-add.sh | 54 ++++++++++++++\n> t/t2401-worktree-prune.sh | 3 +-\n> t/t2402-worktree-list.sh | 22 ++++++\n> t/t2403-worktree-move.sh | 22 ++++++\n> t/t2406-worktree-repair.sh | 26 +++++++\n> t/t2408-worktree-relative.sh | 39 ----------\n> worktree.c | 143 ++++++++++++++++++++----------------\n> worktree.h | 14 ++++\n> 17 files changed, 298 insertions(+), 127 deletions(-)\n> ---\n> base-commit: 6a11438f43469f3815f2f0fc997bd45792ff04c0\n> change-id: 20241025-wt_relative_options-afa41987bc32\n> prerequisite-change-id: 20241020-cleanup-extension-docs-f365868711bf:v1\n> prerequisite-patch-id: 60a443b24e92938b9b6f4a016a7bab87e13bf3ea\n> \n> Best regards,\n\n"},{"id":"506286","messageId":"D58DMTM9YPBE.8L5WT8ZNG1CU@pm.me","threadId":"62421","inReplyTo":"985646c7-8a16-4014-a379-644408157934@gmail.com","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-29T14:58:09Z","receivedAt":"2024-10-29T14:58:15Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote:\n> Hi Caleb\n>\n> On 28/10/2024 19:09, Caleb White wrote:\n>> This patch expands the test coverage by adding cases that specifically\n>> handle relative paths. These tests verify correct behavior in a variety\n>> of operations, including: adding, listing, pruning, moving, and\n>> repairing worktrees with relative paths configured.\n>>\n>> This also adds a test case for reinitializing a repository that has\n>> relative worktrees.\n>\n> It's nice to see new tests being added. If they were added with the code\n> changes they test that would help reader understand the changes being\n> made I think.\n\nI had received feedback that the original patch was too large, so I\nwas trying to split it up into smaller, more digestible pieces. I could \ngo either way so it's really what the reviewers would prefer.\n\nBest,\nCaleb\n\n"},{"id":"506287","messageId":"aeec08e2-29da-4e5e-8a73-7ed7e3a79177@gmail.com","threadId":"62421","inReplyTo":"D58DMTM9YPBE.8L5WT8ZNG1CU@pm.me","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-29T15:43:17Z","receivedAt":"2024-10-29T15:43:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 29/10/2024 14:58, Caleb White wrote:\n> On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote:\n>> Hi Caleb\n>>\n>> On 28/10/2024 19:09, Caleb White wrote:\n>>> This patch expands the test coverage by adding cases that specifically\n>>> handle relative paths. These tests verify correct behavior in a variety\n>>> of operations, including: adding, listing, pruning, moving, and\n>>> repairing worktrees with relative paths configured.\n>>>\n>>> This also adds a test case for reinitializing a repository that has\n>>> relative worktrees.\n>>\n>> It's nice to see new tests being added. If they were added with the code\n>> changes they test that would help reader understand the changes being\n>> made I think.\n> \n> I had received feedback that the original patch was too large, so I\n> was trying to split it up into smaller, more digestible pieces. \n\nOne way to do that would be to convert the \"add\", \"move\" and \"repair\" \nsubcommands in separate patches changing option handling and appropriate \ntests in each one rather than changing all subcommands and their tests \nat once.\n\nBest Wishes\n\nPhillip\n\nI could\n> go either way so it's really what the reviewers would prefer.\n> \n> Best,\n> Caleb\n> \n\n"},{"id":"506297","messageId":"ZyEsfpcnAGCg+W6N@nand.local","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-1-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-29T18:42:06Z","receivedAt":"2024-10-29T18:42:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 28, 2024 at 07:09:37PM +0000, Caleb White wrote:\n> This patch introduces the `--[no-]relative-paths` CLI option for\n> `git worktree {add, move, repair}` commands, as well as the\n> `worktree.useRelativePaths` configuration setting. When enabled,\n> these options allow worktrees to be linked using relative paths,\n> enhancing portability across environments where absolute paths\n> may differ (e.g., containerized setups, shared network drives).\n> Git still creates absolute paths by default, but these options allow\n> users to opt-in to relative paths if desired.\n>\n> Using the `--relative-paths` option with `worktree {move, repair}`\n> will convert absolute paths to relative ones, while `--no-relative-paths`\n> does the reverse. For cases where users want consistency in path\n> handling, the config option `worktree.useRelativePaths` provides\n> a persistent setting.\n\nThis is great. This addresses the main concerns that you and I discussed\nin the earlier round of this series, which was making sure that the new\nbehavior be opt-in, as it breaks backwards compatibility and thus\nrequires a new extension to quarantine older Git versions from touching\nrepositories that list their worktrees out with relative paths.\n\nThis approach makes the most sense to me because it doesn't impose such\na breakage between Git versions when the user doesn't explicitly opt-in\nto the new behavior, which is the right approach to take here IMO.\n\n> In response to reviewer feedback from the initial patch series[1],\n> this revision includes slight refactoring for improved\n> maintainability and clarity in the code base.\n\nGreat :-).\n\n> diff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\n> index 048e349482df6c892055720eb53cdcd6c327b6ed..44b783c2774dc5ff65e3fa232b0c25cd5254876b 100644\n> --- a/Documentation/config/worktree.txt\n> +++ b/Documentation/config/worktree.txt\n> @@ -7,3 +7,8 @@ worktree.guessRemote::\n> \tsuch a branch exists, it is checked out and set as \"upstream\"\n> \tfor the new branch. If no such match can be found, it falls\n> \tback to creating a new branch from the current HEAD.\n\nI would have thought there would be a blank line in between this and the\nsection on worktree.guessRemote. ASCIIDoc doesn't require it because\nthis is a labeled list, but it does improve the readability of the raw\nASCIIDoc itself.\n\nSo not a big deal, but if you end up sending out another version of this\nseries it would be nice to include.\n\n> +worktree.useRelativePaths::\n> +\tIf set to `true`, worktrees will be linked to the repository using\n> +\trelative paths rather than using absolute paths. This is particularly\n> +\tuseful for setups where the repository and worktrees may be moved between\n> +\tdifferent locations or environments.\n\nThis is a good start, but I have a few suggestions on top that I'm\ncurious of your thoughts on. First: what is the default? Users\nshould have some insight into what the default is. Likewise, they should\nknow that that the default behavior does not introduce the repository\nextension, but that setting this configuration to 'true' does.\n\nMaybe something like the following on top?\n\n--- 8< ---\ndiff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\nindex 44b783c277..666cb3c190 100644\n--- a/Documentation/config/worktree.txt\n+++ b/Documentation/config/worktree.txt\n@@ -7,8 +7,13 @@ worktree.guessRemote::\n \tsuch a branch exists, it is checked out and set as \"upstream\"\n \tfor the new branch. If no such match can be found, it falls\n \tback to creating a new branch from the current HEAD.\n+\n worktree.useRelativePaths::\n-\tIf set to `true`, worktrees will be linked to the repository using\n-\trelative paths rather than using absolute paths. This is particularly\n-\tuseful for setups where the repository and worktrees may be moved between\n-\tdifferent locations or environments.\n+\tLink worktrees using relative paths (when \"true\") or absolute\n+\tpaths (when \"false\"). This is particularly useful for setups\n+\twhere the repository and worktrees may be moved between\n+\tdifferent locations or environments. Defaults to \"false\".\n++\n+Note that setting `worktree.useRelativePaths` to \"true\" implies\n+enabling the \"relativeWorktrees\" repository extension, thus making it\n+incompatible with older versions of Git.\n--- >8 ---\n\n> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n> index 70437c815f13852bd2eb862176b8b933e6de0acf..975dc3c46d480480457ec4857988a6b8bc67b647 100644\n> --- a/Documentation/git-worktree.txt\n> +++ b/Documentation/git-worktree.txt\n> @@ -216,6 +216,18 @@ To remove a locked worktree, specify `--force` twice.\n> This can also be set up as the default behaviour by using the\n> `worktree.guessRemote` config option.\n>\n> +--[no-]relative-paths::\n> +\tWorktrees will be linked to the repository using relative paths\n> +\trather than using absolute paths. This is particularly useful for setups\n> +\twhere the repository and worktrees may be moved between different\n> +\tlocations or environments.\n\nThis paragraph is redundant with what you wrote in git-config(1). I\nthink all we want to say is that it overrides the setting of that\nconfiguration variable, and refer users there with linkgit.\n\n> ++\n> +With `repair`, the linking files will be updated if there's an absolute/relative\n> +mismatch, even if the links are correct.\n\nThis is worth keeping.\n\n> +This can also be set up as the default behaviour by using the\n> +`worktree.useRelativePaths` config option.\n> +\n\nThis should get folded into my suggestion above.\n\n> diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh\n> deleted file mode 100755\n> index a3136db7e28cb20926ff44211e246ce625a6e51a..0000000000000000000000000000000000000000\n> --- a/t/t2408-worktree-relative.sh\n> +++ /dev/null\n> @@ -1,39 +0,0 @@\n\nWas removing t2408 intentional here? I don't see the tests being\nre-added elsewhere in this patch (though they may be introduced\nelsewhere later in the series, I haven't read that far yet). Either way,\nit may be worth mentioning in the commit message to avoid confusing\nreaders.\n\n> diff --git a/worktree.c b/worktree.c\n> index 77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7..de5c5e53a5f2a758ddf470b5d6a9ad6c66247181 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -14,6 +14,8 @@\n> #include \"wt-status.h\"\n> #include \"config.h\"\n>\n> +int use_relative_paths = 0;\n\nI wondered whether 'use_relative_paths' should be static, or if we need to extern it in\nfrom somewhere else in the tree. But we do, from worktree.[ch], which\nseems reasonable. It would be nice if there was some way to thread that\ninto the worktree.h API, but I think that this is a reasonable measure\nto take for now.\n\n> +\n> void free_worktree(struct worktree *worktree)\n> {\n> \tif (!worktree)\n> @@ -111,9 +113,9 @@ struct worktree *get_linked_worktree(const char *id,\n> \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n>\n> \tif (!is_absolute_path(worktree_path.buf)) {\n> -\t strbuf_strip_suffix(&path, \"gitdir\");\n> -\t strbuf_addbuf(&path, &worktree_path);\n> -\t strbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n> +\t\tstrbuf_strip_suffix(&path, \"gitdir\");\n> +\t\tstrbuf_addbuf(&path, &worktree_path);\n> +\t\tstrbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n\nWhitespace change?\n\n> \t}\n>\n> \tCALLOC_ARRAY(worktree, 1);\n> @@ -725,12 +727,15 @@ static int is_main_worktree_path(const char *path)\n> * won't know which /worktrees//gitdir to repair. However, we may\n> * be able to infer the gitdir by manually reading /path/to/worktree/.git,\n> * extracting the , and checking if /worktrees/ exists.\n> + *\n> + * Returns -1 on failure and strbuf.len on success.\n> */\n> static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n\nShould this return an ssize_t instead, then? I don't think we're going\nto have worktree paths that are actually larger than INT_MAX, but it\nseems hygienic and good to prevent any accidental overflow issues.\n\nThanks,\nTaylor\n"},{"id":"506309","messageId":"ZyFn9xErajxzQo29@nand.local","threadId":"62421","inReplyTo":"4a316a5f-9360-4f24-843f-bcbb5b3378c4@gmail.com","subject":"Re: [PATCH v2 2/5] worktree: add `write_worktree_linking_files` function","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-29T22:55:51Z","receivedAt":"2024-10-29T22:55:58Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Oct 29, 2024 at 02:52:36PM +0000, Phillip Wood wrote:\n> Hi Caleb\n>\n> On 28/10/2024 19:09, Caleb White wrote:\n> > A new helper function, `write_worktree_linking_files()`, centralizes\n> > the logic for computing and writing either relative or absolute\n> > paths, based on the provided configuration. This function accepts\n> > `strbuf` pointers to both the worktree’s `.git` link and the\n> > repository’s `gitdir`, and then writes the appropriate path to each.\n>\n> That sounds like a useful change. I think it would be better to pass an\n> extra parameter \"use_relative_paths\" rather than relying on a global\n> varibale in worktree.c. Thank you for adding some documentaion for the new\n> function.\n\nGood suggestion. I definitely agree that this is a useful direction.\n\n> > This also teachs `git worktree repair` to fix the linking files if\n> > there is an absolute/relative paths but the links are correct. E.g.,\n> > `git worktree repair` can be used to convert a valid worktree between\n> > absolute and relative paths.\n>\n> This might be better as a separate step so that reviewers can concentrate on\n> the correctness of write_werktree_linking_files() when reviewing this patch.\n\nIndeed. This patch (even though the diffstat isn't overly large) is\nsomewhat noisy just because of the number of spots that needed to be\nadjusted here.\n\nI wonder if another way to split this up (in addition to what you wrote\nabove) might be to introduce the new function and convert one single\ncaller in the first patch. Then subsequent patches can go one callsite\nat a time and convert them to use the new function.\n\nThat way, each patch is easy-ish to verify in isolation. I know that\nresults in some more patches, but I think that the additional clarity I\nimagine we'll get is worth doing so.\n\nThanks,\nTaylor\n"},{"id":"506310","messageId":"ZyFo+e5Vj2TpXNyo@nand.local","threadId":"62421","inReplyTo":"20241028-wt_relative_options-v2-3-33a5021bd7bb@pm.me","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-29T23:00:09Z","receivedAt":"2024-10-29T23:00:12Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 28, 2024 at 07:09:52PM +0000, Caleb White wrote:\n> This patch expands the test coverage by adding cases that specifically\n> handle relative paths. These tests verify correct behavior in a variety\n> of operations, including: adding, listing, pruning, moving, and\n> repairing worktrees with relative paths configured.\n>\n> This also adds a test case for reinitializing a repository that has\n> relative worktrees.\n>\n> Signed-off-by: Caleb White \n> ---\n> t/t0001-init.sh | 17 +++++++++++++----\n> t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++\n> t/t2401-worktree-prune.sh | 3 ++-\n> t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++\n> t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++\n> t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++\n> 6 files changed, 126 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n> index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755\n> --- a/t/t0001-init.sh\n> +++ b/t/t0001-init.sh\n> @@ -435,6 +435,7 @@ sep_git_dir_worktree () {\n> \ttest_when_finished \"rm -rf mainwt linkwt seprepo\" &&\n> \tgit init mainwt &&\n> \ttest_commit -C mainwt gumby &&\n> +\tgit -C mainwt config worktree.useRelativePaths \"$([ \"$2\" = \"relative\" ] && echo true || echo false)\" &&\n\nCan we avoid using '[' and perhaps split this out a little further.\nPerhaps:\n\n if test \"relative\" = $2\n then\n git -C mainwt config worktree.useRelativePaths true\n else\n git -C mainwt config worktree.useRelativePaths false\n fi\n\nI think the duplication is more than worth the clarity here.\n\nThanks,\nTaylor\n"},{"id":"506311","messageId":"D58UM62S8YX0.17RAKH4H6XJSR@pm.me","threadId":"62421","inReplyTo":"ZyFo+e5Vj2TpXNyo@nand.local","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T04:16:36Z","receivedAt":"2024-10-30T04:16:44Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 6:00 PM CDT, Taylor Blau wrote:\n>> diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n>> index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755\n>> --- a/t/t0001-init.sh\n>> +++ b/t/t0001-init.sh\n>> @@ -435,6 +435,7 @@ sep_git_dir_worktree () {\n>> \ttest_when_finished \"rm -rf mainwt linkwt seprepo\" &&\n>> \tgit init mainwt &&\n>> \ttest_commit -C mainwt gumby &&\n>> +\tgit -C mainwt config worktree.useRelativePaths \"$([ \"$2\" = \"relative\" ] && echo true || echo false)\" &&\n>\n> Can we avoid using '[' and perhaps split this out a little further.\n> Perhaps:\n>\n> if test \"relative\" = $2\n> then\n> git -C mainwt config worktree.useRelativePaths true\n> else\n> git -C mainwt config worktree.useRelativePaths false\n> fi\n>\n> I think the duplication is more than worth the clarity here.\n\nSounds good, I'll make that change.\n\nBest,\n\n"},{"id":"506313","messageId":"D58VOSCN68T0.3EZBA39222AUC@pm.me","threadId":"62421","inReplyTo":"ZyEsfpcnAGCg+W6N@nand.local","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:07:01Z","receivedAt":"2024-10-30T05:07:07Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 1:42 PM CDT, Taylor Blau wrote:\n> On Mon, Oct 28, 2024 at 07:09:37PM +0000, Caleb White wrote:\n>> diff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\n>> index 048e349482df6c892055720eb53cdcd6c327b6ed..44b783c2774dc5ff65e3fa232b0c25cd5254876b 100644\n>> --- a/Documentation/config/worktree.txt\n>> +++ b/Documentation/config/worktree.txt\n>> @@ -7,3 +7,8 @@ worktree.guessRemote::\n>> \tsuch a branch exists, it is checked out and set as \"upstream\"\n>> \tfor the new branch. If no such match can be found, it falls\n>> \tback to creating a new branch from the current HEAD.\n>\n> I would have thought there would be a blank line in between this and the\n> section on worktree.guessRemote. ASCIIDoc doesn't require it because\n> this is a labeled list, but it does improve the readability of the raw\n> ASCIIDoc itself.\n>\n> So not a big deal, but if you end up sending out another version of this\n> series it would be nice to include.\n\nI'll add the blank line in the next version of the patch.\n\n>> +worktree.useRelativePaths::\n>> +\tIf set to `true`, worktrees will be linked to the repository using\n>> +\trelative paths rather than using absolute paths. This is particularly\n>> +\tuseful for setups where the repository and worktrees may be moved between\n>> +\tdifferent locations or environments.\n>\n> This is a good start, but I have a few suggestions on top that I'm\n> curious of your thoughts on. First: what is the default? Users\n> should have some insight into what the default is. Likewise, they should\n> know that that the default behavior does not introduce the repository\n> extension, but that setting this configuration to 'true' does.\n>\n> Maybe something like the following on top?\n>\n> --- 8< ---\n> diff --git a/Documentation/config/worktree.txt b/Documentation/config/worktree.txt\n> index 44b783c277..666cb3c190 100644\n> --- a/Documentation/config/worktree.txt\n> +++ b/Documentation/config/worktree.txt\n> @@ -7,8 +7,13 @@ worktree.guessRemote::\n> \tsuch a branch exists, it is checked out and set as \"upstream\"\n> \tfor the new branch. If no such match can be found, it falls\n> \tback to creating a new branch from the current HEAD.\n> +\n> worktree.useRelativePaths::\n> -\tIf set to `true`, worktrees will be linked to the repository using\n> -\trelative paths rather than using absolute paths. This is particularly\n> -\tuseful for setups where the repository and worktrees may be moved between\n> -\tdifferent locations or environments.\n> +\tLink worktrees using relative paths (when \"true\") or absolute\n> +\tpaths (when \"false\"). This is particularly useful for setups\n> +\twhere the repository and worktrees may be moved between\n> +\tdifferent locations or environments. Defaults to \"false\".\n> ++\n> +Note that setting `worktree.useRelativePaths` to \"true\" implies\n> +enabling the \"relativeWorktrees\" repository extension, thus making it\n> +incompatible with older versions of Git.\n\nSounds good to me. I'll update.\n\n>> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt\n>> index 70437c815f13852bd2eb862176b8b933e6de0acf..975dc3c46d480480457ec4857988a6b8bc67b647 100644\n>> --- a/Documentation/git-worktree.txt\n>> +++ b/Documentation/git-worktree.txt\n>> @@ -216,6 +216,18 @@ To remove a locked worktree, specify `--force` twice.\n>> This can also be set up as the default behaviour by using the\n>> `worktree.guessRemote` config option.\n>>\n>> +--[no-]relative-paths::\n>> +\tWorktrees will be linked to the repository using relative paths\n>> +\trather than using absolute paths. This is particularly useful for setups\n>> +\twhere the repository and worktrees may be moved between different\n>> +\tlocations or environments.\n>\n> This paragraph is redundant with what you wrote in git-config(1). I\n> think all we want to say is that it overrides the setting of that\n> configuration variable, and refer users there with linkgit.\n\nI agree. I'll update this paragraph to refer to the config documentation.\n\n>> ++\n>> +With `repair`, the linking files will be updated if there's an absolute/relative\n>> +mismatch, even if the links are correct.\n>\n> This is worth keeping.\n\nKeeping this as is.\n\n>> +This can also be set up as the default behaviour by using the\n>> +`worktree.useRelativePaths` config option.\n>> +\n>\n> This should get folded into my suggestion above.\n\nDone.\n\n>> diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh\n>> deleted file mode 100755\n>> index a3136db7e28cb20926ff44211e246ce625a6e51a..0000000000000000000000000000000000000000\n>> --- a/t/t2408-worktree-relative.sh\n>> +++ /dev/null\n>> @@ -1,39 +0,0 @@\n>\n> Was removing t2408 intentional here? I don't see the tests being\n> re-added elsewhere in this patch (though they may be introduced\n> elsewhere later in the series, I haven't read that far yet). Either way,\n> it may be worth mentioning in the commit message to avoid confusing\n> readers.\n\nYes, this was intentional. This was added in the original round when the\ndefault was changed to use relative paths. I added more comprehensive\ntests in the various worktree files to cover the new functionality. I\nwill make sure to mention this in the commit message.\n\n>> diff --git a/worktree.c b/worktree.c\n>> index 77ff484d3ec48c547ee4e3d958dfa28a52c1eaa7..de5c5e53a5f2a758ddf470b5d6a9ad6c66247181 100644\n>> --- a/worktree.c\n>> +++ b/worktree.c\n>> @@ -14,6 +14,8 @@\n>> #include \"wt-status.h\"\n>> #include \"config.h\"\n>>\n>> +int use_relative_paths = 0;\n>\n> I wondered whether 'use_relative_paths' should be static, or if we need to extern it in\n> from somewhere else in the tree. But we do, from worktree.[ch], which\n> seems reasonable. It would be nice if there was some way to thread that\n> into the worktree.h API, but I think that this is a reasonable measure\n> to take for now.\n\nI can add a getter/setter to the worktree API so we're not using a\nglobal variable.\n\n>> +\n>> void free_worktree(struct worktree *worktree)\n>> {\n>> \tif (!worktree)\n>> @@ -111,9 +113,9 @@ struct worktree *get_linked_worktree(const char *id,\n>> \tstrbuf_strip_suffix(&worktree_path, \"/.git\");\n>>\n>> \tif (!is_absolute_path(worktree_path.buf)) {\n>> -\t strbuf_strip_suffix(&path, \"gitdir\");\n>> -\t strbuf_addbuf(&path, &worktree_path);\n>> -\t strbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n>> +\t\tstrbuf_strip_suffix(&path, \"gitdir\");\n>> +\t\tstrbuf_addbuf(&path, &worktree_path);\n>> +\t\tstrbuf_realpath_forgiving(&worktree_path, path.buf, 0);\n>\n> Whitespace change?\n\nYes, this was added in the original round, I didn't notice that it used\n4 spaces instead of a tab. I had fixed this in v4 of the original round,\nbut this was merged in v3 so I just went ahead and fixed it here.\n\n>> \tCALLOC_ARRAY(worktree, 1);\n>> @@ -725,12 +727,15 @@ static int is_main_worktree_path(const char *path)\n>> * won't know which /worktrees//gitdir to repair. However, we may\n>> * be able to infer the gitdir by manually reading /path/to/worktree/.git,\n>> * extracting the , and checking if /worktrees/ exists.\n>> + *\n>> + * Returns -1 on failure and strbuf.len on success.\n>> */\n>> static int infer_backlink(const char *gitfile, struct strbuf *inferred)\n>\n> Should this return an ssize_t instead, then? I don't think we're going\n> to have worktree paths that are actually larger than INT_MAX, but it\n> seems hygienic and good to prevent any accidental overflow issues.\n\nI thought about this, but you'll run into OS limits long before you hit\nINT_MAX. However, I can make this change to be hygienic.\n\nBest,\nCaleb\n\n"},{"id":"506314","messageId":"D58VRRC59S1E.X68LFI4SFMDA@pm.me","threadId":"62421","inReplyTo":"aeec08e2-29da-4e5e-8a73-7ed7e3a79177@gmail.com","subject":"Re: [PATCH v2 3/5] worktree: add tests for worktrees with relative paths","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:10:56Z","receivedAt":"2024-10-30T05:11:03Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 10:43 AM CDT, phillip.wood123 wrote:\n>> I had received feedback that the original patch was too large, so I\n>> was trying to split it up into smaller, more digestible pieces.\n>\n> One way to do that would be to convert the \"add\", \"move\" and \"repair\"\n> subcommands in separate patches changing option handling and appropriate\n> tests in each one rather than changing all subcommands and their tests\n> at once.\n\nThat makes sense. I'll implement that.\n\nBest,\nCaleb\n\n\n"},{"id":"506315","messageId":"D58VTT1HI35K.P2AJ7CC0FK@pm.me","threadId":"62421","inReplyTo":"2ce3998f-ea16-4fd4-9756-c5849d834b1e@gmail.com","subject":"Re: [PATCH v2 0/5] Allow relative worktree linking to be configured by the user","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:13:35Z","receivedAt":"2024-10-30T05:13:41Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 9:55 AM CDT, Phillip Wood wrote:\n> Hi Caleb\n>\n> Thanks for working on this. I've left so high-level comments on the\n> first three patches. As I said in my comments on patch 1 I think it\n> would be helpful to re-order the changes so that the extension is\n> implementation first and then adjust the implementation to support both\n> relative and absolute paths before adding \"--no-relative-paths\" to \"git\n> worktree add/move/repair\". I'd also like to avoid adding a new global\n> variable\n\nThanks for your review! I appreciate it :).\n\nI'll address your comments on the first three patches and get back to\nyou with a revised series.\n\nBest,\nCaleb\n\n"},{"id":"506316","messageId":"D58W4HPIF8VX.2TWFTU2A2VSYY@pm.me","threadId":"62421","inReplyTo":"0bea06b8-11d0-497f-88e1-153cb61eb06d@gmail.com","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:27:33Z","receivedAt":"2024-10-30T05:27:41Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote:\n>> This patch introduces the `--[no-]relative-paths` CLI option for\n>\n> \"This patch\" is a bit redundant, I'd say \"Introduce a\n> `--[no-]-relative-paths ..`\n\nI removed \"patch\".\n\n>> `git worktree {add, move, repair}` commands, as well as the\n>> `worktree.useRelativePaths` configuration setting. When enabled,\n>> these options allow worktrees to be linked using relative paths,\n>> enhancing portability across environments where absolute paths\n>> may differ (e.g., containerized setups, shared network drives).\n>> Git still creates absolute paths by default, but these options allow\n>> users to opt-in to relative paths if desired.\n>\n> This sounds good, I'm not sure the patch actually implements anything\n> other than the option parsing though. I think it would make sense to add\n> these options at the end of the patch series once the implementation has\n> been changed to support them. I'd start with patches 4 and 5 to add the\n> new extension setting first, then refactor worktree.c to handle creating\n> worktrees with relative or absolute paths and set the extension if\n> appropriate, then add the --relative-path option to \"git worktree\"\n\nThat makes sense, I can reorder the patches as you described.\n\n>> Using the `--relative-paths` option with `worktree {move, repair}`\n>> will convert absolute paths to relative ones, while `--no-relative-paths`\n>> does the reverse. For cases where users want consistency in path\n>> handling, the config option `worktree.useRelativePaths` provides\n>> a persistent setting.\n>>\n>> In response to reviewer feedback from the initial patch series[1],\n>> this revision includes slight refactoring for improved\n>> maintainability and clarity in the code base.\n>\n> Please don't mix cleanups with other code changes as it makes it hard to\n> check that the cleanups don't change the behavior.\n\nFair enough, I'll separate the cleanups into their own patch.\n\n>> +worktree.useRelativePaths::\n>> +\tIf set to `true`, worktrees will be linked to the repository using\n>> +\trelative paths rather than using absolute paths. This is particularly\n>> +\tuseful for setups where the repository and worktrees may be moved between\n>> +\tdifferent locations or environments.\n>\n> I think it would be helpful to spell out the implications of this to the\n> user - namely that you cannot use older versions of git on a repository\n> with worktrees using relative paths and it may break third-party\n> software as well.\n\nI've added a note that you cannot use older versions of Git with\nworktrees when using relative paths.\n\n>> +--[no-]relative-paths::\n>> +\tWorktrees will be linked to the repository using relative paths\n>> +\trather than using absolute paths. This is particularly useful for setups\n>> +\twhere the repository and worktrees may be moved between different\n>> +\tlocations or environments.\n>\n> Again we should spell out the implications of using relative paths.\n\nThis has been revised to simply point to the config docs.\n\n>> +With `repair`, the linking files will be updated if there's an absolute/relative\n>> +mismatch, even if the links are correct.\n>> ++\n>> +This can also be set up as the default behaviour by using the\n>> +`worktree.useRelativePaths` config option.\n>> +\n>> --[no-]track::\n>> \tWhen creating a new branch, if `` is a branch,\n>> \tmark it as \"upstream\" from the new branch. This is the\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n>> \tif (!strcmp(var, \"worktree.guessremote\")) {\n>> \t\tguess_remote = git_config_bool(var, value);\n>> \t\treturn 0;\n>> +\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n>> +\t\tuse_relative_paths = git_config_bool(var, value);\n>\n> As we're trying to remove global variables from libgit.a as part of the\n> libification effort I'd be much happier if \"use_relative_paths\" was\n> declared as a \"static int\" in this file and then passed down to the\n> functions that need it rather than declaring it as a global in \"worktree.c\".\n\nI can create a getter/setter in the worktree API to handle this, but\nI'd rather not pass it as an argument to every function that needs it as\nthat would be a lot of changes. All of these functions would need their\nsignatures updated to include the new parameter:\n\n- `add_worktree()`\n- `update_worktree_location()`\n- `repair_worktree_at_path()`\n- `repair_worktrees()`\n- `repair_worktree()`\n- `write_worktree_linking_files()`\n\n>> diff --git a/t/t2408-worktree-relative.sh b/t/t2408-worktree-relative.sh\n>> deleted file mode 100755\n>\n> There's no explanation for this change in the commit message\n\nI added an explanation for this deletion.\n\nBest,\nCaleb\n\n"},{"id":"506317","messageId":"D58W71B3R4HV.WWTA6V6UJ9IP@pm.me","threadId":"62421","inReplyTo":"4a316a5f-9360-4f24-843f-bcbb5b3378c4@gmail.com","subject":"Re: [PATCH v2 2/5] worktree: add `write_worktree_linking_files` function","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:30:53Z","receivedAt":"2024-10-30T05:30:59Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote:\n> On 28/10/2024 19:09, Caleb White wrote:\n>> This also teachs `git worktree repair` to fix the linking files if\n>> there is an absolute/relative paths but the links are correct. E.g.,\n>> `git worktree repair` can be used to convert a valid worktree between\n>> absolute and relative paths.\n>\n> This might be better as a separate step so that reviewers can\n> concentrate on the correctness of write_werktree_linking_files() when\n> reviewing this patch.\n\nI'm planning on splitting this patch into separate patches for `add`,\n`move`, and `repair` commands. This can likely be done in the same\npatch as the `repair` command as it shouldn't be too difficult to\nfollow at that point.\n\nBest,\nCaleb\n\n>> Signed-off-by: Caleb White \n>> ---\n>> builtin/worktree.c | 11 +----\n>> worktree.c | 118 +++++++++++++++++++++++++++--------------------------\n>> worktree.h | 12 ++++++\n>> 3 files changed, 74 insertions(+), 67 deletions(-)\n>>\n>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> index c1130be5890c905c0b648782a834eb8dfcd79ba5..bb06830d6fe82aa97833c6e87f034115dfaa23bd 100644\n>> --- a/builtin/worktree.c\n>> +++ b/builtin/worktree.c\n>> @@ -417,8 +417,7 @@ static int add_worktree(const char *path, const char *refname,\n>> \t\t\tconst struct add_opts *opts)\n>> {\n>> \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n>> -\tstruct strbuf sb = STRBUF_INIT, sb_tmp = STRBUF_INIT;\n>> -\tstruct strbuf sb_path_realpath = STRBUF_INIT, sb_repo_realpath = STRBUF_INIT;\n>> +\tstruct strbuf sb = STRBUF_INIT;\n>> \tconst char *name;\n>> \tstruct strvec child_env = STRVEC_INIT;\n>> \tunsigned int counter = 0;\n>> @@ -494,10 +493,7 @@ static int add_worktree(const char *path, const char *refname,\n>>\n>> \tstrbuf_reset(&sb);\n>> \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n>> -\tstrbuf_realpath(&sb_path_realpath, path, 1);\n>> -\tstrbuf_realpath(&sb_repo_realpath, sb_repo.buf, 1);\n>> -\twrite_file(sb.buf, \"%s/.git\", relative_path(sb_path_realpath.buf, sb_repo_realpath.buf, &sb_tmp));\n>> -\twrite_file(sb_git.buf, \"gitdir: %s\", relative_path(sb_repo_realpath.buf, sb_path_realpath.buf, &sb_tmp));\n>> +\twrite_worktree_linking_files(sb_git, sb);\n>> \tstrbuf_reset(&sb);\n>> \tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n>> \twrite_file(sb.buf, \"../..\");\n>> @@ -581,12 +577,9 @@ static int add_worktree(const char *path, const char *refname,\n>>\n>> \tstrvec_clear(&child_env);\n>> \tstrbuf_release(&sb);\n>> -\tstrbuf_release(&sb_tmp);\n>> \tstrbuf_release(&symref);\n>> \tstrbuf_release(&sb_repo);\n>> -\tstrbuf_release(&sb_repo_realpath);\n>> \tstrbuf_release(&sb_git);\n>> -\tstrbuf_release(&sb_path_realpath);\n>> \tstrbuf_release(&sb_name);\n>> \tfree_worktree(wt);\n>> \treturn ret;\n>> diff --git a/worktree.c b/worktree.c\n>> index de5c5e53a5f2a758ddf470b5d6a9ad6c66247181..f4cee73d7a1edecafdff30b6d5e2d9dd1365b93e 100644\n>> --- a/worktree.c\n>> +++ b/worktree.c\n>> @@ -381,29 +381,24 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n>> void update_worktree_location(struct worktree *wt, const char *path_)\n>> {\n>> \tstruct strbuf path = STRBUF_INIT;\n>> -\tstruct strbuf repo = STRBUF_INIT;\n>> -\tstruct strbuf file = STRBUF_INIT;\n>> -\tstruct strbuf tmp = STRBUF_INIT;\n>> +\tstruct strbuf dotgit = STRBUF_INIT;\n>> +\tstruct strbuf gitdir = STRBUF_INIT;\n>>\n>> \tif (is_main_worktree(wt))\n>> \t\tBUG(\"can't relocate main worktree\");\n>>\n>> -\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n>> +\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n>> \tstrbuf_realpath(&path, path_, 1);\n>> +\tstrbuf_addf(&dotgit, \"%s/.git\", path.buf);\n>> \tif (fspathcmp(wt->path, path.buf)) {\n>> -\t\tstrbuf_addf(&file, \"%s/gitdir\", repo.buf);\n>> -\t\twrite_file(file.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n>> -\t\tstrbuf_reset(&file);\n>> -\t\tstrbuf_addf(&file, \"%s/.git\", path.buf);\n>> -\t\twrite_file(file.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n>> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n>>\n>> \t\tfree(wt->path);\n>> \t\twt->path = strbuf_detach(&path, NULL);\n>> \t}\n>> \tstrbuf_release(&path);\n>> -\tstrbuf_release(&repo);\n>> -\tstrbuf_release(&file);\n>> -\tstrbuf_release(&tmp);\n>> +\tstrbuf_release(&dotgit);\n>> +\tstrbuf_release(&gitdir);\n>> }\n>>\n>> int is_worktree_being_rebased(const struct worktree *wt,\n>> @@ -582,9 +577,9 @@ static void repair_gitfile(struct worktree *wt,\n>> \t\t\t worktree_repair_fn fn, void *cb_data)\n>> {\n>> \tstruct strbuf dotgit = STRBUF_INIT;\n>> +\tstruct strbuf gitdir = STRBUF_INIT;\n>> \tstruct strbuf repo = STRBUF_INIT;\n>> \tstruct strbuf backlink = STRBUF_INIT;\n>> -\tstruct strbuf tmp = STRBUF_INIT;\n>> \tchar *dotgit_contents = NULL;\n>> \tconst char *repair = NULL;\n>> \tint err;\n>> @@ -600,6 +595,7 @@ static void repair_gitfile(struct worktree *wt,\n>>\n>> \tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n>> \tstrbuf_addf(&dotgit, \"%s/.git\", wt->path);\n>> +\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n>> \tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n>>\n>> \tif (dotgit_contents) {\n>> @@ -617,18 +613,20 @@ static void repair_gitfile(struct worktree *wt,\n>> \t\trepair = _(\".git file broken\");\n>> \telse if (fspathcmp(backlink.buf, repo.buf))\n>> \t\trepair = _(\".git file incorrect\");\n>> +\telse if (use_relative_paths == is_absolute_path(dotgit_contents))\n>> +\t\trepair = _(\".git file absolute/relative path mismatch\");\n>>\n>> \tif (repair) {\n>> \t\tfn(0, wt->path, repair, cb_data);\n>> -\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, wt->path, &tmp));\n>> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n>> \t}\n>>\n>> done:\n>> \tfree(dotgit_contents);\n>> \tstrbuf_release(&repo);\n>> \tstrbuf_release(&dotgit);\n>> +\tstrbuf_release(&gitdir);\n>> \tstrbuf_release(&backlink);\n>> -\tstrbuf_release(&tmp);\n>> }\n>>\n>> static void repair_noop(int iserr UNUSED,\n>> @@ -653,45 +651,30 @@ void repair_worktrees(worktree_repair_fn fn, void *cb_data)\n>>\n>> void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path)\n>> {\n>> -\tstruct strbuf path = STRBUF_INIT;\n>> -\tstruct strbuf repo = STRBUF_INIT;\n>> \tstruct strbuf gitdir = STRBUF_INIT;\n>> \tstruct strbuf dotgit = STRBUF_INIT;\n>> -\tstruct strbuf olddotgit = STRBUF_INIT;\n>> -\tstruct strbuf tmp = STRBUF_INIT;\n>>\n>> \tif (is_main_worktree(wt))\n>> \t\tgoto done;\n>>\n>> -\tstrbuf_realpath(&repo, git_common_path(\"worktrees/%s\", wt->id), 1);\n>> -\tstrbuf_addf(&gitdir, \"%s/gitdir\", repo.buf);\n>> +\tstrbuf_realpath(&gitdir, git_common_path(\"worktrees/%s/gitdir\", wt->id), 1);\n>>\n>> -\tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n>> +\tif (strbuf_read_file(&dotgit, gitdir.buf, 0) < 0)\n>> \t\tgoto done;\n>>\n>> -\tstrbuf_rtrim(&olddotgit);\n>> -\tif (is_absolute_path(olddotgit.buf)) {\n>> -\t\tstrbuf_addbuf(&dotgit, &olddotgit);\n>> -\t} else {\n>> -\t\tstrbuf_addf(&dotgit, \"%s/worktrees/%s/%s\", old_path, wt->id, olddotgit.buf);\n>> +\tstrbuf_rtrim(&dotgit);\n>> +\tif (!is_absolute_path(dotgit.buf)) {\n>> +\t\tstrbuf_insertf(&dotgit, 0, \"%s/worktrees/%s/\", old_path, wt->id);\n>> \t\tstrbuf_realpath_forgiving(&dotgit, dotgit.buf, 0);\n>> \t}\n>>\n>> \tif (!file_exists(dotgit.buf))\n>> \t\tgoto done;\n>>\n>> -\tstrbuf_addbuf(&path, &dotgit);\n>> -\tstrbuf_strip_suffix(&path, \"/.git\");\n>> -\n>> -\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n>> -\twrite_file(gitdir.buf, \"%s\", relative_path(dotgit.buf, repo.buf, &tmp));\n>> +\twrite_worktree_linking_files(dotgit, gitdir);\n>> done:\n>> -\tstrbuf_release(&path);\n>> -\tstrbuf_release(&repo);\n>> \tstrbuf_release(&gitdir);\n>> \tstrbuf_release(&dotgit);\n>> -\tstrbuf_release(&olddotgit);\n>> -\tstrbuf_release(&tmp);\n>> }\n>>\n>> void repair_worktrees_after_gitdir_move(const char *old_path)\n>> @@ -766,13 +749,10 @@ void repair_worktree_at_path(const char *path,\n>> \t\t\t worktree_repair_fn fn, void *cb_data)\n>> {\n>> \tstruct strbuf dotgit = STRBUF_INIT;\n>> -\tstruct strbuf realdotgit = STRBUF_INIT;\n>> \tstruct strbuf backlink = STRBUF_INIT;\n>> \tstruct strbuf inferred_backlink = STRBUF_INIT;\n>> \tstruct strbuf gitdir = STRBUF_INIT;\n>> \tstruct strbuf olddotgit = STRBUF_INIT;\n>> -\tstruct strbuf realolddotgit = STRBUF_INIT;\n>> -\tstruct strbuf tmp = STRBUF_INIT;\n>> \tchar *dotgit_contents = NULL;\n>> \tconst char *repair = NULL;\n>> \tint err;\n>> @@ -784,25 +764,25 @@ void repair_worktree_at_path(const char *path,\n>> \t\tgoto done;\n>>\n>> \tstrbuf_addf(&dotgit, \"%s/.git\", path);\n>> -\tif (!strbuf_realpath(&realdotgit, dotgit.buf, 0)) {\n>> +\tif (!strbuf_realpath(&dotgit, dotgit.buf, 0)) {\n>> \t\tfn(1, path, _(\"not a valid path\"), cb_data);\n>> \t\tgoto done;\n>> \t}\n>>\n>> -\tinfer_backlink(realdotgit.buf, &inferred_backlink);\n>> +\tinfer_backlink(dotgit.buf, &inferred_backlink);\n>> \tstrbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);\n>> -\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(realdotgit.buf, &err));\n>> +\tdotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));\n>> \tif (dotgit_contents) {\n>> \t\tif (is_absolute_path(dotgit_contents)) {\n>> \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n>> \t\t} else {\n>> -\t\t\tstrbuf_addbuf(&backlink, &realdotgit);\n>> +\t\t\tstrbuf_addbuf(&backlink, &dotgit);\n>> \t\t\tstrbuf_strip_suffix(&backlink, \".git\");\n>> \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n>> \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n>> \t\t}\n>> \t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n>> -\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n>> +\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n>> \t\tgoto done;\n>> \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n>> \t\tif (inferred_backlink.len) {\n>> @@ -815,11 +795,11 @@ void repair_worktree_at_path(const char *path,\n>> \t\t\t */\n>> \t\t\tstrbuf_swap(&backlink, &inferred_backlink);\n>> \t\t} else {\n>> -\t\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n>> +\t\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file does not reference a repository\"), cb_data);\n>> \t\t\tgoto done;\n>> \t\t}\n>> \t} else {\n>> -\t\tfn(1, realdotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n>> +\t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git file broken\"), cb_data);\n>> \t\tgoto done;\n>> \t}\n>>\n>> @@ -841,39 +821,35 @@ void repair_worktree_at_path(const char *path,\n>> \t * in the \"copy\" repository. In this case, point the \"copy\" worktree's\n>> \t * .git file at the \"copy\" repository.\n>> \t */\n>> -\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf)) {\n>> +\tif (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))\n>> \t\tstrbuf_swap(&backlink, &inferred_backlink);\n>> -\t}\n>>\n>> \tstrbuf_addf(&gitdir, \"%s/gitdir\", backlink.buf);\n>> \tif (strbuf_read_file(&olddotgit, gitdir.buf, 0) < 0)\n>> \t\trepair = _(\"gitdir unreadable\");\n>> +\telse if (use_relative_paths == is_absolute_path(olddotgit.buf))\n>> +\t\trepair = _(\"gitdir absolute/relative path mismatch\");\n>> \telse {\n>> \t\tstrbuf_rtrim(&olddotgit);\n>> -\t\tif (is_absolute_path(olddotgit.buf)) {\n>> -\t\t\tstrbuf_addbuf(&realolddotgit, &olddotgit);\n>> -\t\t} else {\n>> -\t\t\tstrbuf_addf(&realolddotgit, \"%s/%s\", backlink.buf, olddotgit.buf);\n>> -\t\t\tstrbuf_realpath_forgiving(&realolddotgit, realolddotgit.buf, 0);\n>> +\t\tif (!is_absolute_path(olddotgit.buf)) {\n>> +\t\t\tstrbuf_insertf(&olddotgit, 0, \"%s/\", backlink.buf);\n>> +\t\t\tstrbuf_realpath_forgiving(&olddotgit, olddotgit.buf, 0);\n>> \t\t}\n>> -\t\tif (fspathcmp(realolddotgit.buf, realdotgit.buf))\n>> +\t\tif (fspathcmp(olddotgit.buf, dotgit.buf))\n>> \t\t\trepair = _(\"gitdir incorrect\");\n>> \t}\n>>\n>> \tif (repair) {\n>> \t\tfn(0, gitdir.buf, repair, cb_data);\n>> -\t\twrite_file(gitdir.buf, \"%s\", relative_path(realdotgit.buf, backlink.buf, &tmp));\n>> +\t\twrite_worktree_linking_files(dotgit, gitdir);\n>> \t}\n>> done:\n>> \tfree(dotgit_contents);\n>> \tstrbuf_release(&olddotgit);\n>> -\tstrbuf_release(&realolddotgit);\n>> \tstrbuf_release(&backlink);\n>> \tstrbuf_release(&inferred_backlink);\n>> \tstrbuf_release(&gitdir);\n>> -\tstrbuf_release(&realdotgit);\n>> \tstrbuf_release(&dotgit);\n>> -\tstrbuf_release(&tmp);\n>> }\n>>\n>> int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, timestamp_t expire)\n>> @@ -1034,3 +1010,29 @@ int init_worktree_config(struct repository *r)\n>> \tfree(main_worktree_file);\n>> \treturn res;\n>> }\n>> +\n>> +void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir)\n>> +{\n>> +\tstruct strbuf path = STRBUF_INIT;\n>> +\tstruct strbuf repo = STRBUF_INIT;\n>> +\tstruct strbuf tmp = STRBUF_INIT;\n>> +\n>> +\tstrbuf_addbuf(&path, &dotgit);\n>> +\tstrbuf_strip_suffix(&path, \"/.git\");\n>> +\tstrbuf_realpath(&path, path.buf, 1);\n>> +\tstrbuf_addbuf(&repo, &gitdir);\n>> +\tstrbuf_strip_suffix(&repo, \"/gitdir\");\n>> +\tstrbuf_realpath(&repo, repo.buf, 1);\n>> +\n>> +\tif (use_relative_paths) {\n>> +\t\twrite_file(gitdir.buf, \"%s/.git\", relative_path(path.buf, repo.buf, &tmp));\n>> +\t\twrite_file(dotgit.buf, \"gitdir: %s\", relative_path(repo.buf, path.buf, &tmp));\n>> +\t} else {\n>> +\t\twrite_file(gitdir.buf, \"%s/.git\", path.buf);\n>> +\t\twrite_file(dotgit.buf, \"gitdir: %s\", repo.buf);\n>> +\t}\n>> +\n>> +\tstrbuf_release(&path);\n>> +\tstrbuf_release(&repo);\n>> +\tstrbuf_release(&tmp);\n>> +}\n>> diff --git a/worktree.h b/worktree.h\n>> index 37e65d508ed23d3e7a29850bb938285072a3aaa6..5929089891c97318a8f5329f7938264c717050d5 100644\n>> --- a/worktree.h\n>> +++ b/worktree.h\n>> @@ -217,4 +217,16 @@ void strbuf_worktree_ref(const struct worktree *wt,\n>> */\n>> int init_worktree_config(struct repository *r);\n>>\n>> +/**\n>> + * Write the .git file and gitdir file that links the worktree to the repository.\n>> + *\n>> + * The `dotgit` parameter is the path to the worktree's .git file, and `gitdir`\n>> + * is the path to the repository's `gitdir` file.\n>> + *\n>> + * Example\n>> + * dotgit: \"/path/to/foo/.git\"\n>> + * gitdir: \"/path/to/repo/worktrees/foo/gitdir\"\n>> + */\n>> +void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir);\n>> +\n>> #endif\n>>\n\n\n"},{"id":"506318","messageId":"D58WCL821YEF.1Y68WOUZ8RJLT@pm.me","threadId":"62421","inReplyTo":"ZyFn9xErajxzQo29@nand.local","subject":"Re: [PATCH v2 2/5] worktree: add `write_worktree_linking_files` function","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T05:38:06Z","receivedAt":"2024-10-30T05:38:12Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Tue Oct 29, 2024 at 5:55 PM CDT, Taylor Blau wrote:\n> On Tue, Oct 29, 2024 at 02:52:36PM +0000, Phillip Wood wrote:\n>> On 28/10/2024 19:09, Caleb White wrote:\n>> > A new helper function, `write_worktree_linking_files()`, centralizes\n>> > the logic for computing and writing either relative or absolute\n>> > paths, based on the provided configuration. This function accepts\n>> > `strbuf` pointers to both the worktree’s `.git` link and the\n>> > repository’s `gitdir`, and then writes the appropriate path to each.\n>>\n>> That sounds like a useful change. I think it would be better to pass an\n>> extra parameter \"use_relative_paths\" rather than relying on a global\n>> varibale in worktree.c. Thank you for adding some documentaion for the new\n>> function.\n>\n> Good suggestion. I definitely agree that this is a useful direction.\n\nI didn't particularly care for the global variable either, but that was\nthe easiest way to get up and running. I don't want to just pass this\nparameter around as that will result in changing about 6-7 function\nsignatures. What are some other ways settings have been propagated\ninto the internals? Right now, I'm thinking of just using\na getter/setter in the worktree API, which would remove the global\nvariable.\n\n>> This might be better as a separate step so that reviewers can concentrate on\n>> the correctness of write_werktree_linking_files() when reviewing this patch.\n>\n> Indeed. This patch (even though the diffstat isn't overly large) is\n> somewhat noisy just because of the number of spots that needed to be\n> adjusted here.\n>\n> I wonder if another way to split this up (in addition to what you wrote\n> above) might be to introduce the new function and convert one single\n> caller in the first patch. Then subsequent patches can go one callsite\n> at a time and convert them to use the new function.\n>\n> That way, each patch is easy-ish to verify in isolation. I know that\n> results in some more patches, but I think that the additional clarity I\n> imagine we'll get is worth doing so.\n\nSounds good, I'll split this up into multiple patches for each of the\nsubcommands. I'll also add the tests for that subcommand in the same\npatch for context.\n\nBest,\nCaleb\n\n\n"},{"id":"506335","messageId":"ZyKUEbXedFuzY9Oz@nand.local","threadId":"62421","inReplyTo":"D58W4HPIF8VX.2TWFTU2A2VSYY@pm.me","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-30T20:16:17Z","receivedAt":"2024-10-30T20:16:20Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 30, 2024 at 05:27:33AM +0000, Caleb White wrote:\n> >> diff --git a/builtin/worktree.c b/builtin/worktree.c\n> >> index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n> >> --- a/builtin/worktree.c\n> >> +++ b/builtin/worktree.c\n> >> @@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n> >> \tif (!strcmp(var, \"worktree.guessremote\")) {\n> >> \t\tguess_remote = git_config_bool(var, value);\n> >> \t\treturn 0;\n> >> +\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n> >> +\t\tuse_relative_paths = git_config_bool(var, value);\n> >\n> > As we're trying to remove global variables from libgit.a as part of the\n> > libification effort I'd be much happier if \"use_relative_paths\" was\n> > declared as a \"static int\" in this file and then passed down to the\n> > functions that need it rather than declaring it as a global in \"worktree.c\".\n>\n> I can create a getter/setter in the worktree API to handle this, but\n> I'd rather not pass it as an argument to every function that needs it as\n> that would be a lot of changes. All of these functions would need their\n> signatures updated to include the new parameter:\n>\n> - `add_worktree()`\n> - `update_worktree_location()`\n> - `repair_worktree_at_path()`\n> - `repair_worktrees()`\n> - `repair_worktree()`\n> - `write_worktree_linking_files()`\n\nThere is no reason to have a \"getter\" and \"setter\" for a extern'd\nvariable.\n\nI agree that it would be preferable to have use_relative_paths be a\nstatic int within this compilation unit and to pass it to the above\nfunctions.\n\nThanks,\nTaylor\n"},{"id":"506336","messageId":"D59F4X1I5COT.1QECM7J128ZIG@pm.me","threadId":"62421","inReplyTo":"ZyKUEbXedFuzY9Oz@nand.local","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T20:21:28Z","receivedAt":"2024-10-30T20:21:37Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Wed Oct 30, 2024 at 3:16 PM CDT, Taylor Blau wrote:\n> On Wed, Oct 30, 2024 at 05:27:33AM +0000, Caleb White wrote:\n>> >> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>> >> index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n>> >> --- a/builtin/worktree.c\n>> >> +++ b/builtin/worktree.c\n>> >> @@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n>> >> \tif (!strcmp(var, \"worktree.guessremote\")) {\n>> >> \t\tguess_remote = git_config_bool(var, value);\n>> >> \t\treturn 0;\n>> >> +\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n>> >> +\t\tuse_relative_paths = git_config_bool(var, value);\n>> >\n>> > As we're trying to remove global variables from libgit.a as part of the\n>> > libification effort I'd be much happier if \"use_relative_paths\" was\n>> > declared as a \"static int\" in this file and then passed down to the\n>> > functions that need it rather than declaring it as a global in \"worktree.c\".\n>>\n>> I can create a getter/setter in the worktree API to handle this, but\n>> I'd rather not pass it as an argument to every function that needs it as\n>> that would be a lot of changes. All of these functions would need their\n>> signatures updated to include the new parameter:\n>>\n>> - `add_worktree()`\n>> - `update_worktree_location()`\n>> - `repair_worktree_at_path()`\n>> - `repair_worktrees()`\n>> - `repair_worktree()`\n>> - `write_worktree_linking_files()`\n>\n> There is no reason to have a \"getter\" and \"setter\" for a extern'd\n> variable.\n>\n> I agree that it would be preferable to have use_relative_paths be a\n> static int within this compilation unit and to pass it to the above\n> functions.\n\nIf I created a getter/setter then the variable would no longer be\nextern'd.\n\nTo be clear, you're advocating that I change the function signature\nfor all of the functions listed above to include the new parameter? That\nseems like a lot of parameter bloat when I could just set the variable\nin this compilation unit and access it directly in the\n`write_worktree_linking_files()` function.\n\nBest,\nCaleb\n\n"},{"id":"506337","messageId":"0f91514a-f1cb-40da-a398-56d0fe37600e@gmail.com","threadId":"62421","inReplyTo":"D59F4X1I5COT.1QECM7J128ZIG@pm.me","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-10-30T20:30:57Z","receivedAt":"2024-10-30T20:31:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Caleb\n\nOn 30/10/2024 20:21, Caleb White wrote:\n> On Wed Oct 30, 2024 at 3:16 PM CDT, Taylor Blau wrote:\n>> On Wed, Oct 30, 2024 at 05:27:33AM +0000, Caleb White wrote:\n>>>>> diff --git a/builtin/worktree.c b/builtin/worktree.c\n>>>>> index dae63dedf4cac2621f51f95a39aa456b33acd894..c1130be5890c905c0b648782a834eb8dfcd79ba5 100644\n>>>>> --- a/builtin/worktree.c\n>>>>> +++ b/builtin/worktree.c\n>>>>> @@ -134,6 +134,9 @@ static int git_worktree_config(const char *var, const char *value,\n>>>>> \tif (!strcmp(var, \"worktree.guessremote\")) {\n>>>>> \t\tguess_remote = git_config_bool(var, value);\n>>>>> \t\treturn 0;\n>>>>> +\t} else if (!strcmp(var, \"worktree.userelativepaths\")) {\n>>>>> +\t\tuse_relative_paths = git_config_bool(var, value);\n>>>>\n>>>> As we're trying to remove global variables from libgit.a as part of the\n>>>> libification effort I'd be much happier if \"use_relative_paths\" was\n>>>> declared as a \"static int\" in this file and then passed down to the\n>>>> functions that need it rather than declaring it as a global in \"worktree.c\".\n>>>\n>>> I can create a getter/setter in the worktree API to handle this, but\n>>> I'd rather not pass it as an argument to every function that needs it as\n>>> that would be a lot of changes. All of these functions would need their\n>>> signatures updated to include the new parameter:\n>>>\n>>> - `add_worktree()`\n>>> - `update_worktree_location()`\n>>> - `repair_worktree_at_path()`\n>>> - `repair_worktrees()`\n>>> - `repair_worktree()`\n>>> - `write_worktree_linking_files()`\n>>\n>> There is no reason to have a \"getter\" and \"setter\" for a extern'd\n>> variable.\n>>\n>> I agree that it would be preferable to have use_relative_paths be a\n>> static int within this compilation unit and to pass it to the above\n>> functions.\n> \n> If I created a getter/setter then the variable would no longer be\n> extern'd.\n> \n> To be clear, you're advocating that I change the function signature\n> for all of the functions listed above to include the new parameter? That\n> seems like a lot of parameter bloat\n\nIt's a bit of a pain to have to change the function signatures and pass \nthe parameter down but it's not difficult to do.\n\n> when I could just set the variable\n> in this compilation unit and access it directly in the\n> `write_worktree_linking_files()` function.\n\nThe problem with that is that the variable is still effectively global. \nThe aim of the libification work is to be able to work on more than one \nrepository from a single process while respecting each repository's \nconfig settings.\n\nBest Wishes\n\nPhillip\n\n> Best,\n> Caleb\n> \n\n"},{"id":"506338","messageId":"D59FG357FL61.909ECKSHV9MM@pm.me","threadId":"62421","inReplyTo":"0f91514a-f1cb-40da-a398-56d0fe37600e@gmail.com","subject":"Re: [PATCH v2 1/5] worktree: add CLI/config options for relative path linking","fromName":"Caleb White","fromEmail":"cdwhite3@pm.me","sentAt":"2024-10-30T20:36:03Z","receivedAt":"2024-10-30T20:36:17Z","isPatch":true,"sender":{"key":"cdwhite3@pm.me","avatar":"https://avatars.githubusercontent.com/u/4176520?v=4"},"body":"On Wed Oct 30, 2024 at 3:30 PM CDT, phillip.wood123 wrote:\n> On 30/10/2024 20:21, Caleb White wrote:\n>> If I created a getter/setter then the variable would no longer be\n>> extern'd.\n>>\n>> To be clear, you're advocating that I change the function signature\n>> for all of the functions listed above to include the new parameter? That\n>> seems like a lot of parameter bloat\n>\n> It's a bit of a pain to have to change the function signatures and pass\n> the parameter down but it's not difficult to do.\n>\n>> when I could just set the variable\n>> in this compilation unit and access it directly in the\n>> `write_worktree_linking_files()` function.\n>\n> The problem with that is that the variable is still effectively global.\n> The aim of the libification work is to be able to work on more than one\n> repository from a single process while respecting each repository's\n> config settings.\n>\n\nI see your point. I'll make the changes you've suggested. Thanks for the\nfeedback.\n\nBest,\nCaleb\n\n"}]}