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

CentOS/RHEL: Use dnf if available #395

Merged
merged 2 commits into from
Feb 29, 2024
Merged

CentOS/RHEL: Use dnf if available #395

merged 2 commits into from
Feb 29, 2024

Conversation

kajinamit
Copy link
Contributor

... instead of yum. The yum command is no longer available by default in recent versions.

Closes #394

... instead of yum. The yum command is no longer available by default
in recent versions.

Closes docker#394

Signed-off-by: Takashi Kajinami <[email protected]>
thaJeztah
thaJeztah previously approved these changes Feb 29, 2024
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM - looks like a sane approach, thanks!

@thaJeztah
Copy link
Member

@neersighted PTAL

FWIW; one difference in dnf that I'm aware of is that yum install also updates packages already installed, whereas dnf install doesn't.

This script is not designed to do updates of existing installations, but we're aware of users trying to use it for that purpose. Wondering if we should account for that 🤔

@neersighted
Copy link
Member

It looks like --best should trigger the 'update the packages specified in the command line' behavior that Yum had; I agree we should probably specify it.

@thaJeztah
Copy link
Member

To be fair; we print a warning (and add a delay) if we detect docker was already installed to warn the user "this script might not work for you", but there's a bit of a blurry line there. Would be nice if the script worked for upgrades as well (as we know people try either way), but not sure if we can deliver any promises on that (without a full evaluation of all parts of the script).

@neersighted
Copy link
Member

I agree it's best effort; still, it's somewhat trivial to add the correct flag here. I'll amend this PR.

Signed-off-by: Bjorn Neergaard <[email protected]>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit 3ea1bdc into docker:master Feb 29, 2024
9 checks passed
@kajinamit kajinamit deleted the dnf branch March 4, 2024 10:44
vvoland added a commit to vvoland/docker-install that referenced this pull request Mar 8, 2024
This reverts commit 3ea1bdc, reversing
changes made to 22de4f6.
vvoland added a commit to vvoland/docker-install that referenced this pull request Mar 8, 2024
This reverts commit 3ea1bdc, reversing
changes made to 22de4f6.

Signed-off-by: Paweł Gronowski <[email protected]>
vvoland added a commit to vvoland/docker-install that referenced this pull request Mar 8, 2024
This reverts commit 3ea1bdc, reversing
changes made to 22de4f6.
vvoland added a commit to vvoland/docker-install that referenced this pull request Mar 8, 2024
This reverts commit 3ea1bdc, reversing
changes made to 22de4f6.

Signed-off-by: Paweł Gronowski <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yum is no longer available by default in CentOS Stream 9
3 participants