fix: respect update_before_build=False during builddep (issue#1420)#1741
fix: respect update_before_build=False during builddep (issue#1420)#1741xsuchy wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a workaround for a DNF5 issue where packages are upgraded during build dependency installation even when update_before_build is disabled. It introduces a _get_update_exclude_opts method to generate an exclusion list of installed packages. Feedback suggests restricting this logic to DNF5 for compatibility, improving error handling for the RPM query command, and adding the @tracelog decorator to maintain consistency with the rest of the codebase.
| def _get_update_exclude_opts(self): | ||
| """ | ||
| When update_before_build is disabled, return --exclude options for all | ||
| installed packages so that builddep/install won't upgrade them. | ||
| Works around https://github.com/rpm-software-management/dnf5/issues/1747 | ||
| """ | ||
| if self.config.get('update_before_build', True): | ||
| return [] | ||
|
|
||
| command = [self.config['rpm_command'], "-qa", "--queryformat", "%{NAME}\\n", | ||
| "--root", self.buildroot.make_chroot_path()] | ||
| out, _ = self.buildroot.doOutChroot(command, returnOutput=True, | ||
| printOutput=False, shell=False) | ||
| pkg_names = set(filter(None, out.strip().splitlines())) | ||
| if not pkg_names: | ||
| return [] | ||
| return ['--exclude=' + ','.join(sorted(pkg_names))] |
There was a problem hiding this comment.
The new _get_update_exclude_opts method has a few issues that should be addressed:
- Compatibility: It uses a comma-separated list for
--exclude, which is a DNF5-specific feature. Standard DNF (DNF4), Yum, and MicroDnf do not reliably support comma-separated lists for this option and typically require multiple--excludeflags. Since this workaround is specifically targeting a DNF5 issue, it should be restricted to when DNF5 is the active package manager. - Robustness: The return code of the
rpm -qacommand is ignored. If the command fails (e.g., due to a corrupted RPM DB),outmight contain error messages which would then be incorrectly parsed as package names. - Observability: The method is missing the
@traceLog()decorator, which is consistently used throughout this class for debugging and tracing. - Python 3.6 Compatibility: While not an issue with the current implementation (which uses a single string), if you were to switch to multiple
--excludeflags, be aware that Python 3.6 (still used on RHEL 8) has a limit of 255 arguments for function calls using*args. Using a single comma-separated string for DNF5 avoids this, but limiting the logic to DNF5 is safer.
@traceLog()
def _get_update_exclude_opts(self):
"""
When update_before_build is disabled, return --exclude options for all
installed packages so that builddep/install won't upgrade them.
Works around https://github.com/rpm-software-management/dnf5/issues/1747
"""
if self.config.get('update_before_build', True):
return []
# This workaround is specifically for DNF5 behavior.
# Comma-separated excludes are also a DNF5-specific feature.
if self.buildroot.pkg_manager.name != 'dnf5':
return []
command = [self.config['rpm_command'], "-qa", "--queryformat", "%{NAME}\\n",
"--root", self.buildroot.make_chroot_path()]
out, status = self.buildroot.doOutChroot(command, returnOutput=True,
printOutput=False, shell=False)
if status != 0:
return []
pkg_names = set(filter(None, out.strip().splitlines()))
if not pkg_names:
return []
return ['--exclude=' + ','.join(sorted(pkg_names))]There was a problem hiding this comment.
Addressed the the compatibility by doing this for DNF5 only.
dnf5 builddep upgrades already-installed packages even when update_before_build is disabled. Work around this by excluding all installed package names from repo lookups during dependency installation, so only genuinely missing packages are pulled in. Fixes: rpm-software-management#1420 Assisted-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I don't really like this giant "--exclude" option, please lemme reiterate the problem with dnf. |
dnf5 builddep upgrades already-installed packages even when update_before_build is disabled. Work around this by excluding all installed package names from repo lookups during dependency installation, so only genuinely missing packages are pulled in.
Fixes: #1420