PR#562: Get EPEL10 minor version from koji when it's not available from the branch name - fedpkg

		`@@ -74,17 +74,33 @@`
		`self.mockconfig = 'fedora-%s-%s' % (self._distval, self.localarch)`
		`self.override = 'f%s-override' % self._distval`
		`self._distunset = 'rhel'`
		`+ # Works until RHEL 10`
		`elif re.match(r'el\d$', branch) or \`
		`- re.match(r'epel\d+(?:\.\d+)?$', branch):`
		`- major, minor = re.findall(r'(\d+)(?:\.(\d+))?', branch)[0]`
		`+ re.match(r'epel\d$', branch):`
		`+ self._distval = branch.split('el')[1]`
		`+ self._distvar = 'rhel'`
		`+ self._disttag = 'el%s' % self._distval`
		`+ self.mockconfig = 'epel-%s-%s' % (self._distval, self.localarch)`
		`+ self.override = 'epel%s-override' % self._distval`
		`+ self._distunset = 'fedora'`
		`+ # EPEL 10+ without minor`
		`+ elif re.match(r'epel\d{2,}$', branch):`
		`+ macros = self._get_build_macros('epel%s-candidate' % branch[4:])`
		`+ major, minor = macros['distcore'][3:].split('_')`
		`+ self._distval = major`
		`+ self._distvar = 'rhel'`
		`+ self._disttag = macros['distcore'][1:]`
		`+ self.mockconfig = 'epel-%s-%s' % (major, self.localarch)`
		`+ self.override = 'epel%s.%s-override' % (major, minor)`
		`+ self._distunset = 'fedora'`
		`+ # EPEL 10+ with minor`
		`+ elif re.match(r'epel\d{2,}\.\d+$', branch):`
		`+ major, minor = branch[4:].split('.')`
		`self._distval = major`
		`self._distvar = 'rhel'`
		`- self._disttag = 'el%s' % major if minor == '' \`
		`- else 'el%s_%s' % (major, minor)`
		`- self.mockconfig = 'epel-%s-%s' % (major, self.localarch) if minor == '' \`
		`- else 'epel-%s.%s-%s' % (major, minor, self.localarch)`
		`- self.override = 'epel%s-override' % self._distval if minor == '' \`
		`- else 'epel%s.%s-override' % (major, minor)`
		`+ self._disttag = 'el%s_%s' % (major, minor)`
		`+ self.mockconfig = 'epel-%s.%s-%s' % (major, minor, self.localarch)`
		`+ self.override = 'epel%s.%s-override' % (major, minor)`
		`self._distunset = 'fedora'`
		`elif re.match(r'epel\d+-next$', branch):`
		`self._distval = re.search(r'\d+', branch).group(0)`
		`@@ -161,10 +177,15 @@`
		`"--define", "%s 1" % self._disttag.replace(".", "_")]`
		`# TODO: consider removing macro "%s 1; it has unknown/dubious functionality"`

		`+ # If the runtime is known, and is different from`
		`+ # the target, we need to unset the _runtime_disttag`
		`if self._runtime_disttag:`
		`- if self._disttag != self._runtime_disttag:`
		`- # This means that the runtime is known, and is different from`
		`- # the target, so we need to unset the _runtime_disttag`
		`+ # In the case of rhel like runtimes, only the major needs to be different`
		`+ if re.match(r'^el\d', self._runtime_disttag):`
		`+ if not self._disttag.startswith(self._runtime_disttag):`
		`+ self._rpmdefines.extend(["--eval", "%%undefine %s" %`
		`+ self._runtime_disttag])`
		`+ elif self._disttag != self._runtime_disttag:`
		`self._rpmdefines.extend(["--eval", "%%undefine %s" %`
		`self._runtime_disttag])`

test/test_commands.py

file modified

