bpo-35346, platform: replace os.popen() with subprocess (GH-10786)

vstinner · web-flow · commit 3a521f0b6167 · 2018-12-07T11:10:33.000+01:00
Replace os.popen() with subprocess.check_output() in the platform module:

* platform.uname() (its _syscmd_ver() helper function) now redirects
  stderr to DEVNULL.
* Remove platform.DEV_NULL.
* _syscmd_uname() and _syscmd_file() no longer catch AttributeError.
  The "except AttributeError:" was only needed in Python 2, when
  os.popen() was not always available. In Python 3,
  subprocess.check_output() is always available.
diff --git a/Lib/platform.py b/Lib/platform.py
@@ -119,19 +119,6 @@
 
 ### Globals & Constants
 
-# Determine the platform's /dev/null device
-try:
-    DEV_NULL = os.devnull
-except AttributeError:
-    # os.devnull was added in Python 2.4, so emulate it for earlier
-    # Python versions
-    if sys.platform in ('dos', 'win32', 'win16'):
-        # Use the old CP/M NUL as device name
-        DEV_NULL = 'NUL'
-    else:
-        # Standard Unix uses /dev/null
-        DEV_NULL = '/dev/null'
-
 # Helper for comparing two version number strings.
 # Based on the description of the PHP's version_compare():
 # http://php.net/manual/en/function.version-compare.php
@@ -288,16 +275,15 @@ def _syscmd_ver(system='', release='', version='',
         return system, release, version
 
     # Try some common cmd strings
+    import subprocess
     for cmd in ('ver', 'command /c ver', 'cmd /c ver'):
         try:
-            pipe = os.popen(cmd)
-            info = pipe.read()
-            if pipe.close():
-                raise OSError('command failed')
-            # XXX How can I suppress shell errors from being written
-            #     to stderr ?
-        except OSError as why:
-            #print 'Command %s failed: %s' % (cmd, why)
+            info = subprocess.check_output(cmd,
+                                           stderr=subprocess.DEVNULL,
+                                           text=True,
+                                           shell=True)
+        except (OSError, subprocess.CalledProcessError) as why:
+            #print('Command %s failed: %s' % (cmd, why))
             continue
         else:
             break
@@ -602,16 +588,15 @@ def _syscmd_uname(option, default=''):
     if sys.platform in ('dos', 'win32', 'win16'):
         # XXX Others too ?
         return default
+
+    import subprocess
     try:
-        f = os.popen('uname %s 2> %s' % (option, DEV_NULL))
-    except (AttributeError, OSError):
-        return default
-    output = f.read().strip()
-    rc = f.close()
-    if not output or rc:
+        output = subprocess.check_output(('uname', option),
+                                         stderr=subprocess.DEVNULL,
+                                         text=True)
+    except (OSError, subprocess.CalledProcessError):
         return default
-    else:
-        return output
+    return (output.strip() or default)
 
 def _syscmd_file(target, default=''):
 
@@ -629,17 +614,12 @@ def _syscmd_file(target, default=''):
     import subprocess
     target = _follow_symlinks(target)
     try:
-        proc = subprocess.Popen(['file', target],
-                                stdout=subprocess.PIPE,
-                                stderr=subprocess.STDOUT)
-    except (AttributeError, OSError):
+        output = subprocess.check_output(['file', target],
+                                         stderr=subprocess.DEVNULL,
+                                         encoding='latin-1')
+    except (OSError, subprocess.CalledProcessError):
         return default
-    output = proc.communicate()[0].decode('latin-1')
-    rc = proc.wait()
-    if not output or rc:
-        return default
-    else:
-        return output
+    return (output or default)
 
 ### Information about the used architecture
 
diff --git a/Lib/test/test_platform.py b/Lib/test/test_platform.py
@@ -222,16 +222,16 @@ def test_mac_ver(self):
         res = platform.mac_ver()
 
         if platform.uname().system == 'Darwin':
-            # We're on a MacOSX system, check that
-            # the right version information is returned
-            fd = os.popen('sw_vers', 'r')
-            real_ver = None
-            for ln in fd:
-                if ln.startswith('ProductVersion:'):
-                    real_ver = ln.strip().split()[-1]
+            # We are on a macOS system, check that the right version
+            # information is returned
+            output = subprocess.check_output(['sw_vers'], text=True)
+            for line in output.splitlines():
+                if line.startswith('ProductVersion:'):
+                    real_ver = line.strip().split()[-1]
                     break
-            fd.close()
-            self.assertFalse(real_ver is None)
+            else:
+                self.fail(f"failed to parse sw_vers output: {output!r}")
+
             result_list = res[0].split('.')
             expect_list = real_ver.split('.')
             len_diff = len(result_list) - len(expect_list)
diff --git a/Misc/NEWS.d/next/Library/2018-11-29-12-42-13.bpo-35346.OmTY5c.rst b/Misc/NEWS.d/next/Library/2018-11-29-12-42-13.bpo-35346.OmTY5c.rst
@@ -0,0 +1,2 @@
+:func:`platform.uname` now redirects ``stderr`` to :data:`os.devnull` when
+running external programs like ``cmd /c ver``.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+:func:`platform.uname` now redirects ``stderr`` to :data:`os.devnull` when
	`2`	+running external programs like ``cmd /c ver``.