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

split "extra" kmods into distinct build flow #176

Closed
bsherman opened this issue Apr 16, 2024 · 11 comments · Fixed by #163
Closed

split "extra" kmods into distinct build flow #176

bsherman opened this issue Apr 16, 2024 · 11 comments · Fixed by #163
Assignees
Labels

Comments

@bsherman
Copy link
Contributor

The more kmods we add to Containerfile.common the longer builds take, and, the more likely we break downstream builds for Aurora/Bluefin and others.

Specifically, there are a number of kmods which have been added which are only used in Bazzite. While these are good for us to build, Bazzite builds less frequently and is more actively than managed than some other downstream images.

I propose to add another "stream" of kmod builds to this repo so we'll have:

  • Containerfile.nvidia - as is, just builds nvidia kmod
  • Containerfile.common - will build common/shared kmods (eg, those which were originally in main:kmods images, and/or are used in both Aurora/Bluefin and Bazzite)
  • Containerfile.extra - new addition will be primarily for kmods used in Bazzite or any others we need to build but don't fit in common

The new extra category would NOT blog merge queues for akmods which would allow our hwe and bluefin builds to stay more reliable, even if we have somewhat experimental kmods in extra.

@mulderje
Copy link
Contributor

mulderje commented Apr 17, 2024

Thank you for pulling together the above summary and for chatting on discord (thread link for anyone interested), @bsherman!

I have pulled together a draft PR at #163 and would appreciate your feedback. Please see below for a summary of the changes along with some potential todos:

The PR also includes my additions for the facetimehd-kmod, making it the first entry into "extra." I welcome any feedback the PR, approach, etc. Please also let me know if you prefer me to work on the other todo items within the same PR or keep separate? I didn't want to add too much or go too far down this path before getting feedback.

You can get a preview of the build results in a separate PR on my fork here: mulderje#5

Is the above fork/build in the forks PR the best way for me to handle the build cycle within GH or is there a way for me to kick the PR here?

edit: clarify question on build best practices at the end

@mulderje
Copy link
Contributor

mulderje commented Apr 17, 2024

I took a moment to do an audit of the kmod packages that are being included in the Bazzite and Bluefin images. The below table pulls together a list of any package explicitly included via an rpm-ostree install in either. If the package was in either core image, I put the kmod in "common" and otherwise "extra." Note: this approach would not take into account any dependencies, so please do call out any known packages we should bring over to common.

Please take a look and let me know what you think. If you like the below break out I can either pull into the initial PR (#163) or hold off for the (potential) broader refactor I mentioned above.

Package summary

Package Bazzite Bluefin Main (<39) Stream Notes
ayaneo-platform x common
ayn-platform x common
bmi160 x common
bmi260 x common
bmi323 x common
evdi x x common asus only?
facetimehd (WIP) extra
gasket extra
gcadapter_oc x common
nct6687d x common
nvidia x nvidia only in nvidia build
openrazer x x x common
openrgb extra fsync only?
rtl8814au extra
rtl88xxau extra
ryzen-smu x common
steamdeck extra fsync only? don't see steamdeck or steamdeck-kmod anywhere, but do others with steamdeck*
v4l2loopback x x common fedora version gates
VirtualBox extra
winesync extra fsync only?
wl x x x common
xone x x x common
xpadneo x x common
zenergy x common

COPY lines from Containerfiles

Looking at Bazzite and Bluefin, both copy in all kmods from the RPM directory regardless of installed or not by default. For anything in "extra", we could simply ask that they pull in an extra COPY line for the akmods-extra package. Thoughts on that approach as a starting point?

Bazzite core

COPY --from=ghcr.io/ublue-os/akmods:${AKMODS_FLAVOR}-${FEDORA_MAJOR_VERSION} /rpms /tmp/akmods-rpms

Bazzite nvidia

COPY --from=ghcr.io/ublue-os/akmods-nvidia:${AKMODS_FLAVOR}-${FEDORA_MAJOR_VERSION}-${NVIDIA_MAJOR_VERSION} /rpms /tmp/akmods-rpms

Bluefin core

COPY --from=ghcr.io/ublue-os/akmods:${AKMODS_FLAVOR}-${FEDORA_MAJOR_VERSION} /rpms /tmp/akmods-rpms

build-base.sh calls into install-akmods.sh

Main

COPY --from=ghcr.io/ublue-os/akmods:main-${FEDORA_MAJOR_VERSION} /rpms/ublue-os /tmp/rpms

kmods-install.sh

@bsherman
Copy link
Contributor Author

I have pulled together a draft PR at #163 and would appreciate your feedback. Please see below for a summary of the changes along with some potential todos:

This is looking great! I'm very thankful for you jumping in to help with this! It appears we are communicating well because this looks very much like I would have imagined.

The PR also includes my additions for the facetimehd-kmod, making it the first entry into "extra." I welcome any feedback the PR, approach, etc. Please also let me know if you prefer me to work on the other todo items within the same PR or keep separate? I didn't want to add too much or go too far down this path before getting feedback.

I would prefer to keep adding new features separate from a restructuring of the build, it's just cleaner.

Is the above fork/build in the forks PR the best way for me to handle the build cycle within GH or is there a way for me to kick the PR here?

The approach you are taking with a PR in your fork is good. Thank you again.

Related to a question in discord about PRs blocking your work on this:

  1. I don't think feat: swap to negativo17 as nvidia driver source, again #173 should block this as we've got some testing going on to find out why exactly that set of changes broke Wayland for GNOME/GDM
  2. I DO want feat(ci): fix linux version metadata and make builds more reliable #171 to block this but I need to clean it up since I think the feat: swap to negativo17 as nvidia driver source, again #173 changes got merged there.

As I said in Discord, I'm just a volunteer trying to find time around work and family, so mid-week efforts can be more challenging for me... that said... I don't want to be a blocker and #171 is the next thing on my TODO list.

I'll assign you this issue and add it to our project tracker if you are cool with that.

@bsherman
Copy link
Contributor Author

I took a moment to do an audit of the kmod packages that are being included in the Bazzite and Bluefin images. The below table pulls together a list of any package explicitly included via an rpm-ostree install in either. If the package was in either core image, I put the kmod in "common" and otherwise "extra." Note: this approach would not take into account any dependencies, so please do call out any known packages we should bring over to common.

Love this audit! I'll give feedback on the specifics of which module goes where another time, as I know I would organize slightly differently, but that's easy to change.

COPY lines from Containerfiles

Looking at Bazzite and Bluefin, both copy in all kmods from the RPM directory regardless of installed or not by default. For anything in "extra", we could simply ask that they pull in an extra COPY line for the akmods-extra package. Thoughts on that approach as a starting point?

Yes, this is pretty much what would happen fore Bazzite, or any other consumer of akmods-extra.

If we organize the way I expect, Bluefin won't actually be consuming akmods-extra. Same for our "main" images, they only consume a few of the akmods-common kmods, and even that is only for F38 which goes away soon.

@mulderje
Copy link
Contributor

I would prefer to keep adding new features separate from a restructuring of the build, it's just cleaner.

Sounds good and agreed. I'll get started on drafting a PR for some of the refactoring (e.g. moving build files, building a post script, etc.) that should have no functionality changes. Will post a link here once I'm able to pull together.

As I said in Discord, I'm just a volunteer trying to find time around work and family, so mid-week efforts can be more challenging for me... that said... I don't want to be a blocker and #171 is the next thing on my TODO list.

I'll assign you this issue and add it to our project tracker if you are cool with that.

Yep - sounds good! Just subscribed to #171 to keep track there; makes sense to hold off to me too. Let me know if there is anything I can do to lend a hand.

Love this audit! I'll give feedback on the specifics of which module goes where another time, as I know I would organize slightly differently, but that's easy to change.

Sounds good. Thank you.

@mulderje
Copy link
Contributor

Two quick updates here to lay the foundation for other refactors (also updated the list a few comments up). There is no intended functionality change for either of these PRs as of now, and both are passing builds in my fork.

Please note that depending on the merge order will need to rebase one of these to change the post script directory.

@mulderje
Copy link
Contributor

Love this audit! I'll give feedback on the specifics of which module goes where another time, as I know I would organize slightly differently, but that's easy to change.

FYI just did some updates to the table, marking a few more as common. I also added a few notes. Next up, I'll draft up a PR that puts this into a packages.json-esque format, and we can iterate from there.

@mulderje
Copy link
Contributor

Happy Fedora 40 release day! I have gone through and updated the below package summary with some recent changes, and also rebased #163 to the latest main branch. Please give a shout with any questions/feedback.

Package summary

Package Bazzite Bluefin Main (<39) Stream Notes
ayaneo-platform x common
ayn-platform x common
evdi x x common asus only
facetimehd (WIP) extra #163
gasket extra
kvmfr x common
nct6687d x common
nvidia x nvidia only in nvidia build
openrazer x x x common
openrgb common fsync only. don't actually see explicitly included, but mentioned in a lot of places
rtl8814au extra #185
rtl88xxau extra #185
ryzen-smu x common
v4l2loopback x x common
vhba x common
VirtualBox extra
wl x x x common
xone x x x common
xpadneo x x common
zenergy x common

@bsherman
Copy link
Contributor Author

Happy Fedora 40 release day! I have gone through and updated the below package summary with some recent changes, and also rebased #163 to the latest main branch. Please give a shout with any questions/feedback.

Thanks!

Here's the definition I've been imagined for the streams:

  • nvidia - self-describing, the kmod for nvidia
  • common - any kmod installed by default in bluefin or which was originally in main pre-39
  • extra - any other kmod built here (mostly these have been for bazzite so far, but may include new things)

With that definition in mind, there are several currently marked as "common" in your chart's "stream" column which I think should be marked as extra.

Also, gasket has been removed completely.

@mulderje
Copy link
Contributor

Updated in #163 to reflect the below. I have also pushed ublue-os/bazzite#1009 to ensure we are adding in the additional COPY line there. Please let me know if there is anything else that comes to mind before we can get these pushed.

Package Bazzite Bluefin Main (<39) Stream Notes
ayaneo-platform x extra
ayn-platform x extra
evdi x x common asus only
facetimehd extra #163
framework-laptop extra
kvmfr x extra
nct6687d x extra
nvidia x nvidia only in nvidia build
openrazer x x x common
openrgb extra fsync only
rtl8814au extra #185
rtl88xxau extra #185
ryzen-smu x extra
v4l2loopback x x common
vhba x extra
VirtualBox extra
wl x x x common
xone x x x common
xpadneo x x common
zenergy x extra

@mulderje
Copy link
Contributor

Thanks for the help getting this pushed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants