[PATCH] tests: more completely restore the environment in syshgenv

Adam Simpkins simpkins at fb.com
Thu Jun 29 15:21:10 EDT 2017


On Jun 29, Matt Harbison wrote:
> On Wed, 28 Jun 2017 15:34:56 -0400, Adam Simpkins <simpkins at fb.com> wrote:
> 
> ># HG changeset patch
> ># User Adam Simpkins <simpkins at fb.com>
> ># Date 1498677802 25200
> >#      Wed Jun 28 12:23:22 2017 -0700
> ># Node ID 236c593941ab8034c500039c54bee0b3a19a0cef
> ># Parent  649cb52c8cd94ab98068004e40bf9947de62a0ce
> >tests: more completely restore the environment in syshgenv
> >
> ...
> 
> >diff --git a/tests/run-tests.py b/tests/run-tests.py
> >--- a/tests/run-tests.py
> >+++ b/tests/run-tests.py
> ...
> 
> >@@ -888,6 +896,30 @@
> >         else:
> >             return b'127.0.0.1'
> >+    def _genrestoreenv(self, testenv):
> >+        """Generate a script that can be used by tests to restore
> >the original
> >+        environment."""
> >+        # Put the restoreenv script inside self._threadtmp
> >+        scriptpath = os.path.join(self._threadtmp, 'restoreenv.sh')
> >+        testenv['HGTEST_RESTOREENV'] = scriptpath
> >+
> >+        # Only restore environment variable names that the shell allows
> >+        # us to export.
> >+        name_regex = re.compile('[a-zA-Z][a-zA-Z0-9_]*')
> 
> Did you mean for this regex to be anchored on both ends?  Windows
> complains, unless I explicitly test that '(' is not in the name, but
> I didn't want to leave a hole here:

Yes, you are right, I intended this to be anchored on both sides.  It
is currently anchored at the start due to using re.match(), but it
needs to be anchored at the end too.  I'll send out a patch to
explicitly add both ^ and $.

> --- c:/Users/Matt/projects/hg/tests/test-check-code.t
> +++ c:/Users/Matt/projects/hg/tests/test-check-code.t.err
> @@ -9,6 +9,8 @@
> 
>    $ syshg locate -X contrib/python-zstandard -X
> hgext/fsmonitor/pywatchman |
>    > sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false
> +
> C:\Users\Matt\AppData\Local\Temp\hgtests.jjx4mu\child1\restoreenv.sh:
> line 17: syntax error near unexpected token `X86'
> +
> C:\Users\Matt\AppData\Local\Temp\hgtests.jjx4mu\child1\restoreenv.sh:
> line 17: `PROGRAMFILES(X86)='C:\Program Files (x86)''
>    Skipping i18n/polib.py it has no-che?k-code (glob)
>    Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
>    Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)

That's actually a little interesting, since Windows appears to have an
environment variable with a '(' character in it.  I believe sh doesn't
allow setting variables with '(' characters.  For now I think we'll
just have to live with not restoring these variables.

> Additionally, I wonder if the '\' in the paths need to be escaped.
> Looking at restoreenv.sh has lines like this:
> 
> PSMODULEPATH='C:\Windows\system32\WindowsPowerShell\v1.0\Modules\'
> VS90COMNTOOLS='c:\Program Files (x86)\Microsoft Visual Studio
> 9.0\Common7\Tools\'
> COMMONPROGRAMFILES='C:\Program Files\Common Files'
> 
> My editor wants to color 'VS90COMNTOOLS=' as part of the string on
> the previous line.

That seems like an issue with your editor's coloring to me.
Backslashes inside single-quoted strings are not special characters,
and are treated literally:
http://pubs.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_02_02

-- 
Adam Simpkins
simpkins at fb.com


More information about the Mercurial-devel mailing list