| Message ID | 20240206225122.1095766-7-shyamthakkar001@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add-patch: '@' as a synonym for 'HEAD' | expand |
Hi Ghanshyam On 06/02/2024 22:50, Ghanshyam Thakkar wrote: > The Perl version of the add -i/-p commands has been removed since > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git > add--interactive", 2023-02-07) > > Therefore, Perl prerequisite in t2071-restore-patch and > t7105-reset-patch is not necessary. Thanks for adding this patch. If you do re-roll I've just noticed that one of the tests in t7106-reset-unborn-branch.sh and another in t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't think it is worth re-rolling just for that as we can clean them up separately if needed. Best Wishes Phillip > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > --- > t/t2071-restore-patch.sh | 26 +++++++++++++------------- > t/t7105-reset-patch.sh | 22 +++++++++++----------- > 2 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh > index dbbefc188d..27e85be40a 100755 > --- a/t/t2071-restore-patch.sh > +++ b/t/t2071-restore-patch.sh > @@ -4,7 +4,7 @@ test_description='git restore --patch' > > . ./lib-patch-mode.sh > > -test_expect_success PERL 'setup' ' > +test_expect_success 'setup' ' > mkdir dir && > echo parent >dir/foo && > echo dummy >bar && > @@ -16,28 +16,28 @@ test_expect_success PERL 'setup' ' > save_head > ' > > -test_expect_success PERL 'restore -p without pathspec is fine' ' > +test_expect_success 'restore -p without pathspec is fine' ' > echo q >cmd && > git restore -p <cmd > ' > > # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar' > > -test_expect_success PERL 'saying "n" does nothing' ' > +test_expect_success 'saying "n" does nothing' ' > set_and_save_state dir/foo work head && > test_write_lines n n | git restore -p && > verify_saved_state bar && > verify_saved_state dir/foo > ' > > -test_expect_success PERL 'git restore -p' ' > +test_expect_success 'git restore -p' ' > set_and_save_state dir/foo work head && > test_write_lines n y | git restore -p && > verify_saved_state bar && > verify_state dir/foo head head > ' > > -test_expect_success PERL 'git restore -p with staged changes' ' > +test_expect_success 'git restore -p with staged changes' ' > set_state dir/foo work index && > test_write_lines n y | git restore -p && > verify_saved_state bar && > @@ -56,7 +56,7 @@ do > ' > done > > -test_expect_success PERL 'git restore -p --source=HEAD^' ' > +test_expect_success 'git restore -p --source=HEAD^' ' > set_state dir/foo work index && > # the third n is to get out in case it mistakenly does not apply > test_write_lines n y n | git restore -p --source=HEAD^ && > @@ -64,7 +64,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^' ' > verify_state dir/foo parent index > ' > > -test_expect_success PERL 'git restore -p --source=HEAD^...' ' > +test_expect_success 'git restore -p --source=HEAD^...' ' > set_state dir/foo work index && > # the third n is to get out in case it mistakenly does not apply > test_write_lines n y n | git restore -p --source=HEAD^... && > @@ -72,7 +72,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^...' ' > verify_state dir/foo parent index > ' > > -test_expect_success PERL 'git restore -p handles deletion' ' > +test_expect_success 'git restore -p handles deletion' ' > set_state dir/foo work index && > rm dir/foo && > test_write_lines n y | git restore -p && > @@ -85,21 +85,21 @@ test_expect_success PERL 'git restore -p handles deletion' ' > # dir/foo. There's always an extra 'n' to reject edits to dir/foo in > # the failure case (and thus get out of the loop). > > -test_expect_success PERL 'path limiting works: dir' ' > +test_expect_success 'path limiting works: dir' ' > set_state dir/foo work head && > test_write_lines y n | git restore -p dir && > verify_saved_state bar && > verify_state dir/foo head head > ' > > -test_expect_success PERL 'path limiting works: -- dir' ' > +test_expect_success 'path limiting works: -- dir' ' > set_state dir/foo work head && > test_write_lines y n | git restore -p -- dir && > verify_saved_state bar && > verify_state dir/foo head head > ' > > -test_expect_success PERL 'path limiting works: HEAD^ -- dir' ' > +test_expect_success 'path limiting works: HEAD^ -- dir' ' > set_state dir/foo work head && > # the third n is to get out in case it mistakenly does not apply > test_write_lines y n n | git restore -p --source=HEAD^ -- dir && > @@ -107,7 +107,7 @@ test_expect_success PERL 'path limiting works: HEAD^ -- dir' ' > verify_state dir/foo parent head > ' > > -test_expect_success PERL 'path limiting works: foo inside dir' ' > +test_expect_success 'path limiting works: foo inside dir' ' > set_state dir/foo work head && > # the third n is to get out in case it mistakenly does not apply > test_write_lines y n n | (cd dir && git restore -p foo) && > @@ -115,7 +115,7 @@ test_expect_success PERL 'path limiting works: foo inside dir' ' > verify_state dir/foo head head > ' > > -test_expect_success PERL 'none of this moved HEAD' ' > +test_expect_success 'none of this moved HEAD' ' > verify_saved_head > ' > > diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh > index 7147148138..3691b94d1b 100755 > --- a/t/t7105-reset-patch.sh > +++ b/t/t7105-reset-patch.sh > @@ -5,7 +5,7 @@ test_description='git reset --patch' > TEST_PASSES_SANITIZE_LEAK=true > . ./lib-patch-mode.sh > > -test_expect_success PERL 'setup' ' > +test_expect_success 'setup' ' > mkdir dir && > echo parent > dir/foo && > echo dummy > bar && > @@ -19,14 +19,14 @@ test_expect_success PERL 'setup' ' > > # note: bar sorts before foo, so the first 'n' is always to skip 'bar' > > -test_expect_success PERL 'saying "n" does nothing' ' > +test_expect_success 'saying "n" does nothing' ' > set_and_save_state dir/foo work work && > test_write_lines n n | git reset -p && > verify_saved_state dir/foo && > verify_saved_state bar > ' > > -test_expect_success PERL 'git reset -p' ' > +test_expect_success 'git reset -p' ' > test_write_lines n y | git reset -p >output && > verify_state dir/foo work head && > verify_saved_state bar && > @@ -43,28 +43,28 @@ do > ' > done > > -test_expect_success PERL 'git reset -p HEAD^' ' > +test_expect_success 'git reset -p HEAD^' ' > test_write_lines n y | git reset -p HEAD^ >output && > verify_state dir/foo work parent && > verify_saved_state bar && > test_grep "Apply" output > ' > > -test_expect_success PERL 'git reset -p HEAD^^{tree}' ' > +test_expect_success 'git reset -p HEAD^^{tree}' ' > test_write_lines n y | git reset -p HEAD^^{tree} >output && > verify_state dir/foo work parent && > verify_saved_state bar && > test_grep "Apply" output > ' > > -test_expect_success PERL 'git reset -p HEAD^:dir/foo (blob fails)' ' > +test_expect_success 'git reset -p HEAD^:dir/foo (blob fails)' ' > set_and_save_state dir/foo work work && > test_must_fail git reset -p HEAD^:dir/foo && > verify_saved_state dir/foo && > verify_saved_state bar > ' > > -test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' ' > +test_expect_success 'git reset -p aaaaaaaa (unknown fails)' ' > set_and_save_state dir/foo work work && > test_must_fail git reset -p aaaaaaaa && > verify_saved_state dir/foo && > @@ -76,27 +76,27 @@ test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' ' > # dir/foo. There's always an extra 'n' to reject edits to dir/foo in > # the failure case (and thus get out of the loop). > > -test_expect_success PERL 'git reset -p dir' ' > +test_expect_success 'git reset -p dir' ' > set_state dir/foo work work && > test_write_lines y n | git reset -p dir && > verify_state dir/foo work head && > verify_saved_state bar > ' > > -test_expect_success PERL 'git reset -p -- foo (inside dir)' ' > +test_expect_success 'git reset -p -- foo (inside dir)' ' > set_state dir/foo work work && > test_write_lines y n | (cd dir && git reset -p -- foo) && > verify_state dir/foo work head && > verify_saved_state bar > ' > > -test_expect_success PERL 'git reset -p HEAD^ -- dir' ' > +test_expect_success 'git reset -p HEAD^ -- dir' ' > test_write_lines y n | git reset -p HEAD^ -- dir && > verify_state dir/foo work parent && > verify_saved_state bar > ' > > -test_expect_success PERL 'none of this moved HEAD' ' > +test_expect_success 'none of this moved HEAD' ' > verify_saved_head > ' >
On 07/02/2024 10:50, Phillip Wood wrote: > Hi Ghanshyam > > On 06/02/2024 22:50, Ghanshyam Thakkar wrote: > > The Perl version of the add -i/-p commands has been removed since > > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git > > add--interactive", 2023-02-07) > > > > Therefore, Perl prerequisite in t2071-restore-patch and > > t7105-reset-patch is not necessary. > > Thanks for adding this patch. If you do re-roll I've just noticed that > one of the tests in t7106-reset-unborn-branch.sh and another in > t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't > think it is worth re-rolling just for that as we can clean them up > separately if needed. I didn't cast my net wide enough when I was grepping earlier, t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary PERL prerequisites Best Wishes Phillip
Phillip Wood <phillip.wood123@gmail.com> writes: > On 07/02/2024 10:50, Phillip Wood wrote: >> Hi Ghanshyam >> On 06/02/2024 22:50, Ghanshyam Thakkar wrote: >> > The Perl version of the add -i/-p commands has been removed since >> > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git >> > add--interactive", 2023-02-07) >> > >> > Therefore, Perl prerequisite in t2071-restore-patch and >> > t7105-reset-patch is not necessary. >> Thanks for adding this patch. If you do re-roll I've just noticed >> that one of the tests in t7106-reset-unborn-branch.sh and another in >> t2024-checkout-dwim.sh still have PERL prerequisites as well. I >> don't think it is worth re-rolling just for that as we can clean >> them up separately if needed. > > I didn't cast my net wide enough when I was grepping earlier, > t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary > PERL prerequisites Thanks for helping usher this topic forward. Very much appreciated.
On Wed, Feb 7, 2024 at 8:51 AM Phillip Wood <phillip.wood123@gmail.com> wrote: > On 07/02/2024 10:50, Phillip Wood wrote: > > On 06/02/2024 22:50, Ghanshyam Thakkar wrote: > > > The Perl version of the add -i/-p commands has been removed since > > > 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git > > > add--interactive", 2023-02-07) > > > > > > Therefore, Perl prerequisite in t2071-restore-patch and > > > t7105-reset-patch is not necessary. > > > > Thanks for adding this patch. If you do re-roll I've just noticed that > > one of the tests in t7106-reset-unborn-branch.sh and another in > > t2024-checkout-dwim.sh still have PERL prerequisites as well. I don't > > think it is worth re-rolling just for that as we can clean them up > > separately if needed. > > I didn't cast my net wide enough when I was grepping earlier, > t7514-commit-patch.sh and t3904-stash-patch.sh also have unnecessary > PERL prerequisites Additionally, patch [2/3] drops a PERL prerequisite when it moves an existing test into a loop, but the removal of the prerequisite is not mentioned in the commit message. Presumably, the relocation-into-loop and prerequisite-removal should have been done separately (in patches [2/3] and [3/3], respectively), and that's how I'd suggest doing it.
diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh index dbbefc188d..27e85be40a 100755 --- a/t/t2071-restore-patch.sh +++ b/t/t2071-restore-patch.sh @@ -4,7 +4,7 @@ test_description='git restore --patch' . ./lib-patch-mode.sh -test_expect_success PERL 'setup' ' +test_expect_success 'setup' ' mkdir dir && echo parent >dir/foo && echo dummy >bar && @@ -16,28 +16,28 @@ test_expect_success PERL 'setup' ' save_head ' -test_expect_success PERL 'restore -p without pathspec is fine' ' +test_expect_success 'restore -p without pathspec is fine' ' echo q >cmd && git restore -p <cmd ' # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar' -test_expect_success PERL 'saying "n" does nothing' ' +test_expect_success 'saying "n" does nothing' ' set_and_save_state dir/foo work head && test_write_lines n n | git restore -p && verify_saved_state bar && verify_saved_state dir/foo ' -test_expect_success PERL 'git restore -p' ' +test_expect_success 'git restore -p' ' set_and_save_state dir/foo work head && test_write_lines n y | git restore -p && verify_saved_state bar && verify_state dir/foo head head ' -test_expect_success PERL 'git restore -p with staged changes' ' +test_expect_success 'git restore -p with staged changes' ' set_state dir/foo work index && test_write_lines n y | git restore -p && verify_saved_state bar && @@ -56,7 +56,7 @@ do ' done -test_expect_success PERL 'git restore -p --source=HEAD^' ' +test_expect_success 'git restore -p --source=HEAD^' ' set_state dir/foo work index && # the third n is to get out in case it mistakenly does not apply test_write_lines n y n | git restore -p --source=HEAD^ && @@ -64,7 +64,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^' ' verify_state dir/foo parent index ' -test_expect_success PERL 'git restore -p --source=HEAD^...' ' +test_expect_success 'git restore -p --source=HEAD^...' ' set_state dir/foo work index && # the third n is to get out in case it mistakenly does not apply test_write_lines n y n | git restore -p --source=HEAD^... && @@ -72,7 +72,7 @@ test_expect_success PERL 'git restore -p --source=HEAD^...' ' verify_state dir/foo parent index ' -test_expect_success PERL 'git restore -p handles deletion' ' +test_expect_success 'git restore -p handles deletion' ' set_state dir/foo work index && rm dir/foo && test_write_lines n y | git restore -p && @@ -85,21 +85,21 @@ test_expect_success PERL 'git restore -p handles deletion' ' # dir/foo. There's always an extra 'n' to reject edits to dir/foo in # the failure case (and thus get out of the loop). -test_expect_success PERL 'path limiting works: dir' ' +test_expect_success 'path limiting works: dir' ' set_state dir/foo work head && test_write_lines y n | git restore -p dir && verify_saved_state bar && verify_state dir/foo head head ' -test_expect_success PERL 'path limiting works: -- dir' ' +test_expect_success 'path limiting works: -- dir' ' set_state dir/foo work head && test_write_lines y n | git restore -p -- dir && verify_saved_state bar && verify_state dir/foo head head ' -test_expect_success PERL 'path limiting works: HEAD^ -- dir' ' +test_expect_success 'path limiting works: HEAD^ -- dir' ' set_state dir/foo work head && # the third n is to get out in case it mistakenly does not apply test_write_lines y n n | git restore -p --source=HEAD^ -- dir && @@ -107,7 +107,7 @@ test_expect_success PERL 'path limiting works: HEAD^ -- dir' ' verify_state dir/foo parent head ' -test_expect_success PERL 'path limiting works: foo inside dir' ' +test_expect_success 'path limiting works: foo inside dir' ' set_state dir/foo work head && # the third n is to get out in case it mistakenly does not apply test_write_lines y n n | (cd dir && git restore -p foo) && @@ -115,7 +115,7 @@ test_expect_success PERL 'path limiting works: foo inside dir' ' verify_state dir/foo head head ' -test_expect_success PERL 'none of this moved HEAD' ' +test_expect_success 'none of this moved HEAD' ' verify_saved_head ' diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh index 7147148138..3691b94d1b 100755 --- a/t/t7105-reset-patch.sh +++ b/t/t7105-reset-patch.sh @@ -5,7 +5,7 @@ test_description='git reset --patch' TEST_PASSES_SANITIZE_LEAK=true . ./lib-patch-mode.sh -test_expect_success PERL 'setup' ' +test_expect_success 'setup' ' mkdir dir && echo parent > dir/foo && echo dummy > bar && @@ -19,14 +19,14 @@ test_expect_success PERL 'setup' ' # note: bar sorts before foo, so the first 'n' is always to skip 'bar' -test_expect_success PERL 'saying "n" does nothing' ' +test_expect_success 'saying "n" does nothing' ' set_and_save_state dir/foo work work && test_write_lines n n | git reset -p && verify_saved_state dir/foo && verify_saved_state bar ' -test_expect_success PERL 'git reset -p' ' +test_expect_success 'git reset -p' ' test_write_lines n y | git reset -p >output && verify_state dir/foo work head && verify_saved_state bar && @@ -43,28 +43,28 @@ do ' done -test_expect_success PERL 'git reset -p HEAD^' ' +test_expect_success 'git reset -p HEAD^' ' test_write_lines n y | git reset -p HEAD^ >output && verify_state dir/foo work parent && verify_saved_state bar && test_grep "Apply" output ' -test_expect_success PERL 'git reset -p HEAD^^{tree}' ' +test_expect_success 'git reset -p HEAD^^{tree}' ' test_write_lines n y | git reset -p HEAD^^{tree} >output && verify_state dir/foo work parent && verify_saved_state bar && test_grep "Apply" output ' -test_expect_success PERL 'git reset -p HEAD^:dir/foo (blob fails)' ' +test_expect_success 'git reset -p HEAD^:dir/foo (blob fails)' ' set_and_save_state dir/foo work work && test_must_fail git reset -p HEAD^:dir/foo && verify_saved_state dir/foo && verify_saved_state bar ' -test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' ' +test_expect_success 'git reset -p aaaaaaaa (unknown fails)' ' set_and_save_state dir/foo work work && test_must_fail git reset -p aaaaaaaa && verify_saved_state dir/foo && @@ -76,27 +76,27 @@ test_expect_success PERL 'git reset -p aaaaaaaa (unknown fails)' ' # dir/foo. There's always an extra 'n' to reject edits to dir/foo in # the failure case (and thus get out of the loop). -test_expect_success PERL 'git reset -p dir' ' +test_expect_success 'git reset -p dir' ' set_state dir/foo work work && test_write_lines y n | git reset -p dir && verify_state dir/foo work head && verify_saved_state bar ' -test_expect_success PERL 'git reset -p -- foo (inside dir)' ' +test_expect_success 'git reset -p -- foo (inside dir)' ' set_state dir/foo work work && test_write_lines y n | (cd dir && git reset -p -- foo) && verify_state dir/foo work head && verify_saved_state bar ' -test_expect_success PERL 'git reset -p HEAD^ -- dir' ' +test_expect_success 'git reset -p HEAD^ -- dir' ' test_write_lines y n | git reset -p HEAD^ -- dir && verify_state dir/foo work parent && verify_saved_state bar ' -test_expect_success PERL 'none of this moved HEAD' ' +test_expect_success 'none of this moved HEAD' ' verify_saved_head '
The Perl version of the add -i/-p commands has been removed since 20b813d (add: remove "add.interactive.useBuiltin" & Perl "git add--interactive", 2023-02-07) Therefore, Perl prerequisite in t2071-restore-patch and t7105-reset-patch is not necessary. Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- t/t2071-restore-patch.sh | 26 +++++++++++++------------- t/t7105-reset-patch.sh | 22 +++++++++++----------- 2 files changed, 24 insertions(+), 24 deletions(-)