Move spec file checks from check_source.pl to .py

check_source.pl

@@ -2,8 +2,6 @@
 
 use File::Basename;
 use File::Temp qw/ tempdir  /;
-use XML::Simple;
-use Data::Dumper;
 use Cwd;
 use Text::Diff;
 BEGIN {
@@ -17,39 +15,6 @@ my $old = $ARGV[0];
 my $dir = $ARGV[1];
 my $bname = basename($dir);
 
-my @specs = map basename($_), glob("$dir/*.spec");
-
-if (@specs) {
-    if (!-f "$dir/$bname.changes") {
-        print "$bname.changes is missing. A package submitted as FooBar needs to have a FooBar.changes file with a format created by `osc vc`.\n";
-        $ret = 1;
-    }
-
-    if (!-f "$dir/$bname.spec") {
-        print "$bname.spec is missing. A package submitted as FooBar needs to have a FooBar.spec file.\n";
-        $ret = 1;
-    }
-    exit($ret) if ($ret);
-} else {
-    # package without spec files, eg kiwi only
-    exit($ret);
-}
-
-open(SPEC, "$dir/$bname.spec");
-my $spec = join("", <SPEC>);
-close(SPEC);
-
-if ($spec !~ m/#[*\s]+Copyright\s/) {
-    print "$bname.spec does not appear to contain a Copyright comment. Please stick to the format\n\n";
-    print "# Copyright (c) 2011 Stephan Kulow\n\n";
-    print "or use osc service runall format_spec_file\n";
-    $ret = 1;
-}
-
-if ($spec =~ m/\nVendor:/) {
-    print "$bname.spec contains a Vendor line, this is forbidden.\n";
-    $ret = 1;
-}
 
 # Check that we have for each spec file a changes file - and that at least one
 # contains changes

check_source.py

@@ -214,6 +214,14 @@ class CheckSource(ReviewBot.ReviewBot):
         if not self.check_rpmlint(target_package):
             return False
 
+        specs = [os.path.basename(x) for x in glob.glob(os.path.join(target_package, "*.spec"))]
+        if not specs:
+            # package without spec files e.g kiwi only
+            return True
+
+        if not self.check_spec_policy(target_package, specs):
+            return False
+
         # Run check_source.pl script and interpret output.
         source_checker = os.path.join(CheckSource.SCRIPT_PATH, 'check_source.pl')
         civs = ''
@@ -309,6 +317,35 @@ class CheckSource(ReviewBot.ReviewBot):
                     return False
         return True
 
+    def check_spec_policy(self, directory, specs):
+        bname = os.path.basename(directory)
+        if not os.path.exists(os.path.join(directory, bname + '.changes')):
+            text = f"{bname}.changes is missing. "
+            text += "A package submitted as FooBar needs to have a FooBar.changes file with a format created by `osc vc`."
+            self.review_messages['declined'] = text
+            return False
+
+        specfile = os.path.join(directory, bname + '.spec')
+        if not os.path.exists(specfile):
+            self.review_messages['declined'] = f"{bname}.spec is missing. A package submitted as FooBar needs to have a FooBar.spec file."
+            return False
+
+        for spec in specs:
+            with open(os.path.join(directory, spec), 'r') as f:
+                content = f.read()
+                if not re.search(r'#[*\s]+Copyright\s', content):
+                    text = f"{spec} does not appear to contain a Copyright comment. Please stick to the format\n\n"
+                    text += "# Copyright (c) 2022 Unsong Hero\n\n"
+                    text += "or use osc service runall format_spec_file"
+                    self.review_messages['declined'] = text
+                    return False
+
+                if re.search(r'\nVendor:', content):
+                    self.review_messages['declined'] = "{spec} contains a Vendor line, this is forbidden."
+                    return False
+
+        return True
+
     def source_has_correct_maintainers(self, source_project):
         """Checks whether the source project has the required maintainer
 

tests/check_source_tests.py

@@ -267,6 +267,38 @@ class TestCheckSource(OBSLocal.TestCase):
         self.assertEqual('Services are only allowed if their mode is one of localonly, disabled, buildtime, ' +
                          'manual. Please change the mode of recompress and use `osc service localrun/disabledrun`.', review.comment)
 
+    @pytest.mark.usefixtures("default_config")
+    def test_wrong_name(self):
+        """Declines spec files with wrong name"""
+        self._setup_devel_project(devel_files='blowfish-with-broken-name')
+
+        req_id = self.wf.create_submit_request(self.devel_package.project,
+                                               self.devel_package.name, add_commit=False).reqid
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'new'))
+
+        self.review_bot.set_request_ids([req_id])
+        self.review_bot.check_requests()
+
+        review = self.assertReview(req_id, by_user=(self.bot_user, 'declined'))
+        self.assertEqual("A package submitted as blowfish has to build as 'Name: blowfish' - found Name 'suckfish'", review.comment)
+
+    @pytest.mark.usefixtures("default_config")
+    def test_without_copyright(self):
+        """Declines spec files without copyright"""
+        self._setup_devel_project(devel_files='blowfish-without-copyright')
+
+        req_id = self.wf.create_submit_request(self.devel_package.project,
+                                               self.devel_package.name, add_commit=False).reqid
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'new'))
+
+        self.review_bot.set_request_ids([req_id])
+        self.review_bot.check_requests()
+
+        review = self.assertReview(req_id, by_user=(self.bot_user, 'declined'))
+        self.assertIn("blowfish.spec does not appear to contain a Copyright comment.", review.comment)
+
     def _setup_devel_project(self, maintainer={}, devel_files='blowfish-with-patch-changes',
                              target_files='blowfish'):
         devel_project = self.wf.create_project(SRC_PROJECT, maintainer=maintainer)

tests/fixtures/packages/blowfish-with-broken-name/blowfish.changes

@@ -0,0 +1,5 @@
+-------------------------------------------------------------------
+Thu Jul  8 07:36:30 UTC 2021 - Fisherman <fisherman@opensuse.org>
+
+- Initial version.
+- 1

tests/fixtures/packages/blowfish-with-broken-name/blowfish.spec

@@ -0,0 +1,16 @@
+#
+# Copyright (c) 2020 SUSE LLC
+#
+# This file is under MIT license
+
+
+Name:           suckfish
+Version:        1
+Release:        0
+Summary:        Blowfish
+License:        GPL-2.0-only
+URL:            https://github.com/openSUSE/cockpit-wicked
+Source:         blowfish-1.tar.gz
+BuildArch:      noarch
+
+%changelog

tests/fixtures/packages/blowfish-without-copyright/blowfish.changes

@@ -0,0 +1,5 @@
+-------------------------------------------------------------------
+Thu Jul  8 07:36:30 UTC 2021 - Fisherman <fisherman@opensuse.org>
+
+- Initial version.
+- 1

tests/fixtures/packages/blowfish-without-copyright/blowfish.spec

@@ -0,0 +1,13 @@
+#
+# Hallo!
+
+Name:           blowfish
+Version:        1
+Release:        0
+Summary:        Blowfish
+License:        GPL-2.0-only
+URL:            https://github.com/openSUSE/cockpit-wicked
+Source:         blowfish-1.tar.gz
+BuildArch:      noarch
+
+%changelog