From 797380abd9221c58dc57b7d88f47a48c1bbd63dc Mon Sep 17 00:00:00 2001 From: Cristian Le Date: Thu, 18 Apr 2024 12:04:10 +0200 Subject: [PATCH] fixup! Add pre-commit wrapper Try a simpler approach Signed-off-by: Cristian Le --- .pre-commit-hooks.yaml | 10 ++-- tmt/_pre_commit/__main__.py | 67 ++++++++++++++++++++- tmt/cli.py | 112 ++++++------------------------------ 3 files changed, 88 insertions(+), 101 deletions(-) diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 7252f17774..e3ac757b34 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -1,6 +1,8 @@ - id: tmt-lint name: tmt lint - entry: python -m tmt._pre_commit + # Use a simple wrapper instead of simply `tmt lint` in order to reorder + # some arguments that need to be passed to `tmt` + entry: python -m tmt._pre_commit --pre-check lint --failed-only --source files: '(?:.*\.fmf|.*/\.fmf/version)$' verbose: false pass_filenames: true @@ -9,7 +11,7 @@ - id: tmt-tests-lint name: tmt tests lint - entry: python -m tmt._pre_commit --lint-what tests + entry: python -m tmt._pre_commit --pre-check tests lint --failed-only --source files: '(?:.*\.fmf|.*/\.fmf/version)$' verbose: false pass_filenames: true @@ -18,7 +20,7 @@ - id: tmt-plans-lint name: tmt plans lint - entry: python -m tmt._pre_commit --lint-what plans + entry: python -m tmt._pre_commit ---pre-check plans lint --failed-only --source files: '(?:.*\.fmf|.*/\.fmf/version)$' verbose: false pass_filenames: true @@ -27,7 +29,7 @@ - id: tmt-stories-lint name: tmt stories lint - entry: python -m tmt._pre_commit --lint-what stories + entry: python -m tmt._pre_commit --pre-check stories lint --failed-only --source files: '(?:.*\.fmf|.*/\.fmf/version)$' verbose: false pass_filenames: true diff --git a/tmt/_pre_commit/__main__.py b/tmt/_pre_commit/__main__.py index c352a98f79..d2b1aacd57 100644 --- a/tmt/_pre_commit/__main__.py +++ b/tmt/_pre_commit/__main__.py @@ -1,4 +1,67 @@ -from tmt.cli import pre_commit +import re +import sys + +from tmt.__main__ import run_cli + + +def _run_precommit() -> None: # pyright: ignore[reportUnusedFunction] (used by project.scripts) + """ + A simple wrapper that re-orders cli arguments before :py:func:`run_cli`. + + This utility is needed in order to move arguments like ``--root`` before + the ``lint`` keyword when run by ``pre-commit``. + + Only a limited number of arguments are reordered. + """ + + # Only re-ordering a few known/necessary cli arguments of `main` command + # Could be improved if the click commands can be introspected like with + # `attrs.fields` + # https://github.com/pallets/click/issues/2709 + + argv = sys.argv + # Get the last argument that starts with `-`. Other inputs after that are + # assumed to be filenames (which can be thousands of them) + for last_arg in reversed(range(len(argv))): + if argv[last_arg].startswith("-"): + break + else: + # If there were no args passed, then just `run_cli` + run_cli() + return + + # At this point we need to check and re-order the arguments if needed + for pattern, nargs in [ + (r"(--root$|-r)", 1), + (r"(--root=.*)", 0), + (r"(--verbose$|-v+)", 0), + (r"(--debug$|-d+)", 0), + (r"(--pre-check$)", 0), + ]: + if any(match := re.match(pattern, arg) for arg in argv[1:last_arg + 1]): + assert match + # Actual argument matched + actual_arg = match.group(1) + indx = argv.index(actual_arg) + # Example of reorder args: + # ["tmt", "tests", "lint", "--fix", "--root", "/some/path", "some_file"] + # - argv[0]: name of the program is always first + # ["tmt"] + # - argv[indx:indx+1+nargs]: args to move (pass to `tmt.cli.main`) + # ["--root", "/some/path"] + # - argv[1:indx]: all other keywords (`lint` keyword is in here) + # ["tests", "lint", "--fix"] + # - argv[indx+1+nargs:]: All remaining keywords + # ["some_file"] + # After reorder: + # ["tmt", "--root", "/some/path", "tests", "lint", "--fix", "some_file"] + argv = ( + argv[0:1] + argv[indx: indx + 1 + nargs] + argv[1:indx] + argv[indx + 1 + nargs:] + ) + # Finally move the reordered args to sys.argv and run the cli + sys.argv = argv + run_cli() + if __name__ == "__main__": - pre_commit() + _run_precommit() diff --git a/tmt/cli.py b/tmt/cli.py index 8a4c5d4290..965c5d7671 100644 --- a/tmt/cli.py +++ b/tmt/cli.py @@ -309,6 +309,10 @@ def write_dl( '--force-color', is_flag=True, default=False, help='Forces tmt to use colors in the output and logging.' ) +@option( + '--pre-check', is_flag=True, default=False, hidden=True, + help='Run pre-checks on the git root. (Used by pre-commit wrapper).' + ) def main( click_contex: Context, root: str, @@ -316,6 +320,7 @@ def main( no_color: bool, force_color: bool, show_time: bool, + pre_check: bool, **kwargs: Any) -> None: """ Test Management Tool """ @@ -341,6 +346,18 @@ def main( # Save click context and fmf context for future use tmt.utils.Common.store_cli_invocation(click_contex) + # Run pre-checks + if pre_check: + git_command = tmt.utils.Command('git', 'rev-parse', '--show-toplevel') + git_root = git_command.run(cwd=None, logger=logger).stdout + if not git_root: + raise tmt.utils.RunError("git rev-parse did not produce a path", git_command, 0) + git_root = git_root.strip() + git_command = tmt.utils.Command( + 'git', 'ls-files', '--error-unmatch', f"{git_root}/{root}/.fmf/version" + ) + git_command.run(cwd=None, logger=logger) + # Initialize metadata tree (from given path or current directory) tree = tmt.Tree(logger=logger, path=Path(root)) @@ -2079,98 +2096,3 @@ def completion_zsh(context: Context, install: bool, **kwargs: Any) -> None: def completion_fish(context: Context, install: bool, **kwargs: Any) -> None: """ Setup shell completions for fish """ setup_completion('fish', install) - - -@click.command(context_settings={ - "ignore_unknown_options": True, - "allow_extra_args": True, - }) -@click.pass_context -@click.option( - '-r', '--root', metavar='PATH', show_default=True, default='.', - help='Variable passed to tmt main. See `tmt --help`') -@click.option( - '-v', '--verbose', count=True, default=0, - help='Variable passed to tmt main. See `tmt --help`') -@click.option( - '-d', '--debug', count=True, default=0, - help='Variable passed to tmt main. See `tmt --help`') -@click.option( - '--no-color', is_flag=True, default=False, - help='Variable passed to tmt main. See `tmt --help`' - ) -@click.option( - '--force-color', is_flag=True, default=False, - help='Variable passed to tmt main. See `tmt --help`' - ) -@click.option( - '--version', is_flag=True, - help="Variable passed to tmt main. See `tmt --help`") -@click.option( - '--lint-what', default=None, - type=click.Choice(['tests', 'plans', 'stories']), - help='tmt types to lint') -def pre_commit( - context: click.Context, - root: str, - verbose: int, - debug: int, - version: bool, - no_color: bool, - force_color: bool, - lint_type: Optional[str]) -> None: - """Cli wrapper of tmt lint.""" - # Special handling for --version - if version: - main(['--version']) - - # Create logger to reuse tmt.utils.Command utility - # apply_colors_output is not used because this is recreated and handled by main app instead - apply_colors_output, apply_colors_logging = tmt.log.decide_colorization(no_color, force_color) - logger = tmt.log.Logger.create( - apply_colors_output=apply_colors_output, - apply_colors_logging=apply_colors_logging - ) - logger.add_console_handler() - - # Check that .fmf/version file is present for the specific root requested - try: - git_command = tmt.utils.Command('git', 'rev-parse', '--show-toplevel') - git_root = git_command.run(cwd=None, logger=logger).stdout - if not git_root: - logger.fail("git rev-parse did not produce a path") - sys.exit(1) - git_root = git_root.strip() - git_command = tmt.utils.Command( - 'git', 'ls-files', '--error-unmatch', f"{git_root}/{root}/.fmf/version" - ) - git_command.run(cwd=None, logger=logger) - except tmt.utils.RunError as err: - # Forward git command error for more legible output - if err.stderr: - logger.fail(err.stderr) - sys.exit(err.returncode) - - # Construct the arguments for main cli - args = ['--root', f"{git_root}/{root}"] - if verbose: - args += ['-' + 'v' * verbose] - if debug: - args += ['-' + 'd' * debug] - if no_color: - args += ['--no-color'] - if force_color: - args += ['--force-color'] - - # Pass test/plan/stories before lint - if lint_type: - args += [lint_type] - # Pass lint and default flags of the lint command - args += ['lint', '--source', '--failed-only'] - # Get everything else - if '--source' in context.args: - context.args.remove('--source') - if '--failed-only' in context.args: - context.args.remove('--failed-only') - args += context.args - main(args)