From c8531891a1b2921c78fa3cbd53652f0684e2609a Mon Sep 17 00:00:00 2001
From: Stephan Kulow <coolo@suse.de>
Date: Thu, 24 Mar 2022 21:19:45 +0100
Subject: [PATCH 1/3] Move the 'patch in changes' check from check_source.pl to
 .py

---
 check_source.pl | 63 +++----------------------------------------
 check_source.py | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/check_source.pl b/check_source.pl
index 2a7dd603..6d3aee05 100644
--- a/check_source.pl
+++ b/check_source.pl
@@ -15,17 +15,8 @@ my $old = $ARGV[0];
 my $dir = $ARGV[1];
 my $bname = basename($dir);
 
-my %patches = ();
-
-
 my $odir = getcwd();
 
-chdir($dir) || die "chdir $dir failed";
-for my $patch (glob("*.diff *.patch *.dif")) {
-    $patches{$patch} = 'current';
-}
-chdir($odir) || die "chdir $odir failed";
-
 if (-d "$old") {
 
     chdir($old) || die "chdir $old failed";
@@ -41,15 +32,7 @@ if (-d "$old") {
             }
         }
     }
-    for my $patch (glob("*.diff *.patch *.dif")) {
-        if ($patches{$patch}) {
-            delete $patches{$patch};
-        }
-        else {
-            $patches{$patch} = 'old';
-        }
-    }
-
+   
     chdir($odir) || die "chdir $odir failed";
     chdir($dir) || die "chdir $dir failed";
     for my $spec (glob("*.spec")) {
@@ -85,51 +68,11 @@ if (-d "$old") {
         rename("$spec.new", "$spec") || die "rename failed";
     }
 
-    chdir($dir);
-    my @changes = glob("*.changes");
-    chdir($odir);
-
-    if (%patches) {
-        # parse changes
-        for my $changes (@changes) {
-            my $diff = "";
-            if (! -e "$old/$changes") {
-                $diff = diff "/dev/null", "$dir/$changes";
-            }
-            else {
-                $diff = diff "$old/$changes", "$dir/$changes";
-            }
-            for my $line (split(/\n/, $diff)) {
-                # Check if the line mentions a patch being added (starts with +)
-                # or removed (starts with -)
-                next unless $line =~ m/^[+-]/;
-                # In any of those cases, remove the patch from the list
-                $line =~ s/^[+-]//;
-                for my $patch (keys %patches) {
-                    if (index($line, $patch) != -1) {
-                        delete $patches{$patch};
-                    }
-                }
-            }
-        }
-    }
-    # still some left?
-    if (%patches) {
-        $ret = 1;
-        for my $patch (keys %patches) {
-            # wording stolen from Raymond's declines :)
-            if ($patches{$patch} eq 'current') {
-                print "A patch ($patch) is being added without this addition being mentioned in the changelog.\n";
-            }
-            else {
-                print "A patch ($patch) is being deleted without this removal being mentioned in the changelog.\n";
-            }
-        }
-    }
 }
 
 my $tmpdir = tempdir("obs-XXXXXXX", TMPDIR => 1, CLEANUP => 1);
-chdir($dir) || die 'tempdir failed';
+chdir($odir) || die "chdir $odir failed";
+chdir($dir) || die "chdir $dir failed";
 if (system("/usr/lib/obs/service/download_files","--enforceupstream", "yes", "--enforcelocal", "yes", "--outdir", $tmpdir)) {
     print "Source URLs are not valid. Try \"osc service runall download_files\".\n";
     $ret = 2;
diff --git a/check_source.py b/check_source.py
index 21313b68..c48a7a4c 100755
--- a/check_source.py
+++ b/check_source.py
@@ -1,5 +1,6 @@
 #!/usr/bin/python3
 
+import difflib
 import glob
 import os
 import re
@@ -225,6 +226,9 @@ class CheckSource(ReviewBot.ReviewBot):
         if not self.run_source_validator('_old', target_package):
             return False
 
+        if not self.detect_mentioned_patches('_old', target_package):
+            return False
+
         # Run check_source.pl script and interpret output.
         source_checker = os.path.join(CheckSource.SCRIPT_PATH, 'check_source.pl')
         civs = ''
@@ -574,6 +578,73 @@ class CheckSource(ReviewBot.ReviewBot):
 
         return True
 
+    def difflines(self, oldf, newf):
+        with open(oldf, 'r') as f:
+            oldl = f.readlines()
+        with open(newf, 'r') as f:
+            newl = f.readlines()
+        return list(difflib.unified_diff(oldl, newl))
+
+    def detect_mentioned_patches(self, old, directory):
+        # new packages have different rules
+        if not os.path.isdir(old):
+            return True
+        opatches = self.list_patches(old)
+        npatches = self.list_patches(directory)
+
+        cpatches = opatches.intersection(npatches)
+        opatches -= cpatches
+        npatches -= cpatches
+
+        if not npatches and not opatches:
+            return True
+
+        patches_to_mention = dict()
+        for p in opatches:
+            patches_to_mention[p] = 'old'
+        for p in npatches:
+            patches_to_mention[p] = 'new'
+        for changes in glob.glob(os.path.join(directory, '*.changes')):
+            base = os.path.basename(changes)
+            oldchanges = os.path.join(old, base)
+            if os.path.exists(oldchanges):
+                diff = self.difflines(oldchanges, changes)
+            else:
+                with open(changes, 'r') as f:
+                    diff = ['+' + l for l in f.readlines()]
+            for line in diff:
+                pass
+                # Check if the line mentions a patch being added (starts with +)
+                # or removed (starts with -)
+                if not re.match(r'[+-]', line):
+                    continue
+                # In any of those cases, remove the patch from the list
+                line = line[1:].strip()
+                for patch in patches_to_mention:
+                    if line.find(patch) >= 0:
+                        del patches_to_mention[patch]
+                        break
+
+        if not patches_to_mention:
+            return True
+
+        lines = []
+        for patch, state in patches_to_mention.items():
+            # wording stolen from Raymond's declines :)
+            if state == 'new':
+                lines.append(f"A patch ({patch}) is being added without this addition being mentioned in the changelog.")
+            else:
+                lines.append(f"A patch ({patch}) is being deleted without this removal being mentioned in the changelog.")
+        self.review_messages['declined'] = '\n'.join(lines)
+        return False
+
+    def list_patches(self, directory):
+        ret = set()
+        for ext in ['*.diff', '*.patch', '*.dif']:
+            for file in glob.glob(os.path.join(directory, ext)):
+                ret.add(os.path.basename(file))
+        return ret
+
 
 class CommandLineInterface(ReviewBot.CommandLineInterface):
 

From ed87520710229d2fc95e08e5d20f23a03d68feff Mon Sep 17 00:00:00 2001
From: Stephan Kulow <coolo@suse.de>
Date: Fri, 25 Mar 2022 07:39:17 +0100
Subject: [PATCH 2/3] Ignore patches that are listed as sources

The patch live cycle does not apply to them
---
 check_source.pl                               |   3 ---
 check_source.py                               |  22 ++++++++++++++++--
 tests/check_source_tests.py                   |  19 +++++++++++++++
 .../blowfish-1.tar.gz                         | Bin 0 -> 216 bytes
 .../blowfish-patch-as-source/blowfish.changes |  10 ++++++++
 .../blowfish-patch-as-source/blowfish.spec    |  17 ++++++++++++++
 .../blowfish-patch-as-source/test.patch       |   1 +
 7 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 tests/fixtures/packages/blowfish-patch-as-source/blowfish-1.tar.gz
 create mode 100644 tests/fixtures/packages/blowfish-patch-as-source/blowfish.changes
 create mode 100644 tests/fixtures/packages/blowfish-patch-as-source/blowfish.spec
 create mode 100644 tests/fixtures/packages/blowfish-patch-as-source/test.patch

diff --git a/check_source.pl b/check_source.pl
index 6d3aee05..14e933d3 100644
--- a/check_source.pl
+++ b/check_source.pl
@@ -44,9 +44,6 @@ if (-d "$old") {
             if (m/^Source/) {
                 my $line = $_;
                 $line =~ s/^(Source[0-9]*)\s*:\s*//;
-                if ($patches{$line}) {
-                   delete $patches{$line};
-                }
                 my $prefix = $1;
                 my $parsedline = $ps->{lc $prefix};
                 if (defined $thash{$parsedline}) {
diff --git a/check_source.py b/check_source.py
index c48a7a4c..97c337c0 100755
--- a/check_source.py
+++ b/check_source.py
@@ -226,7 +226,7 @@ class CheckSource(ReviewBot.ReviewBot):
         if not self.run_source_validator('_old', target_package):
             return False
 
-        if not self.detect_mentioned_patches('_old', target_package):
+        if not self.detect_mentioned_patches('_old', target_package, specs):
             return False
 
         # Run check_source.pl script and interpret output.
@@ -585,7 +585,18 @@ class CheckSource(ReviewBot.ReviewBot):
             newl = f.readlines()
         return list(difflib.unified_diff(oldl, newl))
 
-    def detect_mentioned_patches(self, old, directory):
+    def _mentioned_sources(self, directory, specs):
+        sources = set()
+        for spec in specs:
+            with open(os.path.join(directory, spec)) as f:
+                for line in f:
+                    m = re.match(r'Source[0-9]*\s*:\s*(.*)$', line)
+                    if not m:
+                        continue
+                    sources.add(m.group(1))
+        return sources
+
+    def detect_mentioned_patches(self, old, directory, specs):
         # new packages have different rules
         if not os.path.isdir(old):
             return True
@@ -625,6 +636,13 @@ class CheckSource(ReviewBot.ReviewBot):
                         del patches_to_mention[patch]
                         break
 
+        # if a patch is mentioned as source, we ignore it
+        sources = self._mentioned_sources(directory, specs)
+        sources |= self._mentioned_sources(old, specs)
+
+        for s in sources:
+            patches_to_mention.pop(s, None)
+
         if not patches_to_mention:
             return True
 
diff --git a/tests/check_source_tests.py b/tests/check_source_tests.py
index 8085d422..ea6303a4 100644
--- a/tests/check_source_tests.py
+++ b/tests/check_source_tests.py
@@ -151,6 +151,25 @@ class TestCheckSource(OBSLocal.TestCase):
         self.assertReview(req_id, by_user=(self.bot_user, 'accepted'))
         self.assertReview(req_id, by_group=(REVIEW_TEAM, 'new'))
 
+    @pytest.mark.usefixtures("default_config")
+    def test_patch_as_source(self):
+        """Accepts a request if a new patch is a source"""
+        # switch target and devel, so basically do revert of changes done
+        # with patch and changes
+        self._setup_devel_project(devel_files='blowfish-patch-as-source',
+                                  target_files='blowfish')
+
+        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()
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'accepted'))
+        self.assertReview(req_id, by_group=(REVIEW_TEAM, 'new'))
+
     @pytest.mark.usefixtures("required_source_maintainer")
     def test_no_source_maintainer(self):
         """Declines the request when the 'required_maintainer' is not maintainer of the source project
diff --git a/tests/fixtures/packages/blowfish-patch-as-source/blowfish-1.tar.gz b/tests/fixtures/packages/blowfish-patch-as-source/blowfish-1.tar.gz
new file mode 100644
index 0000000000000000000000000000000000000000..94fff08bd3128a12905a224c182cb1c1093ccba3
GIT binary patch
literal 216
zcmV;}04M(+iwFP!000001MSqk3c@fHhT+U!MX#XtBxzy?cSmobXpjmzv=zL3qasSN
z;!vuE_ZgDmg!~Y`lg`)Wc@@fSsGC%Kv3}l$rd31AB4_Pwj=FOu+AFc5weeoWsVol8
zspLH9Eqa`qwyfh-#yIN_=^{C_)gdg!nfL*PEOhFv6h-dLEEda8#Z7PV*U!S21`qoi
zFZmar-*Yd|eh}rI{2QY)m5h?fNB;M9sJ7K{7wW!su|s~?r`y~n;`;xj6#xJL00000
S00000V7e~$+pz8cC;$Le&uKpZ

literal 0
HcmV?d00001

diff --git a/tests/fixtures/packages/blowfish-patch-as-source/blowfish.changes b/tests/fixtures/packages/blowfish-patch-as-source/blowfish.changes
new file mode 100644
index 00000000..f1036fc6
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-patch-as-source/blowfish.changes
@@ -0,0 +1,10 @@
+-------------------------------------------------------------------
+Fri Mar 25 06:18:56 UTC 2022 - Stephan Kulow <coolo@suse.com>
+
+- Not mentioning the patch!
+
+-------------------------------------------------------------------
+Thu Jul  8 07:36:30 UTC 2021 - Fisherman <fisherman@opensuse.org>
+
+- Initial version.
+- 1
diff --git a/tests/fixtures/packages/blowfish-patch-as-source/blowfish.spec b/tests/fixtures/packages/blowfish-patch-as-source/blowfish.spec
new file mode 100644
index 00000000..90a301b0
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-patch-as-source/blowfish.spec
@@ -0,0 +1,17 @@
+#
+# Copyright (c) 2020 SUSE LLC
+#
+# This file is under MIT license
+
+
+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
+Source1:        test.patch
+BuildArch:      noarch
+
+%changelog
diff --git a/tests/fixtures/packages/blowfish-patch-as-source/test.patch b/tests/fixtures/packages/blowfish-patch-as-source/test.patch
new file mode 100644
index 00000000..6fcb6ba2
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-patch-as-source/test.patch
@@ -0,0 +1 @@
+Not really a patch

From ae658fefe20bec0c71ac46bbcaf122af4e2f134e Mon Sep 17 00:00:00 2001
From: Stephan Kulow <coolo@suse.de>
Date: Fri, 25 Mar 2022 07:56:35 +0100
Subject: [PATCH 3/3] Move download URL check to check_source.py

And with that, check_source.pl is gone
---
 CONTENTS.md                                   |   2 +-
 check_source.pl                               |  78 --------------
 check_source.py                               |  97 ++++++++++--------
 dist/package/openSUSE-release-tools.spec      |   4 -
 tests/check_source_tests.py                   |  31 ++++++
 .../blowfish-1.tar.gz                         | Bin 0 -> 216 bytes
 .../blowfish.changes                          |   5 +
 .../blowfish-with-existing-url/blowfish.spec  |  16 +++
 .../blowfish-with-urls/blowfish-1.tar.gz      | Bin 0 -> 216 bytes
 .../blowfish-with-urls/blowfish.changes       |  10 ++
 .../packages/blowfish-with-urls/blowfish.spec |  16 +++
 11 files changed, 133 insertions(+), 126 deletions(-)
 delete mode 100644 check_source.pl
 create mode 100644 tests/fixtures/packages/blowfish-with-existing-url/blowfish-1.tar.gz
 create mode 100644 tests/fixtures/packages/blowfish-with-existing-url/blowfish.changes
 create mode 100644 tests/fixtures/packages/blowfish-with-existing-url/blowfish.spec
 create mode 100644 tests/fixtures/packages/blowfish-with-urls/blowfish-1.tar.gz
 create mode 100644 tests/fixtures/packages/blowfish-with-urls/blowfish.changes
 create mode 100644 tests/fixtures/packages/blowfish-with-urls/blowfish.spec

diff --git a/CONTENTS.md b/CONTENTS.md
index 905aba5f..af6266c7 100644
--- a/CONTENTS.md
+++ b/CONTENTS.md
@@ -263,7 +263,7 @@ Inspects built RPMs from staging projects.
 Checks for usual mistakes and problems in the source packages submitted by users. Used also as
 review bot that assigns reviews (?).
 
-* Sources: [check_source.py](check_source.py) and [check_source.pl](check_source.pl)
+* Sources: [check_source.py](check_source.py)
 * Documentation: [docs/check_source.asciidoc](docs/check_source.asciidoc)
 * Package: openSUSE-release-tools-check-source
 * Usage: [gocd](https://github.com/openSUSE/openSUSE-release-tools/search?q=path%3A%2Fgocd+check_source)
diff --git a/check_source.pl b/check_source.pl
deleted file mode 100644
index 14e933d3..00000000
--- a/check_source.pl
+++ /dev/null
@@ -1,78 +0,0 @@
-#! /usr/bin/perl
-
-use File::Basename;
-use File::Temp qw/ tempdir  /;
-use Cwd;
-use Text::Diff;
-BEGIN {
-    unshift @INC, ($::ENV{'BUILD_DIR'} || '/usr/lib/build');
-}
-use Build;
-
-my $ret = 0;
-
-my $old = $ARGV[0];
-my $dir = $ARGV[1];
-my $bname = basename($dir);
-
-my $odir = getcwd();
-
-if (-d "$old") {
-
-    chdir($old) || die "chdir $old failed";
-    my $cf = Build::read_config("x86_64", "/usr/lib/build/configs/default.conf");
-
-    my %thash = ();
-    for my $spec (glob("*.spec")) {
-        my $ps = Build::Rpm::parse($cf, $spec);
-
-        while (my ($k, $v) = each %$ps) {
-            if ($k =~ m/^source/) {
-                $thash{$v} = 1;
-            }
-        }
-    }
-   
-    chdir($odir) || die "chdir $odir failed";
-    chdir($dir) || die "chdir $dir failed";
-    for my $spec (glob("*.spec")) {
-        my $ps = Build::Rpm::parse($cf, $spec);
-        open(OSPEC, "$spec");
-        open(NSPEC, ">$spec.new");
-        while (<OSPEC>) {
-            chomp;
-            if (m/^Source/) {
-                my $line = $_;
-                $line =~ s/^(Source[0-9]*)\s*:\s*//;
-                my $prefix = $1;
-                my $parsedline = $ps->{lc $prefix};
-                if (defined $thash{$parsedline}) {
-                    my $file = $line;
-                    my $bname = basename($file);
-                    print NSPEC "$prefix: $bname\n";
-                }
-                else {
-                    print NSPEC "$_\n";
-                }
-            }
-            else {
-                print NSPEC "$_\n";
-            }
-        }
-        close(OSPEC);
-        close(NSPEC);
-        system(("cp", "$spec", "$spec.beforeurlstrip"));
-        rename("$spec.new", "$spec") || die "rename failed";
-    }
-
-}
-
-my $tmpdir = tempdir("obs-XXXXXXX", TMPDIR => 1, CLEANUP => 1);
-chdir($odir) || die "chdir $odir failed";
-chdir($dir) || die "chdir $dir failed";
-if (system("/usr/lib/obs/service/download_files","--enforceupstream", "yes", "--enforcelocal", "yes", "--outdir", $tmpdir)) {
-    print "Source URLs are not valid. Try \"osc service runall download_files\".\n";
-    $ret = 2;
-}
-
-exit($ret);
diff --git a/check_source.py b/check_source.py
index 97c337c0..a9eceebe 100755
--- a/check_source.py
+++ b/check_source.py
@@ -7,12 +7,12 @@ import re
 import shutil
 import subprocess
 import sys
+import tempfile
 
 from lxml import etree as ET
 
 import osc.conf
 import osc.core
-from osc.util.helper import decode_list
 from osclib.conf import Config
 from osclib.core import devel_project_get
 from osclib.core import devel_project_fallback
@@ -183,13 +183,11 @@ class CheckSource(ReviewBot.ReviewBot):
         os.makedirs(dir)
         os.chdir(dir)
 
-        old_info = {'version': None}
         try:
             CheckSource.checkout_package(self.apiurl, target_project, target_package, pathname=dir,
                                          server_service_files=True, expand_link=True)
             shutil.rmtree(os.path.join(target_package, '.osc'))
             os.rename(target_package, '_old')
-            old_info = self.package_source_parse(target_project, target_package)
         except HTTPError as e:
             if e.code == 404:
                 self.logger.info('target package does not exist %s/%s' % (target_project, target_package))
@@ -229,51 +227,27 @@ class CheckSource(ReviewBot.ReviewBot):
         if not self.detect_mentioned_patches('_old', 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 = ''
-        new_version = None
-        if old_info['version'] and old_info['version'] != new_info['version']:
-            new_version = new_info['version']
-            civs += "NEW_VERSION='{}' ".format(new_version)
-        civs += 'LC_ALL=C perl %s _old %s 2>&1' % (source_checker, target_package)
-        p = subprocess.Popen(civs, shell=True, stdout=subprocess.PIPE, close_fds=True)
-        ret = os.waitpid(p.pid, 0)[1]
-        checked = decode_list(p.stdout.readlines())
-
-        output = '  '.join(checked).replace('\033', '')
-        os.chdir('/tmp')
-
-        # ret = 0 : Good
-        # ret = 1 : Bad
-        # ret = 2 : Bad but can be non-fatal in some cases
-        if ret > 1 and target_project.startswith('openSUSE:Leap:') and (source_project.startswith('SUSE:SLE-15:') or
-                                                                        source_project.startswith('openSUSE:Factory')):
-            pass
-        elif ret != 0:
-            shutil.rmtree(dir)
-            self.review_messages['declined'] = "Output of check script:\n" + output
+        if not self.check_urls('_old', target_package, specs):
             return False
 
         shutil.rmtree(dir)
         self.review_messages['accepted'] = 'Check script succeeded'
 
-        if len(checked):
-            self.review_messages['accepted'] += "\n\nOutput of check script (non-fatal):\n" + output
+        if self.skip_add_reviews:
+            return True
 
-        if not self.skip_add_reviews:
-            if self.add_review_team and self.review_team is not None:
-                self.add_review(self.request, by_group=self.review_team, msg='Please review sources')
+        if self.add_review_team and self.review_team is not None:
+            self.add_review(self.request, by_group=self.review_team, msg='Please review sources')
 
-            if self.only_changes():
-                self.logger.debug('only .changes modifications')
-                if self.staging_group and self.review_user in group_members(self.apiurl, self.staging_group):
-                    if not self.dryrun:
-                        osc.core.change_review_state(self.apiurl, str(self.request.reqid), 'accepted',
-                                                     by_group=self.staging_group,
-                                                     message='skipping the staging process since only .changes modifications')
-                else:
-                    self.logger.debug('unable to skip staging review since not a member of staging group')
+        if self.only_changes():
+            self.logger.debug('only .changes modifications')
+            if self.staging_group and self.review_user in group_members(self.apiurl, self.staging_group):
+                if not self.dryrun:
+                    osc.core.change_review_state(self.apiurl, str(self.request.reqid), 'accepted',
+                                                 by_group=self.staging_group,
+                                                 message='skipping the staging process since only .changes modifications')
+            else:
+                self.logger.debug('unable to skip staging review since not a member of staging group')
 
         return True
 
@@ -578,6 +552,40 @@ class CheckSource(ReviewBot.ReviewBot):
 
         return True
 
+    def _snipe_out_existing_urls(self, old, directory, specs):
+        if not os.path.isdir(old):
+            return
+        oldsources = self._mentioned_sources(old, specs)
+        for spec in specs:
+            specfn = os.path.join(directory, spec)
+            nspecfn = specfn + '.new'
+            wf = open(nspecfn, 'w')
+            with open(specfn) as rf:
+                for line in rf:
+                    m = re.match(r'(Source[0-9]*\s*):\s*(.*)$', line)
+                    print(line, m)
+                    if m and m.group(2) in oldsources:
+                        wf.write(m.group(1) + ":" + os.path.basename(m.group(2)) + "\n")
+                        continue
+                    wf.write(line)
+            wf.close()
+            os.rename(nspecfn, specfn)
+
+    def check_urls(self, old, directory, specs):
+        self._snipe_out_existing_urls(old, directory, specs)
+        oldcwd = os.getcwd()
+        with tempfile.TemporaryDirectory() as tmpdir:
+            os.chdir(directory)
+            res = subprocess.run(["/usr/lib/obs/service/download_files", "--enforceupstream",
+                                  "yes", "--enforcelocal", "yes", "--outdir", tmpdir], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+            if res.returncode:
+                self.review_messages['declined'] = "Source URLs are not valid. Try `osc service runall download_files`.\n" + \
+                    res.stdout.decode('utf-8')
+                os.chdir(oldcwd)
+                return False
+        os.chdir(oldcwd)
+        return True
+
     def difflines(self, oldf, newf):
         with open(oldf, 'r') as f:
             oldl = f.readlines()
@@ -588,7 +596,10 @@ class CheckSource(ReviewBot.ReviewBot):
     def _mentioned_sources(self, directory, specs):
         sources = set()
         for spec in specs:
-            with open(os.path.join(directory, spec)) as f:
+            specfn = os.path.join(directory, spec)
+            if not os.path.exists(specfn):
+                continue
+            with open(specfn) as f:
                 for line in f:
                     m = re.match(r'Source[0-9]*\s*:\s*(.*)$', line)
                     if not m:
@@ -622,7 +633,7 @@ class CheckSource(ReviewBot.ReviewBot):
                 diff = self.difflines(oldchanges, changes)
             else:
                 with open(changes, 'r') as f:
-                    diff = ['+' + l for l in f.readlines()]
+                    diff = ['+' + line for line in f.readlines()]
             for line in diff:
                 pass
                 # Check if the line mentions a patch being added (starts with +)
diff --git a/dist/package/openSUSE-release-tools.spec b/dist/package/openSUSE-release-tools.spec
index 212b1bcb..ca586098 100644
--- a/dist/package/openSUSE-release-tools.spec
+++ b/dist/package/openSUSE-release-tools.spec
@@ -103,9 +103,7 @@ OBS product release announcer for generating email diffs summaries.
 %package check-source
 Summary:        Check source review bot
 Group:          Development/Tools/Other
-# check_source.pl
 Requires:       obs-service-download_files
-Requires:       obs-service-format_spec_file
 Requires:       obs-service-source_validator
 Requires:       osclib = %{version}
 Requires:       perl-Text-Diff
@@ -373,7 +371,6 @@ exit 0
 %exclude %{_datadir}/%{source_dir}/abichecker
 %exclude %{_datadir}/%{source_dir}/%{announcer_filename}
 %exclude %{_datadir}/%{source_dir}/check_maintenance_incidents.py
-%exclude %{_datadir}/%{source_dir}/check_source.pl
 %exclude %{_datadir}/%{source_dir}/check_source.py
 %exclude %{_datadir}/%{source_dir}/devel-project.py
 %exclude %{_datadir}/%{source_dir}/metrics
@@ -411,7 +408,6 @@ exit 0
 
 %files check-source
 %{_bindir}/osrt-check_source
-%{_datadir}/%{source_dir}/check_source.pl
 %{_datadir}/%{source_dir}/check_source.py
 
 %files maintenance
diff --git a/tests/check_source_tests.py b/tests/check_source_tests.py
index ea6303a4..9e51e9dd 100644
--- a/tests/check_source_tests.py
+++ b/tests/check_source_tests.py
@@ -366,6 +366,37 @@ class TestCheckSource(OBSLocal.TestCase):
         review = self.assertReview(req_id, by_user=(self.bot_user, 'declined'))
         self.assertEqual("Attention, README is not mentioned in spec files as source or patch.", review.comment)
 
+    @pytest.mark.usefixtures("default_config")
+    def test_source_urls(self):
+        """Declines invalid source URLs"""
+        self._setup_devel_project(devel_files='blowfish-with-urls')
+
+        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("Source URLs are not valid. Try `osc service runall download_files`.\nblowfish-1.tar.gz", review.comment)
+
+    @pytest.mark.usefixtures("default_config")
+    def test_existing_source_urls(self):
+        """Accepts invalid source URLs if previously present"""
+        self._setup_devel_project(devel_files='blowfish-with-urls', target_files='blowfish-with-existing-url')
+
+        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()
+
+        self.assertReview(req_id, by_user=(self.bot_user, 'accepted'))
+
     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)
diff --git a/tests/fixtures/packages/blowfish-with-existing-url/blowfish-1.tar.gz b/tests/fixtures/packages/blowfish-with-existing-url/blowfish-1.tar.gz
new file mode 100644
index 0000000000000000000000000000000000000000..94fff08bd3128a12905a224c182cb1c1093ccba3
GIT binary patch
literal 216
zcmV;}04M(+iwFP!000001MSqk3c@fHhT+U!MX#XtBxzy?cSmobXpjmzv=zL3qasSN
z;!vuE_ZgDmg!~Y`lg`)Wc@@fSsGC%Kv3}l$rd31AB4_Pwj=FOu+AFc5weeoWsVol8
zspLH9Eqa`qwyfh-#yIN_=^{C_)gdg!nfL*PEOhFv6h-dLEEda8#Z7PV*U!S21`qoi
zFZmar-*Yd|eh}rI{2QY)m5h?fNB;M9sJ7K{7wW!su|s~?r`y~n;`;xj6#xJL00000
S00000V7e~$+pz8cC;$Le&uKpZ

literal 0
HcmV?d00001

diff --git a/tests/fixtures/packages/blowfish-with-existing-url/blowfish.changes b/tests/fixtures/packages/blowfish-with-existing-url/blowfish.changes
new file mode 100644
index 00000000..45f636c9
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-with-existing-url/blowfish.changes
@@ -0,0 +1,5 @@
+-------------------------------------------------------------------
+Thu Jul  8 07:36:30 UTC 2021 - Fisherman <fisherman@opensuse.org>
+
+- Initial version.
+- 1
diff --git a/tests/fixtures/packages/blowfish-with-existing-url/blowfish.spec b/tests/fixtures/packages/blowfish-with-existing-url/blowfish.spec
new file mode 100644
index 00000000..2ffcffea
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-with-existing-url/blowfish.spec
@@ -0,0 +1,16 @@
+#
+# Copyright (c) 2020 SUSE LLC
+#
+# This file is under MIT license
+
+
+Name:           blowfish
+Version:        1
+Release:        0
+Summary:        Blowfish
+License:        GPL-2.0-only
+URL:            https://github.com/openSUSE/cockpit-wicked
+Source:         https://example.com/blowfish-%{version}.tar.gz
+BuildArch:      noarch
+
+%changelog
diff --git a/tests/fixtures/packages/blowfish-with-urls/blowfish-1.tar.gz b/tests/fixtures/packages/blowfish-with-urls/blowfish-1.tar.gz
new file mode 100644
index 0000000000000000000000000000000000000000..94fff08bd3128a12905a224c182cb1c1093ccba3
GIT binary patch
literal 216
zcmV;}04M(+iwFP!000001MSqk3c@fHhT+U!MX#XtBxzy?cSmobXpjmzv=zL3qasSN
z;!vuE_ZgDmg!~Y`lg`)Wc@@fSsGC%Kv3}l$rd31AB4_Pwj=FOu+AFc5weeoWsVol8
zspLH9Eqa`qwyfh-#yIN_=^{C_)gdg!nfL*PEOhFv6h-dLEEda8#Z7PV*U!S21`qoi
zFZmar-*Yd|eh}rI{2QY)m5h?fNB;M9sJ7K{7wW!su|s~?r`y~n;`;xj6#xJL00000
S00000V7e~$+pz8cC;$Le&uKpZ

literal 0
HcmV?d00001

diff --git a/tests/fixtures/packages/blowfish-with-urls/blowfish.changes b/tests/fixtures/packages/blowfish-with-urls/blowfish.changes
new file mode 100644
index 00000000..ecb7b430
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-with-urls/blowfish.changes
@@ -0,0 +1,10 @@
+-------------------------------------------------------------------
+Fri Mar 25 07:43:21 UTC 2022 - Stephan Kulow <coolo@suse.com>
+
+- I have a cool new URL!
+
+-------------------------------------------------------------------
+Thu Jul  8 07:36:30 UTC 2021 - Fisherman <fisherman@opensuse.org>
+
+- Initial version.
+- 1
diff --git a/tests/fixtures/packages/blowfish-with-urls/blowfish.spec b/tests/fixtures/packages/blowfish-with-urls/blowfish.spec
new file mode 100644
index 00000000..2ffcffea
--- /dev/null
+++ b/tests/fixtures/packages/blowfish-with-urls/blowfish.spec
@@ -0,0 +1,16 @@
+#
+# Copyright (c) 2020 SUSE LLC
+#
+# This file is under MIT license
+
+
+Name:           blowfish
+Version:        1
+Release:        0
+Summary:        Blowfish
+License:        GPL-2.0-only
+URL:            https://github.com/openSUSE/cockpit-wicked
+Source:         https://example.com/blowfish-%{version}.tar.gz
+BuildArch:      noarch
+
+%changelog