Move changes check from check_source.pl to .py

check_source.pl

@@ -16,31 +16,6 @@ my $dir = $ARGV[1];
 my $bname = basename($dir);
 
 
-# Check that we have for each spec file a changes file - and that at least one
-# contains changes
-my $changes_updated = 0;
-for my $spec (@specs) {
-    $changes = $spec;
-    $changes =~ s/\.spec$/.changes/;
-
-    # new or deleted .changes files also count
-    if ((-f "$old/$changes") != (-f "$dir/$changes")) {
-        $changes_updated = 1;
-        last;
-    }
-    elsif ((-f "$old/$changes") && (-f "$dir/$changes")) {
-        if (system(("cmp", "-s", "$old/$changes", "$dir/$changes"))) {
-            $changes_updated = 1;
-            last;
-        }
-    }
-}
-
-if (!$changes_updated) {
-    print "No changelog. Please use 'osc vc' to update the changes file(s).\n";
-    $ret = 1;
-}
-
 if ($spec !~ m/\n%changelog\s/ && $spec != m/\n%changelog$/) {
     print "$bname.spec does not contain a %changelog line. We don't want a changelog in the spec file, but the %changelog section needs to be present\n";
     $ret = 1;

check_source.py

@@ -219,7 +219,7 @@ class CheckSource(ReviewBot.ReviewBot):
             # package without spec files e.g kiwi only
             return True
 
-        if not self.check_spec_policy(target_package, specs):
+        if not self.check_spec_policy('_old', target_package, specs):
             return False
 
         # Run check_source.pl script and interpret output.
@@ -317,7 +317,7 @@ class CheckSource(ReviewBot.ReviewBot):
                     return False
         return True
 
-    def check_spec_policy(self, directory, specs):
+    def check_spec_policy(self, old, directory, specs):
         bname = os.path.basename(directory)
         if not os.path.exists(os.path.join(directory, bname + '.changes')):
             text = f"{bname}.changes is missing. "
@@ -330,6 +330,7 @@ class CheckSource(ReviewBot.ReviewBot):
             self.review_messages['declined'] = f"{bname}.spec is missing. A package submitted as FooBar needs to have a FooBar.spec file."
             return False
 
+        changes_updated = False
         for spec in specs:
             with open(os.path.join(directory, spec), 'r') as f:
                 content = f.read()
@@ -344,6 +345,23 @@ class CheckSource(ReviewBot.ReviewBot):
                     self.review_messages['declined'] = "{spec} contains a Vendor line, this is forbidden."
                     return False
 
+            # Check that we have for each spec file a changes file - and that at least one
+            # contains changes
+            changes = spec.replace('.spec', '.changes')
+
+            # new or deleted .changes files also count
+            old_exists = os.path.exists(os.path.join(old, changes))
+            new_exists = os.path.exists(os.path.join(directory, changes))
+            if old_exists != new_exists:
+                changes_updated = True
+            elif old_exists and new_exists:
+                if subprocess.run(["cmp", "-s", os.path.join(old, changes), os.path.join(directory, changes)]).returncode:
+                    changes_updated = True
+
+        if not changes_updated:
+            self.review_messages['declined'] = "No changelog. Please use 'osc vc' to update the changes file(s)."
+            return False
+
         return True
 
     def source_has_correct_maintainers(self, source_project):

tests/check_source_tests.py

@@ -299,6 +299,22 @@ class TestCheckSource(OBSLocal.TestCase):
         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)
 
+    @pytest.mark.usefixtures("default_config")
+    def test_no_changelog(self):
+        """Declines submit request with just changed spec file"""
+        self._setup_devel_project(devel_files='blowfish-without-changes-update')
+
+        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("No changelog. Please use 'osc vc' to update the changes file(s).", 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-without-changes-update/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-changes-update/blowfish.spec

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