Controller: make gerrit action format agnostic

lib/rift/Controller.py

@@ -858,10 +858,15 @@ def action_gerrit(args, config, staff, modules):
             pkg = ProjectPackages.get(names[1], config, staff, modules)
             if (filepath == os.path.relpath(pkg.buildfile) and
                 not patchedfile.is_deleted_file):
-                Spec(pkg.buildfile, config=config).analyze(review, pkg.dir)
+                pkg.load()
+                try:
+                    pkg.analyze(review, pkg.dir)
+                except NotImplementedError:
+                    logging.info("Skipping package format %s which does not "
+                                 "support static analysis", pkg.format)
 
     # Push review
-    review.msg_header = 'rpmlint analysis'
+    review.msg_header = 'rift static analysis'
     review.push(config, args.change, args.patchset)
 
 def action_sync(args, config):

lib/rift/package/_base.py

@@ -254,6 +254,10 @@ def add_changelog_entry(self, maintainer, comment, bump):
         """Must be implemented in concrete children classes when supported."""
         raise NotImplementedError
 
+    def analyze(self, review, configdir):
+        """Must be implemented in concrete children classes when supported."""
+        raise NotImplementedError
+
     def supports_arch(self, arch):
         """
         Return True if package does not exclude any architecture or the given

lib/rift/package/rpm.py

@@ -148,6 +148,10 @@ def has_real_variants(self):
         assert(len(self.variants))  # Cannot be called with empty variants list.
         return len(self.variants) > 1 or self.variants[0] != _DEFAULT_VARIANT
 
+    def analyze(self, review, configdir):
+        assert self.spec is not None
+        self.spec.analyze(review, configdir)
+
     def supports_arch(self, arch):
         """
         Returns True if provided architecture is listed in package spec file's

tests/TestUtils.py

@@ -99,7 +99,9 @@
 {% if exclusive_arch %}
 ExclusiveArch:  {{ exclusive_arch }}
 {% endif -%}
+{% if arch == 'noarch' -%}
 BuildArch:      {{ arch }}
+{% endif -%}
 {% for build_require in build_requires | default(['br-package']) %}
 BuildRequires:  {{ build_require }}
 {% endfor %}
@@ -399,9 +401,10 @@ def make_temp_filename():
     """Return a temporary name for a file."""
     return (tempfile.mkstemp(prefix='rift-test-'))[1]
 
-def make_temp_file(text, delete=True):
+def make_temp_file(text, delete=True, suffix=None):
     """ Create a temporary file with the provided text."""
-    tmp = tempfile.NamedTemporaryFile(prefix='rift-test-', delete=delete)
+    tmp = tempfile.NamedTemporaryFile(prefix='rift-test-', delete=delete,
+                                      suffix=suffix)
     tmp.write(text.encode())
     tmp.flush()
     return tmp

tests/package/base.py

@@ -12,6 +12,7 @@
     ActionableArchPackage,
 )
 from ..TestUtils import RiftProjectTestCase
+from rift.Gerrit import Review
 
 
 class PackageTestingConcrete(Package):
@@ -112,6 +113,14 @@ def test_add_changelog_entry(self):
         with self.assertRaises(NotImplementedError):
             pkg.add_changelog_entry("Myself", "Package modification", False)
 
+    def test_analyze(self):
+        """ Test Package analyze (not implemented) """
+        pkg = PackageTestingConcrete(
+            'pkg', self.config, self.staff, self.modules, 'rpm'
+        )
+        with self.assertRaises(NotImplementedError):
+            pkg.analyze(Review(), pkg.dir)
+
 
 class ActionableArchPackageTest(RiftProjectTestCase):
 

tests/package/rpm.py

@@ -10,6 +10,8 @@
 from rift.run import RunResult
 from rift.TestResults import TestResults
 from rift.Config import _DEFAULT_VARIANT
+from rift.Gerrit import Review
+
 from ..TestUtils import RiftProjectTestCase, make_temp_file, gen_rpm_spec
 
 
@@ -300,6 +302,63 @@ def test_supports_arch_wo_exclusive_arch(self):
         self.assertTrue(pkg.supports_arch('x86_64'))
         self.assertTrue(pkg.supports_arch('aarch64'))
 
+    def test_analyze(self):
+        """ Test PackageRPM analyze success """
+        pkgname = 'pkg'
+        pkg = PackageRPM(pkgname, self.config, self.staff, self.modules)
+        pkgfile = make_temp_file(textwrap.dedent("""
+            package:
+                maintainers:
+                - Myself
+                module: Great module
+                reason: Missing package
+                origin: Company
+            """))
+        spec_file = make_temp_file(
+            gen_rpm_spec(
+                name=pkgname,
+                version="1.0",
+                release="1",
+                arch="x86_64",
+            ),
+            suffix='.spec'
+        )
+        pkg.buildfile = spec_file.name
+        pkg.load(infopath = pkgfile.name)
+        review = Mock(spec=Review)
+        pkg.analyze(review, pkg.dir)
+        review.invalidate.assert_not_called()
+
+    def test_analyze_invalidate(self):
+        """ Test PackageRPM analyze failure """
+        pkgname = 'pkg'
+        pkg = PackageRPM(pkgname, self.config, self.staff, self.modules)
+        pkgfile = make_temp_file(textwrap.dedent("""
+            package:
+                maintainers:
+                - Myself
+                module: Great module
+                reason: Missing package
+                origin: Company
+            """))
+        # Use $$RPM_SOURCE_DIR and $RPM_BUILD_ROOT in build steps in order to
+        # produce error in both rpmlint v1 and v2.
+        spec_file = make_temp_file(
+            gen_rpm_spec(
+                name=pkgname,
+                version="1.0",
+                release="1",
+                arch="x86_64",
+                buildsteps="$RPM_SOURCE_DIR\n$RPM_BUILD_ROOT",
+            ),
+            suffix='.spec'
+        )
+        pkg.buildfile = spec_file.name
+        pkg.load(infopath = pkgfile.name)
+        review = Mock(spec=Review)
+        pkg.analyze(review, pkg.dir)
+        review.invalidate.assert_called_once()
+
     def test_for_arch(self):
         """ Test PackageRPM for_arch() returns ActionableArchPackageRPM object. """
         pkgname = 'pkg'