Merge branch 'master' into jbrill-msvc-fixes

Manually resolved conflicts in CHANGES.txt and SCons/Tool/MSCommon/vc.py.

Author: jcbrill

CHANGES.txt

@@ -38,6 +38,10 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       module was refactored.
     - Add arm64 to the MSVS supported architectures list for VS2017 and later to be
       consistent with the current documentation of MSVS_ARCH.
+    - Fix an issue with an unhandled MissingConfiguration exception due to an msvc
+      registry query that returns a path that does not exist.  Multiple invocation
+      paths were not prepared to handle the MissingConfiguration exception.  The
+      MissingConfiguration exception type was removed.
     - For msvc version specifications without an 'Exp' suffix, an express installation
       is used when no other edition is detected for the msvc version.
     - VS2015 Express (14.1Exp) may not have been detected. The registry keys written
@@ -110,7 +114,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       If YACC_GRAPH_FILE_SUFFIX is not set, it will respect YACCVCGFILESUFFIX.
 
   From Philipp Maierhöfer:
-    - Fix gfortran tool initialization. Defaults to using binary named gfortran 
+    - Fix gfortran tool initialization. Defaults to using binary named gfortran
       as would be expected, and properly set's SHFORTRAN flags to include -fPIC
       where previously it was only doing so for the other fortran versions (F77,..)
 
@@ -194,6 +198,13 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       than source).  Docs updated.  Fixes #4326 and #4327.
     - Cleaned up dblite module (checker warnings, etc.).
     - Some cleanup in the FortranCommon tool.
+    - Rewrite the internal _subproc routine - a new scons_subproc_run() now
+      makes use of Python's subprocess.run in a more natural way, getting
+      around some of the issues with attempted context manager use, fetching
+      output, etc. - we let the subprocess module do the hard work,
+      since it's well debugged and supported.  _subproc is no longer
+      called by internal code, but remains in place in case there are builds
+      which call to it (even though it was never "published API").
     - Changed the message about scons -H to clarify it shows built-in options.
     - Improve CacheDir() Documentation.
     - Release-building setup tweaked. (most of) the targets listed in
@@ -217,9 +228,17 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       (Enabled/specified by SCONS_CACHE_MSVC_CONFIG).
       The existing cache will be discarded if there's a decode error reading it.
       It's possible there's a race condition creating this issue in certain CI
-      builds.
-      Also add a simple filesystem-based locking protocol to try to
-      avoide the problem occuring.
+      builds.  Also add a simple filesystem-based locking protocol to try to
+      avoid the problem occuring.
+    - Update the first two chapters on building with SCons in the User Guide.
+    - Update docs on Export/Import - make sure mutable/immutable status has
+      been mentioned.
+    - Some cleanup to the Util package, including renaming SCons.Util.types
+      to SCons.Util.sctypes to avoid any possible confusion with the
+      Python stdlib types module.
+    - TeX tests: skip tests that use makeindex or epstopdf not installed, or
+      if `kpsewhich glossaries.sty` fails.
+
 
   From Jonathon Reinhart:
     - Fix another instance of `int main()` in CheckLib() causing failures

RELEASE.txt

@@ -80,6 +80,12 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   The "--warn=missing-sconscript" commandline option is no longer available
   as the warning was part of the transitional phase.
 - Add missing directories to searched paths for mingw installs
+- SCons.Util.types renamed to to SCons.Util.sctypes to avoid any possible
+  confusion with the Python stdlib "types" module. Note that it was briefly
+  (for 4.5.x only) possible to import directly from SCons.Util.types,
+  although the preferred usage remains to import from SCons.Util only.
+  Any code that did the direct import will have to change to import from
+  SCons.Util.sctypes.
 
 FIXES
 -----
@@ -128,7 +134,7 @@ FIXES
   with the python logging module was refactored to prevent test failure.
 - MSVS: Add arm64 to the MSVS supported architectures list for VS2017 and later to be
   consistent with the current documentation of MSVS_ARCH.
