-
Notifications
You must be signed in to change notification settings - Fork 350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support DNF5's config-manager #5657
Conversation
Multilib test is not working ATM - see rhinstaller/kickstart-tests#1192 |
9033e4e
to
673a27f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the missing test and verify that the collect_requirements
is correctly implemented. It seems to me that DNF payload might be solving this directly.
Otherwise looks great!
def test_multilib_policy_dnf4(self, execute): | ||
"""Update the multilib policy on pre-dnf5 systems.""" | ||
execute.return_value = 0 | ||
|
||
with tempfile.TemporaryDirectory() as sysroot: | ||
data = PackagesConfigurationData() | ||
data.multilib_policy = MULTILIB_POLICY_ALL | ||
dnf_manager = Mock(spec=DNFManager) | ||
dnf_manager.is_package_available.return_value = False | ||
|
||
task = UpdateDNFConfigurationTask(sysroot, data) | ||
task = UpdateDNFConfigurationTask(sysroot, data, dnf_manager) | ||
task.run() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great to check also if the required plugin package is correctly requested.
def collect_requirements(self): | ||
"""Return installation requirements for this module. | ||
|
||
:return: a list of requirements | ||
""" | ||
requirements = [] | ||
|
||
if self.dnf_manager.is_package_available("dnf5"): | ||
plugins_name = "dnf5-modules" | ||
else: | ||
plugins_name = "dnf-plugins-core" | ||
|
||
if self._packages_configuration.multilib_policy != MULTILIB_POLICY_BEST: | ||
requirements.append( | ||
Requirement.for_package(plugins_name, reason="Needed to enable multilib support.") | ||
) | ||
|
||
return requirements | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not be the correct way at the end. I looked on the code and it seems that we should follow path:
return collect_remote_requirements() \ |
This PR is stale because it has been open 60 days with no activity. |
673a27f
to
b4bae77
Compare
b4bae77
to
a0903a7
Compare
a0903a7
to
6f39cb0
Compare
6f39cb0
to
5b27e24
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM otherwise
Fedora rawhide is installing DNF5 on new systems. Payload that sets up multilib support on the system runs on the installed system, thus using DNF5. DNF5 changed config-manager plugin syntax. This commit makes sure that it works with both DNF4 as well as with DNF5.
5b27e24
to
a83757f
Compare
/kickstart-test --testtype smoke |
Fedora rawhide is installing DNF5 on new systems. Payload that sets up multilib support on the system runs on the installed system, thus using DNF5. DNF5 changed config-manager plugin syntax. This commit makes sure that it works with both DNF4 as well as with DNF5.