From 08a89892121d92b98b3f837116d5cea71b088bd4 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 26 Oct 2022 22:24:41 +0100 Subject: [PATCH] Add warning message about outdated linter version (#2615) Checks every 24h if current version is latest and prints warning message if so. If HTTP requests fails, no error is displayed. --- .config/dictionary.txt | 1 + .pre-commit-config.yaml | 2 +- .pylintrc | 7 +++ src/ansiblelint/__main__.py | 13 ++-- src/ansiblelint/app.py | 9 ++- src/ansiblelint/config.py | 120 ++++++++++++++++++++++++++++++++++++ test/test_strict.py | 7 ++- 7 files changed, 149 insertions(+), 10 deletions(-) diff --git a/.config/dictionary.txt b/.config/dictionary.txt index 44e0d9e6e6..c1a3afd897 100644 --- a/.config/dictionary.txt +++ b/.config/dictionary.txt @@ -186,6 +186,7 @@ ungrouped unignored unimported unindented +uninstallation unjinja unlex unnormalized diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 77a26a1179..fc55b6e52b 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -161,7 +161,7 @@ repos: plugins/.* )$ - repo: https://github.com/pycqa/pylint - rev: v2.15.3 + rev: v2.15.5 hooks: - id: pylint additional_dependencies: diff --git a/.pylintrc b/.pylintrc index c27006e083..8e25094302 100644 --- a/.pylintrc +++ b/.pylintrc @@ -1,3 +1,7 @@ +[MAIN] +extension-pkg-allow-list = + black.parsing, + [IMPORTS] preferred-modules = py:pathlib, @@ -12,6 +16,9 @@ ignore-paths=^src/ansiblelint/_version.*$ [MESSAGES CONTROL] +# increase from default is 50 which is too aggressive +max-statements = 60 + disable = # On purpose disabled as we rely on black line-too-long, diff --git a/src/ansiblelint/__main__.py b/src/ansiblelint/__main__.py index 6614140900..813e716073 100755 --- a/src/ansiblelint/__main__.py +++ b/src/ansiblelint/__main__.py @@ -41,7 +41,7 @@ from ansiblelint._mockings import _perform_mockings_cleanup from ansiblelint.app import get_app from ansiblelint.color import console, console_options, reconfigure, render_yaml -from ansiblelint.config import options +from ansiblelint.config import get_version_warning, options from ansiblelint.constants import EXIT_CONTROL_C_RC, LOCK_TIMEOUT_RC from ansiblelint.file_utils import abspath, cwd, normpath from ansiblelint.skip_utils import normalize_tag @@ -88,10 +88,6 @@ def initialize_options(arguments: list[str] | None = None) -> None: new_options = cli.get_config(arguments or []) new_options.cwd = pathlib.Path.cwd() - if new_options.version: - print(f"ansible-lint {__version__} using ansible {ansible_version()}") - sys.exit(0) - if new_options.colored is None: new_options.colored = should_do_markup() @@ -187,6 +183,13 @@ def main(argv: list[str] | None = None) -> int: # noqa: C901 console_options["force_terminal"] = options.colored reconfigure(console_options) + if options.version: + console.print( + f"ansible-lint [repr.number]{__version__}[/] using ansible [repr.number]{ansible_version()}[/]" + ) + console.print(get_version_warning()) + sys.exit(0) + initialize_logger(options.verbosity) _logger.debug("Options: %s", options) _logger.debug(os.getcwd()) diff --git a/src/ansiblelint/app.py b/src/ansiblelint/app.py index ad726f879e..0699d15aef 100644 --- a/src/ansiblelint/app.py +++ b/src/ansiblelint/app.py @@ -13,7 +13,7 @@ from ansiblelint import formatters from ansiblelint._mockings import _perform_mockings from ansiblelint.color import console, console_stderr, render_yaml -from ansiblelint.config import PROFILES +from ansiblelint.config import PROFILES, get_version_warning from ansiblelint.config import options as default_options from ansiblelint.constants import RULE_DOC_URL, SUCCESS_RC, VIOLATIONS_FOUND_RC from ansiblelint.errors import MatchError @@ -204,8 +204,8 @@ def report_outcome(self, result: LintResult, mark_as_success: bool = False) -> i return SUCCESS_RC if mark_as_success else VIOLATIONS_FOUND_RC - @staticmethod def report_summary( # pylint: disable=too-many-branches,too-many-locals + self, summary: SummarizedResults, changed_files_count: int, files_count: int, @@ -290,6 +290,11 @@ def report_summary( # pylint: disable=too-many-branches,too-many-locals msg += f", and fixed {summary.fixed} issue(s)" msg += f" on {files_count} files." + if not self.options.offline: + version_warning = get_version_warning() + if version_warning: + msg += f"\n{version_warning}" + console_stderr.print(msg) diff --git a/src/ansiblelint/config.py b/src/ansiblelint/config.py index 96f6c813d0..e0a32ce190 100644 --- a/src/ansiblelint/config.py +++ b/src/ansiblelint/config.py @@ -1,15 +1,32 @@ """Store configuration options as a singleton.""" from __future__ import annotations +import json +import logging import os import re +import sys +import time +import urllib.request +import warnings from argparse import Namespace from functools import lru_cache from pathlib import Path from typing import Any +from urllib.error import HTTPError, URLError +from packaging.version import Version + +from ansiblelint import __version__ from ansiblelint.loaders import yaml_from_file +_logger = logging.getLogger(__name__) + + +CACHE_DIR = ( + os.path.expanduser(os.environ.get("XDG_CONFIG_CACHE", "~/.cache")) + "/ansible-lint" +) + DEFAULT_WARN_LIST = [ "avoid-implicit", "experimental", @@ -171,3 +188,106 @@ def parse_ansible_version(stdout: str) -> tuple[str, str | None]: if match: return match.group(1), None return "", f"FATAL: Unable parse ansible cli version: {stdout}" + + +def in_venv() -> bool: + """Determine whether Python is running from a venv.""" + if hasattr(sys, "real_prefix"): + return True + pfx = getattr(sys, "base_prefix", sys.prefix) + return pfx != sys.prefix + + +def guess_install_method() -> str: + """Guess if pip upgrade command should be used.""" + pip = "" + if in_venv(): + _logger.debug("Found virtualenv, assuming `pip3 install` will work.") + pip = f"pip install --upgrade {__package__}" + elif __file__.startswith(os.path.expanduser("~/.local/lib")): + _logger.debug( + "Found --user installation, assuming `pip3 install --user` will work." + ) + pip = f"pip3 install --user --upgrade {__package__}" + + # By default we assume pip is not safe to be used + use_pip = False + package_name = "ansible-lint" + try: + # Use pip to detect if is safe to use it to upgrade the package. + # We do imports here to for performance and reasons, and also in order + # to avoid errors if pip internals change. Also we want to avoid having + # to add pip as a dependency, so we make use of it only when present. + + # trick to avoid runtime warning from inside pip: _distutils_hack/__init__.py:33: UserWarning: Setuptools is replacing distutils. + with warnings.catch_warnings(record=True): + warnings.simplefilter("always") + # pylint: disable=import-outside-toplevel + from pip._internal.exceptions import UninstallationError + from pip._internal.metadata import get_default_environment + from pip._internal.req.req_uninstall import uninstallation_paths + + try: + dist = get_default_environment().get_distribution(package_name) + if dist: + logging.debug("Found %s dist", dist) + for _ in uninstallation_paths(dist): + use_pip = True + else: + logging.debug("Skipping %s as it is not installed.", package_name) + use_pip = False + except UninstallationError as exc: + logging.debug(exc) + use_pip = False + except ImportError: + use_pip = False + + # We only want to recommend pip for upgrade if it looks safe to do so. + return pip if use_pip else "" + + +def get_version_warning() -> str: + """Display warning if current version is outdated.""" + msg = "" + data = {} + current_version = Version(__version__) + if not os.path.exists(CACHE_DIR): + os.makedirs(CACHE_DIR) + cache_file = f"{CACHE_DIR}/latest.json" + refresh = True + if os.path.exists(cache_file): + age = time.time() - os.path.getmtime(cache_file) + if age < 24 * 60 * 60: + refresh = False + with open(cache_file, encoding="utf-8") as f: + data = json.load(f) + + if refresh or not data: + release_url = ( + "https://api.github.com/repos/ansible/ansible-lint/releases/latest" + ) + try: + with urllib.request.urlopen(release_url) as url: + data = json.load(url) + with open(cache_file, "w", encoding="utf-8") as f: + json.dump(data, f) + except (URLError, HTTPError) as exc: + _logger.debug( + "Unable to fetch latest version from %s due to: %s", release_url, exc + ) + return "" + + html_url = data["html_url"] + new_version = Version(data["tag_name"][1:]) # removing v prefix from tag + # breakpoint() + + if current_version > new_version: + msg = "[dim]You are using a pre-release version of ansible-lint.[/]" + elif current_version < new_version: + msg = f"""[warning]A new release of ansible-lint is available: [red]{current_version}[/] → [green][link={html_url}]{new_version}[/][/][/]""" + + pip = guess_install_method() + if pip: + msg += f" Upgrade by running: [info]{pip}[/]" + + return msg diff --git a/test/test_strict.py b/test/test_strict.py index 77a9b44562..bd3cbd1c34 100644 --- a/test/test_strict.py +++ b/test/test_strict.py @@ -19,5 +19,8 @@ def test_strict(strict: bool, returncode: int, message: str) -> None: result = run_ansible_lint(*args) assert result.returncode == returncode assert "key-order[task]" in result.stdout - summary_line = result.stderr.splitlines()[-1] - assert message in summary_line + for summary_line in result.stderr.splitlines(): + if summary_line.startswith(message): + break + else: + pytest.fail(f"Failed to find {message} inside stderr output")