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 support for dnf5 (in preparation of fedora 41) #441

Merged
merged 5 commits into from
Sep 12, 2024

Conversation

thaJeztah
Copy link
Member

dnf, yum: set global options before command

The -y and -q options are global options, and can be set before the
command that's run. There's some ambiguity in the USAGE output of different
versions (yum, dnf, dnf5)

yum always outputs the same usage, but shows that options go before
the command;

yum --help
Usage: yum [options] COMMAND

yum install --help
Usage: yum [options] COMMAND

dnf (dnf4) is ambiguous; when showing usage for install it shows the
options to be set after the command;

dnf install --help
usage: dnf install [-c [config file]] [-q] [-v] [--version] ....

but the global --help shows them to be before the command;

dnf --help
usage: dnf [options] COMMAND


General DNF options:
  ...
  -q, --quiet           quiet operation
  -v, --verbose         verbose operation
  ,,,
  --setopt SETOPTS      set arbitrary config and repo options

dnf5 is more explicit about global vs per-command options;

dnf --help
Usage:
  dnf5 [GLOBAL OPTIONS] <COMMAND> ...

dnf install --help
Usage:
  dnf5 [GLOBAL OPTIONS] install [OPTIONS] [ARGUMENTS]

Testing shows that older versions (dnf4 and yum) handle both fine,
and because dnf5 (per the above) prefers global options to go before
the command, we can use that convention in this script.

apt-get: set global options before command

The -y and -q options are global options, and can be set before the
command that's run;

apt-get --help
...
Usage: apt-get [options] command
       apt-get [options] install|remove pkg1 [pkg2 ...]
       apt-get [options] source pkg1 [pkg2 ...]

add support for dnf5

Fedora 41 and up use the new dnf5 as default, which is a rewrite of
the dnf commands with different options;

+ dnf config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo
  Unknown argument "--add-repo" for command "config-manager". Add "--help" for more information about the arguments.
  make: *** [Makefile:95: verify] Error 2
  script returned exit code 2

This patch:

  • adds a check for dnf5
  • removes some indirection through env-vars, and instead inlines the code

Note that with CentOS 7 being EOL, we can probably remove the yum variant
from the script, but I left those in place for now.

don't install weak-dependencies for dnf-core-utils

We only need the config-manager, but it comes with a large number of weak
dependencies;

dnf -q install dnf-plugins-core
Transaction Summary:
Installing:       78 packages

Disable weak dependencies to skip those, as they're not essential for this
script;

dnf -q --setopt=install_weak_deps=False install dnf-plugins-core
Transaction Summary:
Installing:       32 packages

add workaround for dnf5 addrepo bug

The addrepo command has a bug that causes it to fail if the .repo file
contains empty lines, causing it to fail;

dnf config-manager addrepo --from-repofile="https://download.docker.com/linux/fedora/docker-ce.repo"
Error in added repository configuration file. Cannot set repository option "#1=
": Option "#1" not found

Use a temporary file and strip empty lines as a workaround until the bug
is fixed.

The `-y` and `-q` options are global options, and can be set before the
command that's run. There's some ambiguity in the USAGE output of different
versions (`yum`, `dnf`, `dnf5`)

yum always outputs the same usage, but shows that options go _before_
the command;

    yum --help
    Usage: yum [options] COMMAND

    yum install --help
    Usage: yum [options] COMMAND

dnf (dnf4) is ambiguous; when showing usage for `install` it shows the
options to be set _after_ the command;

    dnf install --help
    usage: dnf install [-c [config file]] [-q] [-v] [--version] ....

but the global `--help` shows them to be before the command;

    dnf --help
    usage: dnf [options] COMMAND

    General DNF options:
      ...
      -q, --quiet           quiet operation
      -v, --verbose         verbose operation
      ,,,
      --setopt SETOPTS      set arbitrary config and repo options

dnf5 is more explicit about global vs per-command options;

    dnf --help
    Usage:
      dnf5 [GLOBAL OPTIONS] <COMMAND> ...

    dnf install --help
    Usage:
      dnf5 [GLOBAL OPTIONS] install [OPTIONS] [ARGUMENTS]

