| Message ID | 20241028-wt_relative_options-v2-3-33a5021bd7bb@pm.me (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Allow relative worktree linking to be configured by the user | expand |
Hi Caleb On 28/10/2024 19:09, Caleb White wrote: > This patch expands the test coverage by adding cases that specifically > handle relative paths. These tests verify correct behavior in a variety > of operations, including: adding, listing, pruning, moving, and > repairing worktrees with relative paths configured. > > This also adds a test case for reinitializing a repository that has > relative worktrees. It's nice to see new tests being added. If they were added with the code changes they test that would help reader understand the changes being made I think. Best Wishes Phillip > Signed-off-by: Caleb White <cdwhite3@pm.me> > --- > t/t0001-init.sh | 17 +++++++++++++---- > t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++ > t/t2401-worktree-prune.sh | 3 ++- > t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++ > t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++ > t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++ > 6 files changed, 126 insertions(+), 5 deletions(-) > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -435,6 +435,7 @@ sep_git_dir_worktree () { > test_when_finished "rm -rf mainwt linkwt seprepo" && > git init mainwt && > test_commit -C mainwt gumby && > + git -C mainwt config worktree.useRelativePaths "$([ "$2" = "relative" ] && echo true || echo false)" && > git -C mainwt worktree add --detach ../linkwt && > git -C "$1" init --separate-git-dir ../seprepo && > git -C mainwt rev-parse --git-common-dir >expect && > @@ -442,12 +443,20 @@ sep_git_dir_worktree () { > test_cmp expect actual > } > > -test_expect_success 're-init to move gitdir with linked worktrees' ' > - sep_git_dir_worktree mainwt > +test_expect_success 're-init to move gitdir with linked worktrees (absolute)' ' > + sep_git_dir_worktree mainwt absolute > ' > > -test_expect_success 're-init to move gitdir within linked worktree' ' > - sep_git_dir_worktree linkwt > +test_expect_success 're-init to move gitdir within linked worktree (absolute)' ' > + sep_git_dir_worktree linkwt absolute > +' > + > +test_expect_success 're-init to move gitdir with linked worktrees (relative)' ' > + sep_git_dir_worktree mainwt relative > +' > + > +test_expect_success 're-init to move gitdir within linked worktree (relative)' ' > + sep_git_dir_worktree linkwt relative > ' > > test_expect_success MINGW '.git hidden' ' > diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh > index cfc4aeb1798c6d023909cec771e5b74e983af5ea..630c13230b3cc762ce8d943e22be8891aa2b1871 100755 > --- a/t/t2400-worktree-add.sh > +++ b/t/t2400-worktree-add.sh > @@ -1207,4 +1207,45 @@ test_expect_success '"add" with initialized submodule, with submodule.recurse se > git -C project-clone -c submodule.recurse worktree add ../project-5 > ' > > +test_expect_success 'can create worktrees with relative paths' ' > + test_when_finished "git worktree remove relative" && > + git 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 && > + echo "../../../relative/.git" >expect && > + test_cmp expect actual > + > +' > + > +test_expect_success 'can create worktrees with absolute paths' ' > + git config worktree.useRelativePaths true && > + git worktree add ./relative && > + cat relative/.git >actual && > + echo "gitdir: ../.git/worktrees/relative" >expect && > + test_cmp expect actual && > + git worktree add --no-relative-paths ./absolute && > + cat absolute/.git >actual && > + echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect && > + test_cmp expect actual > +' > + > +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 && > + cd .. && > + mv repo moved && > + cd moved/wt1 && > + git status >out 2>err && > + test_must_be_empty err > + ) > +' > + > test_done > diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh > index 976d048e3efc74be9cd909ce76d552b3944d2e10..5eb52b9abbf29514dc082c260ebb7a5e8e63aae0 100755 > --- a/t/t2401-worktree-prune.sh > +++ b/t/t2401-worktree-prune.sh > @@ -120,11 +120,12 @@ test_expect_success 'prune duplicate (main/linked)' ' > ! test -d .git/worktrees/wt > ' > > -test_expect_success 'not prune proper worktrees when run inside linked worktree' ' > +test_expect_success 'not prune proper worktrees inside linked worktree with relative paths' ' > test_when_finished rm -rf repo wt_ext && > git init repo && > ( > cd repo && > + git config worktree.useRelativePaths true && > echo content >file && > git add file && > git commit -m msg && > diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh > index 33ea9cb21ba07c9563530b54da06753eaa993fe2..780daa6cd6351f8fa9434619cc212aade8f01420 100755 > --- a/t/t2402-worktree-list.sh > +++ b/t/t2402-worktree-list.sh > @@ -261,6 +261,7 @@ test_expect_success 'broken main worktree still at the top' ' > ' > > test_expect_success 'linked worktrees are sorted' ' > + test_when_finished "rm -rf sorted" && > mkdir sorted && > git init sorted/main && > ( > @@ -280,6 +281,27 @@ test_expect_success 'linked worktrees are sorted' ' > test_cmp expected sorted/main/actual > ' > > +test_expect_success 'linked worktrees with relative paths are shown with absolute paths' ' > + test_when_finished "rm -rf sorted" && > + mkdir sorted && > + git init sorted/main && > + ( > + cd sorted/main && > + test_tick && > + test_commit new && > + git worktree add --relative-paths ../first && > + git worktree add ../second && > + git worktree list --porcelain >out && > + grep ^worktree out >actual > + ) && > + cat >expected <<-EOF && > + worktree $(pwd)/sorted/main > + worktree $(pwd)/sorted/first > + worktree $(pwd)/sorted/second > + EOF > + test_cmp expected sorted/main/actual > +' > + > test_expect_success 'worktree path when called in .git directory' ' > git worktree list >list1 && > git -C .git worktree list >list2 && > diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh > index 901342ea09b51a8e832f1109fbb737df84283aa2..6ce9ed3f1e6b3f73d2a290e770233eec30221fe5 100755 > --- a/t/t2403-worktree-move.sh > +++ b/t/t2403-worktree-move.sh > @@ -247,4 +247,26 @@ test_expect_success 'not remove a repo with initialized submodule' ' > ) > ' > > +test_expect_success 'move worktree with absolute path to relative path' ' > + git 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 && > + git worktree move relative relative2 && > + cat relative2/.git >actual && > + echo "gitdir: ../.git/worktrees/absolute" >expect && > + test_cmp expect actual > +' > + > +test_expect_success 'move worktree with relative path to absolute path' ' > + git 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_done > diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh > index 7686e60f6ad186519b275f11a5e14064c905b207..84451e903b2ef3c645c0311faf055c846588baf6 100755 > --- a/t/t2406-worktree-repair.sh > +++ b/t/t2406-worktree-repair.sh > @@ -216,4 +216,30 @@ test_expect_success 'repair copied main and linked worktrees' ' > test_cmp dup/linked.expect dup/linked/.git > ' > > +test_expect_success 'repair absolute worktree to use relative paths' ' > + test_when_finished "rm -rf main side sidemoved" && > + test_create_repo main && > + test_commit -C main init && > + git -C main worktree add --detach ../side && > + echo "../../../../sidemoved/.git" >expect-gitdir && > + echo "gitdir: ../main/.git/worktrees/side" >expect-gitfile && > + mv side sidemoved && > + git -C main worktree repair --relative-paths ../sidemoved && > + test_cmp expect-gitdir main/.git/worktrees/side/gitdir && > + test_cmp expect-gitfile sidemoved/.git > +' > + > +test_expect_success 'repair relative worktree to use absolute paths' ' > + test_when_finished "rm -rf main side sidemoved" && > + test_create_repo main && > + test_commit -C main init && > + git -C main worktree add --relative-paths --detach ../side && > + echo "$(pwd)/sidemoved/.git" >expect-gitdir && > + echo "gitdir: $(pwd)/main/.git/worktrees/side" >expect-gitfile && > + mv side sidemoved && > + git -C main worktree repair ../sidemoved && > + test_cmp expect-gitdir main/.git/worktrees/side/gitdir && > + test_cmp expect-gitfile sidemoved/.git > +' > + > test_done >
On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote: > Hi Caleb > > On 28/10/2024 19:09, Caleb White wrote: >> This patch expands the test coverage by adding cases that specifically >> handle relative paths. These tests verify correct behavior in a variety >> of operations, including: adding, listing, pruning, moving, and >> repairing worktrees with relative paths configured. >> >> This also adds a test case for reinitializing a repository that has >> relative worktrees. > > It's nice to see new tests being added. If they were added with the code > changes they test that would help reader understand the changes being > made I think. I had received feedback that the original patch was too large, so I was trying to split it up into smaller, more digestible pieces. I could go either way so it's really what the reviewers would prefer. Best, Caleb
On 29/10/2024 14:58, Caleb White wrote: > On Tue Oct 29, 2024 at 9:52 AM CDT, Phillip Wood wrote: >> Hi Caleb >> >> On 28/10/2024 19:09, Caleb White wrote: >>> This patch expands the test coverage by adding cases that specifically >>> handle relative paths. These tests verify correct behavior in a variety >>> of operations, including: adding, listing, pruning, moving, and >>> repairing worktrees with relative paths configured. >>> >>> This also adds a test case for reinitializing a repository that has >>> relative worktrees. >> >> It's nice to see new tests being added. If they were added with the code >> changes they test that would help reader understand the changes being >> made I think. > > I had received feedback that the original patch was too large, so I > was trying to split it up into smaller, more digestible pieces. One way to do that would be to convert the "add", "move" and "repair" subcommands in separate patches changing option handling and appropriate tests in each one rather than changing all subcommands and their tests at once. Best Wishes Phillip I could > go either way so it's really what the reviewers would prefer. > > Best, > Caleb >
On Mon, Oct 28, 2024 at 07:09:52PM +0000, Caleb White wrote: > This patch expands the test coverage by adding cases that specifically > handle relative paths. These tests verify correct behavior in a variety > of operations, including: adding, listing, pruning, moving, and > repairing worktrees with relative paths configured. > > This also adds a test case for reinitializing a repository that has > relative worktrees. > > Signed-off-by: Caleb White <cdwhite3@pm.me> > --- > t/t0001-init.sh | 17 +++++++++++++---- > t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++ > t/t2401-worktree-prune.sh | 3 ++- > t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++ > t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++ > t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++ > 6 files changed, 126 insertions(+), 5 deletions(-) > > diff --git a/t/t0001-init.sh b/t/t0001-init.sh > index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755 > --- a/t/t0001-init.sh > +++ b/t/t0001-init.sh > @@ -435,6 +435,7 @@ sep_git_dir_worktree () { > test_when_finished "rm -rf mainwt linkwt seprepo" && > git init mainwt && > test_commit -C mainwt gumby && > + git -C mainwt config worktree.useRelativePaths "$([ "$2" = "relative" ] && echo true || echo false)" && Can we avoid using '[' and perhaps split this out a little further. Perhaps: if test "relative" = $2 then git -C mainwt config worktree.useRelativePaths true else git -C mainwt config worktree.useRelativePaths false fi I think the duplication is more than worth the clarity here. Thanks, Taylor
On Tue Oct 29, 2024 at 6:00 PM CDT, Taylor Blau wrote: >> diff --git a/t/t0001-init.sh b/t/t0001-init.sh >> index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755 >> --- a/t/t0001-init.sh >> +++ b/t/t0001-init.sh >> @@ -435,6 +435,7 @@ sep_git_dir_worktree () { >> test_when_finished "rm -rf mainwt linkwt seprepo" && >> git init mainwt && >> test_commit -C mainwt gumby && >> + git -C mainwt config worktree.useRelativePaths "$([ "$2" = "relative" ] && echo true || echo false)" && > > Can we avoid using '[' and perhaps split this out a little further. > Perhaps: > > if test "relative" = $2 > then > git -C mainwt config worktree.useRelativePaths true > else > git -C mainwt config worktree.useRelativePaths false > fi > > I think the duplication is more than worth the clarity here. Sounds good, I'll make that change. Best,
On Tue Oct 29, 2024 at 10:43 AM CDT, phillip.wood123 wrote: >> I had received feedback that the original patch was too large, so I >> was trying to split it up into smaller, more digestible pieces. > > One way to do that would be to convert the "add", "move" and "repair" > subcommands in separate patches changing option handling and appropriate > tests in each one rather than changing all subcommands and their tests > at once. That makes sense. I'll implement that. Best, Caleb
diff --git a/t/t0001-init.sh b/t/t0001-init.sh index 0178aa62a41f1606f2382a4a10ab593ccf11e0e8..e21b9aa5e7f4599af8b20165b50896c9a49ba7ea 100755 --- a/t/t0001-init.sh +++ b/t/t0001-init.sh @@ -435,6 +435,7 @@ sep_git_dir_worktree () { test_when_finished "rm -rf mainwt linkwt seprepo" && git init mainwt && test_commit -C mainwt gumby && + git -C mainwt config worktree.useRelativePaths "$([ "$2" = "relative" ] && echo true || echo false)" && git -C mainwt worktree add --detach ../linkwt && git -C "$1" init --separate-git-dir ../seprepo && git -C mainwt rev-parse --git-common-dir >expect && @@ -442,12 +443,20 @@ sep_git_dir_worktree () { test_cmp expect actual } -test_expect_success 're-init to move gitdir with linked worktrees' ' - sep_git_dir_worktree mainwt +test_expect_success 're-init to move gitdir with linked worktrees (absolute)' ' + sep_git_dir_worktree mainwt absolute ' -test_expect_success 're-init to move gitdir within linked worktree' ' - sep_git_dir_worktree linkwt +test_expect_success 're-init to move gitdir within linked worktree (absolute)' ' + sep_git_dir_worktree linkwt absolute +' + +test_expect_success 're-init to move gitdir with linked worktrees (relative)' ' + sep_git_dir_worktree mainwt relative +' + +test_expect_success 're-init to move gitdir within linked worktree (relative)' ' + sep_git_dir_worktree linkwt relative ' test_expect_success MINGW '.git hidden' ' diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index cfc4aeb1798c6d023909cec771e5b74e983af5ea..630c13230b3cc762ce8d943e22be8891aa2b1871 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -1207,4 +1207,45 @@ test_expect_success '"add" with initialized submodule, with submodule.recurse se git -C project-clone -c submodule.recurse worktree add ../project-5 ' +test_expect_success 'can create worktrees with relative paths' ' + test_when_finished "git worktree remove relative" && + git 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 && + echo "../../../relative/.git" >expect && + test_cmp expect actual + +' + +test_expect_success 'can create worktrees with absolute paths' ' + git config worktree.useRelativePaths true && + git worktree add ./relative && + cat relative/.git >actual && + echo "gitdir: ../.git/worktrees/relative" >expect && + test_cmp expect actual && + git worktree add --no-relative-paths ./absolute && + cat absolute/.git >actual && + echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect && + test_cmp expect actual +' + +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 && + cd .. && + mv repo moved && + cd moved/wt1 && + git status >out 2>err && + test_must_be_empty err + ) +' + test_done diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh index 976d048e3efc74be9cd909ce76d552b3944d2e10..5eb52b9abbf29514dc082c260ebb7a5e8e63aae0 100755 --- a/t/t2401-worktree-prune.sh +++ b/t/t2401-worktree-prune.sh @@ -120,11 +120,12 @@ test_expect_success 'prune duplicate (main/linked)' ' ! test -d .git/worktrees/wt ' -test_expect_success 'not prune proper worktrees when run inside linked worktree' ' +test_expect_success 'not prune proper worktrees inside linked worktree with relative paths' ' test_when_finished rm -rf repo wt_ext && git init repo && ( cd repo && + git config worktree.useRelativePaths true && echo content >file && git add file && git commit -m msg && diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh index 33ea9cb21ba07c9563530b54da06753eaa993fe2..780daa6cd6351f8fa9434619cc212aade8f01420 100755 --- a/t/t2402-worktree-list.sh +++ b/t/t2402-worktree-list.sh @@ -261,6 +261,7 @@ test_expect_success 'broken main worktree still at the top' ' ' test_expect_success 'linked worktrees are sorted' ' + test_when_finished "rm -rf sorted" && mkdir sorted && git init sorted/main && ( @@ -280,6 +281,27 @@ test_expect_success 'linked worktrees are sorted' ' test_cmp expected sorted/main/actual ' +test_expect_success 'linked worktrees with relative paths are shown with absolute paths' ' + test_when_finished "rm -rf sorted" && + mkdir sorted && + git init sorted/main && + ( + cd sorted/main && + test_tick && + test_commit new && + git worktree add --relative-paths ../first && + git worktree add ../second && + git worktree list --porcelain >out && + grep ^worktree out >actual + ) && + cat >expected <<-EOF && + worktree $(pwd)/sorted/main + worktree $(pwd)/sorted/first + worktree $(pwd)/sorted/second + EOF + test_cmp expected sorted/main/actual +' + test_expect_success 'worktree path when called in .git directory' ' git worktree list >list1 && git -C .git worktree list >list2 && diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh index 901342ea09b51a8e832f1109fbb737df84283aa2..6ce9ed3f1e6b3f73d2a290e770233eec30221fe5 100755 --- a/t/t2403-worktree-move.sh +++ b/t/t2403-worktree-move.sh @@ -247,4 +247,26 @@ test_expect_success 'not remove a repo with initialized submodule' ' ) ' +test_expect_success 'move worktree with absolute path to relative path' ' + git 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 && + git worktree move relative relative2 && + cat relative2/.git >actual && + echo "gitdir: ../.git/worktrees/absolute" >expect && + test_cmp expect actual +' + +test_expect_success 'move worktree with relative path to absolute path' ' + git 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_done diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh index 7686e60f6ad186519b275f11a5e14064c905b207..84451e903b2ef3c645c0311faf055c846588baf6 100755 --- a/t/t2406-worktree-repair.sh +++ b/t/t2406-worktree-repair.sh @@ -216,4 +216,30 @@ test_expect_success 'repair copied main and linked worktrees' ' test_cmp dup/linked.expect dup/linked/.git ' +test_expect_success 'repair absolute worktree to use relative paths' ' + test_when_finished "rm -rf main side sidemoved" && + test_create_repo main && + test_commit -C main init && + git -C main worktree add --detach ../side && + echo "../../../../sidemoved/.git" >expect-gitdir && + echo "gitdir: ../main/.git/worktrees/side" >expect-gitfile && + mv side sidemoved && + git -C main worktree repair --relative-paths ../sidemoved && + test_cmp expect-gitdir main/.git/worktrees/side/gitdir && + test_cmp expect-gitfile sidemoved/.git +' + +test_expect_success 'repair relative worktree to use absolute paths' ' + test_when_finished "rm -rf main side sidemoved" && + test_create_repo main && + test_commit -C main init && + git -C main worktree add --relative-paths --detach ../side && + echo "$(pwd)/sidemoved/.git" >expect-gitdir && + echo "gitdir: $(pwd)/main/.git/worktrees/side" >expect-gitfile && + mv side sidemoved && + git -C main worktree repair ../sidemoved && + test_cmp expect-gitdir main/.git/worktrees/side/gitdir && + test_cmp expect-gitfile sidemoved/.git +' + test_done
This patch expands the test coverage by adding cases that specifically handle relative paths. These tests verify correct behavior in a variety of operations, including: adding, listing, pruning, moving, and repairing worktrees with relative paths configured. This also adds a test case for reinitializing a repository that has relative worktrees. Signed-off-by: Caleb White <cdwhite3@pm.me> --- t/t0001-init.sh | 17 +++++++++++++---- t/t2400-worktree-add.sh | 41 +++++++++++++++++++++++++++++++++++++++++ t/t2401-worktree-prune.sh | 3 ++- t/t2402-worktree-list.sh | 22 ++++++++++++++++++++++ t/t2403-worktree-move.sh | 22 ++++++++++++++++++++++ t/t2406-worktree-repair.sh | 26 ++++++++++++++++++++++++++ 6 files changed, 126 insertions(+), 5 deletions(-)