PR#597: mockbuild: allow deafulting to local Mock configs (in fedpkg) - rpkg

rpkg

#597 mockbuild: allow deafulting to local Mock configs (in fedpkg)

Merged 3 years ago by onosek. Opened 3 years ago by praiskup.

praiskup/rpkg opt-in-koji-mock-config into master

mockbuild: allow enforcing local mock config in fedpkg

Pavel Raiskup • 3 years ago

12e13bf

pyrpkg/__init__.py

file modified

+31 -7

		`@@ -2800,7 +2800,32 @@`
		`raise rpkgError('Failed to remove temporary directory'`
		`' %s. Reason: %s.' % (tmp_dir, error))`

		`- def mockbuild(self, mockargs=[], root=None, hashtype=None, shell=None):`
		`+ @staticmethod`
		`+ def use_local_mock_config(root, force_local):`
		`+ """`
		`+ Decide if local mock configuration should be used, based on Mock root`
		`+ name and command-line arguments.`
		`+`
		`+ :param str root: mock chroot, the -r <chroot> argument`
		`+ :param bool force_local: enforce download of the Mock configuration`
		`+ from Kojihub (when False), or enforce local Mock config (when`
		`+ True). If None, local configuration is used if the config file`
		`+ exists.`
		`+ :return: bool, True for local config, False for downloaded config.`
		`+ """`
		`+ if force_local is not None:`
		`+ return force_local`
		`+`
		`+ chroot_cfg = '/etc/mock/%s.cfg' % root`
		`+ home_chroot_cfg = '~/.config/mock/%s.cfg' % root`
		`+ home_chroot_cfg = os.path.expanduser(home_chroot_cfg)`
		`+ if os.path.exists(chroot_cfg) or os.path.exists(home_chroot_cfg):`
		`+ return True`
		`+ return False`
		`+`
		`+`
		`+ def mockbuild(self, mockargs=[], root=None, hashtype=None, shell=None,`
		`+ force_local_mock_config=None):`
		`"""Build the package in mock, using mockargs`

		`Log the output and returns nothing`
		`@@ -2812,6 +2837,9 @@`
		`:param str hashtype: used to generate SRPM only if there is no SRPM`
		`generated before.`
		`:param bool shell: indicate whether to go into chroot.`
		`+ :param bool koji_config: enforce download of the Mock configuration`
		`+ from Kojihub (True), or enforce local Mock config (False). If`
		`+ None local configuration is used if present.`

		`.. versionadded:: 1.56`
		`Parameter shell.`
		`@@ -2829,12 +2857,8 @@`
		`config_dir = None`
		`if not root:`
		`root = self.mockconfig`
		`- chroot_cfg = '/etc/mock/%s.cfg' % root`
		`- home_chroot_cfg = '~/.config/mock/%s.cfg' % root`
		`- home_chroot_cfg = os.path.expanduser(home_chroot_cfg)`
		`- if not os.path.exists(chroot_cfg) and not os.path.exists(home_chroot_cfg):`
		`- self.log.debug('Mock config %s was not found. Going to'`
		`- ' request koji to create new one.', chroot_cfg)`
		`+ if not self.use_local_mock_config(root, force_local_mock_config):`
		`+ self.log.debug('Going to download Mock config from Kojihub')`
		`try:`
		`config_dir = self._config_dir_basic(root=root)`
		`except rpkgError as error:`

pyrpkg/cli.py

file modified

+15 -3

		`@@ -1118,6 +1118,17 @@`
		`"extra_args", default=None, nargs=argparse.REMAINDER,`
		`help="Custom arguments that are passed to the 'mock'. "`
		`"Use '--' to separate them from other arguments.")`
		`+ mock_config_group = mockbuild_parser.add_mutually_exclusive_group()`
		`+ mock_config_group.add_argument(`
		`+ '--use-koji-mock-config', default=None, dest="local_mock_config",`
		`+ action='store_false',`
		`+ help="Download Mock configuration from Kojihub, instead of using "`
		`+ "the local Mock configuration in mock-core-configs.rpm.")`
		`+ mock_config_group.add_argument(`
		`+ '--use-local-mock-config', default=None, dest="local_mock_config",`
		`+ action='store_true',`
		`+ help="Enforce use of local Mock configuration.")`
		`+`
		`mockbuild_parser.set_defaults(command=self.mockbuild)`

		`def register_mock_config(self):`
		`@@ -2308,9 +2319,10 @@`
		`# there were no args`
		`pass`
		`try:`
		`- self.cmd.mockbuild(mockargs, self.args.root,`
		`- hashtype=self.args.hash,`
		`- shell=self.args.shell)`
		`+ self.cmd.mockbuild(`
		`+ mockargs, self.args.root, hashtype=self.args.hash,`
		`+ shell=self.args.shell,`
		`+ force_local_mock_config=self.args.local_mock_config)`
		`except Exception as e:`
		`raise rpkgError(e)`

praiskup commented 3 years ago

See the commit message.

praiskup commented 3 years ago

Testing CI RPM build will be here: https://src.fedoraproject.org/rpms/rpkg/pull-request/38

churchyard commented 3 years ago

I agree this is an improvement for fedpkg. The automatic koji config might be required to use rhpkg and rhpkg users might be surprised by this change. centpkg also appears to use the koji config (but I'd prefer if it used the centos-stream-9 config on disk).

praiskup commented 3 years ago

Ideas?

churchyard commented 3 years ago

Maybe the default could be determined on the frontend level?

praiskup commented 3 years ago

So keep the koji_config=None, keep the old local config detection mechanism by default ON, and disable it in fedpkg? Sounds OK.

I can take a look at this, but this will require another "protected" method defined here
that will be designed to be overridden in Fedpkg. Before I start hacking on this, and
on fedpkg, I'd like to have ACK from rpkg maintainers that this actually makes sense.

Or anyone else wants to take a look at the patch?

ngompa commented 3 years ago

I'm not an rpkg maintainer, but this makes sense to me. Moreover, fedpkg should grow some capability to let the user set the base distro for EPEL and EPEL Next targets by configuring the prefix to look up.

A simple example:

[fedpkg.epel-distros]
base = "alma"
next = "centos-stream"

That will allow for fedpkg to automatically look up the correct configs based on our {distro}+epel(-next)-<releasever>.cfg naming convention.

rebased onto 9adacc8813dd8be22830e3180e8bd4065fac337c

3 years ago

praiskup commented 3 years ago

OK, I updated this anyway ... that was not so huge pile of work, though I bet cleaning that up will take more work...

Corresponding patch for fedpkg: https://src.fedoraproject.org/rpms/fedpkg/pull-request/35

praiskup commented 3 years ago

Testing packages are in dnf copr enable praiskup/fedpkg-and-no-epel-8-config

rebased onto 12e13bf

3 years ago

rebased onto 12e13bf

3 years ago

praiskup commented 3 years ago

Can I do something about the code? Fedora community thinks that it's better to block Mock in updates-testing, rather than have broken fedpkg mockbuild in epel8 branch. So it would be nice to find a way out from this quickly (CentOS 8 mirrors will be off soon).