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

Add DnfTool for using dnf package manager #5791

Merged
merged 8 commits into from Oct 28, 2019
Merged

Conversation

@ShadowMitia
Copy link
Contributor

@ShadowMitia ShadowMitia commented Sep 21, 2019

This is required for Fedora starting with Fedora 31 as yum will be removed.
Added Redhat and Centos distributions for future-proofing.

Haven't been able to run the tests properly yet. There might be things missing still.

Changelog: Feature: Support for DNF system package manager (Fedora 31+ and others) when present.
Docs: conan-io/docs#1462

This is required for Fedora starting with Fedora 31 as yum will be removed.
Added Redhat and Centos distributions for future-proofing.
@CLAassistant
Copy link

@CLAassistant CLAassistant commented Sep 21, 2019

CLA assistant check
All committers have signed the CLA.

Other systems don't always use dnf and might need more precise adjustements.
@ShadowMitia
Copy link
Contributor Author

@ShadowMitia ShadowMitia commented Sep 24, 2019

I'm only dealing with Fedora now, other systems don't always have dnf installed (contrary to what I first thought). They might need more adjustements than I'm capable to deal with, having no experience with those systems.

conans/client/tools/oss.py Show resolved Hide resolved
@ShadowMitia ShadowMitia requested a review from lasote Oct 10, 2019
@lasote
Copy link
Contributor

@lasote lasote commented Oct 23, 2019

I've open a PR in your fork @ShadowMitia please merge and we will merge here later.

@lasote lasote assigned jgsogo and unassigned lasote Oct 25, 2019
@lasote lasote requested review from jgsogo and removed request for jgsogo and lasote Oct 25, 2019
Copy link
Member

@jgsogo jgsogo left a comment

Thanks @ShadowMitia and @lasote for the implementation. Just consider the suggestion about the comment, but it can be merged.

It requires adding DNF to the docs (should be listed here https://docs.conan.io/en/latest/reference/conanfile/methods.html#systempackagetool) and also an entry in the changelog to know that Fedora now will use dnf first and fallback to yum.

@@ -150,6 +157,18 @@ def system_package_tool_test(self):
spt.update()
self.assertEqual(runner.command_called, "sudo -A apt-get update")

# Fake a dnf exe and check if for fedora it used it
Copy link
Member

@jgsogo jgsogo Oct 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok leaving the fake executable, but we can leave a FIXME so anyone motivated can improve it mocking the system call.

Suggested change
# Fake a dnf exe and check if for fedora it used it
# Fake a dnf executable and check Fedora uses it. # FIXME: Mock which("dnf") function

Copy link
Contributor

@lasote lasote Oct 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would say Mock which("dnf") function if you dare. I'm pleased if you explain it to me how to do it.

@lasote lasote merged commit 6324117 into conan-io:develop Oct 28, 2019
2 checks passed
@jgsogo jgsogo removed their assignment Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants