-
Notifications
You must be signed in to change notification settings - Fork 4
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: Add akmods module #89
Conversation
Credits: @c0deplayer This is not ideal as it does not support custom kernels & it involves editing Containerfile. I believe there is no other way but to make users edit Containerfile for those files to be even pulled of. I would like this to be through the recipe only, so I will put this as a draft until some better ideas come.
Hmm, it fails here with these errors: Error
|
The akmods repo example installs the akmods with the commands below. Maybe the wildcard is required?
|
I was just looking at if there'd be a way to install the akmods directly using the module without Containerfile changes, and there is a COPR but it doesn't have everything and I'm not sure if it'd just work. With a compiler-based approach this would be easier. I guess I should work on that sometime... |
@xynydev I have great news. Module actually works! Here's proof it works: I think this is good to merge, I don't see what more of a changes is needed. If you catch something that needs update, please let me know. Edit: I remembered. We need to solve ${IMAGE_MAJOR_VERSION} tag in Containerfile for pulling akmods files instead of fixed Fedora version. However, adding IMAGE_MAJOR_VERSION as an argument only works when building an image offline. Github Action fails. So I think it's good as it is (not yet, see below). It's a shame that akmods repo doesn't have |
I added support for Surface & Asus images as I noticed they don't install kernel-devel-matched package. It auto-detects if the base image is right before installing kmods. However, it seems that BASE_IMAGE variable is not recognized in cloud for some reason, hence builds failing. |
This finally fixes akmods module
Installing kernel-devel-matched actually works for all images & it's the more proper way of installing akmods. This time, it's really ready to merge. |
This would avoid editing README if some other compatible image gets announced or discontinued.
#!/usr/bin/env bash | ||
set -oue pipefail | ||
|
||
BASED_IMAGE=$(echo "${BASE_IMAGE}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the point of this? The BASE_IMAGE
and BASED_IMAGE
variable are identical, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, there is no need for this.
I can correct this in another PR.
Credits: @c0deplayer
This is not ideal as it does not support custom kernels & it involves editing Containerfile.
Custom kernels is something that upstream should solve I believe, so it's not a concern we should direct to.
I believe there is no other way but to make users edit Containerfile for those files to be even pulled of.
I would like this to be through the recipe only, so I will put this as a draft until some better ideas come.
2nd part of this work:
blue-build/legacy-template#212