Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

marusak
Copy link
Contributor

@marusak marusak commented May 20, 2024

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.

@marusak
Copy link
Contributor Author

marusak commented May 20, 2024

Multilib test is not working ATM - see rhinstaller/kickstart-tests#1192

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.
Copy link
Member

@jkonecny12 jkonecny12 left a 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!

Comment on lines +480 to 491
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()
Copy link
Member

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.

Comment on lines +375 to +393
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

Copy link
Member

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() \

Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants