From db1eacdaacb9c8f6889f46bc1c6af155b81ad72a Mon Sep 17 00:00:00 2001 From: Zach White Date: Wed, 19 May 2021 15:24:46 -0700 Subject: 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 * fix the cpp invocation for older python * allow for unprompted installation * make sure qmk new-keyboard works on windows Co-authored-by: Ryan --- lib/python/qmk/cli/cformat.py | 10 +++++----- lib/python/qmk/cli/clean.py | 6 ++++-- lib/python/qmk/cli/compile.py | 5 +++-- lib/python/qmk/cli/doctor.py | 4 ++-- lib/python/qmk/cli/flash.py | 5 +++-- lib/python/qmk/cli/generate/docs.py | 14 ++++++++------ lib/python/qmk/cli/multibuild.py | 5 +++-- lib/python/qmk/cli/new/keyboard.py | 2 +- lib/python/qmk/cli/pyformat.py | 8 ++++---- lib/python/qmk/cli/pytest.py | 6 +++--- 10 files changed, 36 insertions(+), 29 deletions(-) (limited to 'lib/python/qmk/cli') 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 -- cgit v1.2.3