+87 -3

		`@@ -266,17 +266,27 @@`

		`self.assert_rpmdefines()`

		`+ @patch('pyrpkg.Commands.kojisession', new_callable=PropertyMock)`
		`@patch('pyrpkg.Commands.branch_merge', new_callable=PropertyMock)`
		`- def test_load_epel10_dist_tag(self, branch_merge):`
		`+ def test_load_epel10_dist_tag(self, branch_merge, kojisession):`
		`branch_merge.return_value = 'epel10'`

		`+ self.cmd._kojisession = Mock()`
		`+ koji_session = kojisession.return_value`
		`+ koji_session.getBuildConfig.return_value = {`
		`+ # minimal subset of the real koji response`
		`+ 'extra': {`
		`+ 'rpm.macro.distcore': '.el10_3',`
		`+ },`
		`+ }`
		`+`
		`self.cmd.load_rpmdefines()`

		`self.assertEqual('10', self.cmd._distval)`
		`self.assertEqual('rhel', self.cmd._distvar)`
		`- self.assertEqual('el10', self.cmd._disttag)`
		`+ self.assertEqual('el10_3', self.cmd._disttag)`
		`self.assertEqual('epel-10-i686', self.cmd.mockconfig)`
		`- self.assertEqual('epel10-override', self.cmd.override)`
		`+ self.assertEqual('epel10.3-override', self.cmd.override)`
		`self.assertTrue(hasattr(self.cmd, '_distunset'))`

		`self.assert_rpmdefines()`
		`@@ -359,6 +369,80 @@`
		`self.assert_rpmdefines(extra_rpmdefines)`


		`+ class TestLoadRpmDefinesRuntimes(CommandTestCase):`
		`+ """Test Commands.load_rpmdefines"""`
		`+`
		`+ def setUp(self):`
		`+ super(TestLoadRpmDefinesRuntimes, self).setUp()`
		`+`
		`+ self.localarch = patch(`
		`+ 'pyrpkg.Commands.localarch',`
		`+ new_callable=PropertyMock,`
		`+ return_value='i686')`
		`+ self.localarch.start()`
		`+`
		`+ self.cmd = self.make_commands()`
		`+`
		`+ def tearDown(self):`
		`+ self.localarch.stop()`
		`+ super(TestLoadRpmDefinesRuntimes, self).tearDown()`
		`+`
		`+ def assert_rpmdefines(self, extra_rpmdefines=[]):`
		`+ """Assert Commands._rpmdefines after calling load_rpmdefines"""`
		`+ expected_rpmdefines = [`
		`+ "--define", "_sourcedir %s" % self.cmd.layout.sourcedir,`
		`+ "--define", "_specdir %s" % self.cmd.layout.specdir,`
		`+ "--define", "_builddir %s" % self.cmd.layout.builddir,`
		`+ "--define", "_srcrpmdir %s" % self.cmd.layout.srcrpmdir,`
		`+ "--define", "_rpmdir %s" % self.cmd.layout.rpmdir,`
		`+ "--define", "_rpmfilename %s" % self.cmd.layout.rpmfilename,`
		`+ "--define", "dist %%{?distprefix}.%s" % self.cmd._disttag,`
		`+ "--define", "%s %s" % (self.cmd._distvar, self.cmd._distval),`
		`+ "--eval", "%%undefine %s" % self.cmd._distunset,`
		`+ "--define", "%s 1" % self.cmd._disttag.replace(".", "_")`
		`+ ]`
		`+ expected_rpmdefines.extend(extra_rpmdefines)`
		`+ self.assertEqual(expected_rpmdefines, self.cmd._rpmdefines)`
		`+`
		`+ @patch('fedpkg.Commands._determine_runtime_env')`
		`+ @patch('pyrpkg.Commands.branch_merge', new_callable=PropertyMock)`
		`+ def test_load_fc40_on_el10_8_runtime(self, branch_merge, _determine_runtime_env):`
		`+ _determine_runtime_env.return_value = 'fc40'`
		`+ branch_merge.return_value = 'epel10.8'`
		`+`
		`+ self.cmd.load_rpmdefines()`
		`+ extra_rpmdefines = [`
		`+ "--eval", "%%undefine %s" % self.cmd._runtime_disttag]`
		`+ self.assert_rpmdefines(extra_rpmdefines)`
		`+`
		`+ @patch('fedpkg.Commands._determine_runtime_env')`
		`+ @patch('pyrpkg.Commands.kojisession', new_callable=PropertyMock)`
		`+ @patch('pyrpkg.Commands.branch_merge', new_callable=PropertyMock)`
		`+ def test_load_epel10_on_el10_runtime(self, branch_merge, kojisession, _determine_runtime_env):`
		`+ _determine_runtime_env.return_value = 'el10'`
		`+ branch_merge.return_value = 'epel10'`
		`+`
		`+ self.cmd._kojisession = Mock()`
		`+ koji_session = kojisession.return_value`
		`+ koji_session.getBuildConfig.return_value = {`
		`+ # minimal subset of the real koji response`
		`+ 'extra': {`
		`+ 'rpm.macro.distcore': '.el10_8',`
		`+ },`
		`+ }`
		`+ self.cmd.load_rpmdefines()`
		`+ self.assert_rpmdefines()`
		`+`
		`+ @patch('fedpkg.Commands._determine_runtime_env')`
		`+ @patch('pyrpkg.Commands.branch_merge', new_callable=PropertyMock)`
		`+ def test_load_epel10_4_on_el10_runtime(self, branch_merge, _determine_runtime_env):`
		`+ _determine_runtime_env.return_value = 'el10'`
		`+ branch_merge.return_value = 'epel10.4'`
		`+`
		`+ self.cmd.load_rpmdefines()`
		`+ self.assert_rpmdefines()`
		`+`
		`+`
		`class TestFindRawhideBranch(CommandTestCase):`
		`"""Test Commands._findrawhidebranch"""`

