diff options
| author | Zach White <[email protected]> | 2021-05-19 15:24:46 -0700 | 
|---|---|---|
| committer | GitHub <[email protected]> | 2021-05-19 15:24:46 -0700 | 
| commit | db1eacdaacb9c8f6889f46bc1c6af155b81ad72a (patch) | |
| tree | cd32a69a04c7ff93a11941d82aef3ce31c2b7523 /lib/python | |
| parent | a9aec546c873fa5a2cb1d9a10878aca71818b609 (diff) | |
| download | qmk_firmware-db1eacdaacb9c8f6889f46bc1c6af155b81ad72a.tar.gz qmk_firmware-db1eacdaacb9c8f6889f46bc1c6af155b81ad72a.zip | |
Align our subprocess usage with current best practices. (#12940)0.12.50
* Align our subprocess usage with current best practices.
* remove unused import
* Apply suggestions from code review
Co-authored-by: Ryan <[email protected]>
* fix the cpp invocation for older python
* allow for unprompted installation
* make sure qmk new-keyboard works on windows
Co-authored-by: Ryan <[email protected]>
Diffstat (limited to 'lib/python')
| -rw-r--r-- | lib/python/qmk/cli/cformat.py | 10 | ||||
| -rw-r--r-- | lib/python/qmk/cli/clean.py | 6 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/compile.py | 5 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/doctor.py | 4 | ||||
| -rw-r--r-- | lib/python/qmk/cli/flash.py | 5 | ||||
| -rw-r--r-- | lib/python/qmk/cli/generate/docs.py | 14 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/multibuild.py | 5 | ||||
| -rw-r--r-- | lib/python/qmk/cli/new/keyboard.py | 2 | ||||
| -rwxr-xr-x | lib/python/qmk/cli/pyformat.py | 8 | ||||
| -rw-r--r-- | lib/python/qmk/cli/pytest.py | 6 | ||||
| -rw-r--r-- | lib/python/qmk/commands.py | 23 | ||||
| -rw-r--r-- | lib/python/qmk/keymap.py | 9 | ||||
| -rw-r--r-- | lib/python/qmk/os_helpers/__init__.py | 5 | ||||
| -rw-r--r-- | lib/python/qmk/os_helpers/linux/__init__.py | 3 | ||||
| -rw-r--r-- | lib/python/qmk/submodules.py | 17 | ||||
| -rw-r--r-- | lib/python/qmk/tests/test_cli_commands.py | 9 | 
16 files changed, 58 insertions, 73 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 diff --git a/lib/python/qmk/commands.py b/lib/python/qmk/commands.py index 8c3f95ea21..ee049e8af7 100644 --- a/lib/python/qmk/commands.py +++ b/lib/python/qmk/commands.py @@ -2,11 +2,9 @@  """  import json  import os -import platform -import subprocess -import shlex  import shutil  from pathlib import Path +from subprocess import DEVNULL  from time import strftime  from milc import cli @@ -94,7 +92,7 @@ def get_git_version(repo_dir='.', check_dir='.'):      git_describe_cmd = ['git', 'describe', '--abbrev=6', '--dirty', '--always', '--tags']      if Path(check_dir).exists(): -        git_describe = cli.run(git_describe_cmd, cwd=repo_dir) +        git_describe = cli.run(git_describe_cmd, stdin=DEVNULL, cwd=repo_dir)          if git_describe.returncode == 0:              return git_describe.stdout.strip() @@ -224,20 +222,3 @@ def parse_configurator_json(configurator_file):              user_keymap['layout'] = aliases[orig_keyboard]['layouts'][user_keymap['layout']]      return user_keymap - - -def run(command, *args, **kwargs): -    """Run a command with subprocess.run -    """ -    platform_id = platform.platform().lower() - -    if isinstance(command, str): -        raise TypeError('`command` must be a non-text sequence such as list or tuple.') - -    if 'windows' in platform_id: -        safecmd = map(str, command) -        safecmd = map(shlex.quote, safecmd) -        safecmd = ' '.join(safecmd) -        command = [os.environ.get('SHELL', '/usr/bin/bash'), '-c', safecmd] - -    return subprocess.run(command, *args, **kwargs) diff --git a/lib/python/qmk/keymap.py b/lib/python/qmk/keymap.py index 4ad9ffb591..ac7951082e 100644 --- a/lib/python/qmk/keymap.py +++ b/lib/python/qmk/keymap.py @@ -1,9 +1,9 @@  """Functions that help you work with QMK keymaps.  """  import json -import subprocess  import sys  from pathlib import Path +from subprocess import DEVNULL  import argcomplete  from milc import cli @@ -12,7 +12,6 @@ from pygments.token import Token  from pygments import lex  import qmk.path -import qmk.commands  from qmk.keyboard import find_keyboard_from_dir, rules_mk  # The `keymap.c` template to use when a keyboard doesn't have its own @@ -361,7 +360,7 @@ def list_keymaps(keyboard, c=True, json=True, additional_files=None, fullpath=Fa      return sorted(names) -def _c_preprocess(path, stdin=None): +def _c_preprocess(path, stdin=DEVNULL):      """ Run a file through the C pre-processor      Args: @@ -371,7 +370,9 @@ def _c_preprocess(path, stdin=None):      Returns:          the stdout of the pre-processor      """ -    pre_processed_keymap = qmk.commands.run(['cpp', path] if path else ['cpp'], stdin=stdin, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) +    cmd = ['cpp', str(path)] if path else ['cpp'] +    pre_processed_keymap = cli.run(cmd, stdin=stdin) +      return pre_processed_keymap.stdout diff --git a/lib/python/qmk/os_helpers/__init__.py b/lib/python/qmk/os_helpers/__init__.py index 3f64a63a3a..3e98db3c32 100644 --- a/lib/python/qmk/os_helpers/__init__.py +++ b/lib/python/qmk/os_helpers/__init__.py @@ -3,10 +3,9 @@  from enum import Enum  import re  import shutil -import subprocess +from subprocess import DEVNULL  from milc import cli -from qmk.commands import run  from qmk import submodules  from qmk.constants import QMK_FIRMWARE @@ -142,7 +141,7 @@ def is_executable(command):      # Make sure the command can be executed      version_arg = ESSENTIAL_BINARIES[command].get('version_arg', '--version') -    check = run([command, version_arg], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, timeout=5, universal_newlines=True) +    check = cli.run([command, version_arg], combined_output=True, stdin=DEVNULL, timeout=5)      ESSENTIAL_BINARIES[command]['output'] = check.stdout diff --git a/lib/python/qmk/os_helpers/linux/__init__.py b/lib/python/qmk/os_helpers/linux/__init__.py index 9e73964e47..008654ab0f 100644 --- a/lib/python/qmk/os_helpers/linux/__init__.py +++ b/lib/python/qmk/os_helpers/linux/__init__.py @@ -5,7 +5,6 @@ import shutil  from milc import cli  from qmk.constants import QMK_FIRMWARE -from qmk.commands import run  from qmk.os_helpers import CheckStatus @@ -132,7 +131,7 @@ def check_modem_manager():      """      if check_systemd(): -        mm_check = run(["systemctl", "--quiet", "is-active", "ModemManager.service"], timeout=10) +        mm_check = cli.run(["systemctl", "--quiet", "is-active", "ModemManager.service"], timeout=10)          if mm_check.returncode == 0:              return True      else: diff --git a/lib/python/qmk/submodules.py b/lib/python/qmk/submodules.py index be51a68043..6a272dae50 100644 --- a/lib/python/qmk/submodules.py +++ b/lib/python/qmk/submodules.py @@ -1,7 +1,6 @@  """Functions for working with QMK's submodules.  """ - -import subprocess +from milc import cli  def status(): @@ -18,7 +17,7 @@ def status():      status is None when the submodule doesn't exist, False when it's out of date, and True when it's current      """      submodules = {} -    git_cmd = subprocess.run(['git', 'submodule', 'status'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, timeout=30, universal_newlines=True) +    git_cmd = cli.run(['git', 'submodule', 'status'], timeout=30)      for line in git_cmd.stdout.split('\n'):          if not line: @@ -53,19 +52,19 @@ def update(submodules=None):          # Update everything          git_sync_cmd.append('--recursive')          git_update_cmd.append('--recursive') -        subprocess.run(git_sync_cmd, check=True) -        subprocess.run(git_update_cmd, check=True) +        cli.run(git_sync_cmd, check=True) +        cli.run(git_update_cmd, check=True)      else:          if isinstance(submodules, str):              # Update only a single submodule              git_sync_cmd.append(submodules)              git_update_cmd.append(submodules) -            subprocess.run(git_sync_cmd, check=True) -            subprocess.run(git_update_cmd, check=True) +            cli.run(git_sync_cmd, check=True) +            cli.run(git_update_cmd, check=True)          else:              # Update submodules in a list              for submodule in submodules: -                subprocess.run(git_sync_cmd + [submodule], check=True) -                subprocess.run(git_update_cmd + [submodule], check=True) +                cli.run([*git_sync_cmd, submodule], check=True) +                cli.run([*git_update_cmd, submodule], check=True) diff --git a/lib/python/qmk/tests/test_cli_commands.py b/lib/python/qmk/tests/test_cli_commands.py index 741551e5ec..d13e42d408 100644 --- a/lib/python/qmk/tests/test_cli_commands.py +++ b/lib/python/qmk/tests/test_cli_commands.py @@ -1,15 +1,14 @@  import platform +from subprocess import DEVNULL -from subprocess import STDOUT, PIPE - -from qmk.commands import run +from milc import cli  is_windows = 'windows' in platform.platform().lower()  def check_subcommand(command, *args):      cmd = ['bin/qmk', command, *args] -    result = run(cmd, stdout=PIPE, stderr=STDOUT, universal_newlines=True) +    result = cli.run(cmd, stdin=DEVNULL, combined_output=True)      return result @@ -18,7 +17,7 @@ def check_subcommand_stdin(file_to_read, command, *args):      """      with open(file_to_read, encoding='utf-8') as my_file:          cmd = ['bin/qmk', command, *args] -        result = run(cmd, stdin=my_file, stdout=PIPE, stderr=STDOUT, universal_newlines=True) +        result = cli.run(cmd, stdin=my_file, combined_output=True)      return result | 
