From a6f196440ad8c9b65a5bbc31a88d8c80f2ba2ded Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 27 Jul 2022 10:56:46 +0530 Subject: [PATCH 1/2] refactor(cli): Commands Resolution The implementations so far were hacks that worked for the most used commands but broken down when challenged or expected to maintain documented usages [eg: custom app commands]. The current implementation consists of a two step approach: 1. figure out command name that user is trying to execute 2. pass the directive to the app (bench, frappe or other) that consists of the cmd --- For tackling #1, get_cmd_from_sysargv contains exhaustive rules that cover all (that i know and ive come across) combinations of valid frappe commands. For problem #2, a simple check in click's Group object does the trick. Tested with possible known commands with combinations of context flags and params, with bench, frappe & external app's commands --- bench/cli.py | 59 ++++++++++++++++++++++------------------- bench/utils/__init__.py | 46 +++++++++++++++++++++++++------- 2 files changed, 68 insertions(+), 37 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index 9cf946cc..bfe6f4c3 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -1,6 +1,8 @@ # imports - standard imports import atexit +from contextlib import contextmanager import json +from logging import Logger import os import pwd import sys @@ -25,7 +27,7 @@ from bench.utils import ( is_root, log, setup_logging, - parse_sys_argv, + get_cmd_from_sysargv, ) from bench.utils.bench import get_env_cmd @@ -35,23 +37,41 @@ verbose = False is_envvar_warn_set = None from_command_line = False # set when commands are executed via the CLI bench.LOG_BUFFER = [] -sys_argv = None change_uid_msg = "You should not run this command as root" src = os.path.dirname(__file__) +@contextmanager +def execute_cmd(check_for_update=True, command: str = None, logger: Logger = None): + if check_for_update: + atexit.register(check_latest_version) + + try: + yield + except BaseException as e: + return_code = getattr(e, "code", 1) + + if isinstance(e, Exception): + click.secho(f"ERROR: {e}", fg="red") + + if return_code: + logger.warning(f"{command} executed with exit code {return_code}") + + raise e + + def cli(): - global from_command_line, bench_config, is_envvar_warn_set, verbose, sys_argv + global from_command_line, bench_config, is_envvar_warn_set, verbose from_command_line = True command = " ".join(sys.argv) argv = set(sys.argv) is_envvar_warn_set = not (os.environ.get("BENCH_DEVELOPER") or os.environ.get("CI")) is_cli_command = len(sys.argv) > 1 and not argv.intersection({"src", "--version"}) - sys_argv = parse_sys_argv() + cmd_from_sys = get_cmd_from_sysargv() - if "--verbose" in sys_argv.options: + if "--verbose" in argv: verbose = True change_working_directory() @@ -69,8 +89,8 @@ def cli(): if ( is_envvar_warn_set and is_cli_command - and is_dist_editable(bench.PROJECT_NAME) and not bench_config.get("developer_mode") + and is_dist_editable(bench.PROJECT_NAME) ): log( "bench is installed in editable mode!\n\nThis is not the recommended mode" @@ -95,31 +115,14 @@ def cli(): print(get_frappe_help()) return - if ( - sys_argv.commands.intersection(get_cached_frappe_commands()) - or sys_argv.commands.intersection(get_frappe_commands()) - ): + if cmd_from_sys in bench_command.commands: + with execute_cmd(check_for_update=not is_cli_command, command=command, logger=logger): + bench_command() + elif cmd_from_sys in get_frappe_commands(): frappe_cmd() - - if sys.argv[1] in Bench(".").apps: + else: app_cmd() - if not is_cli_command: - atexit.register(check_latest_version) - - try: - bench_command() - except BaseException as e: - return_code = getattr(e, "code", 1) - - if isinstance(e, Exception): - click.secho(f"ERROR: {e}", fg="red") - - if return_code: - logger.warning(f"{command} executed with exit code {return_code}") - - raise e - def check_uid(): if cmd_requires_root() and not is_root(): diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index 85746877..cd323ca1 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -165,7 +165,7 @@ def which(executable: str, raise_err: bool = False) -> str: return exec_ -def setup_logging(bench_path=".") -> "logger": +def setup_logging(bench_path=".") -> logging.Logger: LOG_LEVEL = 15 logging.addLevelName(LOG_LEVEL, "LOG") @@ -529,13 +529,41 @@ class _dict(dict): return _dict(dict(self).copy()) -def parse_sys_argv(): - sys_argv = _dict(options=set(), commands=set()) +def get_cmd_from_sysargv(): + """Identify and segregate tokens to options and command - for c in sys.argv[1:]: - if c.startswith("-"): - sys_argv.options.add(c) - else: - sys_argv.commands.add(c) + For Command: `bench --profile --site frappeframework.com migrate --no-backup` + sys.argv: ["/home/frappe/.local/bin/bench", "--profile", "--site", "frappeframework.com", "migrate", "--no-backup"] + Actual command run: migrate - return sys_argv + """ + # context is passed as options to frappe's bench_helper + from bench.bench import Bench + frappe_context = _dict( + params={"--site"}, + flags={"--verbose", "--profile", "--force"} + ) + cmd_from_ctx = None + sys_argv = sys.argv[1:] + skip_next = False + + for arg in sys_argv: + if skip_next: + skip_next = False + continue + + if arg in frappe_context.flags: + continue + + elif arg in frappe_context.params: + skip_next = True + continue + + if sys_argv.index(arg) == 0 and arg in Bench(".").apps: + continue + + cmd_from_ctx = arg + + break + + return cmd_from_ctx From 8a0b78451b3ef33db342c3081ee371bc29e4a82f Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Wed, 27 Jul 2022 14:21:44 +0530 Subject: [PATCH 2/2] fix: Add support for options on bench main group Options like --use-feature, --version are tested and support maintained by the changes defined in this commit --- bench/cli.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/bench/cli.py b/bench/cli.py index bfe6f4c3..b874532a 100755 --- a/bench/cli.py +++ b/bench/cli.py @@ -109,20 +109,31 @@ def cli(): ): log("Command not being executed in bench directory", level=3) - if in_bench and len(sys.argv) > 1: - if sys.argv[1] == "--help": - print(click.Context(bench_command).get_help()) + if len(sys.argv) == 1 or sys.argv[1] == "--help": + print(click.Context(bench_command).get_help()) + if in_bench: print(get_frappe_help()) - return + return - if cmd_from_sys in bench_command.commands: - with execute_cmd(check_for_update=not is_cli_command, command=command, logger=logger): - bench_command() - elif cmd_from_sys in get_frappe_commands(): + _opts = [x.opts + x.secondary_opts for x in bench_command.params] + opts = {item for sublist in _opts for item in sublist} + + # handle usages like `--use-feature='feat-x'` and `--use-feature 'feat-x'` + if cmd_from_sys and cmd_from_sys.split("=", 1)[0].strip() in opts: + bench_command() + + if cmd_from_sys in bench_command.commands: + with execute_cmd(check_for_update=not is_cli_command, command=command, logger=logger): + bench_command() + + if in_bench: + if cmd_from_sys in get_frappe_commands(): frappe_cmd() else: app_cmd() + bench_command() + def check_uid(): if cmd_requires_root() and not is_root():