From a6427cd508a9ea52cf3cf2ce24de03f2388bd675 Mon Sep 17 00:00:00 2001 From: Otto Urpelainen Date: Jul 18 2021 09:14:40 +0000 Subject: Do not download unused sources during command 'sources' Command 'sources' used to download all files listed in the sources file. This is waste of resources, because it is a common packager workflow to first update the specfile then get the new source with 'spectool -g *.spec' and then do 'fedpkg mockbuild'. In that situation, thesources file always lists stale files, downloading those never achieves anything useful. This commit improves the situation by avoiding download of files not actually used in the specfile. The test suite had a strange configuration where specfile did not have any sources, but sources file had an entry for 'readme.patch'. This led to failures with the new feature. Fixed by adding 'readme.patch' as a source in the specfile. Resolves #559 Signed-off-by: Otto Urpelainen --- diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py index a7d834d..1943ef1 100644 --- a/pyrpkg/__init__.py +++ b/pyrpkg/__init__.py @@ -46,6 +46,7 @@ from pyrpkg.errors import (HashtypeMixingError, UnknownTargetError, rpkgAuthError, rpkgError) from pyrpkg.lookaside import CGILookasideCache from pyrpkg.sources import SourcesFile +from pyrpkg.spec import SpecFile from pyrpkg.utils import (cached_property, extract_srpm, find_me, is_file_tracked, is_lookaside_eligible_file, log_result) @@ -2026,6 +2027,7 @@ class Commands(object): outdir = self.path sourcesf = SourcesFile(self.sources_filename, self.source_entry_type) + specf = SpecFile(os.path.join(self.path, self.spec)) args = dict() if self.lookaside_request_params: @@ -2044,6 +2046,9 @@ class Commands(object): "Error: Attempting a download '{0}' that would override a git tracked file. " "Either remove the corresponding line from 'sources' file to keep the git " "tracked one or 'git rm' the file to allow the download.".format(outfile)) + if (entry.file not in specf.sources): + self.log.info("Not downloading unused %s" % entry.file) + continue self.lookasidecache.download( self.ns_repo_name if self.lookaside_namespaced else self.repo_name, entry.file, entry.hash, outfile, diff --git a/pyrpkg/lookaside.py b/pyrpkg/lookaside.py index b112957..4bc81de 100644 --- a/pyrpkg/lookaside.py +++ b/pyrpkg/lookaside.py @@ -152,6 +152,7 @@ class CGILookasideCache(object): if os.path.exists(outfile): if self.file_is_valid(outfile, hash, hashtype=hashtype): + self.log.info("Not downloading already downloaded %s" % filename) return self.log.info("Downloading %s", filename) diff --git a/pyrpkg/spec.py b/pyrpkg/spec.py new file mode 100644 index 0000000..9ab5bb4 --- /dev/null +++ b/pyrpkg/spec.py @@ -0,0 +1,54 @@ +# spec.py - Simple specfile parser that finds source file names +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by the +# Free Software Foundation; either version 2 of the License, or (at your +# option) any later version. See http://www.gnu.org/copyleft/gpl.html for +# the full text of the license. + +import re +import subprocess + +from pyrpkg.errors import rpkgError + + +class SpecFile(object): + """Simple specfile parser that finds source file names""" + + re = re.compile(R'^source[0-9]*:\s*(?P.*)\s*$', re.IGNORECASE) + + def __init__(self, spec): + self.spec = spec + self.sources = [] + + self.parse() + + def parse(self): + """Call rpmspec and find source tags from the result.""" + stdout = run(self.spec) + for line in stdout.splitlines(): + m = self.re.match(line) + if not m: + continue + + # Forget domain and path, only store file name + val = m.group('val').split('/')[-1] + self.sources.append(val) + + +def run(spec): + cmdline = ['rpmspec', '-P', spec] + try: + process = subprocess.Popen(cmdline, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = process.communicate(input) + except Exception as e: + raise rpkgError('Error running rpmspec on "%s": %s' % (spec, e)) + + retcode = process.poll() + if retcode: + raise rpkgError('Error running rpmspec on "%s", return code %s' + % (spec, retcode)) + + return stdout.decode('utf-8') diff --git a/tests/test_spec.py b/tests/test_spec.py new file mode 100644 index 0000000..15e3730 --- /dev/null +++ b/tests/test_spec.py @@ -0,0 +1,55 @@ +import os +import shutil +import unittest +import tempfile + +from pyrpkg import spec +from pyrpkg.errors import rpkgError + + +class SpecFileTestCase(unittest.TestCase): + def setUp(self): + self.workdir = tempfile.mkdtemp(prefix='rpkg-tests.') + self.specfile = os.path.join(self.workdir, self._testMethodName) + + # Write common header + spec_fd = open(self.specfile, "w") + spec_fd.write( + "Name: test-spec\n" + "Version: 0.0.1\n" + "Release: 1\n" + "Summary: test specfile\n" + "License: BSD\n" + "\n" + "%description\n" + "foo\n" + "\n") + spec_fd.close() + + def tearDown(self): + shutil.rmtree(self.workdir) + return + + def test_parse(self): + # Write some sources + spec_fd = open(self.specfile, "a") + spec_fd.write( + "Source0: https://example.com/tarball.tar.gz\n" + "Source1: https://example.com/subdir/LICENSE.txt\n" + "Source2: https://another.domain.com/source.tar.gz\n") + spec_fd.close() + + s = spec.SpecFile(self.specfile) + actual = s.sources + expected = ["tarball.tar.gz", "LICENSE.txt", "source.tar.gz"] + self.assertEqual(len(actual), len(expected)) + self.assertTrue(all([a == b for a, b in zip(actual, expected)])) + + def test_invalid_specfile(self): + # Overwrite the specfile, removing mandatory fields + # Parsing such invalid specfile fails + spec_fd = open(self.specfile, "w") + spec_fd.write("Foo: Bar\n") + spec_fd.close() + + self.assertRaises(rpkgError, spec.SpecFile, [self.specfile]) diff --git a/tests/utils.py b/tests/utils.py index c686040..f5e8091 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -36,7 +36,7 @@ Name: docpkg Version: 1.2 Release: 2%{dist} License: GPL -#Source0: +Source0: https://example.com/readme.patch #Patch0: Group: Applications/Productivity BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)