lint: add new action to check for errors in spec

This patch adds new lint module and associated `rdopkg lint` action.

.spec sanity checks moved to the new lint module.

rpmlint was integrated.

Fixes: #115

Change-Id: I4a3be67e051a4e637d53c8a77c888ae7b1f6d511

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
 ~~~~~~~~~~~~~
 

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)")
+           ])
+]

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)

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."

rdopkg/shell.py

@@ -97,13 +97,15 @@ def run(action_runner, cargs, version=None):
             exception.DuplicatePatchesBaseError,
             exception.FileNotFound,
             exception.IncompleteChangelog,
+            exception.InvalidLintCheck,
             exception.InvalidPatchesBaseRef,
             exception.InvalidPackageFilter,
             exception.InvalidRDOPackage,
             exception.InvalidReleaseBumpIndex,
             exception.InvalidGitRef,
             exception.InvalidQuery,
             exception.InvalidUsage,
+            exception.LintProblemsFound,
             exception.ManualResolutionNeeded,
             exception.NoPatchesChanged,
             exception.NoDistgitChangesFound,

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))

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 <[email protected]> 1.2.3-42
+- Update to upstream 1.2.3
+- Oh no, there's a %macro in changelog
+
+* Tue Mar 25 2014 Jakub Ruzicka <[email protected]> 1.2.2-1
+- Update to upstream 1.2.2

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