-- FORTRAN: Fix gfortran tool initialization. Defaults to using binary named gfortran 
+- FORTRAN: Fix gfortran tool initialization. Defaults to using binary named gfortran
   as would be expected, and properly set's SHFORTRAN flags to include -fPIC
   where previously it was only doing so for the other fortran versions (F77,..)
 - MSCommon: Added more error handling while reading msvc config cache.
@@ -171,6 +177,9 @@ DOCUMENTATION
   usage, which had no problem).
 - Changed the message about scons -H to clarify it shows built-in options only.
 - Cachedir description updated.
+- Updated the first two chapters on building with SCons in the User Guide.
+- Clarify that exported variables are shared - if mutable, changes made in
+  an SConscript are visible everywhere that takes a refereence to that object.
 
 DEVELOPMENT
 -----------
@@ -185,6 +194,8 @@ DEVELOPMENT
   doesn't use so it can be instantiated by unittests and others.
 - Added more type annotations to internal routines.
 - Cleaned up dblite module (checker warnings, etc.).
+- TeX tests: skip tests that use makeindex or epstopdf not installed, or
+  if `kpsewhich glossaries.sty` fails.
 
 Thanks to the following contributors listed below for their contributions to this release.
 ==========================================================================================

SCons/Action.py

@@ -824,13 +824,120 @@ def _resolve_shell_env(env, target, source):
     return ENV
 
 
-def _subproc(scons_env, cmd, error: str='ignore', **kw):
-    """Wrapper for subprocess which pulls from construction env.
+def scons_subproc_run(
+    scons_env, *args, error: str = None, **kwargs
+) -> subprocess.CompletedProcess:
+    """Run a command with arguments using an SCons execution environment.
+
+    Does an underlyng call to :func:`subprocess.run` to run a command and
+    returns a :class:`subprocess.CompletedProcess` instance with the results.
+    Use when an external command needs to run in an "SCons context" -
+    that is, with a crafted execution environment, rather than the user's
+    existing environment, particularly when you need to collect output
+    back. Typical case: run a tool's "version" option to find out which
+    version you have installed.
+
+    If supplied, the ``env`` keyword argument passes an
+    execution environment to process into appropriate form before it is
+    supplied to :mod:`subprocess`; if omitted, *scons_env* is used to derive
+    a suitable default.  The other keyword arguments are passed through,
+    except that SCons legacy ``error` keyword is remapped to
+    the subprocess ``check` keyword.  The caller is responsible for
+    setting up the desired arguments  for :func:`subprocess.run`.
+
+    This function retains the legacy behavior of returning something
+    vaguely usable even in the face of complete failure: it synthesizes
+    a :class:`~subprocess.CompletedProcess` instance in this case.
+
+    A subset of interesting keyword arguments follows; see the Python
+    documentation of :mod:`subprocess` for a complete list.
+
+    Keyword Arguments:
+       stdout: (and *stderr*, *stdin*) if set to :const:`subprocess.PIPE`.
+          send input to or collect output from the relevant stream in
+          the subprocess; the default ``None`` does no redirection
+          (i.e. output or errors may go to the console or log file,
+          but is not captured); if set to :const:`subprocess.DEVNULL`
+          they are explicitly thrown away. ``capture_output`` is a
+          synonym for setting both ``stdout`` and ``stderr``
+          to :const:`~subprocess.PIPE`.
+       text: open *stdin*, *stdout*, *stderr* in text mode. Default
+          is binary mode. ``universal_newlines`` is a synonym -
+          note ``text`` is not understood before Python 3.7.
+       encoding: specifies an encoding. Changes to text mode.
+       errors: specified error handling. Changes to text mode.
+       input: a byte sequence to be passed to *stdin*, unless text
+          mode is enabled, in which case it must be a string.
+       shell: if true, the command is executed through the shell.
+       check: if true and the subprocess exits with a non-zero exit
+          code, raise a :exc:`subprocess.CalledProcessError` exception.
+          Otherwise (the default) in case of an :exc:`OSError`, report the
+          exit code in the :class:`~Subprocess.CompletedProcess` instance.
+
+    .. versionadded:: 4.6
+    """
+    # Figure out the execution environment to use
+    ENV = kwargs.get('env', None)
+    if ENV is None:
+        ENV = get_default_ENV(scons_env)
+    kwargs['env'] = SCons.Util.sanitize_shell_env(ENV)
+
+    # backwards-compat with _subproc: accept 'error', map it to
+    # ``check`` and remove, since it would not be recognized by run()
+    check = kwargs.get('check')
+    if check and error:
+        raise ValueError('error and check arguments may not both be used.')
+    if check is None:
+        check = False  # always supply some value, to pacify checkers (pylint).
+    if error is not None:
+        if error == 'raise':
+            check = True
+        del kwargs['error']
+    kwargs['check'] = check
+
+    # Ensure that the ENV values are all strings:
+    new_env = {}
+    for key, value in ENV.items():
+        if is_List(value):
+            # If the value is a list, then we assume it is a path list,
+            # because that's a pretty common list-like value to stick
+            # in an environment variable:
+            value = SCons.Util.flatten_sequence(value)
+            new_env[key] = os.pathsep.join(str(v) for v in value)
+        else:
+            # If it's not a list type, "convert" it to str. This is
+            # harmless if it's already a string, but gets us the right
+            # thing for Dir and File instances and will produce something
+            # reasonable for just about everything else:
+            new_env[key] = str(value)
+    kwargs['env'] = new_env
+
+    # Some tools/tests expect no failures for things like missing files
+    # unless raise/check is set. If set, let subprocess.run go ahead and
+    # kill things, else catch and construct a dummy CompletedProcess instance.
+    if check:
+        cp = subprocess.run(*args, **kwargs)
+    else:
+        try:
+            cp = subprocess.run(*args, **kwargs)
+        except OSError as exc:
+            argline = ' '.join(*args)
+            cp = subprocess.CompletedProcess(argline, 1)
+            cp.stdout = ""
+            cp.stderr = ""
+
+    return cp
+
+
+def _subproc(scons_env, cmd, error='ignore', **kw):
+    """Wrapper for subprocess.Popen which pulls from construction env.
 
     Use for calls to subprocess which need to interpolate values from
     an SCons construction environment into the environment passed to
     subprocess.  Adds an an error-handling argument.  Adds ability
     to specify std{in,out,err} with "'devnull'" tag.
