Clean up some more Python 2 code

* _subproc is a little simpler as subprocess.DEVNULL is sure to br
  defined.  Adjusted the docstring to better show the purpose.
* CacheDir didn't need the Py2 branch of getting the config.
* MSCommon can use a more specific exception.
* packaging can now unconditionally use inspect.getfullargspec.

Signed-off-by: Mats Wichmann <mats@linux.com>

6 files changed, 22 insertions, 96 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 7c5c9c2..d661015 100755
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -76,11 +76,12 @@ From Rob Boehne
     - Accommodate VS 2017 Express - it's got a more liberal license then VS
       Community, so some people prefer it (from 2019, no more Express)
     - vswhere call should also now work even if programs aren't on the C: drive.
-    - Add an alternate warning message cl.exe is not found and msvc config
+    - Add an alternate warning message if cl.exe is not found and msvc config
       cache is in use (SCONS_CACHE_MSVC_CONFIG was given) - config cache
       may be out of date.
     - Fixed bug where changing TEXTFILESUFFIX would cause Substfile() to rebuild. (Github Issue #3540)
     - Script/Main.py now uses importlib instead of imp module.
+    - Drop some Python 2-isms.
 
 
 RELEASE 3.1.2 - Mon, 17 Dec 2019 02:06:27 +0000
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 8a8cf27..ab901f6 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -105,6 +105,7 @@ import pickle
 import re
 import sys
 import subprocess
+from subprocess import DEVNULL
 import itertools
 import inspect
 from collections import OrderedDict
@@ -751,30 +752,20 @@ def get_default_ENV(env):
         return default_ENV
 
 
-def _subproc(scons_env, cmd, error = 'ignore', **kw):
-    """Do common setup for a subprocess.Popen() call
+def _subproc(scons_env, cmd, error='ignore', **kw):
+    """Wrapper for subprocess which pulls from construction env.
 
-    This function is still in draft mode.  We're going to need something like
-    it in the long run as more and more places use subprocess, but I'm sure
-    it'll have to be tweaked to get the full desired functionality.
-    one special arg (so far?), 'error', to tell what to do with exceptions.
+    Use for calls to subprocess which need to interpolate values from
+    an SCons construction enviroment into the environment passed to
+    subprocess.  Adds an an error-handling argument.  Adds ability
+    to specify std{in,out,err} with "'devnull'" tag.
     """
-    # allow std{in,out,err} to be "'devnull'".  This is like
-    # subprocess.DEVNULL, which does not exist for Py2. Use the
-    # subprocess one if possible.
-    # Clean this up when Py2 support is dropped
-    try:
-        from subprocess import DEVNULL
-    except ImportError:
-        DEVNULL = None
-
+    # TODO: just uses subprocess.DEVNULL now, we can drop the "devnull"
+    # string now - it is a holdover from Py2, which didn't have DEVNULL.
     for stream in 'stdin', 'stdout', 'stderr':
         io = kw.get(stream)
         if is_String(io) and io == 'devnull':
-            if DEVNULL:
-                kw[stream] = DEVNULL
-            else:
-                kw[stream] = open(os.devnull, "r+")
+            kw[stream] = DEVNULL
 
     # Figure out what shell environment to use
     ENV = kw.get('env', None)
diff --git a/src/engine/SCons/CacheDir.py b/src/engine/SCons/CacheDir.py
index 9bb7ef3..117f6b0 100644
--- a/src/engine/SCons/CacheDir.py
+++ b/src/engine/SCons/CacheDir.py
@@ -135,9 +135,6 @@ def CachePushFunc(target, source, env):
 
 CachePush = SCons.Action.Action(CachePushFunc, None)
 
-# Nasty hack to cut down to one warning for each cachedir path that needs
-# upgrading.
-warned = dict()
 
 class CacheDir(object):
 
@@ -159,12 +156,12 @@ class CacheDir(object):
         if path is None:
             return
 
-        self._readconfig3(path)
+        self._readconfig(path)
 
 
-    def _readconfig3(self, path):
+    def _readconfig(self, path):
         """
-        Python3 version of reading the cache config.
+        Read the cache config.
 
         If directory or config file do not exist, create.  Take advantage
         of Py3 capability in os.makedirs() and in file open(): just try
@@ -201,64 +198,6 @@ class CacheDir(object):
                 raise SCons.Errors.SConsEnvironmentError(msg)
 
 
-    def _readconfig2(self, path):
-        """
-        Python2 version of reading cache config.
-
-        See if there is a config file in the cache directory. If there is,
-        use it. If there isn't, and the directory exists and isn't empty,
-        produce a warning. If the directory does not exist or is empty,
-        write a config file.
-
-        :param path: path to the cache directory
-        """
-        config_file = os.path.join(path, 'config')
-        if not os.path.exists(config_file):
-            # A note: There is a race hazard here if two processes start and
-            # attempt to create the cache directory at the same time. However,
-            # Python 2.x does not give you the option to do exclusive file
-            # creation (not even the option to error on opening an existing
-            # file for writing...). The ordering of events here is an attempt
-            # to alleviate this, on the basis that it's a pretty unlikely
-            # occurrence (would require two builds with a brand new cache
-            # directory)
-            if os.path.isdir(path) and any(f != "config" for f in os.listdir(path)):
-                self.config['prefix_len'] = 1
-                # When building the project I was testing this on, the warning
-                # was output over 20 times. That seems excessive
-                global warned
-                if self.path not in warned:
-                    msg = "Please upgrade your cache by running " +\
-                          "scons-configure-cache.py " +  self.path
-                    SCons.Warnings.warn(SCons.Warnings.CacheVersionWarning, msg)
-                    warned[self.path] = True
-            else:
-                if not os.path.isdir(path):
-                    try:
-                        os.makedirs(path)
-                    except OSError:
-                        # If someone else is trying to create the directory at
-                        # the same time as me, bad things will happen
-                        msg = "Failed to create cache directory " + path
-                        raise SCons.Errors.SConsEnvironmentError(msg)
-
-                self.config['prefix_len'] = 2
-                if not os.path.exists(config_file):
-                    try:
-                        with open(config_file, 'w') as config:
-                            json.dump(self.config, config)
-                    except Exception:
-                        msg = "Failed to write cache configuration for " + path
-                        raise SCons.Errors.SConsEnvironmentError(msg)
-        else:
-            try:
-                with open(config_file) as config:
-                    self.config = json.load(config)
-            except ValueError:
-                msg = "Failed to read cache configuration for " + path
-                raise SCons.Errors.SConsEnvironmentError(msg)
-
-
     def CacheDebug(self, fmt, target, cachefile):
         if cache_debug != self.current_cache_debug:
             if cache_debug == '-':
diff --git a/src/engine/SCons/CacheDirTests.py b/src/engine/SCons/CacheDirTests.py
index ff22d01..a3114c1 100644
--- a/src/engine/SCons/CacheDirTests.py
+++ b/src/engine/SCons/CacheDirTests.py
@@ -168,7 +168,7 @@ class ExceptionTestCase(unittest.TestCase):
         os.remove(old_config)
         
         try:
-            self._CacheDir._readconfig3(self._CacheDir.path)
+            self._CacheDir._readconfig(self._CacheDir.path)
             assert False, "Should have raised exception and did not"
         except SCons.Errors.SConsEnvironmentError as e:
             assert str(e) == "Failed to write cache configuration for {}".format(self._CacheDir.path)
diff --git a/src/engine/SCons/Tool/MSCommon/common.py b/src/engine/SCons/Tool/MSCommon/common.py
index 505136e..7f832c6 100644
--- a/src/engine/SCons/Tool/MSCommon/common.py
+++ b/src/engine/SCons/Tool/MSCommon/common.py
@@ -64,8 +64,7 @@ def read_script_env_cache():
         try:
             with open(CONFIG_CACHE, 'r') as f:
                 envcache = json.load(f)
-        # TODO can use more specific FileNotFoundError when py2 dropped
-        except IOError:
+        except FileNotFoundError:
             # don't fail if no cache file, just proceed without it
             pass
     return envcache
diff --git a/src/engine/SCons/Tool/packaging/__init__.py b/src/engine/SCons/Tool/packaging/__init__.py
index 5795396..f06027d 100644
--- a/src/engine/SCons/Tool/packaging/__init__.py
+++ b/src/engine/SCons/Tool/packaging/__init__.py
@@ -27,6 +27,10 @@ SCons Packaging Tool.
 
 __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
+import os
+import importlib
+from inspect import getfullargspec
+
 import SCons.Defaults
 import SCons.Environment
 from SCons.Variables import *
@@ -34,8 +38,6 @@ from SCons.Errors import *
 from SCons.Util import is_List, make_path_relative
 from SCons.Warnings import warn, Warning
 
-import os
-import importlib
 
 __all__ = [
     'src_targz', 'src_tarbz2', 'src_tarxz', 'src_zip',
@@ -168,13 +170,7 @@ def Package(env, target=None, source=None, **kw):
         # this exception means that a needed argument for the packager is
         # missing. As our packagers get their "tags" as named function
         # arguments we need to find out which one is missing.
-        #TODO: getargspec deprecated in Py3. cleanup when Py2.7 dropped.
-        try:
-            from inspect import getfullargspec
-            argspec = getfullargspec(packager.package)
-        except ImportError:
-            from inspect import getargspec
-            argspec = getargspec(packager.package)
+        argspec = getfullargspec(packager.package)
         args = argspec.args
         if argspec.defaults:
             # throw away arguments with default values