dherrera commented a month ago

When using the plain epel10 branch, we still need to set the minor version for both the dist_tag and override values.
As sugested by @carlwgeorge in #559 we can get this value from koji like how eln gets it's distval.
This PR does this by using the distcore macro value available from koji for the epel10 build target, which provides that information.

These changes should fix #559

1 new commit added

Fix EPEL10 branch expression to cover 2 or more digits.

a month ago

1 new commit added

Get macros from epel-candidate build target.

a month ago

carlwgeorge commented on line 18 of fedpkg/__init__.py a month ago

branch.split('el')[1] could be simplified to branch[4:].

carlwgeorge commented on line 19 of fedpkg/__init__.py a month ago

We could accomplish the same thing without resorting to regex.

major, minor = macros['distcore'][3:].split('_')

carlwgeorge commented on line 22 of fedpkg/__init__.py a month ago

Similar to above, macros['distcore'].split('.')[1] could be simplified to macros['distcore'][1:].

carlwgeorge commented on line 28 of fedpkg/__init__.py a month ago

Similar to above, a regex is probably overkill for this. How about this instead?

major, minor = branch[4:].split('.')

rebased onto 19feb58

a month ago

onosek commented a month ago

Hi, I rebased this on the top of #560 as it was suggested at #559.

1 new commit added

Improvements on string parsing

a month ago

pretty please pagure-ci rebuild

a month ago

carlwgeorge commented a month ago

This looks good to me. :thumbsup:

5 new commits added

Improvements on string parsing
Get macros from epel-candidate build target.
Fix EPEL10 branch expression to cover 2 or more digits.
Use minor value from koji when no minor in branch name
Fix minor requirements for EPEL10+ in test

17 days ago

2 new commits added

Handle rhel runtimes when minor version exists in disttag
Add tests cases for runtime_disttag removal when working with epel10

16 days ago

dherrera commented 16 days ago

After checking the changes from #560 , I noticed that some changes were also required to make sure that disttags don't get removed on rhel-like runtimes when dealing with minor versions.