Align our subprocess usage with current best practices. (#12940)

* Align our subprocess usage with current best practices.

* remove unused import

* Apply suggestions from code review

Co-authored-by: Ryan <fauxpark@gmail.com>

* fix the cpp invocation for older python

* allow for unprompted installation

* make sure qmk new-keyboard works on windows

Co-authored-by: Ryan <fauxpark@gmail.com>

10 files changed, 36 insertions, 29 deletions

diff --git a/lib/python/qmk/cli/cformat.py b/lib/python/qmk/cli/cformat.py
index 15158d9c7d..efeb459676 100644
--- a/lib/python/qmk/cli/cformat.py
+++ b/lib/python/qmk/cli/cformat.py
@@ -1,8 +1,8 @@
 """Format C code according to QMK's style.
 """
-import subprocess
 from os import path
 from shutil import which
+from subprocess import CalledProcessError, DEVNULL, Popen, PIPE
 
 from argcomplete.completers import FilesCompleter
 from milc import cli
@@ -34,7 +34,7 @@ def find_diffs(files):
 
     for file in files:
         cli.log.debug('Checking for changes in %s', file)
-        clang_format = subprocess.Popen([find_clang_format(), file], stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+        clang_format = Popen([find_clang_format(), file], stdout=PIPE, stderr=PIPE, universal_newlines=True)
         diff = cli.run(['diff', '-u', f'--label=a/{file}', f'--label=b/{file}', str(file), '-'], stdin=clang_format.stdout, capture_output=True)
 
         if diff.returncode != 0:
@@ -51,11 +51,11 @@ def cformat_run(files):
     clang_format = [find_clang_format(), '-i']
 
     try:
-        cli.run(clang_format + list(map(str, files)), check=True, capture_output=False)
+        cli.run([*clang_format, *map(str, files)], check=True, capture_output=False, stdin=DEVNULL)
         cli.log.info('Successfully formatted the C code.')
         return True
 
-    except subprocess.CalledProcessError as e:
+    except CalledProcessError as e:
         cli.log.error('Error formatting C code!')
         cli.log.debug('%s exited with returncode %s', e.cmd, e.returncode)
         cli.log.debug('STDOUT:')
@@ -111,7 +111,7 @@ def cformat(cli):
 
     else:
         git_diff_cmd = ['git', 'diff', '--name-only', cli.args.base_branch, *core_dirs]
-        git_diff = cli.run(git_diff_cmd)
+        git_diff = cli.run(git_diff_cmd, stdin=DEVNULL)
 
         if git_diff.returncode != 0:
             cli.log.error("Error running %s", git_diff_cmd)
diff --git a/lib/python/qmk/cli/clean.py b/lib/python/qmk/cli/clean.py
index 9096529fde..72b7ffe810 100644
--- a/lib/python/qmk/cli/clean.py
+++ b/lib/python/qmk/cli/clean.py
@@ -1,6 +1,8 @@
 """Clean the QMK firmware folder of build artifacts.
 """
-from qmk.commands import run, create_make_target
+from subprocess import DEVNULL
+
+from qmk.commands import create_make_target
 from milc import cli
 
 
@@ -9,4 +11,4 @@ from milc import cli
 def clean(cli):
     """Runs `make clean` (or `make distclean` if --all is passed)
     """
-    run(create_make_target('distclean' if cli.args.all else 'clean'))
+    cli.run(create_make_target('distclean' if cli.args.all else 'clean'), capture_output=False, stdin=DEVNULL)
diff --git a/lib/python/qmk/cli/compile.py b/lib/python/qmk/cli/compile.py
index 23ca4e00a6..7a45e77214 100755
--- a/lib/python/qmk/cli/compile.py
+++ b/lib/python/qmk/cli/compile.py
@@ -2,6 +2,8 @@
 
 You can compile a keymap already in the repo or using a QMK Configurator export.
 """
+from subprocess import DEVNULL
+
 from argcomplete.completers import FilesCompleter
 from milc import cli
 
@@ -31,8 +33,7 @@ def compile(cli):
     """
     if cli.args.clean and not cli.args.filename and not cli.args.dry_run:
         command = create_make_command(cli.config.compile.keyboard, cli.config.compile.keymap, 'clean')
-        # FIXME(skullydazed/anyone): Remove text=False once milc 1.0.11 has had enough time to be installed everywhere.
-        cli.run(command, capture_output=False, text=False)
+        cli.run(command, capture_output=False, stdin=DEVNULL)
 
     # Build the environment vars
     envs = {}
diff --git a/lib/python/qmk/cli/doctor.py b/lib/python/qmk/cli/doctor.py
index 4a2e2010f7..9e10570620 100755
--- a/lib/python/qmk/cli/doctor.py
+++ b/lib/python/qmk/cli/doctor.py
@@ -3,12 +3,12 @@
 Check out the user's QMK environment and make sure it's ready to compile.
 """
 import platform
+from subprocess import DEVNULL
 
 from milc import cli
 from milc.questions import yesno
 from qmk import submodules
 from qmk.constants import QMK_FIRMWARE
-from qmk.commands import run
 from qmk.os_helpers import CheckStatus, check_binaries, check_binary_versions, check_submodules, check_git_repo
 
 
@@ -93,7 +93,7 @@ def doctor(cli):
 
     if not bin_ok:
         if yesno('Would you like to install dependencies?', default=True):
-            run(['util/qmk_install.sh'])
+            cli.run(['util/qmk_install.sh', '-y'], stdin=DEVNULL, capture_output=False)
             bin_ok = check_binaries()
 
     if bin_ok:
diff --git a/lib/python/qmk/cli/flash.py b/lib/python/qmk/cli/flash.py
index 1b67840616..1b2932a5b2 100644
--- a/lib/python/qmk/cli/flash.py
+++ b/lib/python/qmk/cli/flash.py
@@ -3,6 +3,7 @@
 You can compile a keymap already in the repo or using a QMK Configurator export.
 A bootloader must be specified.
 """
+from subprocess import DEVNULL
 
 from argcomplete.completers import FilesCompleter
 from milc import cli
@@ -55,7 +56,7 @@ def flash(cli):
     """
     if cli.args.clean and not cli.args.filename and not cli.args.dry_run:
         command = create_make_command(cli.config.flash.keyboard, cli.config.flash.keymap, 'clean')
-        cli.run(command, capture_output=False)
+        cli.run(command, capture_output=False, stdin=DEVNULL)
 
     # Build the environment vars
     envs = {}
@@ -98,7 +99,7 @@ def flash(cli):
         cli.log.info('Compiling keymap with {fg_cyan}%s', ' '.join(command))
         if not cli.args.dry_run:
             cli.echo('\n')
-            compile = cli.run(command, capture_output=False, text=True)
+            compile = cli.run(command, capture_output=False, stdin=DEVNULL)
             return compile.returncode
 
     else:
diff --git a/lib/python/qmk/cli/generate/docs.py b/lib/python/qmk/cli/generate/docs.py
index a59a24db50..749336fea5 100644
--- a/lib/python/qmk/cli/generate/docs.py
+++ b/lib/python/qmk/cli/generate/docs.py
@@ -1,8 +1,8 @@
 """Build QMK documentation locally
 """
 import shutil
-import subprocess
 from pathlib import Path
+from subprocess import DEVNULL
 
 from milc import cli
 
@@ -24,14 +24,16 @@ def generate_docs(cli):
     shutil.copytree(DOCS_PATH, BUILD_PATH)
 
     # When not verbose we want to hide all output
-    args = {'check': True}
-    if not cli.args.verbose:
-        args.update({'stdout': subprocess.DEVNULL, 'stderr': subprocess.STDOUT})
+    args = {
+        'capture_output': False if cli.config.general.verbose else True,
+        'check': True,
+        'stdin': DEVNULL,
+    }
 
     cli.log.info('Generating internal docs...')
 
     # Generate internal docs
-    subprocess.run(['doxygen', 'Doxyfile'], **args)
-    subprocess.run(['moxygen', '-q', '-a', '-g', '-o', BUILD_PATH / 'internals_%s.md', 'doxygen/xml'], **args)
+    cli.run(['doxygen', 'Doxyfile'], **args)
+    cli.run(['moxygen', '-q', '-a', '-g', '-o', BUILD_PATH / 'internals_%s.md', 'doxygen/xml'], **args)
 
     cli.log.info('Successfully generated internal docs to %s.', BUILD_PATH)
diff --git a/lib/python/qmk/cli/multibuild.py b/lib/python/qmk/cli/multibuild.py
index a4f0a0cc03..46594c0997 100755
--- a/lib/python/qmk/cli/multibuild.py
+++ b/lib/python/qmk/cli/multibuild.py
@@ -4,6 +4,7 @@ This will compile everything in parallel, for testing purposes.
 """
 import re
 from pathlib import Path
+from subprocess import DEVNULL
 
 from milc import cli
 
@@ -35,7 +36,7 @@ def multibuild(cli):
 
     make_cmd = _find_make()
     if cli.args.clean:
-        cli.run([make_cmd, 'clean'], capture_output=False, text=False)
+        cli.run([make_cmd, 'clean'], capture_output=False, stdin=DEVNULL)
 
     builddir = Path(QMK_FIRMWARE) / '.build'
     makefile = builddir / 'parallel_kb_builds.mk'
@@ -75,4 +76,4 @@ all: {keyboard_safe}_binary
             )
             # yapf: enable
 
-    cli.run([make_cmd, '-j', str(cli.args.parallel), '-f', makefile, 'all'], capture_output=False, text=False)
+    cli.run([make_cmd, '-j', str(cli.args.parallel), '-f', makefile, 'all'], capture_output=False, stdin=DEVNULL)
diff --git a/lib/python/qmk/cli/new/keyboard.py b/lib/python/qmk/cli/new/keyboard.py
index cab0198fb7..ae4445ca48 100644
--- a/lib/python/qmk/cli/new/keyboard.py
+++ b/lib/python/qmk/cli/new/keyboard.py
@@ -8,4 +8,4 @@ def new_keyboard(cli):
     """Creates a new keyboard
     """
     # TODO: replace this bodge to the existing script
-    cli.run(['util/new_keyboard.sh'], capture_output=False)
+    cli.run(['util/new_keyboard.sh'], stdin=None, capture_output=False)
diff --git a/lib/python/qmk/cli/pyformat.py b/lib/python/qmk/cli/pyformat.py
index 02581f0d85..abe5f6de19 100755
--- a/lib/python/qmk/cli/pyformat.py
+++ b/lib/python/qmk/cli/pyformat.py
@@ -1,8 +1,8 @@
 """Format python code according to QMK's style.
 """
-from milc import cli
+from subprocess import CalledProcessError, DEVNULL
 
-import subprocess
+from milc import cli
 
 
 @cli.argument('-n', '--dry-run', arg_only=True, action='store_true', help="Flag only, don't automatically format.")
@@ -13,11 +13,11 @@ def pyformat(cli):
     edit = '--diff' if cli.args.dry_run else '--in-place'
     yapf_cmd = ['yapf', '-vv', '--recursive', edit, 'bin/qmk', 'lib/python']
     try:
-        cli.run(yapf_cmd, check=True, capture_output=False)
+        cli.run(yapf_cmd, check=True, capture_output=False, stdin=DEVNULL)
         cli.log.info('Python code in `bin/qmk` and `lib/python` is correctly formatted.')
         return True
 
-    except subprocess.CalledProcessError:
+    except CalledProcessError:
         if cli.args.dry_run:
             cli.log.error('Python code in `bin/qmk` and `lib/python` incorrectly formatted!')
         else:
diff --git a/lib/python/qmk/cli/pytest.py b/lib/python/qmk/cli/pytest.py
index 50a1d70a4f..bdb336b9a7 100644
--- a/lib/python/qmk/cli/pytest.py
+++ b/lib/python/qmk/cli/pytest.py
@@ -2,7 +2,7 @@
 
 QMK script to run unit and integration tests against our python code.
 """
-import subprocess
+from subprocess import DEVNULL
 
 from milc import cli
 
@@ -11,7 +11,7 @@ from milc import cli
 def pytest(cli):
     """Run several linting/testing commands.
     """
-    nose2 = subprocess.run(['nose2', '-v'])
-    flake8 = subprocess.run(['flake8', 'lib/python', 'bin/qmk'])
+    nose2 = cli.run(['nose2', '-v'], capture_output=False, stdin=DEVNULL)
+    flake8 = cli.run(['flake8', 'lib/python', 'bin/qmk'], capture_output=False, stdin=DEVNULL)
 
     return flake8.returncode | nose2.returncode