Hi Caleb
On 26/11/2024 01:51, Caleb White wrote:
Changes in v5:
- Added docs to `--relative-paths` option.
- Added test coverage for `repair_worktrees()` and relative paths.
- Move `strbuf_reset` call in `infer_backlink()`.
- Cleaned up tests.
- Slight stylistic changes.
- Tweaked commit messages.
- Updated base to 090d24e9af.
Thanks for re-rolling, these changes sound good. Below is the
range-diff of what is in seen today compared to last week. I've left
it untrimmed so other people can check what's changed and I've added a
couple of comments. The only thing I'm worried about is the deletion
of a check for setting extensions.relativeWorktrees in patch 5 the
rest of the changes look good, thank you for the extra test checks and
log messages.
Best Wishes
Phillip
1: 67a622c7f2 ! 1: 0986f98022 setup: correctly reinitialize repository version
@@ Commit message
config to ensure that the repository version is set correctly.
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## setup.c ##
@@ setup.c: void initialize_repository_version(int hash_algo,
2: 85964909b0 ! 2: c36e1a59fa worktree: add `relativeWorktrees` extension
@@ Commit message
Suggested-by: Junio C Hamano <gitster@xxxxxxxxx>
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## Documentation/config/extensions.txt ##
@@ Documentation/config/extensions.txt: Note that this setting should only be set by linkgit:git-init[1] or
3: a614c39333 ! 3: 5b19b63040 worktree: refactor infer_backlink return
@@ Commit message
[1]: https://lore.kernel.org/git/20241007-wt_relative_paths-v3-0-622cf18c45eb@xxxxx
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## worktree.c ##
@@ worktree.c: struct worktree *get_linked_worktree(const char *id,
@@ worktree.c: static int is_main_worktree_path(const char *path)
{
struct strbuf actual = STRBUF_INIT;
const char *id;
-
-+ strbuf_reset(inferred);
- if (strbuf_read_file(&actual, gitfile, 0) < 0)
- goto error;
- if (!starts_with(actual.buf, "gitdir:"))
@@ worktree.c: static int infer_backlink(const char *gitfile, struct strbuf *inferred)
- id++; /* advance past '/' to point at <id> */
- if (!*id)
- goto error;
-- strbuf_reset(inferred);
- strbuf_git_common_path(inferred, the_repository, "worktrees/%s", id);
- if (!is_directory(inferred->buf))
goto error;
strbuf_release(&actual);
4: 83ea6d7ba0 ! 4: ec143ae00e worktree: add `write_worktree_linking_files()` function
@@ Commit message
is linked with relative paths.
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## worktree.c ##
@@ worktree.c: int init_worktree_config(struct repository *r)
free(main_worktree_file);
return res;
}
+
-+void write_worktree_linking_files(struct strbuf dotgit,
-+ struct strbuf gitdir,
++void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,
+ int use_relative_paths)
+{
+ struct strbuf path = STRBUF_INIT;
@@ worktree.h: void strbuf_worktree_ref(const struct worktree *wt,
+ * dotgit: "/path/to/foo/.git"
+ * gitdir: "/path/to/repo/worktrees/foo/gitdir"
+ */
-+void write_worktree_linking_files(struct strbuf dotgit,
-+ struct strbuf gitdir,
++void write_worktree_linking_files(struct strbuf dotgit, struct strbuf gitdir,
+ int use_relative_paths);
+
#endif
5: 36d01dca84 ! 5: 237206b08f worktree: add relative cli/config options to `add` command
@@ Commit message
written for the various worktree operations in their own files.
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## Documentation/config/worktree.txt ##
@@ Documentation/config/worktree.txt: worktree.guessRemote::
@@ Documentation/git-worktree.txt: To remove a locked worktree, specify `--force` t
`worktree.guessRemote` config option.
+--[no-]relative-paths::
++ Link worktrees using relative paths or absolute paths (default).
+ Overrides the `worktree.useRelativePaths` config option, see
+ linkgit:git-config[1].
+
@@ t/t2400-worktree-add.sh: test_expect_success '"add" with initialized submodule,
+test_expect_success 'can create worktrees with relative paths' '
+ test_when_finished "git worktree remove relative" &&
-+ git config worktree.useRelativePaths false &&
++ test_config worktree.useRelativePaths false &&
+ git worktree add --relative-paths ./relative &&
-+ cat relative/.git >actual &&
+ echo "gitdir: ../.git/worktrees/relative" >expect &&
-+ test_cmp expect actual &&
-+ cat .git/worktrees/relative/gitdir >actual &&
++ test_cmp expect relative/.git &&
+ echo "../../../relative/.git" >expect &&
-+ test_cmp expect actual
-+
++ test_cmp expect .git/worktrees/relative/gitdir
+'
+
+test_expect_success 'can create worktrees with absolute paths' '
-+ git config worktree.useRelativePaths true &&
++ test_config worktree.useRelativePaths true &&
+ git worktree add ./relative &&
-+ cat relative/.git >actual &&
+ echo "gitdir: ../.git/worktrees/relative" >expect &&
-+ test_cmp expect actual &&
++ test_cmp expect relative/.git &&
+ git worktree add --no-relative-paths ./absolute &&
-+ cat absolute/.git >actual &&
+ echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect &&
-+ test_cmp expect actual
++ test_cmp expect absolute/.git &&
++ echo "$(pwd)/absolute/.git" >expect &&
++ test_cmp expect .git/worktrees/absolute/gitdir
+'
+
+test_expect_success 'move repo without breaking relative internal links' '
+ test_when_finished rm -rf repo moved &&
+ git init repo &&
+ (
+ cd repo &&
-+ git config worktree.useRelativePaths true &&
+ test_commit initial &&
-+ git worktree add wt1 &&
++ git worktree add --relative-paths wt1 &&
+ cd .. &&
+ mv repo moved &&
+ cd moved/wt1 &&
-+ git status >out 2>err &&
++ git worktree list >out 2>err &&
+ test_must_be_empty err
+ )
+'
@@ t/t2400-worktree-add.sh: test_expect_success '"add" with initialized submodule,
+ git init repo &&
+ git -C repo commit --allow-empty -m base &&
+ git -C repo worktree add --relative-paths ./foo &&
-+ git -C repo config get core.repositoryformatversion >actual &&
-+ echo 1 >expected &&
-+ test_cmp expected actual &&
-+ git -C repo config get extensions.relativeworktrees >actual &&
-+ echo true >expected &&
-+ test_cmp expected actual
++ test_cmp_config -C repo 1 core.repositoryformatversion
+'
We have lost the check for extensions.relativeworktrees
here. Although we don't set worktree.useRelativePaths anymore we
should still set the extension as we pase --relative-paths to "git
worktree add"
+
test_done
6: 34401d680c ! 6: 8e4307f520 worktree: add relative cli/config options to `move` command
@@ Commit message
the new path will be relative (and visa-versa).
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## builtin/worktree.c ##
@@ builtin/worktree.c: static int move_worktree(int ac, const char **av, const char *prefix)
@@ t/t2403-worktree-move.sh: test_expect_success 'not remove a repo with initialize
'
+test_expect_success 'move worktree with absolute path to relative path' '
-+ git config worktree.useRelativePaths false &&
++ test_config worktree.useRelativePaths false &&
+ git worktree add ./absolute &&
+ git worktree move --relative-paths absolute relative &&
-+ cat relative/.git >actual &&
+ echo "gitdir: ../.git/worktrees/absolute" >expect &&
-+ test_cmp expect actual &&
-+ git config worktree.useRelativePaths true &&
++ test_cmp expect relative/.git &&
++ echo "../../../relative/.git" >expect &&
++ test_cmp expect .git/worktrees/absolute/gitdir &&
++ test_config worktree.useRelativePaths true &&
+ git worktree move relative relative2 &&
-+ cat relative2/.git >actual &&
+ echo "gitdir: ../.git/worktrees/absolute" >expect &&
-+ test_cmp expect actual
++ test_cmp expect relative2/.git &&
++ echo "../../../relative2/.git" >expect &&
++ test_cmp expect .git/worktrees/absolute/gitdir
+'
+
+test_expect_success 'move worktree with relative path to absolute path' '
-+ git config worktree.useRelativePaths true &&
++ test_config worktree.useRelativePaths true &&
+ git worktree move --no-relative-paths relative2 absolute &&
-+ cat absolute/.git >actual &&
+ echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect &&
-+ test_cmp expect actual
++ test_cmp expect absolute/.git &&
++ echo "$(pwd)/absolute/.git" >expect &&
++ test_cmp expect .git/worktrees/absolute/gitdir
+'
+
test_done
@@ worktree.c: int validate_worktree(const struct worktree *wt, struct strbuf *errm
}
-void update_worktree_location(struct worktree *wt, const char *path_)
-+void update_worktree_location(struct worktree *wt,
-+ const char *path_,
++void update_worktree_location(struct worktree *wt, const char *path_,
+ int use_relative_paths)
{
struct strbuf path = STRBUF_INIT;
@@ worktree.c: int validate_worktree(const struct worktree *wt, struct strbuf *errm
## worktree.h ##
@@ worktree.h: int validate_worktree(const struct worktree *wt,
+ /*
* Update worktrees/xxx/gitdir with the new path.
*/
- void update_worktree_location(struct worktree *wt,
+-void update_worktree_location(struct worktree *wt,
- const char *path_);
-+ const char *path_,
++void update_worktree_location(struct worktree *wt, const char *path_,
+ int use_relative_paths);
typedef void (* worktree_repair_fn)(int iserr, const char *path,
7: 30e9e41061 ! 7: 4f951f4088 worktree: add relative cli/config options to `repair` command
@@ Commit message
even if the original path was correct. This allows a user to covert
existing worktrees between absolute/relative as desired.
+ To simplify things, both linking files are written when one of the files
+ needs to be repaired. In some cases, this fixes the other file before it
+ is checked, in other cases this results in a correct file being written
+ with the same contents.
+
Thanks for adding this, I think it will be really helpful for anyone
trying to understand this change in the future.
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## Documentation/git-worktree.txt ##
@@ Documentation/git-worktree.txt: This can also be set up as the default behaviour by using the
- --[no-]relative-paths::
+ Link worktrees using relative paths or absolute paths (default).
Overrides the `worktree.useRelativePaths` config option, see
linkgit:git-config[1].
++
@@ t/t2406-worktree-repair.sh: test_expect_success 'repair copied main and linked w
test_cmp dup/linked.expect dup/linked/.git
'
++test_expect_success 'repair worktree with relative path with missing gitfile' '
++ test_when_finished "rm -rf main wt" &&
++ test_create_repo main &&
++ git -C main config worktree.useRelativePaths true &&
++ test_commit -C main init &&
++ git -C main worktree add --detach ../wt &&
++ rm wt/.git &&
++ test_path_is_missing wt/.git &&
++ git -C main worktree repair &&
++ echo "gitdir: ../main/.git/worktrees/wt" >expect &&
++ test_cmp expect wt/.git
++'
++
+test_expect_success 'repair absolute worktree to use relative paths' '
+ test_when_finished "rm -rf main side sidemoved" &&
+ test_create_repo main &&
@@ worktree.c: int other_head_refs(each_ref_fn fn, void *cb_data)
*/
static void repair_gitfile(struct worktree *wt,
- worktree_repair_fn fn, void *cb_data)
-+ worktree_repair_fn fn,
-+ void *cb_data,
++ worktree_repair_fn fn, void *cb_data,
+ int use_relative_paths)
{
struct strbuf dotgit = STRBUF_INIT;
@@ worktree.c: static ssize_t infer_backlink(const char *gitfile, struct strbuf *in
*/
void repair_worktree_at_path(const char *path,
- worktree_repair_fn fn, void *cb_data)
-+ worktree_repair_fn fn,
-+ void *cb_data,
++ worktree_repair_fn fn, void *cb_data,
+ int use_relative_paths)
{
struct strbuf dotgit = STRBUF_INIT;
8: 298327f538 ! 8: 28eb7f66b2 worktree: refactor `repair_worktree_after_gitdir_move()`
@@ Commit message
`write_worktree_linking_files` function. It also preserves the
relativity of the linking files; e.g., if an existing worktree used
absolute paths then the repaired paths will be absolute (and visa-versa).
+ `repair_worktree_after_gitdir_move()` is used to repair both sets of
+ worktree linking files if the `.git` directory is moved during a
+ re-initialization using `git init`.
Thanks for the extra explanation
This also adds a test case for reinitializing a repository that has
relative worktrees.
Signed-off-by: Caleb White <cdwhite3@xxxxx>
- Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
+ Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
## t/t0001-init.sh ##
@@ t/t0001-init.sh: test_expect_success SYMLINKS 're-init to move gitdir symlink' '
@@ t/t0001-init.sh: test_expect_success SYMLINKS 're-init to move gitdir symlink' '
git init mainwt &&
+ if test "relative" = $2
+ then
-+ git -C mainwt config worktree.useRelativePaths true
++ test_config -C mainwt worktree.useRelativePaths true
+ else
-+ git -C mainwt config worktree.useRelativePaths false
++ test_config -C mainwt worktree.useRelativePaths false
+ fi
test_commit -C mainwt gumby &&
git -C mainwt worktree add --detach ../linkwt &&
- Link to v4: https://lore.kernel.org/r/20241031-wt_relative_options-v4-0-07a3dc0f02a3@xxxxx
Changes in v4:
- Fixed failing test in ci
- Link to v3: https://lore.kernel.org/r/20241031-wt_relative_options-v3-0-3e44ccdf64e6@xxxxx
Changes in v3:
- Split patches into smaller edits.
- Moved tests into the patches with the relevant code changes.
- Removed global `use_relative_paths` and instead pass parameter to functions.
- Changed `infer_backlink` return type from `int` to `ssize_t`.
- Updated `worktree.useRelativePaths` and `--relative-paths` descriptions.
- Reordered patches
- Link to v2: https://lore.kernel.org/r/20241028-wt_relative_options-v2-0-33a5021bd7bb@xxxxx
Changes in v2:
- Fixed a bug where repositories with valid extensions would be downgraded
to v0 during reinitialization, causing future operations to fail.
- Split patch [1/2] into three separate patches.
- Updated cover letter and commit messages.
- Updated documentation wording.
- Link to v1: https://lore.kernel.org/r/20241025-wt_relative_options-v1-0-c3005df76bf9@xxxxx
---
Caleb White (8):
setup: correctly reinitialize repository version
worktree: add `relativeWorktrees` extension
worktree: refactor infer_backlink return
worktree: add `write_worktree_linking_files()` function
worktree: add relative cli/config options to `add` command
worktree: add relative cli/config options to `move` command
worktree: add relative cli/config options to `repair` command
worktree: refactor `repair_worktree_after_gitdir_move()`
Documentation/config/extensions.txt | 6 ++
Documentation/config/worktree.txt | 10 +++
Documentation/git-worktree.txt | 8 ++
builtin/worktree.c | 29 ++++---
repository.c | 1 +
repository.h | 1 +
setup.c | 39 ++++++---
setup.h | 1 +
t/t0001-init.sh | 22 ++++-
t/t2400-worktree-add.sh | 45 +++++++++++
t/t2401-worktree-prune.sh | 3 +-
t/t2402-worktree-list.sh | 22 +++++
t/t2403-worktree-move.sh | 25 ++++++
t/t2406-worktree-repair.sh | 39 +++++++++
t/t2408-worktree-relative.sh | 39 ---------
t/t5504-fetch-receive-strict.sh | 6 +-
worktree.c | 157 ++++++++++++++++++++----------------
worktree.h | 22 ++++-
18 files changed, 333 insertions(+), 142 deletions(-)
---
base-commit: 090d24e9af6e9f59c3f7bee97c42bb1ae3c7f559
change-id: 20241025-wt_relative_options-afa41987bc32
Best regards,
