From 8667d5379161183b306bdd4a6733c666cd2ef310 Mon Sep 17 00:00:00 2001
From: Otto Liljalaakso <otto.liljalaakso@iki.fi>
Date: Sun, 2 Apr 2023 17:21:00 +0300
Subject: [PATCH 1/2] Use release's rpmdefines in unused sources check
Conditional Source: tags are problematic and, in fact, forbidden in at
least Fedora. However, there are packages that conditionalize packages
based on macros such as %{rhel} or %{fedora}. 'x-pkg sources' did not
handle such packages correctly, because when the specfile was parsed
to check for unused sources, values for those macros were not set. This
was different from other commands which set such macros based on the
value of --release parameter or Git branch name.
Improve support for conditional Source: tags by using the standard set
of rpmdefines when the specfile is parsed in 'fedpkg sources'.
Fixes: #671
JIRA: RHELCMP-11465
Merges: https://pagure.io/rpkg/pull-request/678
Signed-off-by: Otto Liljalaakso <otto.liljalaakso@iki.fi>
---
pyrpkg/__init__.py | 21 +++++++++++++++------
pyrpkg/spec.py | 12 +++++++-----
tests/test_cli.py | 21 ++++++++++++++++++++-
tests/test_spec.py | 8 ++++++--
4 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/pyrpkg/__init__.py b/pyrpkg/__init__.py
index 3f934d3..817ef33 100644
--- a/pyrpkg/__init__.py
+++ b/pyrpkg/__init__.py
@@ -2261,13 +2261,22 @@ class Commands(object):
sourcesf = SourcesFile(self.sources_filename, self.source_entry_type)
try:
- specf = SpecFile(os.path.join(self.layout.specdir, self.spec),
- self.layout.sourcedir)
- spec_parsed = True
- except Exception:
- self.log.warning("Parsing specfile for used sources failed. "
- "Falling back to downloading all sources.")
+ # Try resolving rpmdefines separately. This produces a clear error
+ # message in the common failure case of custom branch name.
+ self.rpmdefines
+ except Exception as err:
+ self.log.warning("Parsing specfile for used sources failed: %s" % err)
+ self.log.warning("Falling back to downloading all sources.")
spec_parsed = False
+ else:
+ try:
+ specf = SpecFile(os.path.join(self.layout.specdir, self.spec),
+ self.rpmdefines)
+ spec_parsed = True
+ except Exception:
+ self.log.warning("Parsing specfile for used sources failed. "
+ "Falling back to downloading all sources.")
+ spec_parsed = False
args = dict()
if self.lookaside_request_params:
diff --git a/pyrpkg/spec.py b/pyrpkg/spec.py
index d72f1fb..5400de3 100644
--- a/pyrpkg/spec.py
+++ b/pyrpkg/spec.py
@@ -18,16 +18,16 @@ class SpecFile(object):
r'^((source[0-9]*|patch[0-9]*)\s*:\s*(?P<val>.*))\s*$',
re.IGNORECASE)
- def __init__(self, spec, sourcedir):
+ def __init__(self, spec, rpmdefines):
self.spec = spec
- self.sourcedir = sourcedir
+ self.rpmdefines = rpmdefines
self.sources = []
self.parse()
def parse(self):
"""Call rpmspec and find source tags from the result."""
- stdout = run(self.spec, self.sourcedir)
+ stdout = run(self.spec, self.rpmdefines)
for line in stdout.splitlines():
m = self.sourcefile_expression.match(line)
if not m:
@@ -38,8 +38,10 @@ class SpecFile(object):
self.sources.append(val)
-def run(spec, sourcedir):
- cmdline = ['rpmspec', '--define', "_sourcedir %s" % sourcedir, '-P', spec]
+def run(spec, rpmdefines):
+ cmdline = ['rpmspec']
+ cmdline.extend(rpmdefines)
+ cmdline.extend(['-P', spec])
try:
process = subprocess.Popen(cmdline,
stdout=subprocess.PIPE,
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 02620ef..58df047 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -1607,6 +1607,25 @@ class TestSources(LookasideCacheMock, CliTestCase):
def test_unused_sources_are_not_downloaded(self):
self._upload_unused()
+ cli_cmd = ['rpkg', '--path', self.cloned_repo_path, 'sources']
+ with patch('sys.argv', new=cli_cmd):
+ with patch('pyrpkg.Commands.rpmdefines',
+ new=['--define', '_sourcedir %s' % self.cloned_repo_path]):
+ cli = self.new_cli()
+ with patch('pyrpkg.lookaside.CGILookasideCache.download',
+ new=self.lookasidecache_download):
+ cli.sources()
+
+ path = os.path.join(self.cloned_repo_path, 'unused.patch')
+ self.assertFalse(os.path.exists(path))
+
+ @patch('pyrpkg.Commands.load_rpmdefines')
+ def test_download_sources_including_unused(self, rpmdefines):
+ self._upload_unused()
+ # SpecFile parsing executes 'rpmspec', that needs '--define' arguments from rpmdefines
+ # when rpmdefines raises eception, SpecFile parsing fails --> all sources are downloaded.
+ rpmdefines.side_effect = rpkgError
+
cli_cmd = ['rpkg', '--path', self.cloned_repo_path, 'sources']
with patch('sys.argv', new=cli_cmd):
cli = self.new_cli()
@@ -1615,7 +1634,7 @@ class TestSources(LookasideCacheMock, CliTestCase):
cli.sources()
path = os.path.join(self.cloned_repo_path, 'unused.patch')
- self.assertFalse(os.path.exists(path))
+ self.assertTrue(os.path.exists(path))
def test_force_option_downloads_unused_sources(self):
self._upload_unused()
diff --git a/tests/test_spec.py b/tests/test_spec.py
index eefc475..0c7907a 100644
--- a/tests/test_spec.py
+++ b/tests/test_spec.py
@@ -10,6 +10,10 @@ from pyrpkg.errors import rpkgError
class SpecFileTestCase(unittest.TestCase):
def setUp(self):
self.workdir = tempfile.mkdtemp(prefix='rpkg-tests.')
+ self.rpmdefines = ["--define", "_sourcedir %s" % self.workdir,
+ "--define", "_specdir %s" % self.workdir,
+ "--define", "_builddir %s" % self.workdir,
+ "--eval", "%%undefine rhel"]
self.specfile = os.path.join(self.workdir, self._testMethodName)
# Write common header
@@ -43,7 +47,7 @@ class SpecFileTestCase(unittest.TestCase):
"PAtch999: https://remote.patch-sourcce.org/another-patch.bz2\n")
spec_fd.close()
- s = spec.SpecFile(self.specfile, self.workdir)
+ s = spec.SpecFile(self.specfile, self.rpmdefines)
actual = s.sources
expected = [
"tarball.tar.gz",
@@ -65,4 +69,4 @@ class SpecFileTestCase(unittest.TestCase):
self.assertRaises(rpkgError,
spec.SpecFile,
self.specfile,
- self.workdir)
+ self.rpmdefines)
--
2.40.0