Testing shows that older versions (`dnf4` and `yum`) handle both fine,
and because `dnf5` (per the above) prefers global options to go before
the command, we can use that convention in this script.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
The `-y` and `-q` options are global options, and can be set before the
command that's run;

    apt-get --help
    ...
    Usage: apt-get [options] command
           apt-get [options] install|remove pkg1 [pkg2 ...]
           apt-get [options] source pkg1 [pkg2 ...]

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah self-assigned this Sep 4, 2024
install.sh Outdated
Comment on lines 560 to 561
if command_exists dnf5; then
$sh_c "dnf -y -q install dnf-plugins-core"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering.. Should we actually use dnf instead of dnf5 as a command here?

Is there a possibility that both dnf5 and dnf are available, but dnf still points to the old dnf (maybe on systems upgraded from fedora <41?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question! I was somewhat wondering if both could be present, and if there's a good way to determine which one is the default.

I tried to avoid doing some string-matching in dnf --version as that felt more hacky (and brittle).

To some extent, I don't know if they can really live side-by-side. Or at least; knowing now that they use a different approach to configure repositories; docker/docker-ce-packaging#1058 (comment)

Interesting; looks like setopt is taking a slightly different approach, and doesn't update the original .repo file (/etc/yum.repos.d/docker-ce.repo) but instead uses override files (/usr/share/dnf5/repos.override.d or /etc/dnf/repos.override.d).

From https://dnf5.readthedocs.io/en/latest/dnf5_plugins/config-manager.8.html

Screenshot 2024-09-05 at 13 03 46

If those override files are only considered by dnf5, but ignored by dnf, then potentially they would be having a different view of the world (different repositories enabled/disabled) 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me try what happens if I use an override file with dnf, and if it understands those!

Copy link
Member Author

Choose a reason for hiding this comment

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

So I gave it a try, but it looks like mixing/matching dnf and dnf5 doesn't fair well. I took the /etc/dnf/repos.override.d/99-config_manager.repo that was created by dnf5, and copied it into a fedora 40 container;

dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core
dnf config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo

mkdir -p /etc/dnf/repos.override.d
cp 99-config_manager.repo /etc/dnf/repos.override.d/99-config_manager.repo
dnf makecache
dnf -y -q --best install docker-ce docker-ce-cli containerd.io docker-compose-plugin docker-ce-rootless-extras docker-buildx-plugin
Error: Unable to find a match: docker-ce docker-ce-cli containerd.io docker-compose-plugin docker-ce-rootless-extras docker-buildx-plugin

Content of that file;

cat /etc/dnf/repos.override.d/99-config_manager.repo
# Generated by dnf5 config-manager.
# Do not modify this file manually, use dnf5 config-manager instead.
[docker-ce-nightly]
enabled=0
[docker-ce-nightly-debuginfo]
enabled=0
[docker-ce-nightly-source]
enabled=0
[docker-ce-stable]
enabled=0
[docker-ce-stable-debuginfo]
enabled=0
[docker-ce-stable-source]
enabled=0
[docker-ce-test]
enabled=1
[docker-ce-test-debuginfo]
enabled=0
[docker-ce-test-source]
enabled=0

Copy link
Member Author

Choose a reason for hiding this comment

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

So while it probably doesn't do "harm" to call dnf5 instead of dnf, it also likely doesn't fix the issue (unless there's other good ways to verify if dnf means dnf5 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could check if it's a symlink;

ls -la /usr/bin/dnf
lrwxrwxrwx 1 root root 4 Aug  2 00:00 /usr/bin/dnf -> dnf5

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't bother too much TBH. My main point here is that we don't lose anything by calling dnf5 directly, and considering that we're doing it in a if command_exists dnf5 branch, I think it's actually more correct to actually exec dnf5.

Copy link
Member Author

Choose a reason for hiding this comment

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

hm.. yeah, that's also a good point. Question there is though if we then should make all calls to dnf -> dnf5, or only the ones that differ; basically the only incompatible ones are around config-manager and enabling/disabling repos. So the minimal change would be;

diff --git a/install.sh b/install.sh
index 66dfa42..ff6c2a5 100755
--- a/install.sh
+++ b/install.sh
@@ -559,18 +559,18 @@ do_install() {
                                fi
                                if command_exists dnf5; then
                                        # $sh_c "dnf -y -q --setopt=install_weak_deps=False install dnf-plugins-core"
-                                       # $sh_c "dnf config-manager addrepo --save-filename=docker-ce.repo --from-repofile='$repo_file_url'"
+                                       # $sh_c "dnf5 config-manager addrepo --save-filename=docker-ce.repo --from-repofile='$repo_file_url'"

                                        $sh_c "dnf -y -q --setopt=install_weak_deps=False install curl dnf-plugins-core"
                                        # FIXME(thaJeztah); strip empty lines as workaround for https://github.com/rpm-software-management/dnf5/issues/1603
                                        TMP_REPO_FILE="$(mktemp --dry-run)"
                                        $sh_c "curl -fsSL '$repo_file_url' | tr -s '\n' > '${TMP_REPO_FILE}'"
-                                       $sh_c "dnf config-manager addrepo --save-filename=docker-ce.repo --overwrite --from-repofile='${TMP_REPO_FILE}'"
+                                       $sh_c "dnf5 config-manager addrepo --save-filename=docker-ce.repo --overwrite --from-repofile='${TMP_REPO_FILE}'"
                                        $sh_c "rm -f '${TMP_REPO_FILE}'"

                                        if [ "$CHANNEL" != "stable" ]; then
-                                               $sh_c "dnf config-manager setopt 'docker-ce-*.enabled=0'"
-                                               $sh_c "dnf config-manager setopt 'docker-ce-$CHANNEL.enabled=1'"
+                                               $sh_c "dnf5 config-manager setopt 'docker-ce-*.enabled=0'"
+                                               $sh_c "dnf5 config-manager setopt 'docker-ce-$CHANNEL.enabled=1'"
                                        fi
                                        $sh_c "dnf makecache"
                                elif command_exists dnf; then

However, if we'd want all commands to use dnf5, we'd need to (re)introduce the $pkg_manager variable to parameterise all other parts (I was hoping to get rid of that after we remove yum from the script).

Copy link
Contributor

Choose a reason for hiding this comment

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

Using dnf5 only in the if command_exists dnf5 branch is 100% sufficient for me 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

@vvoland updated; PTAL 🤗

Fedora 41 and up use the new dnf5 as default, which is a rewrite of
the dnf commands with different options;

    + dnf config-manager --add-repo https://download.docker.com/linux/fedora/docker-ce.repo
      Unknown argument "--add-repo" for command "config-manager". Add "--help" for more information about the arguments.
      make: *** [Makefile:95: verify] Error 2
      script returned exit code 2

This patch:

- adds a check for dnf5
- removes some indirection through env-vars, and instead inlines the code

Note that with CentOS 7 being EOL, we can probably remove the `yum` variant
from the script, but I left those in place for now.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
We only need the config-manager, but it comes with a large number of weak
dependencies;

    dnf -q install dnf-plugins-core
    Transaction Summary:
    Installing:       78 packages

Disable weak dependencies to skip those, as they're not essential for this
script;

    dnf -q --setopt=install_weak_deps=False install dnf-plugins-core
    Transaction Summary:
    Installing:       32 packages

Signed-off-by: Sebastiaan van Stijn <[email protected]>
The addrepo command has a bug that causes it to fail if the `.repo` file
contains empty lines, causing it to fail;

    dnf config-manager addrepo --from-repofile="https://download.docker.com/linux/fedora/docker-ce.repo"
    Error in added repository configuration file. Cannot set repository option "docker#1=
    ": Option "docker#1" not found

Use a temporary file and strip empty lines as a workaround until the bug
is fixed.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah merged commit 992da63 into docker:master Sep 12, 2024
9 checks passed
@thaJeztah thaJeztah deleted the add_dnf5_support branch September 12, 2024 07:48
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.

3 participants