+
+    .. deprecated:: 4.6
     """
     # TODO: just uses subprocess.DEVNULL now, we can drop the "devnull"
     # string now - it is a holdover from Py2, which didn't have DEVNULL.

SCons/ActionTests.py

@@ -41,7 +41,7 @@ def __call__(self) -> None:
 import sys
 import types
 import unittest
-import subprocess
+from subprocess import PIPE
 
 import SCons.Action
 import SCons.Environment
@@ -2327,17 +2327,9 @@ def test_code_contents(self) -> None:
         self.assertEqual(c, expected[sys.version_info[:2]])
 
     def test_uncaught_exception_bubbles(self):
-        """Test that _subproc bubbles uncaught exceptions"""
-
+        """Test that scons_subproc_run bubbles uncaught exceptions"""
         try:
-            pobj = SCons.Action._subproc(
-                Environment(),
-                None,
-                stdin='devnull',
-                stderr='devnull',
-                stdout=subprocess.PIPE,
-            )
-            pobj.wait()
+            cp = SCons.Action.scons_subproc_run(Environment(), None, stdout=PIPE)
         except EnvironmentError:
             pass
         except Exception:

SCons/Environment.py

@@ -36,6 +36,7 @@
 import re
 import shlex
 from collections import UserDict, deque
+from subprocess import PIPE, DEVNULL
 
 import SCons.Action
 import SCons.Builder
@@ -770,29 +771,24 @@ def backtick(self, command) -> str:
         Raises:
             OSError: if the external command returned non-zero exit status.
         """
-
-        import subprocess
-
         # common arguments
         kw = {
-            "stdin": "devnull",
-            "stdout": subprocess.PIPE,
-            "stderr": subprocess.PIPE,
+            "stdin": DEVNULL,
+            "stdout": PIPE,
+            "stderr": PIPE,
             "universal_newlines": True,
         }
         # if the command is a list, assume it's been quoted
         # othewise force a shell
         if not is_List(command):
             kw["shell"] = True
         # run constructed command
-        p = SCons.Action._subproc(self, command, **kw)
-        out, err = p.communicate()
-        status = p.wait()
-        if err:
-            sys.stderr.write("" + err)
-        if status:
-            raise OSError("'%s' exited %d" % (command, status))
-        return out
+        cp = SCons.Action.scons_subproc_run(self, command, **kw)
+        if cp.stderr:
+            sys.stderr.write(cp.stderr)
+        if cp.returncode:
+            raise OSError(f'{command!r} exited {cp.returncode}')
+        return cp.stdout
 
 
     def AddMethod(self, function, name=None) -> None:

SCons/EnvironmentTests.py

@@ -649,7 +649,7 @@ def test_backtick(self) -> None:
             try:
                 env.backtick(cmd)
             except OSError as e:
-                assert str(e) == "'%s' exited 1" % cmd, str(e)
+                assert str(e) == f'{cmd!r} exited 1', str(e)
             else:
                 self.fail("did not catch expected OSError")
 

SCons/Errors.py

@@ -27,7 +27,8 @@
 """
 
 import shutil
-import SCons.Util
+
+from SCons.Util.sctypes import to_String, is_String
 
 # Note that not all Errors are defined here, some are at the point of use
 
@@ -77,7 +78,7 @@ def __init__(self,
 
         # py3: errstr should be string and not bytes.
 
-        self.errstr = SCons.Util.to_String(errstr)
+        self.errstr = to_String(errstr)
         self.status = status
         self.exitstatus = exitstatus
         self.filename = filename
@@ -189,7 +190,7 @@ def convert_to_BuildError(status, exc_info=None):
             status=2,
             exitstatus=2,
             exc_info=exc_info)
-    elif SCons.Util.is_String(status):
+    elif is_String(status):
         buildError = BuildError(
             errstr=status,
             status=2,

SCons/Node/FS.py

@@ -1204,7 +1204,7 @@ def islink(self, path) -> bool:
 
     if hasattr(os, 'readlink'):
 
-        def readlink(self, file):
+        def readlink(self, file) -> str:
             return os.readlink(file)
 
     else:

SCons/Platform/aix.py

@@ -28,7 +28,7 @@
 selection method.
 """
 
-import subprocess
+from subprocess import PIPE
 
 from . import posix
 
@@ -47,22 +47,21 @@ def get_xlc(env, xlc=None, packages=[]):
         xlc = xlc[0]
     for package in packages:
         # find the installed filename, which may be a symlink as well
-        pipe = SCons.Action._subproc(env, ['lslpp', '-fc', package],
-                stdin = 'devnull',
-                stderr = 'devnull',
-                universal_newlines=True,
-                stdout = subprocess.PIPE)
+        cp = SCons.Action.scons_subproc_run(
+            env, ['lslpp', '-fc', package], universal_newlines=True, stdout=PIPE
+        )
         # output of lslpp is something like this:
         #     #Path:Fileset:File
         #     /usr/lib/objrepos:vac.C 6.0.0.0:/usr/vac/exe/xlCcpp
         #     /usr/lib/objrepos:vac.C 6.0.0.0:/usr/vac/bin/xlc_r -> /usr/vac/bin/xlc
-        for line in pipe.stdout:
+        for line in cp.stdout.splitlines():
             if xlcPath:
-                continue # read everything to let lslpp terminate
+                continue  # read everything to let lslpp terminate
             fileset, filename = line.split(':')[1:3]
             filename = filename.split()[0]
-            if ('/' in xlc and filename == xlc) \
-            or ('/' not in xlc and filename.endswith('/' + xlc)):
+            if ('/' in xlc and filename == xlc) or (
+                '/' not in xlc and filename.endswith('/' + xlc)
+            ):
                 xlcVersion = fileset.split()[1]
                 xlcPath, sep, xlc = filename.rpartition('/')
     return (xlcPath, xlc, xlcVersion)