From 0b41c13509b5547028f141d869e10199566a1228 Mon Sep 17 00:00:00 2001 From: Joel Challis Date: Thu, 20 Oct 2022 14:35:27 +0100 Subject: [PATCH] [CLI] Ensure consistent clean behaviour (#18781) --- lib/python/qmk/cli/compile.py | 57 +++++++++-------------- lib/python/qmk/cli/flash.py | 85 +++++++++++++---------------------- lib/python/qmk/commands.py | 35 +++++++++++++-- 3 files changed, 84 insertions(+), 93 deletions(-) diff --git a/lib/python/qmk/cli/compile.py b/lib/python/qmk/cli/compile.py index 95118e6687..9e7629906f 100755 --- a/lib/python/qmk/cli/compile.py +++ b/lib/python/qmk/cli/compile.py @@ -2,14 +2,13 @@ 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 import qmk.path from qmk.decorators import automagic_keyboard, automagic_keymap -from qmk.commands import compile_configurator_json, create_make_command, parse_configurator_json +from qmk.commands import compile_configurator_json, create_make_command, parse_configurator_json, build_environment from qmk.keyboard import keyboard_completer, keyboard_folder from qmk.keymap import keymap_completer @@ -31,48 +30,32 @@ def compile(cli): If a keyboard and keymap are provided this command will build a firmware based on that. """ - if cli.args.clean and not cli.args.filename and not cli.args.dry_run: - if cli.config.compile.keyboard and cli.config.compile.keymap: - command = create_make_command(cli.config.compile.keyboard, cli.config.compile.keymap, 'clean') - cli.run(command, capture_output=False, stdin=DEVNULL) - # Build the environment vars - envs = {} - for env in cli.args.env: - if '=' in env: - key, value = env.split('=', 1) - envs[key] = value - else: - cli.log.warning('Invalid environment variable: %s', env) + envs = build_environment(cli.args.env) # Determine the compile command - command = None + commands = [] if cli.args.filename: # If a configurator JSON was provided generate a keymap and compile it user_keymap = parse_configurator_json(cli.args.filename) - command = compile_configurator_json(user_keymap, parallel=cli.config.compile.parallel, **envs) + commands = [compile_configurator_json(user_keymap, parallel=cli.config.compile.parallel, clean=cli.args.clean, **envs)] - else: - if cli.config.compile.keyboard and cli.config.compile.keymap: - # Generate the make command for a specific keyboard/keymap. - command = create_make_command(cli.config.compile.keyboard, cli.config.compile.keymap, parallel=cli.config.compile.parallel, **envs) + elif cli.config.compile.keyboard and cli.config.compile.keymap: + # Generate the make command for a specific keyboard/keymap. + if cli.args.clean: + commands.append(create_make_command(cli.config.compile.keyboard, cli.config.compile.keymap, 'clean', **envs)) + commands.append(create_make_command(cli.config.compile.keyboard, cli.config.compile.keymap, parallel=cli.config.compile.parallel, **envs)) - elif not cli.config.compile.keyboard: - cli.log.error('Could not determine keyboard!') - elif not cli.config.compile.keymap: - cli.log.error('Could not determine keymap!') - - # Compile the firmware, if we're able to - if command: - cli.log.info('Compiling keymap with {fg_cyan}%s', ' '.join(command)) - if not cli.args.dry_run: - cli.echo('\n') - # FIXME(skullydazed/anyone): Remove text=False once milc 1.0.11 has had enough time to be installed everywhere. - compile = cli.run(command, capture_output=False, text=False) - return compile.returncode - - else: + if not commands: cli.log.error('You must supply a configurator export, both `--keyboard` and `--keymap`, or be in a directory for a keyboard or keymap.') - cli.echo('usage: qmk compile [-h] [-b] [-kb KEYBOARD] [-km KEYMAP] [filename]') + cli.print_help() return False + + cli.log.info('Compiling keymap with {fg_cyan}%s', ' '.join(commands[-1])) + if not cli.args.dry_run: + cli.echo('\n') + for command in commands: + ret = cli.run(command, capture_output=False) + if ret.returncode: + return ret.returncode diff --git a/lib/python/qmk/cli/flash.py b/lib/python/qmk/cli/flash.py index c39f4b36d4..40bfbdab56 100644 --- a/lib/python/qmk/cli/flash.py +++ b/lib/python/qmk/cli/flash.py @@ -3,15 +3,13 @@ You can compile a keymap already in the repo or using a QMK Configurator export. A bootloader must be specified. """ -from subprocess import DEVNULL -import sys - from argcomplete.completers import FilesCompleter + from milc import cli import qmk.path from qmk.decorators import automagic_keyboard, automagic_keymap -from qmk.commands import compile_configurator_json, create_make_command, parse_configurator_json +from qmk.commands import compile_configurator_json, create_make_command, parse_configurator_json, build_environment from qmk.keyboard import keyboard_completer, keyboard_folder from qmk.flashers import flasher @@ -75,59 +73,40 @@ def flash(cli): return False except KeyboardInterrupt: cli.log.info('Ctrl-C was pressed, exiting...') - sys.exit(0) + return True - else: - if cli.args.clean and not cli.args.filename and not cli.args.dry_run: - if cli.config.flash.keyboard and cli.config.flash.keymap: - command = create_make_command(cli.config.flash.keyboard, cli.config.flash.keymap, 'clean') - cli.run(command, capture_output=False, stdin=DEVNULL) + if cli.args.bootloaders: + # Provide usage and list bootloaders + cli.print_help() + print_bootloader_help() + return False - # Build the environment vars - envs = {} - for env in cli.args.env: - if '=' in env: - key, value = env.split('=', 1) - envs[key] = value - else: - cli.log.warning('Invalid environment variable: %s', env) + # Build the environment vars + envs = build_environment(cli.args.env) - # Determine the compile command - command = '' + # Determine the compile command + commands = [] - if cli.args.bootloaders: - # Provide usage and list bootloaders - cli.echo('usage: qmk flash [-h] [-b] [-n] [-kb KEYBOARD] [-km KEYMAP] [-bl BOOTLOADER] [filename]') - print_bootloader_help() - return False + if cli.args.filename: + # If a configurator JSON was provided generate a keymap and compile it + user_keymap = parse_configurator_json(cli.args.filename) + commands = [compile_configurator_json(user_keymap, cli.args.bootloader, parallel=cli.config.flash.parallel, clean=cli.args.clean, **envs)] - if cli.args.filename: - # Handle compiling a configurator JSON - user_keymap = parse_configurator_json(cli.args.filename) - keymap_path = qmk.path.keymap(user_keymap['keyboard']) - command = compile_configurator_json(user_keymap, cli.args.bootloader, parallel=cli.config.flash.parallel, **envs) + elif cli.config.flash.keyboard and cli.config.flash.keymap: + # Generate the make command for a specific keyboard/keymap. + if cli.args.clean: + commands.append(create_make_command(cli.config.flash.keyboard, cli.config.flash.keymap, 'clean', **envs)) + commands.append(create_make_command(cli.config.flash.keyboard, cli.config.flash.keymap, cli.args.bootloader, parallel=cli.config.flash.parallel, **envs)) - cli.log.info('Wrote keymap to {fg_cyan}%s/%s/keymap.c', keymap_path, user_keymap['keymap']) + if not commands: + cli.log.error('You must supply a configurator export, both `--keyboard` and `--keymap`, or be in a directory for a keyboard or keymap.') + cli.print_help() + return False - else: - if cli.config.flash.keyboard and cli.config.flash.keymap: - # Generate the make command for a specific keyboard/keymap. - command = create_make_command(cli.config.flash.keyboard, cli.config.flash.keymap, cli.args.bootloader, parallel=cli.config.flash.parallel, **envs) - - elif not cli.config.flash.keyboard: - cli.log.error('Could not determine keyboard!') - elif not cli.config.flash.keymap: - cli.log.error('Could not determine keymap!') - - # Compile the firmware, if we're able to - if command: - 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, stdin=DEVNULL) - return compile.returncode - - else: - cli.log.error('You must supply a configurator export, both `--keyboard` and `--keymap`, or be in a directory for a keyboard or keymap.') - cli.echo('usage: qmk flash [-h] [-b] [-n] [-kb KEYBOARD] [-km KEYMAP] [-bl BOOTLOADER] [filename]') - return False + cli.log.info('Compiling keymap with {fg_cyan}%s', ' '.join(commands[-1])) + if not cli.args.dry_run: + cli.echo('\n') + for command in commands: + ret = cli.run(command, capture_output=False) + if ret.returncode: + return ret.returncode diff --git a/lib/python/qmk/commands.py b/lib/python/qmk/commands.py index 2ab506c710..07826a4866 100644 --- a/lib/python/qmk/commands.py +++ b/lib/python/qmk/commands.py @@ -107,7 +107,7 @@ def get_make_parallel_args(parallel=1): return parallel_args -def compile_configurator_json(user_keymap, bootloader=None, parallel=1, **env_vars): +def compile_configurator_json(user_keymap, bootloader=None, parallel=1, clean=False, **env_vars): """Convert a configurator export JSON file into a C file and then compile it. Args: @@ -129,7 +129,6 @@ def compile_configurator_json(user_keymap, bootloader=None, parallel=1, **env_va # e.g.: qmk compile - < keyboards/clueboard/california/keymaps/default/keymap.json user_keymap["keymap"] = user_keymap.get("keymap", "default_json") - # Write the keymap.c file keyboard_filesafe = user_keymap['keyboard'].replace('/', '_') target = f'{keyboard_filesafe}_{user_keymap["keymap"]}' keyboard_output = Path(f'{KEYBOARD_OUTPUT_PREFIX}{keyboard_filesafe}') @@ -137,8 +136,25 @@ def compile_configurator_json(user_keymap, bootloader=None, parallel=1, **env_va keymap_dir = keymap_output / 'src' keymap_json = keymap_dir / 'keymap.json' + if clean: + if keyboard_output.exists(): + shutil.rmtree(keyboard_output) + if keymap_output.exists(): + shutil.rmtree(keymap_output) + + # begin with making the deepest folder in the tree keymap_dir.mkdir(exist_ok=True, parents=True) - keymap_json.write_text(json.dumps(user_keymap), encoding='utf-8') + + # Compare minified to ensure consistent comparison + new_content = json.dumps(user_keymap, separators=(',', ':')) + if keymap_json.exists(): + old_content = json.dumps(json.loads(keymap_json.read_text(encoding='utf-8')), separators=(',', ':')) + if old_content == new_content: + new_content = None + + # Write the keymap.json file if different + if new_content: + keymap_json.write_text(new_content, encoding='utf-8') # Return a command that can be run to make the keymap and flash if given verbose = 'true' if cli.config.general.verbose else 'false' @@ -210,6 +226,19 @@ def parse_configurator_json(configurator_file): return user_keymap +def build_environment(args): + """Common processing for cli.args.env + """ + envs = {} + for env in args: + if '=' in env: + key, value = env.split('=', 1) + envs[key] = value + else: + cli.log.warning('Invalid environment variable: %s', env) + return envs + + def in_virtualenv(): """Check if running inside a virtualenv. Based on https://stackoverflow.com/a/1883251