[clang] [clang-tools-extra] [lldb] [llvm] [NFC] Prefer subprocess.DEVNULL over os.devnull (PR #106500)

Nicolas van Kempen via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 28 23:44:40 PDT 2024


https://github.com/nicovank created https://github.com/llvm/llvm-project/pull/106500


There is no need to support Python 2.7 anymore, Python 3.3+ has `subprocess.DEVNULL`. This is good practice and also prevents file handles from staying open unnecessarily.

Also remove a couple unused or unneeded `__future__` imports.


>From f0fa35d2a955ba4971e94e3c8bf1a6e0453e8bf9 Mon Sep 17 00:00:00 2001
From: Nicolas van Kempen <nvankemp at gmail.com>
Date: Thu, 29 Aug 2024 02:44:13 -0400
Subject: [PATCH] [NFC] Prefer subprocess.DEVNULL over os.devnull

There is no need to support Python 2.7 anymore, Python 3.3+ has `subprocess.DEVNULL`. This is good practice and also prevents file handles from staying open unnecessarily.

Also remove a couple unused or unneeded `__future__` imports.
---
 .../clang-tidy/tool/run-clang-tidy.py            | 10 ++++------
 clang/docs/tools/generate_formatted_state.py     |  6 ++----
 clang/tools/scan-view/share/startfile.py         |  2 +-
 clang/utils/creduce-clang-crash.py               |  4 +---
 lldb/bindings/interface/SBErrorDocstrings.i      |  6 ++++--
 .../packages/Python/lldbsuite/test/decorators.py |  6 ++----
 lldb/packages/Python/lldbsuite/test/lldbtest.py  |  3 +--
 llvm/utils/UpdateTestChecks/common.py            | 16 +++++++---------
 llvm/utils/git/pre-push.py                       | 15 ++-------------
 llvm/utils/gn/gn.py                              |  2 +-
 10 files changed, 25 insertions(+), 45 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
index 48401ba5ea42a9..b702eece37002b 100755
--- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -511,12 +511,10 @@ async def main() -> None:
         )
         invocation.append("-list-checks")
         invocation.append("-")
-        if args.quiet:
-            # Even with -quiet we still want to check if we can call clang-tidy.
-            with open(os.devnull, "w") as dev_null:
-                subprocess.check_call(invocation, stdout=dev_null)
-        else:
-            subprocess.check_call(invocation)
+        # Even with -quiet we still want to check if we can call clang-tidy.
+        subprocess.check_call(
+            invocation, stdout=subprocess.DEVNULL if args.quiet else None
+        )
     except:
         print("Unable to run clang-tidy.", file=sys.stderr)
         sys.exit(1)
diff --git a/clang/docs/tools/generate_formatted_state.py b/clang/docs/tools/generate_formatted_state.py
index 66cebbf7af33a4..2de43dc383f557 100755
--- a/clang/docs/tools/generate_formatted_state.py
+++ b/clang/docs/tools/generate_formatted_state.py
@@ -78,8 +78,6 @@ def get_style(count, passed):
      - {style2}`{percent}%`
 """
 
-FNULL = open(os.devnull, "w")
-
 
 with open(DOC_FILE, "wb") as output:
     cleanfiles = open(CLEAN_FILE, "wb")
@@ -101,8 +99,8 @@ def get_style(count, passed):
                 # interested in it, just the return code.
                 git_check = subprocess.Popen(
                     ["git", "ls-files", "--error-unmatch", act_sub_dir],
-                    stdout=FNULL,
-                    stderr=FNULL,
+                    stdout=subprocess.DEVNULL,
+                    stderr=subprocess.DEVNULL,
                 )
                 if git_check.wait() != 0:
                     print("Skipping directory: ", act_sub_dir)
diff --git a/clang/tools/scan-view/share/startfile.py b/clang/tools/scan-view/share/startfile.py
index d63e69280e90dd..c72475e8b6212e 100644
--- a/clang/tools/scan-view/share/startfile.py
+++ b/clang/tools/scan-view/share/startfile.py
@@ -48,7 +48,7 @@ def _invoke(self, cmdline):
             or sys.platform[:3] == "win"
             or sys.platform == "darwin"
         ):
-            inout = file(os.devnull, "r+")
+            inout = subprocess.DEVNULL
         else:
             # for TTY programs, we need stdin/out
             inout = None
diff --git a/clang/utils/creduce-clang-crash.py b/clang/utils/creduce-clang-crash.py
index db4a3435a3aef7..180dfbeab224e9 100755
--- a/clang/utils/creduce-clang-crash.py
+++ b/clang/utils/creduce-clang-crash.py
@@ -8,7 +8,6 @@
   *.test.sh -- interestingness test for C-Reduce
 """
 
-from __future__ import print_function
 from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
@@ -228,8 +227,7 @@ def check_interestingness(self):
         testfile = os.path.abspath(self.testfile)
 
         # Check that the test considers the original file interesting
-        with open(os.devnull, "w") as devnull:
-            returncode = subprocess.call(testfile, stdout=devnull)
+        returncode = subprocess.call(testfile, stdout=subprocess.DEVNULL)
         if returncode:
             sys.exit("The interestingness test does not pass for the original file.")
 
diff --git a/lldb/bindings/interface/SBErrorDocstrings.i b/lldb/bindings/interface/SBErrorDocstrings.i
index b64c3d64c6c77b..c272ffb7605ffb 100644
--- a/lldb/bindings/interface/SBErrorDocstrings.i
+++ b/lldb/bindings/interface/SBErrorDocstrings.i
@@ -10,8 +10,10 @@ For example (from test/python_api/hello_world/TestHelloWorld.py), ::
 
         # Spawn a new process and don't display the stdout if not in TraceOn() mode.
         import subprocess
