diff --git a/doc/rdopkg.1.adoc b/doc/rdopkg.1.adoc index 7e676200..aced0c4a 100644 --- a/doc/rdopkg.1.adoc +++ b/doc/rdopkg.1.adoc @@ -241,6 +241,33 @@ and corresponding `Resolves:` lines in commit message. More use cases can be found in link:rdopkg-feature-new-version.7.html[rdopkg-feature-new-version(7)]. + +ACTION: lint +~~~~~~~~~~~~ + +Run checks for errors in current distgit. + +Available checks selectable with `--lint-checks`: + +* `sanity`: internal rdopkg sanity checks on the .spec +* `rpmlint`: run `rpmlint` tool on the .spec +* `all`: run all available checks (default) + +Available error levels selectable with `--error-level` affect the exit code: + +* `E`: exit with code 23 when linting error is found (default) +* `W`: exit with code 23 when linttng error or warning is found +* `-`: only print errors/warnings, always returns 0 + +*Most of the time you probably want just:* + + rdopkg lint + +*Example of only running rpmlint with `W` error level:* + + rdopkg lint --lint-checks rpmlint --error-level W + + ACTION: clone ~~~~~~~~~~~~~ diff --git a/rdopkg/actions/lint/__init__.py b/rdopkg/actions/lint/__init__.py new file mode 100644 index 00000000..1eb0df6a --- /dev/null +++ b/rdopkg/actions/lint/__init__.py @@ -0,0 +1,17 @@ +from rdopkg.action import Action, Arg + + +ACTIONS = [ + Action('lint', help="check for common .spec problems", + optional_args=[ + Arg('spec_fn', positional=True, nargs='?', + help="a .spec file to check"), + Arg('lint_checks', metavar='CHECKS', + help="comma-separated lists of checks to perform " + "(checks: sanity, rpmlint, all)"), + Arg('error_level', metavar='E|W|-', + help="'E' to halt on errors (default), " + "'W' to halt on errors and warnings, " + "'-' to ignore errors (affects return value)") + ]) +] diff --git a/rdopkg/actions/lint/actions.py b/rdopkg/actions/lint/actions.py new file mode 100644 index 00000000..9202efcf --- /dev/null +++ b/rdopkg/actions/lint/actions.py @@ -0,0 +1,16 @@ +""" +Check for common .spec problems +""" +from rdopkg.utils import specfile +from rdopkg.utils import lint as _lint + + +def lint(spec_fn=None, lint_checks=None, error_level='E'): + if not spec_fn: + spec_fn = specfile.spec_fn() + if lint_checks: + checks = lint_checks.split(',') + else: + checks = None + hints = _lint.lint(spec_fn, checks=checks) + _lint.lint_report(hints, error_level=error_level) diff --git a/rdopkg/exception.py b/rdopkg/exception.py index 6fe06b71..87558290 100644 --- a/rdopkg/exception.py +++ b/rdopkg/exception.py @@ -211,3 +211,12 @@ class NoPatchesChanged(RdopkgException): class NoDistgitChangesFound(RdopkgException): msg_fmt = "No distgit changes found" + + +class InvalidLintCheck(RdopkgException): + msg_fmt = "Invalid lint check: %(check)s" + + +class LintProblemsFound(RdopkgException): + exit_code = 23 + msg_fmt = "Lint problems detected." diff --git a/rdopkg/shell.py b/rdopkg/shell.py index 73c3e9a3..955e1aa5 100644 --- a/rdopkg/shell.py +++ b/rdopkg/shell.py @@ -97,6 +97,7 @@ def run(action_runner, cargs, version=None): exception.DuplicatePatchesBaseError, exception.FileNotFound, exception.IncompleteChangelog, + exception.InvalidLintCheck, exception.InvalidPatchesBaseRef, exception.InvalidPackageFilter, exception.InvalidRDOPackage, @@ -104,6 +105,7 @@ def run(action_runner, cargs, version=None): exception.InvalidGitRef, exception.InvalidQuery, exception.InvalidUsage, + exception.LintProblemsFound, exception.ManualResolutionNeeded, exception.NoPatchesChanged, exception.NoDistgitChangesFound, diff --git a/rdopkg/utils/lint.py b/rdopkg/utils/lint.py new file mode 100644 index 00000000..b39e654e --- /dev/null +++ b/rdopkg/utils/lint.py @@ -0,0 +1,165 @@ +""" +Check for common .spec problems +""" + +from collections import defaultdict +import re + +from rdopkg import exception +from rdopkg.utils import log +from rdopkg.utils.cmd import run +from rdopkg import helpers + + +class LintHint(object): + def __init__(self, location, level, msg): + self.location = location + self.level = level + self.msg = msg + + def __repr__(self): + return ('%s: %s: %s' % + (self.location, self.level, self.msg)) + + +RE_RPMLINT_HINT = r'(.*):\s+([EW]):\s+(.+)$' +RE_RPMLINT_SUMMARY = (r'\d+ packages and \d+ specfiles checked; ' + r'\d+ errors?, \d+ warnings?.') + + +def rpmlint_check(*args): + # run rpmlint and return linting hints it found + hints = [] + cmd = ['rpmlint'] + if args: + cmd += args + try: + out = run(*cmd, fatal=False, log_fail=False) + except exception.CommandNotFound: + raise exception.CommandNotFound( + msg="Unable to run rpmlint checks because rpmlint is missing.") + for line in out.splitlines(): + m = re.match(RE_RPMLINT_HINT, line) + if m: + hints.append(LintHint(location=m.group(1), + level=m.group(2), + msg=m.group(3))) + continue + m = re.match(RE_RPMLINT_SUMMARY, line) + if m: + # ignore final rpmlint summary + continue + hints.append(LintHint(location='rpmlint', level='W', msg=( + 'Failed to parse rpmlint output: %s' % line))) + return hints + + +def sanity_check_buildarch(txt): + # make sure BuildArch is AFTER SourceX and PatchX lines, + # otherwise %{patches} macro is empty which causes trouble + bm = re.search(r'^BuildArch:', txt, flags=re.M) + if not bm: + return True + bi = bm.start() + sm = re.search(r'^Source\d+:', txt, flags=re.M) + if sm: + si = sm.start() + if bi < si: + return False + pm = re.search(r'^Patch\d+:', txt, flags=re.M) + if pm: + pi = pm.start() + if bi < pi: + return False + return True + + +def sanity_check_patches_base(txt): + # duplicate patches_base might lead to unexpected behavior + bases = re.findall('^#\s*patches_base', txt, flags=re.M) + if len(bases) > 1: + return False + return True + + +def sanity_check(spec_fn): + # perform rdopkg sanity checks for common problems + hints = [] + try: + txt = open(spec_fn, 'r').read() + except Exception as e: + hints.append(LintHint( + spec_fn, 'E', str(e))) + return hints + if not sanity_check_buildarch(txt): + hints.append(LintHint(spec_fn, 'E', ( + "buildarch-before-sources: Due to mysterious" + "ways of rpm, BuildArch needs to be placed " + "AFTER SourceX and PatchX lines in .spec file, " + "otherwise %%{patches} macro will be empty " + "and both %%autosetup and `git am %%{patches}` will fail. " + "Please move BuildArch AFTER SourceX and PatchX lines."))) + if not sanity_check_patches_base(txt): + hints.append(LintHint(spec_fn, 'E', ( + "duplicate-patches-base: Please make sure to only have one " + "# patches_base= entry in .spec file to avoid problems."))) + return hints + + +LINT_CHECKS = { + 'sanity': sanity_check, + 'rpmlint': rpmlint_check, +} + + +def lint(spec_fn, checks=None): + if not checks or checks == 'all': + # use all checks by default + checks = LINT_CHECKS.keys() + hints = [] + for ch in checks: + try: + check = LINT_CHECKS[ch] + except KeyError: + raise exception.InvalidLintCheck(check=ch) + hints += check(spec_fn) + return hints + + +def lint_report(hints, error_level=None): + # print a linting report based on passed hints + # optionally raise error depending on error_level: + # * 'E': raise when errors found + # * 'W': raise when errors or warnings are found + # * other: don't raise error, only print report + hint_dict = defaultdict(list) + for hint in hints: + hint_dict[hint.level].append(hint) + + errors = hint_dict.get('E', []) + n_errors = len(errors) + if errors: + print("\n%d ERRORS found:" % n_errors) + for e in errors: + print(e) + + warns = hint_dict.get('W', []) + n_warns = len(warns) + if warns: + print("\n%d WARNINGS found:" % n_warns) + for e in warns: + print(e) + + if n_errors == 0 and n_warns == 0: + print('no linting errors found \o/') + return + + critical_error = False + if error_level == 'E' and n_errors > 0: + critical_error = True + if error_level == 'W' and (n_errors > 0 or n_warns > 0): + critical_error = True + if critical_error: + raise exception.LintProblemsFound( + "Linting problems detected: %d errors, %d warnings" % ( + n_errors, n_warns)) diff --git a/tests/assets/spec/lint/foo.spec b/tests/assets/spec/lint/foo.spec new file mode 100644 index 00000000..6216ad0c --- /dev/null +++ b/tests/assets/spec/lint/foo.spec @@ -0,0 +1,62 @@ +Name: foo +Epoch: 2077 +Version: 1.2.3 +Release: 42%{?dist} +Summary: Some package, dude + +Group: Development/Languages +License: ASL 2.0 +URL: http://notreallyavaliddomain.name/foo + +BuildArch: noarch + +Source0: %{name}/%{name}-%{version}.tar.gz + +Patch0001: 0001-something.patch +Patch0002: 0002-something-else.patch + +BuildRequires: python-setuptools +BuildRequires: python2-devel + +Requires: python-argparse +Requires: python-iso8601 +Requires: python-prettytable + +PreReq: is-deprecated + +%description +This is foo! This is foo! This is foo! This is foo! This is foo! This is foo! +This is foo! This is foo! This is foo! + +%setup -q + + +%prep +%setup -q + +%patch0001 -p1 +%patch0002 -p1 + +# We provide version like this in order to remove runtime dep on pbr. +sed -i s/REDHATNOVACLIENTVERSION/%{version}/ novaclient/__init__.py + +%build +%{__python} setup.py build + +%install +%{__python} setup.py install -O1 --skip-build --root %{buildroot} + + +%files +%doc README.rst +%{_bindir}/foo +# hardcoded library path +/usr/lib/share/foo + +%changelog +* Mon Apr 07 2014 Jakub Ruzicka 1.2.3-42 +- Update to upstream 1.2.3 +- Oh no, there's a %macro in changelog + +* Tue Mar 25 2014 Jakub Ruzicka 1.2.2-1 +- Update to upstream 1.2.2 diff --git a/tests/test_lint.py b/tests/test_lint.py new file mode 100644 index 00000000..0a2090ba --- /dev/null +++ b/tests/test_lint.py @@ -0,0 +1,71 @@ +import pytest +import subprocess + +from rdopkg.cli import rdopkg + +import test_common as common + + +RPMLINT_AVAILABLE = False +try: + subprocess.call(["rpmlint", "--version"]) + RPMLINT_AVAILABLE = True +except Exception as e: + pass + + +def _assert_sanity_out(o): + assert 'ERRORS found:' in o + assert 'E: buildarch-before-sources' in o + + +def _assert_rpmlint_out(o): + # check expected errors were raised + assert 'ERRORS found:' in o + assert 'E: hardcoded-library-path' in o + assert 'E: specfile-error warning:' in o + assert 'prereq is deprecated: PreReq' in o + # check expected warnings were raised + assert 'WARNINGS found:' in o + assert 'W: macro-in-%changelog' in o + assert 'W: setup-not-in-prep' in o + # make sure parsing rpmlint output was a success + assert 'Failed to parse rpmlint output' not in o + + +def test_lint_sanity(tmpdir, capsys): + dist_path = common.prep_spec_test(tmpdir, 'lint') + rv = -1 + with dist_path.as_cwd(): + rv = rdopkg('lint', '--lint-checks', 'sanity') + cap = capsys.readouterr() + o = cap.out + _assert_sanity_out(o) + assert rv == 23 + + +@pytest.mark.skipif('RPMLINT_AVAILABLE == False') +def test_lint_rpmlint(tmpdir, capsys): + dist_path = common.prep_spec_test(tmpdir, 'lint') + rv = -1 + with dist_path.as_cwd(): + rv = rdopkg('lint', '--lint-checks', 'rpmlint') + cap = capsys.readouterr() + o = cap.out + _assert_rpmlint_out(o) + assert rv == 23 + + +@pytest.mark.skipif('RPMLINT_AVAILABLE == False') +def test_lint_all(tmpdir, capsys): + # run `rdopkg lint` on tests/assets/spec/lint.spec + dist_path = common.prep_spec_test(tmpdir, 'lint') + rv = -1 + with dist_path.as_cwd(): + rv = rdopkg('lint') + cap = capsys.readouterr() + o = cap.out + _assert_rpmlint_out(o) + _assert_sanity_out(o) + # linting problems are indicated by exit code 23 + assert rv == 23