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

dnfjson: enable loading of optional metadata when needed #702

Merged
merged 2 commits into from
May 29, 2024

Conversation

achilleas-k
Copy link
Member

@achilleas-k achilleas-k commented May 22, 2024

dnfjson: add OptionalMetadata property to depsolve args

New property on the arguments object that controls the types of metadata
to download for repositories and use when depsolving.

See osbuild/osbuild#1775


dnfjson: enable filelists for older distro versions

Enable filelists in the optional metadata for EL7, EL8, EL9, and Fedora 39.

When running osbuild-depsolve-dnf with the libdnf version that's
included in these distro versions, this option will have no effect since
it is enabled by default. However, when depsolving packages for these
distro versions with newer versions of libdnf, the filelists are
disabled and can cause issues with packages that declare dependencies on
filepaths.

The cutoff point is a little fuzzy for CentOS and Fedora. Depsolving
all our test manifests for Fedora 39 and CentOS Stream 9 without
filelists works, but since the packaging guidelines of not depending on
filepaths went into effect in EL10 and Fedora 40, it's possible there
are packages in F39 and C9S that don't conform yet, so it's probably
safer to keep filelists enabled for these distro versions.


Requires osbuild/osbuild#1775

New property on the arguments object that controls the types of metadata
to download for repositories and use when depsolving.

See osbuild/osbuild#1775
@achilleas-k achilleas-k requested review from thozza and bcl May 22, 2024 10:56
@bcl
Copy link
Contributor

bcl commented May 22, 2024

I guess we don't really have a way to express what version of osbuild-depsolve-dnf this depends on other than making sure to set a new Requires version in osbuild-composer when this is imported, right?

@thozza
Copy link
Member

thozza commented May 23, 2024

I'm wondering what would be the downside of enabling the downloading of filelists explicitly in all cases?

pkg/dnfjson/dnfjson.go Outdated Show resolved Hide resolved
@achilleas-k
Copy link
Member Author

I'm wondering what would be the downside of enabling the downloading of filelists explicitly in all cases?

The only downside is the size, which, on a slow connection, can make osbuild-composer very slow to start (building the initial rpmmd cache). Right now, this switch will make startup and depsolving faster on Fedora 40 and RHEL/CentOS 10, and I think that's nice to have.

Enable filelists in the optional metadata for EL7, EL8, EL9, and Fedora
39.

When running osbuild-depsolve-dnf with the libdnf version that's
included in these distro versions, this option will have no effect since
it is enabled by default.  However, when depsolving packages for these
distro versions with newer versions of libdnf, the filelists are
disabled and can cause issues with packages that declare dependencies on
filepaths.

The cutoff point is a little fuzzy for CentOS and Fedora.  Depsolving
all our test manifests for Fedora 39 and CentOS Stream 9 without
filelists works, but since the packaging guidelines of not depending on
filepaths went into effect in EL10 and Fedora 40, it's possible there
are packages in F39 and C9S that don't conform yet, so it's probably
safer to keep filelists enabled for these distro versions.
@achilleas-k
Copy link
Member Author

I guess we don't really have a way to express what version of osbuild-depsolve-dnf this depends on other than making sure to set a new Requires version in osbuild-composer when this is imported, right?

Yes, exactly. However, since osbuild-depsolve-dnf's json deserialization isn't strict, adding this new option to the request (or any unknown property) has no effect and just gets ignored. And on the depsolver side, the property is optional, so this change is non-breaking in any direction:

  • New composer adds the option to an old depsolver, the depsolver ignores it.
  • Old composer with new depsolver will never specify the option.

Both cases will behave like the change was never made.

@achilleas-k achilleas-k requested a review from thozza May 27, 2024 15:28
Copy link
Contributor

@bcl bcl left a comment

Choose a reason for hiding this comment

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

Looks good!

@bcl bcl added this pull request to the merge queue May 28, 2024
Merged via the queue into osbuild:main with commit b002d25 May 29, 2024
16 of 18 checks passed
@achilleas-k achilleas-k deleted the depsolve-filelists branch May 30, 2024 16:58
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