Patchwork [2,of,8] tests: quote paths in shell script hooks

login
register
mail settings
Submitter Matt Harbison
Date April 2, 2017, 11:12 p.m.
Message ID <54fd6d15228e8e2f8e73.1491174768@Envy>
Download mbox | patch
Permalink /patch/19912/
State Accepted
Headers show

Comments

Matt Harbison - April 2, 2017, 11:12 p.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1491074606 14400
#      Sat Apr 01 15:23:26 2017 -0400
# Node ID 54fd6d15228e8e2f8e735beac0db0930c60af9ad
# Parent  46f601b676cecfcc318f6852fe6ab6962703d2be
tests: quote paths in shell script hooks

Without the quoting, MSYS will remove the '\' directory separators, and the repo
can't be opened.
Jun Wu - April 3, 2017, 12:41 a.m.
Excerpts from Matt Harbison's message of 2017-04-02 19:12:48 -0400:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1491074606 14400
> #      Sat Apr 01 15:23:26 2017 -0400
> # Node ID 54fd6d15228e8e2f8e735beac0db0930c60af9ad
> # Parent  46f601b676cecfcc318f6852fe6ab6962703d2be
> tests: quote paths in shell script hooks
> 
> Without the quoting, MSYS will remove the '\' directory separators, and the repo
> can't be opened.
> 
> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
> --- a/tests/test-bookmarks.t
> +++ b/tests/test-bookmarks.t
> @@ -924,9 +924,9 @@
>  
>    $ cat > $TESTTMP/checkpending.sh <<EOF
>    > echo "@repo"
> -  > hg -R $TESTTMP/repo bookmarks
> +  > hg -R "$TESTTMP/repo" bookmarks
>    > echo "@unrelated"
> -  > hg -R $TESTTMP/unrelated bookmarks
> +  > hg -R "$TESTTMP/unrelated" bookmarks

Maybe this is worth a check-code rule.

>    > exit 1 # to avoid adding new bookmark for subsequent tests
>    > EOF
>  
> diff --git a/tests/test-hook.t b/tests/test-hook.t
> --- a/tests/test-hook.t
> +++ b/tests/test-hook.t
> @@ -857,9 +857,9 @@
>  
>    $ cat > $TESTTMP/checkpending.sh <<EOF
>    > echo '@a'
> -  > hg -R $TESTTMP/a tip -q
> +  > hg -R "$TESTTMP/a" tip -q
>    > echo '@a/nested'
> -  > hg -R $TESTTMP/a/nested tip -q
> +  > hg -R "$TESTTMP/a/nested" tip -q
>    > exit 1 # to avoid adding new revision for subsequent tests
>    > EOF
>    $ hg init nested
> diff --git a/tests/test-phases.t b/tests/test-phases.t
> --- a/tests/test-phases.t
> +++ b/tests/test-phases.t
> @@ -609,9 +609,9 @@
>  
>    $ cat > $TESTTMP/checkpending.sh <<EOF
>    > echo '@initialrepo'
> -  > hg -R $TESTTMP/initialrepo phase 7
> +  > hg -R "$TESTTMP/initialrepo" phase 7
>    > echo '@push-dest'
> -  > hg -R $TESTTMP/push-dest phase 6
> +  > hg -R "$TESTTMP/push-dest" phase 6
>    > exit 1 # to avoid changing phase for subsequent tests
>    > EOF
>    $ cd ../initialrepo
> diff --git a/tests/test-share.t b/tests/test-share.t
> --- a/tests/test-share.t
> +++ b/tests/test-share.t
> @@ -168,11 +168,11 @@
>  
>    $ cat > $TESTTMP/checkbookmarks.sh <<EOF
>    > echo "@repo1"
> -  > hg -R $TESTTMP/repo1 bookmarks
> +  > hg -R "$TESTTMP/repo1" bookmarks
>    > echo "@repo2"
> -  > hg -R $TESTTMP/repo2 bookmarks
> +  > hg -R "$TESTTMP/repo2" bookmarks
>    > echo "@repo3"
> -  > hg -R $TESTTMP/repo3 bookmarks
> +  > hg -R "$TESTTMP/repo3" bookmarks
>    > exit 1 # to avoid adding new bookmark for subsequent tests
>    > EOF
>
Matt Harbison - April 3, 2017, 12:45 a.m.
On Sun, 02 Apr 2017 20:41:29 -0400, Jun Wu <quark@fb.com> wrote:

> Excerpts from Matt Harbison's message of 2017-04-02 19:12:48 -0400:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1491074606 14400
>> #      Sat Apr 01 15:23:26 2017 -0400
>> # Node ID 54fd6d15228e8e2f8e735beac0db0930c60af9ad
>> # Parent  46f601b676cecfcc318f6852fe6ab6962703d2be
>> tests: quote paths in shell script hooks
>>
>> Without the quoting, MSYS will remove the '\' directory separators, and  
>> the repo
>> can't be opened.
>>
>> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
>> --- a/tests/test-bookmarks.t
>> +++ b/tests/test-bookmarks.t
>> @@ -924,9 +924,9 @@
>>
>>    $ cat > $TESTTMP/checkpending.sh <<EOF
>>    > echo "@repo"
>> -  > hg -R $TESTTMP/repo bookmarks
>> +  > hg -R "$TESTTMP/repo" bookmarks
>>    > echo "@unrelated"
>> -  > hg -R $TESTTMP/unrelated bookmarks
>> +  > hg -R "$TESTTMP/unrelated" bookmarks
>
> Maybe this is worth a check-code rule.

It would definitely be nice, but I'm not sure how to do that and limit the  
scope.  Not every instance of $TESTTMP/.. needs quoting- just the ones in  
things that will be executed by the shell.
Katsunori FUJIWARA - April 4, 2017, 5:39 p.m.
At Sun, 02 Apr 2017 20:45:58 -0400,
Matt Harbison wrote:
> 
> On Sun, 02 Apr 2017 20:41:29 -0400, Jun Wu <quark@fb.com> wrote:
> 
> > Excerpts from Matt Harbison's message of 2017-04-02 19:12:48 -0400:
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison@yahoo.com>
> >> # Date 1491074606 14400
> >> #      Sat Apr 01 15:23:26 2017 -0400
> >> # Node ID 54fd6d15228e8e2f8e735beac0db0930c60af9ad
> >> # Parent  46f601b676cecfcc318f6852fe6ab6962703d2be
> >> tests: quote paths in shell script hooks
> >>
> >> Without the quoting, MSYS will remove the '\' directory separators, and  
> >> the repo
> >> can't be opened.
> >>
> >> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
> >> --- a/tests/test-bookmarks.t
> >> +++ b/tests/test-bookmarks.t
> >> @@ -924,9 +924,9 @@
> >>
> >>    $ cat > $TESTTMP/checkpending.sh <<EOF
> >>    > echo "@repo"
> >> -  > hg -R $TESTTMP/repo bookmarks
> >> +  > hg -R "$TESTTMP/repo" bookmarks
> >>    > echo "@unrelated"
> >> -  > hg -R $TESTTMP/unrelated bookmarks
> >> +  > hg -R "$TESTTMP/unrelated" bookmarks
> >
> > Maybe this is worth a check-code rule.
> 
> It would definitely be nice, but I'm not sure how to do that and limit the  
> scope.  Not every instance of $TESTTMP/.. needs quoting- just the ones in  
> things that will be executed by the shell.

With current check-code implementation, all here-doc fragments in *.t
script are completely ignored, because all non-whitespace letters in
here-doc are replaced with 'x' by rephere() at filtering stage of
checkfile().

  https://www.mercurial-scm.org/repo/hg/file/68f263f52d2e/contrib/check-code.py#l96

This ignorance may cause overlooking outdated here-doc python code in
*.t script, too.

In fact, I have a suspending series to check here-doc fragments in *.t
script. I'll finish it in a hurry!

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Matt Harbison - April 5, 2017, 3:34 a.m.
On Tue, 04 Apr 2017 13:39:54 -0400, FUJIWARA Katsunori  
<foozy@lares.dti.ne.jp> wrote:

> At Sun, 02 Apr 2017 20:45:58 -0400,
> Matt Harbison wrote:
>>
>> On Sun, 02 Apr 2017 20:41:29 -0400, Jun Wu <quark@fb.com> wrote:
>>
>> > Excerpts from Matt Harbison's message of 2017-04-02 19:12:48 -0400:
>> >> # HG changeset patch
>> >> # User Matt Harbison <matt_harbison@yahoo.com>
>> >> # Date 1491074606 14400
>> >> #      Sat Apr 01 15:23:26 2017 -0400
>> >> # Node ID 54fd6d15228e8e2f8e735beac0db0930c60af9ad
>> >> # Parent  46f601b676cecfcc318f6852fe6ab6962703d2be
>> >> tests: quote paths in shell script hooks
>> >>
>> >> Without the quoting, MSYS will remove the '\' directory separators,  
>> and
>> >> the repo
>> >> can't be opened.
>> >>
>> >> diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
>> >> --- a/tests/test-bookmarks.t
>> >> +++ b/tests/test-bookmarks.t
>> >> @@ -924,9 +924,9 @@
>> >>
>> >>    $ cat > $TESTTMP/checkpending.sh <<EOF
>> >>    > echo "@repo"
>> >> -  > hg -R $TESTTMP/repo bookmarks
>> >> +  > hg -R "$TESTTMP/repo" bookmarks
>> >>    > echo "@unrelated"
>> >> -  > hg -R $TESTTMP/unrelated bookmarks
>> >> +  > hg -R "$TESTTMP/unrelated" bookmarks
>> >
>> > Maybe this is worth a check-code rule.
>>
>> It would definitely be nice, but I'm not sure how to do that and limit  
>> the
>> scope.  Not every instance of $TESTTMP/.. needs quoting- just the ones  
>> in
>> things that will be executed by the shell.
>
> With current check-code implementation, all here-doc fragments in *.t
> script are completely ignored, because all non-whitespace letters in
> here-doc are replaced with 'x' by rephere() at filtering stage of
> checkfile().
>
>   https://www.mercurial-scm.org/repo/hg/file/68f263f52d2e/contrib/check-code.py#l96
>
> This ignorance may cause overlooking outdated here-doc python code in
> *.t script, too.
>
> In fact, I have a suspending series to check here-doc fragments in *.t
> script. I'll finish it in a hurry!

OK, thanks!  No rush from me though.  There are still more than a handful  
of broken tests on Windows, so I'm not too worried about this case.


>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t
--- a/tests/test-bookmarks.t
+++ b/tests/test-bookmarks.t
@@ -924,9 +924,9 @@ 
 
   $ cat > $TESTTMP/checkpending.sh <<EOF
   > echo "@repo"
-  > hg -R $TESTTMP/repo bookmarks
+  > hg -R "$TESTTMP/repo" bookmarks
   > echo "@unrelated"
-  > hg -R $TESTTMP/unrelated bookmarks
+  > hg -R "$TESTTMP/unrelated" bookmarks
   > exit 1 # to avoid adding new bookmark for subsequent tests
   > EOF
 
diff --git a/tests/test-hook.t b/tests/test-hook.t
--- a/tests/test-hook.t
+++ b/tests/test-hook.t
@@ -857,9 +857,9 @@ 
 
   $ cat > $TESTTMP/checkpending.sh <<EOF
   > echo '@a'
-  > hg -R $TESTTMP/a tip -q
+  > hg -R "$TESTTMP/a" tip -q
   > echo '@a/nested'
-  > hg -R $TESTTMP/a/nested tip -q
+  > hg -R "$TESTTMP/a/nested" tip -q
   > exit 1 # to avoid adding new revision for subsequent tests
   > EOF
   $ hg init nested
diff --git a/tests/test-phases.t b/tests/test-phases.t
--- a/tests/test-phases.t
+++ b/tests/test-phases.t
@@ -609,9 +609,9 @@ 
 
   $ cat > $TESTTMP/checkpending.sh <<EOF
   > echo '@initialrepo'
-  > hg -R $TESTTMP/initialrepo phase 7
+  > hg -R "$TESTTMP/initialrepo" phase 7
   > echo '@push-dest'
-  > hg -R $TESTTMP/push-dest phase 6
+  > hg -R "$TESTTMP/push-dest" phase 6
   > exit 1 # to avoid changing phase for subsequent tests
   > EOF
   $ cd ../initialrepo
diff --git a/tests/test-share.t b/tests/test-share.t
--- a/tests/test-share.t
+++ b/tests/test-share.t
@@ -168,11 +168,11 @@ 
 
   $ cat > $TESTTMP/checkbookmarks.sh <<EOF
   > echo "@repo1"
-  > hg -R $TESTTMP/repo1 bookmarks
+  > hg -R "$TESTTMP/repo1" bookmarks
   > echo "@repo2"
-  > hg -R $TESTTMP/repo2 bookmarks
+  > hg -R "$TESTTMP/repo2" bookmarks
   > echo "@repo3"
-  > hg -R $TESTTMP/repo3 bookmarks
+  > hg -R "$TESTTMP/repo3" bookmarks
   > exit 1 # to avoid adding new bookmark for subsequent tests
   > EOF