-        popen = subprocess.Popen([self.exe, 'abc', 'xyz'],
-                                 stdout = open(os.devnull, 'w') if not self.TraceOn() else None)
+        popen = subprocess.Popen(
+            [self.exe, 'abc', 'xyz'],
+            stdout=subprocess.DEVNULL if not self.TraceOn() else None,
+        )
 
         listener = lldb.SBListener('my.attach.listener')
         error = lldb.SBError()
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 0e8ca159efd55d..834f01aaa61e6b 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -467,9 +467,8 @@ def should_skip_simulator_test():
         if lldbplatformutil.getHostPlatform() not in ["darwin", "macosx"]:
             return "simulator tests are run only on darwin hosts."
         try:
-            DEVNULL = open(os.devnull, "w")
             output = subprocess.check_output(
-                ["xcodebuild", "-showsdks"], stderr=DEVNULL
+                ["xcodebuild", "-showsdks"], stderr=subprocess.DEVNULL
             ).decode("utf-8")
             if re.search("%ssimulator" % platform, output):
                 return None
@@ -1094,9 +1093,8 @@ def skipUnlessFeature(feature):
     def is_feature_enabled():
         if platform.system() == "Darwin":
             try:
-                DEVNULL = open(os.devnull, "w")
                 output = subprocess.check_output(
-                    ["/usr/sbin/sysctl", feature], stderr=DEVNULL
+                    ["/usr/sbin/sysctl", feature], stderr=subprocess.DEVNULL
                 ).decode("utf-8")
                 # If 'feature: 1' was output, then this feature is available and
                 # the test should not be skipped.
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index b57c3bdd87c83c..e0da7cbd1ddd6e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -31,7 +31,6 @@
 import abc
 from functools import wraps
 import gc
-import glob
 import io
 import json
 import os.path
@@ -416,7 +415,7 @@ def launch(self, executable, args, extra_env):
 
         self._proc = Popen(
             [executable] + args,
-            stdout=open(os.devnull) if not self._trace_on else None,
+            stdout=DEVNULL if not self._trace_on else None,
             stdin=PIPE,
             env=env,
         )
diff --git a/llvm/utils/UpdateTestChecks/common.py b/llvm/utils/UpdateTestChecks/common.py
index eb212ed304e9db..34d859f9ed9d99 100644
--- a/llvm/utils/UpdateTestChecks/common.py
+++ b/llvm/utils/UpdateTestChecks/common.py
@@ -1,11 +1,8 @@
-from __future__ import print_function
-
 import argparse
 import bisect
 import collections
 import copy
 import glob
-import itertools
 import os
 import re
 import subprocess
@@ -517,12 +514,13 @@ def invoke_tool(exe, cmd_args, ir, preprocess_cmd=None, verbose=False):
                     sep="",
                     file=sys.stderr,
                 )
-            # Python 2.7 doesn't have subprocess.DEVNULL:
-            with open(os.devnull, "w") as devnull:
-                pp = subprocess.Popen(
-                    preprocess_cmd, shell=True, stdin=devnull, stdout=subprocess.PIPE
-                )
-                ir_file = pp.stdout
+            pp = subprocess.Popen(
+                preprocess_cmd,
+                shell=True,
+                stdin=subprocess.DEVNULL,
+                stdout=subprocess.PIPE,
+            )
+            ir_file = pp.stdout
 
         if isinstance(cmd_args, list):
             args = [applySubstitutions(a, substitutions) for a in cmd_args]
diff --git a/llvm/utils/git/pre-push.py b/llvm/utils/git/pre-push.py
index d7ae3767d2923d..dfa009dd1a6f62 100755
--- a/llvm/utils/git/pre-push.py
+++ b/llvm/utils/git/pre-push.py
@@ -27,7 +27,6 @@
 """
 
 import argparse
-import os
 import shutil
 import subprocess
 import sys
@@ -70,14 +69,6 @@ def ask_confirm(prompt):
         return query.lower() == "y"
 
 
-def get_dev_null():
-    """Lazily create a /dev/null fd for use in shell()"""
-    global dev_null_fd
-    if dev_null_fd is None:
-        dev_null_fd = open(os.devnull, "w")
-    return dev_null_fd
-
-
 def shell(
     cmd,
     strip=True,
@@ -95,10 +86,8 @@ def shell(
         cwd_msg = " in %s" % cwd
     log_verbose("Running%s: %s" % (cwd_msg, " ".join(quoted_cmd)))
 
-    err_pipe = subprocess.PIPE
-    if ignore_errors:
-        # Silence errors if requested.
-        err_pipe = get_dev_null()
+    # Silence errors if requested.
+    err_pipe = subprocess.DEVNULL if ignore_errors else subprocess.PIPE
 
     start = time.time()
     p = subprocess.Popen(
diff --git a/llvm/utils/gn/gn.py b/llvm/utils/gn/gn.py
index 290c6941bceea2..6b7919b7faeb92 100755
--- a/llvm/utils/gn/gn.py
+++ b/llvm/utils/gn/gn.py
@@ -42,7 +42,7 @@ def main():
     if (
         subprocess.call(
             "gn --version",
-            stdout=open(os.devnull, "w"),
+            stdout=subprocess.DEVNULL,
             stderr=subprocess.STDOUT,
             shell=True,
         )



More information about the cfe-commits mailing list