1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
|
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
|
