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

feat: dnf module #361

Open
xynydev opened this issue Nov 18, 2024 · 19 comments · May be fixed by #377
Open

feat: dnf module #361

xynydev opened this issue Nov 18, 2024 · 19 comments · May be fixed by #377
Labels
priority: medium Needs to be done soon state: approved Approved to proceed. type: feature Brand new functionality, features, pages, workflows, endpoints, etc.

Comments

@xynydev
Copy link
Member

xynydev commented Nov 18, 2024

A dnf module will eventually replace the rpm-ostree module. We should start investigating usage of dnf5 on atomic Fedora systems to determine a reasonable timeline for development of such a module.

https://fedoramagazine.org/whats-new-for-fedora-atomic-desktops-in-fedora-41/#first-step-towards-bootable-containers-dnf5-and-bootc

@xynydev xynydev added type: feature Brand new functionality, features, pages, workflows, endpoints, etc. state: approved Approved to proceed. priority: medium Needs to be done soon labels Nov 18, 2024
@fiftydinar
Copy link
Collaborator

fiftydinar commented Nov 18, 2024

It functions similarly to rpm-ostree when I tested it.

The only advantage I see is that we can enable copr repos with simpler command dnf copr enable maintainer/repo instead of manual curls.

Basically, when we would replace rpm-ostree occurences with dnf in the current module, users wouldn't notice any change in functionality except a bit different logs.

@xynydev
Copy link
Member Author

xynydev commented Nov 19, 2024

Basically, when we would replace rpm-ostree occurences with dnf in the current module

I would rather create a new module called dnf. We may keep most of the configuration and even code the same, but there's no reason to keep the legacy name of rpm-ostree around.

@fiftydinar
Copy link
Collaborator

fiftydinar commented Nov 21, 2024

I'm in for making a new dnf module & announcing it to the users in blog, Discord, Mastodon & BlueSky (if it's used?).

Current structure from rpm-ostree module looks good & can be ported to dnf, except there needs to be some minor corrections in recipe format & some code logic replacements are needed.

Minor recipe format corrections

Repos

Currently, in rpm-ostree we just download the repos with curl directly to /etc/yum.repos.d/.

dnf has copr plugin to download & enable COPR repos.

  • We can continue to use the same method of downloading repos using curl to /etc/yum.repos.d/
  • Or switch to more native method of using dnf5 -y copr enable user/package
    • In case of dnf5 copr command usage, we can either
      • keep the same structure of repos array in recipe, so when it detects that it's copr URL, it will use dnf5 copr command
      • same as above, but users can just name the array copr: user/package instead of the full URL & we will detect that (this one is the best approach imo).
      • or add the new YAML array called repos-copr or similar, where users would just input user & package

Replacing packages

Currently, we use rpm-ostree override replace, which supports replacing packages from repo, direct URL & maybe local RPMs too.

It seems that there is no alternative for replacing packages in dnf.

dnf5 swap "${rem_packages}" "${inst_packages}" only does
rpm-ostree override remove "${rem_packages}" --install "${inst_packages}".

Everything else

Everything else looks relatively native.

@tulilirockz
Copy link
Contributor

dnf5 has behavioural differences from rpm-ostree, as it probably does not install Recommends: packages by default! I ended up figuring that out when I was testing out the dnf5 migration on https://github.com/ublue-os/main and https://github.com/ublue-os/bluefin.

@tulilirockz
Copy link
Contributor

rpm-ostree override replace can be replaced by dnf -y remove "${rem_packages}" && dnf -y install "${inst_packages}"

@tulilirockz
Copy link
Contributor

