From 5ee4d6dbd030efad41974acb91060429ca695f10 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Fri, 2 Oct 2020 13:52:02 +0100 Subject: [PATCH] Enable progressive mode Adds a new option --progressive that ease adoption of the linter as it will only report failure if it detects an increase number of violations since previous commit. --- README.rst | 19 ++++++++++ lib/ansiblelint/__main__.py | 70 ++++++++++++++++++++++++++++------- lib/ansiblelint/cli.py | 6 +++ lib/ansiblelint/file_utils.py | 12 ++++++ 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/README.rst b/README.rst index d0868bb8b1..0e061fa9c8 100644 --- a/README.rst +++ b/README.rst @@ -94,6 +94,8 @@ The following is the output from ``ansible-lint --help``, providing an overview -q quieter, although not silent output -p parseable output in the format of pep8 --parseable-severity parseable output including severity of rule + --progressive Return success if it detects a reduction in number of violations compared with + previous git commit. This feature works only on git repository clones. -r RULESDIR Specify custom rule directories. Add -R to keep using embedded rules from /usr/local/lib/python3.8/site-packages/ansiblelint/rules -R Keep default rules when using -r @@ -111,6 +113,23 @@ The following is the output from ``ansible-lint --help``, providing an overview -c CONFIG_FILE Specify configuration file to use. Defaults to ".ansible-lint" --version show program's version number and exit +Progressive mode +---------------- + +In order to ease tool adoption, git users can enable the progressive mode using +``--progressive`` option. This makes the linter return a success even if +some failures are found, as long the total number of violations did not +increase since the previous commit. + +As expected, this mode makes the linter run twice if it finds any violations. +The second run is performed against a temporary git working copy that contains +the previous commit. All the violations that were already present are removed +from the list and the final result is displayed. + +The most notable benefit introduced by this mode it does not prevent merging +new code while allowing developer to address historical violation at his own +speed. + CI/CD ----- diff --git a/lib/ansiblelint/__main__.py b/lib/ansiblelint/__main__.py index cc0d5c3848..ecc8ea45fa 100755 --- a/lib/ansiblelint/__main__.py +++ b/lib/ansiblelint/__main__.py @@ -24,13 +24,16 @@ import logging import os import pathlib +import subprocess import sys +from contextlib import contextmanager from typing import TYPE_CHECKING, List, Set, Type from rich.markdown import Markdown from ansiblelint import cli, formatters from ansiblelint.color import console, console_stderr +from ansiblelint.file_utils import cwd from ansiblelint.generate_docs import rules_as_rich, rules_as_rst from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner @@ -151,23 +154,28 @@ def main() -> int: skip.update(str(s).split(',')) options.skip_list = frozenset(skip) - if not options.playbook: - # no args triggers auto-detection mode - playbooks = get_playbooks_and_roles(options=options) - else: - playbooks = sorted(set(options.playbook)) - - matches = list() - checked_files: Set[str] = set() - for playbook in playbooks: - runner = Runner(rules, playbook, options.tags, - options.skip_list, options.exclude_paths, - options.verbosity, checked_files) - matches.extend(runner.run()) + matches = _get_matches(rules, options) # Assure we do not print duplicates and the order is consistent matches = sorted(set(matches)) + mark_as_success = False + if matches and options.progressive: + _logger.info( + "Matches found, running again on previos revision in order to detect regressions") + with _previous_revision(): + old_matches = _get_matches(rules, options) + # remove old matches from current list + if len(old_matches) > len(matches): + _logger.warning( + "Total violation(s) reducted from %s to %s since previous " + "commit, will mark result as success", + len(old_matches), len(matches)) + mark_as_success = True + current_len = len(matches) + matches = list(set(matches) - set(old_matches)) + _logger.warning("Removed %s previously known violation(s)", current_len - len(matches)) + for match in matches: print(formatter.format(match, options.colored)) @@ -177,12 +185,46 @@ def main() -> int: for match in matches: print(formatter.format(match)) - if matches: + if matches and not mark_as_success: return report_outcome(matches, options=options) else: return 0 +def _get_matches(rules: RulesCollection, options: "Namespace") -> list: + + if not options.playbook: + # no args triggers auto-detection mode + playbooks = get_playbooks_and_roles(options=options) + else: + playbooks = sorted(set(options.playbook)) + + matches = list() + checked_files: Set[str] = set() + for playbook in playbooks: + runner = Runner(rules, playbook, options.tags, + options.skip_list, options.exclude_paths, + options.verbosity, checked_files) + matches.extend(runner.run()) + return matches + + +@contextmanager +def _previous_revision(): + """Create or updates a temporary workdir containing the previous revious.""" + worktree_dir = ".cache/old-rev" + revision = subprocess.run( + ["git", "rev-parse", "HEAD^1"], + check=True, + universal_newlines=True).stdout + p = pathlib.Path(worktree_dir) + p.mkdir(parents=True, exist_ok=True) + os.system(f"git worktree add -f {worktree_dir}") + with cwd(worktree_dir): + os.system(f"git checkout {revision}") + yield + + if __name__ == "__main__": try: sys.exit(main()) diff --git a/lib/ansiblelint/cli.py b/lib/ansiblelint/cli.py index 6886605992..02ed496ccd 100644 --- a/lib/ansiblelint/cli.py +++ b/lib/ansiblelint/cli.py @@ -111,6 +111,12 @@ def get_cli_parser() -> argparse.ArgumentParser: default=False, action='store_true', help="parseable output including severity of rule") + parser.add_argument('--progressive', dest='progressive', + default=False, + action='store_true', + help="Return success if it detects a reduction in number" + " of violations compared with previous git commit. This " + "feature works only on git repository clones.") parser.add_argument('-r', action=AbspathArgAction, dest='rulesdir', default=[], type=Path, help="Specify custom rule directories. Add -R " diff --git a/lib/ansiblelint/file_utils.py b/lib/ansiblelint/file_utils.py index c464191069..f25382f3ff 100644 --- a/lib/ansiblelint/file_utils.py +++ b/lib/ansiblelint/file_utils.py @@ -1,5 +1,6 @@ """Utility functions related to file operations.""" import os +from contextlib import contextmanager def normpath(path) -> str: @@ -11,3 +12,14 @@ def normpath(path) -> str: """ # convertion to string in order to allow receiving non string objects return os.path.relpath(str(path)) + + +@contextmanager +def cwd(path): + """Context manager for temporary changing current working directory.""" + old_pwd = os.getcwd() + os.chdir(path) + try: + yield + finally: + os.chdir(old_pwd)