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: Add akmods module #212

Merged
merged 5 commits into from
Jan 14, 2024
Merged

Conversation

fiftydinar
Copy link
Contributor

@fiftydinar fiftydinar commented Dec 25, 2023

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/modules#89

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.
@xynydev
Copy link
Member

xynydev commented Dec 27, 2023

How big are the akmods, like, would it hurt to just always COPY them in the Containerfile (as is done with bling rpms and files). This would make just the module be used to configure akmods installation.

Also, the tag part could be main-${IMAGE_MAJOR_VERSION}, but if someone wanted to build main, asus & surface images they'd need to edit build.yml...

@fiftydinar
Copy link
Contributor Author

How big are the akmods, like, would it hurt to just always COPY them in the Containerfile (as is done with bling rpms and files). This would make just the module be used to configure akmods installation.

It's 2MB when I pulled it with podman. Not big imo

Screenshot from 2023-12-27 10-39-59

@xynydev
Copy link
Member

xynydev commented Dec 27, 2023

Then I'd probably pull main by default and just document it well that you need to change that if you're building asus or surface.

@fiftydinar
Copy link
Contributor Author

Then I'd probably pull main by default and just document it well that you need to change that if you're building asus or surface.

That's only needed if users want to use older version of Fedora as a base (GTS as the most used option of those).

Users can use -nvidia, -asus, -surface etc. builds as a base if they want those akmods.

I did the change to address this:
https://github.com/ublue-os/bling/blob/d76cca4f3abc7e0730c45e3d820d36b236194c89/modules/akmods/README.md

@fiftydinar fiftydinar changed the title feat: Add kmods installer module feat: Add akmods module Dec 27, 2023
@fiftydinar
Copy link
Contributor Author

How big are the akmods, like, would it hurt to just always COPY them in the Containerfile (as is done with bling rpms and files). This would make just the module be used to configure akmods installation.

Also, the tag part could be main-${IMAGE_MAJOR_VERSION}, but if someone wanted to build main, asus & surface images they'd need to edit build.yml...

I forgot to mention, when I set ${IMAGE_MAJOR_VERSION}, it fails

The guy who originally made akmods module used this workaround:

https://github.com/C0dePlayer/silverflow/blob/live/Containerfile

@fiftydinar fiftydinar marked this pull request as ready for review January 10, 2024 11:34
@fiftydinar fiftydinar requested a review from castrojo as a code owner January 10, 2024 11:34
@fiftydinar
Copy link
Contributor Author

fiftydinar commented Jan 10, 2024

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 (2nd part of the PR still needs some fixes though).

It's a shame that akmods repo doesn't have latest tag available though. So you need to edit this manually whenever new Fedora gets a release. I filled an issue on this.

Part of adding support for Surface & Asus images.
@fiftydinar
Copy link
Contributor Author

If this "latest" tag is not considered in akmods repo, I believe this is ready to merge, as 2nd part in bling is finally ready.

xynydev added a commit to blue-build/modules that referenced this pull request Jan 14, 2024
* feat: Add kmods installer module

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.

* Rename kmods installer to akmods

* Update README

Address: 

blue-build/legacy-template#212 (comment)

#89 (comment)

* Fix README typo

* Remove non-needed space for yml in README

* Add support for Surface & Asus images

* Clarify tagged base image warning better

* Clarify tagged base image warning better pt.2

* There is no need to fetch main-nvidia build for now

* Use simpler =~ for conditioning instead of grep & sed

This finally fixes akmods module

* Install kernel-devel-matched for all builds

* Assure that Surface installs their version of kernel-devel-matched

* Mention that framework images can be used as a base

* Delete duplicate warning message

* Remove non-needed explanation on why only Universal Blue builds are supported

* Clarify 1st warning better

* Clarify `main` akmods compatibility better

This would avoid editing README if some other compatible image gets announced or discontinued.

* docs(akmods): grammar fixes

* docs: add link to akmod tag matrix

---------

Co-authored-by: xyny <[email protected]>
@xynydev
Copy link
Member

xynydev commented Jan 14, 2024

To merge, please update the branch @fiftydinar

@xynydev xynydev enabled auto-merge (squash) January 14, 2024 16:46
@fiftydinar
Copy link
Contributor Author

To merge, please update the branch @fiftydinar

Updated

@xynydev xynydev merged commit df4f330 into blue-build:template Jan 14, 2024
2 checks passed
@fiftydinar fiftydinar deleted the kmods-installer branch January 14, 2024 17:25
xynydev referenced this pull request in xynydev/linuXYZ Jan 14, 2024
feat: Add akmods module (#212)
elgabo86 referenced this pull request in elgabo86/gablue Jan 14, 2024
feat: Add akmods module (#212)
Craftidore referenced this pull request in Craftidore/shale Jan 27, 2024
* feat: Add kmods installer module

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.

* Update Containerfile

Related to this:

https://github.com/ublue-os/bling/blob/d76cca4f3abc7e0730c45e3d820d36b236194c89/modules/akmods/README.md

* Clarify change of akmod version better

Part of adding support for Surface & Asus images.
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.

2 participants