Moving to dnf copr should be completely fine, too! There are some repos that sadly do not work rn like the ublue-os/akmods one that gets funky w/ the dnf copr behaviour (see: ublue-os/bluefin#1954)

@fiftydinar
Copy link
Collaborator

fiftydinar commented Nov 21, 2024

dnf5 has behavioural differences from rpm-ostree, as it probably does not install Recommends: packages by default! I ended up figuring that out when I was testing out the dnf5 migration on https://github.com/ublue-os/main and https://github.com/ublue-os/bluefin.

According to dnf defaults, the default is to install weak dependencies by default, just like rpm-ostree.
install_weak_deps=True

If  enabled, when a new package is about to be installed, all packages linked by weak dependency relation
(Recommends or Supplements flags) with this package will be pulled into the transaction.

I also personally didn't notice any difference in this behavior when I tested it a bit.

rpm-ostree override replace can be replaced by dnf -y remove "${rem_packages}" && dnf -y install "${inst_packages}"

That's basically what dnf swap does (except swap only supports swapping 1 package as I see now...).

But if there is no alternative, then sure, that can be also used & adjusted to match the replace behavior from recipe, without user noticing any difference.

Moving to dnf copr should be completely fine, too! There are some repos that sadly do not work rn like the ublue-os/akmods one that gets funky w/ the dnf copr behaviour (see: ublue-os/bluefin#1954)

Interesting, thanks for noting.

@fiftydinar
Copy link
Collaborator

I noticed in this Bazzite's PR:
https://github.com/ublue-os/bazzite/pull/1905/files

That they are using dnf5 -y upgrade --repo for replacing packages from the repo, so I think we found the command replacement for this actually.

@xynydev
Copy link
Member Author

xynydev commented Nov 23, 2024

dnf has copr plugin to download & enable COPR repos.

I think we could do this simple change:

  • keep the repos: array
  • for each element, check if it starts with http(s)://, and use the curl method if so
  • if not, give the repo to dnf5 -y copr enable and see what happens
    • if it was configured correctly, the repo should be enabled, but if there's a problem it should error out and we can print a nice error message in the logs

@fiftydinar
Copy link
Collaborator

  • if not, give the repo to dnf5 -y copr enable and see what happens

I think a flaw in this method is support for repo files which are directly provided in /files/rpm-ostree/

So I think that my suggestion of using copr: user/package format is better.

@fiftydinar
Copy link
Collaborator

@gmpinder We will probably need to add --mount=type=cache,dst=/var/cache/libdnf5 in Containerfile when this gets merged

@gmpinder
Copy link
Member

@gmpinder We will probably need to add --mount=type=cache,dst=/var/cache/libdnf5 in Containerfile when this gets merged

Sounds good, that'll be an easy addition.

@xynydev
Copy link
Member Author

xynydev commented Nov 24, 2024

I think a flaw in this method is support for repo files which are directly provided in /files/rpm-ostree/

Oh right, I forgot that. If we assume copr repo names don't include the .repo suffix, but repo files all do, we use that as another way to detect it.

Adding another key would work too I guess, but then we'd have to debate the whole config structure related to that again lol.

@fiftydinar
Copy link
Collaborator

fiftydinar commented Nov 25, 2024

We should hold a bit releasing dnf module until all issues are resolved with it. rpm-ostree should be still well supported for F42 at minimum.

ublue-os/bluefin#1954 (comment)

Draft PR can be made though, to get us more ready.

@fiftydinar fiftydinar linked a pull request Dec 22, 2024 that will close this issue
@gmpinder
Copy link
Member

So I think that my suggestion of using copr: user/package format is better.

I'm still of the opinion that we should have a separate top-level copr property that takes a list. Right now it's coded to be in the repos section and requires the user to define it like COPR repo/namespace. This, imho, could get confusing for users and be error prone if extra whitespace is added. I think we should encode as much semantics into the spec as we can to allow for easier readability and validation.

@tulilirockz
Copy link
Contributor

tulilirockz commented Dec 24, 2024

I am also with @gmpinder in this one. It's both kind of confusing and having some typing-strict way to check if the repo is a COPR would be much better. Like, I think the best way would be to do something like this

# (...)
repos:
    - name: "myuser/chongus"
      copr: true # or something dunno
    - "other-regular-repo.repo"
    - anotherone.repo
# (...)

@fiftydinar
Copy link
Collaborator

I'm still of the opinion that we should have a separate top-level copr property that takes a list. Right now it's coded to be in the repos section and requires the user to define it like COPR repo/namespace. This, imho, could get confusing for users and be error prone if extra whitespace is added. I think we should encode as much semantics into the spec as we can to allow for easier readability and validation.

I thought about adding manipulation flag options to repos, like we have for install, remove & replace.

I think that having distinct copr: would be better in that case. We would use dnf copr to provide enable & disable repo options.

For repos:, those would be enable, disable, changing priority & maybe something else too.

I am also with @gmpinder in this one. It's both kind of confusing and having some typing-strict way to check if the repo > is a COPR would be much better. Like, I think the best way would be to do something like this

# (...)
repos:
    - name: "myuser/chongus"
      copr: true # or something dunno
    - "other-regular-repo.repo"
    - anotherone.repo
# (...)

I think that distinct copr: is better, as I proposed having repo manipulation as outlined above.

@gmpinder
Copy link
Member

I thought about adding manipulation flag options to repos, like we have for install, remove & replace.

I think that having distinct copr: would be better in that case. We would use dnf copr to provide enable & disable repo options.

For repos:, those would be enable, disable, changing priority & maybe something else too.

I'd be down with this. Especially if there are flows where you'd want to enable/disable repos.

@fiftydinar
Copy link
Collaborator

fiftydinar commented Dec 25, 2024

I thought about adding manipulation flag options to repos, like we have for install, remove & replace.
I think that having distinct copr: would be better in that case. We would use dnf copr to provide enable & disable repo options.
For repos:, those would be enable, disable, changing priority & maybe something else too.

I'd be down with this. Especially if there are flows where you'd want to enable/disable repos.

I added separate copr: array now.

I'll add some manipulation options to copr: & repos: later. Recipe format will need to change a bit for this for those arrays, so any suggestion for that would be great & which options should be added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: medium Needs to be done soon state: approved Approved to proceed. type: feature Brand new functionality, features, pages, workflows, endpoints, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants