tools/osbuild-depsolve-dnf: enable loading of optional metadata

[achilleas-k]

This is required so that callers can enable loading of filelists when using newer versions of libdnf with old repositories or packages that specify dependencies on files. For example, depsolving against RHEL 9.3 repos on Fedora 40 fails to resolve platform-python with the message

nothing provides /usr/libexec/platform-python needed by platform-python-...

Further reading:

• https://libdnf.readthedocs.io/en/stable/tutorial-py/#case-for-loading-the-filelists
• https://github.com/rpm-software-management/dnf/releases/tag/4.19.0
• https://dnf.readthedocs.io/en/stable/user_faq.html#starting-with-fedora-40-i-noticed-repository-metadata-is-synchronized-much-faster-what-happened
• https://fedoraproject.org/wiki/Changes/DNFConditionalFilelists

[achilleas-k]

Here's a small depsolve request that demonstrates the issue:

{
  "command": "depsolve",
  "arch": "x86_64",
  "module_platform_id": "platform:el9",
  "releasever": "9",
  "cachedir": "/tmp/rpmmd.cache",
  "arguments": {
    "repos": [
      {
        "id": "baseos",
        "name": "baseos",
        "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-baseos-n9.3-20230808",
        "check_gpg": false
      },
      {
        "id": "appstream",
        "name": "appstream",
        "baseurl": "https://rpmrepo.osbuild.org/v2/mirror/rhvpn/el9/el9-x86_64-appstream-n9.3-20230808",
        "check_gpg": false
      }
    ],
    "transactions": [
      {
        "package-specs": [
          "insights-client"
        ]
      }
    ]
  }
}

$ rpm -qa "*dnf*"
dnf-data-4.19.2-1.fc40.noarch
libdnf-0.73.1-1.fc40.x86_64
python3-libdnf-0.73.1-1.fc40.x86_64
python3-dnf-4.19.2-1.fc40.noarch
dnf-4.19.2-1.fc40.noarch

$ ./osbuild-depsolve-dnf < ./depsolve.json
error depsolve
DepsolveError: There was a problem depsolving insights-client:
 Problem: conflicting requests
  - nothing provides /usr/libexec/platform-python needed by insights-client-3.2.0-1.el9.noarch from appstream
{"kind": "DepsolveError", "reason": "There was a problem depsolving insights-client: \n Problem: conflicting requests\n  - nothing provides /usr/libexec/platform-python needed by insights-client-3.2.0-1.el9.noarch from appstream"}

Make sure the cache directory is empty before testing.

[achilleas-k]

Maybe this would be better as an option on the depsolve request so that we can get the speed and efficiency of depsolving without downloading filelists when we know we don't need them and still be able to depsolve older distro versions when we need to.

[bcl]

I think that's the best option, maybe even a generic option to control what metadata is needed, set by the distro definition. I wouldn't want to unnecessarily download them when they aren't needed -- I don't actually remember why I set it in the dnf5 version, it was likely just a side effect of trying to get things working with the current repos.

[achilleas-k]

Made it an option for both dnf and dnf5. This is a change in the default behaviour for the dnf5 depsolver, but I think that's okay given we don't use it in prod yet.

Discussions on the name of the option are welcome.
Currently it's a boolean called repo-filelists.
Should we make it a list of strings that the caller can enable instead?

{
  "command": "depsolve",
  "arch": "...",
  "module_platform_id": "...",
  "cachedir": "...",
  "optional-metadata": ["comps", "filelists"],
  "arguments": {
    "repos": [ ... ],
    "transactions": [ ... ]
  }
}

[bcl]

I like the example above, optional-metadata seems fine, or extra-metadata maybe? Either one does a good job describing that it's adding, not overriding anything.

[achilleas-k]

I think extra-metadata is clearer (extra) but optional-metadata matches the option name, which I like.
Difficult decision. I'll toss a coin and update the PR.

[achilleas-k]

I went with optional-metadata to match the dnf option name and moved it to the arguments part of the request.

{
  "command": "depsolve",
  "arch": "...",
  "module_platform_id": "...",
  "cachedir": "...",
  "arguments": {
    "repos": [ ... ],
    "optional-metadata": ["comps", "filelists"],
    "transactions": [ ... ]
  }
}

Moving it to the arguments makes more sense since it is an option that is only relevant for the depsolve command.

[bcl]

Looks good. Schutzbot not happy though, not really sure why.

[achilleas-k]

Looks good. Schutzbot not happy though, not really sure why.

Happy now.

[mvo5]

This looks nice! How much extra work would it be if the existing test_depsolve.py be extended to include the "optional-metadata" in the files and the test?

[achilleas-k]

This looks nice! How much extra work would it be if the existing test_depsolve.py be extended to include the "optional-metadata" in the files and the test?

Not sure. I think it might be as simple as enabling the option (or not) and checking if the local cache includes any files called *filelists* (or not) after the depsolve.
Let me poke at it a bit and see.

[achilleas-k]

I added a test but there's a small issue. The test doesn't check for the version of dnf, so if it's run with a version older than 4.19.0, it will fail. It's also tricky to check which version we're testing against because in out GitHub tests we use tox, which sets up venvs, so if we import dnf to check its version, we'll be checking the version in the venv, but the test is actually run against the system dnf version because we shell out to osbuild-depsolve-dnf, which runs in the system-wide env.

[achilleas-k]

I added a test but there's a small issue. The test doesn't check for the version of dnf, so if it's run with a version older than 4.19.0, it will fail. It's also tricky to check which version we're testing against because in out GitHub tests we use tox, which sets up venvs, so if we import dnf to check its version, we'll be checking the version in the venv, but the test is actually run against the system dnf version because we shell out to osbuild-depsolve-dnf, which runs in the system-wide env.

Now that I think about it, this might be an issue with release testing in EL9, though I'm not sure what tests we run there.

Perhaps we should exclude it here: tests/unit-tests/run-unit-tests.sh

[mvo5]

Thank you!

[bcl]

Looks good!