adding another line
adding another line..
you can see it skips to installing akmods and mock, so i don't believe it's working
115 Replies
@EyeCantCU threaded
uh... yeah i think the problem is the
rpm-ostree > dnf symlink
if i remove that, at least remove it before the kernel swap i get totally different errors
Nooooooooooooo
Wait what
this is good 🙂
The kernel package there is literally kernel 😦
progress
Progress indeed
ok, i'm copying this from
asus repo's Containerfile to see how it compares
same excpt for the addition of "kernel-uki-virt" in akmodsInteresting. I had to remove uki-virt in ASUS
the copr isn't loaded 🤦♂️
😦
So it never even grabs the kernel
i'll have a PR in a moment
curios why removed teh uki-virt?
It depends on a version of systemd not in 38. Could probably add a version check and throw it in 39
k
yeah... this kernel replacement works better when the asus-linux copr is provided 😉
Hahaha
I'd imagine
so, i've got the asus kmods building
now i'm working on surface
Oh lord, I never even checked for the copr.
I saw the section for them and merely assumed it was already there
Good catch, thank you
what i don't know is why the error didn't fail the build, it just moved on
I'm very surprised it didn't blow up with an error. You'd think it would
Yeah, I can't even layer shit half the time without something going wrong. But I could go ahead and install a kernel that doesn't exist? OK
GitHub
fix: add missing asus copr by bsherman · Pull Request #73 · ublue-o...
This should fix the ASUS build but Surface is still not quite right.
note that surface is NOT working right yet
i was getting this:
but i need to feed my kid so you guys can fix, or i can look later
I'll look :). Thank you for fixing ASUS!
Dude
Why did. That just get merged?
uhh does it not check build status??
that's fine, I'll fix it
LOL
You approved but I said it was broke LOL
I could have drafted I guess
yeah but despite that it should fail to build and never merge, no?
Yeah except we need to figure out why it's not failig
lmao it was only queued
we're good
cancelled
Con you convert to draft plz? I'm on phone waiting in line for food for kid
done
this doesn't make a lot of sense to me:
that looks like it's trying to replace surface-kernel.rpm
It's a package with a crazy high kernel version number
To trick it into allowing the package to be replaced
Straight from the Surface Linux peeps
We could try to replace it similar to the ASUS kernel but I remember that being troublesome for one reason or another
I remember why it's weird like that. rpm-ostree override replace kernel --remove kernel-* --install kernel-surface-* never worked as it should
Ok, looks like it installs now
I think we need kenrel-devel for each here
what does
rpm-ostree cliwrap install-to-root / do? i wonder if that's swallowing our errorThat installs the kernel directly to the root of the image
It's needed otherwise the kernel doesn't install at all
Also, I noticed 39 for Surface was enabled. Disabled that since we can't build it since the repo isn't populated yet
makes sense
It's there but nothing's in it
We also need to restore the symlink when we're done since a kmod needs it
well, that's funny
dnf is getting installed
see https://github.com/ublue-os/akmods/actions/runs/6306385816/job/17121442466#step:7:270 so i don't think we need the symlink 🙂
I wonder what this is about though: https://github.com/ublue-os/akmods/actions/runs/6306489021/job/17121712947#step:7:1262
🤯
Have to brainstorm the surface kernel install because it's building for the fake kernel we tell it to replace ;_;
i'm fine with restoring the dnf symlink
i had just removed for testing
Weird stuff
OH!
I now know how the Simpsons Hit and Run devs felt
it's needed on 37 maybe?
Possibly
Since that seems to be a trend
We could filter it out to just 37 and kill it in two months
Or more like a month
yeah, please re-add symlink for only 37 if you are in there editing now
i need to run and grab some groceries
Okay. Will do
Sounds good
i'm doing dad duty tonight
All righty
Reading an issue over in surface kernel. If they had named it just kernel like the Asus kernel we could outright replace it but they didn't name the package that as they didn't want to conflict with the base kernel... so this is why we use this convoluted installation method
interesting
ok, so this https://github.com/ublue-os/akmods/actions/runs/6306756774/job/17122383081#step:7:272 is why i removed the dnf symlink... i think we need to add it after setting up the custom kernels, but before installking the akmods/mock packages
Just saw and pushed 🙂
At least we have main and Asus now lol
That... that is so confusing
https://github.com/ublue-os/akmods/actions/runs/6306801253/job/17122491572#step:7:769
Do we need some sort of voodoo uninstall dnf, symlink, reinstall junk?
OH
Before akmods and mock
i DID have some uninstall voodoo, but i think that's gone now that we build nvidia versions in distinct jobs
hah
symlink can't be at the end
Moved it up a little
if it's at the end it fails to create because dnf is already installed
Before the akmods package gets installed
It's a dep
yep, current location looks good
Awesome
do you ever run these locally?
Going to do that for Surface because I am sick of watching builds fail
I think I know what I need to do and I hate it
Change all the kernel version queries via rpm to look for kernel-surface instead in surface builds
But I'll get to it 🙂
Yeah... That was it
i have to see the diff 🙂
🙂 https://github.com/ublue-os/akmods/pull/73/commits/3e668d3c8f774fb66762302fad079a2963d3711c
Ack.... Now Asus is failing lmao
Oh bad gateway
Should be okay
Got us down from like 29 jobs to 19
Wait whaaa
that's a bit ugly
I'm not sure how else we'd fix it
i'm wondering if we could set the "KERNEL_NAME" based on KERNEL_FLAVOR maybe?
I think we could
Although... I'm not sure of an effecient way of handling that. Could be added as a workflow env variable and passed as a build arg depending on what's in the matrix
Something like:
then
Then we just throw ${KERNEL_NAME} in all the build-kmods in the string for the RPM query
I'm 50/50 on this actually working but let's try it
hmm
yeah, i'm concerned about doing another variable in workflow, i was thinking if we could set it in the Containerfile somehow, since we already have KERNEL_FLAVOR and KERNEL_NAME is based on it
I'm not sure of a way to do that. We can't change the value of ARGS unless they are set during podman build/the workflow. I've tried with RUN and then changing based on that but to no avail
I can see why that's concerning though
Because in any typical env if someone doesn't set both, Surface just won't compile
We could have like KERNEL_NAME=$1 in all the scripts and pass it as a script arg based on KENREL_FLAVOR in the Containerfile
So two things
I missed doing this in the Nvidia script
Annnd Surface 37 doesn't like where we symlink dnf
Enter should be backspace
Or hmm... caps lock should be backspace
I'm off to bed but here's a good laugh: https://github.com/ublue-os/akmods/pull/73/commits/5ce5c5ee1d9dbb32c755bbeb6afed0f8f5145973
At this point I'm asking myself... how much do we really want to support Surface on 37 for 1 month
naw
imo
Disabled. And I guess we can change kernel name when it is an arg but it all has to be in one run statement so no checking for args in all the scripts and passing it needed
If we don't handle it that way, then it doesn't persist
And it still has to be an explicit argument otherwise it doesn't get passed
Does that make sense? I don't feel like I'm saying what I mean very well
i think i'm following you
looking at current state of PR for a refresh
the current state isn't really bad... the only thing we lose is ability to cache a build layer per kmod build
but that isn't a problem in CI only matters when doing local testing, and frankly, when I test locally i comment out all but one or two of the kmod builds to make things to faster, then uncomment and do a final test before pushign
This is what I've got now. Simplifies things a little bit: https://github.com/ublue-os/akmods/pull/73/commits/b32bfeace20574aee398b4875b20429e8af870b0
That's true. Everything would be in one layer
no
Okay
i mean, yes, but
at the end we copy only the bits we want into a scratch base anyway
so the build layers are temporary in this case
Ah so that's how that works
That's pretty cool
yeah, that's why the akmod images aren't stupid big
I was wondering about that
With all the layers, I thought they'd be huge
But if it's just pulling what it needs that's nifty
now you know... when you want to start a container image from nothing...
FROM scratch is the key 😄Super helpful. Thank you
welcome
Looks like everything is good now. We should probably rename the PR haha
fix: Everything
lol
renamed
Awesome. Should be good to go unless there are any touchups you think we need
i'm re-reading PR again now
i think we don't need
ARG KERNEL_NAME="kernel" in containerfiles anymore?We'd have to restore KERNEL_NAME=$1 in all the scripts and pass it like it was, but we can if you want
i don't think
let me test
also, were you wanting to exclude F37 from surface/asus builds?
oh you did, i misread that
coolio
Yeah. ASUS doesn't have a 37 kernel and I didn't see much reason to build it for Surface for one month
yep
agree
Especially since 38 is vastly improved over 37
and EOL
effecitvely
Yup
i pushed small change to demonstrate, but...
sinc you added KERNEL_NAME as a variable within the RUN directive ... that is the variable/value which is used for all the kmod builds, NOT the top-level containerfile build arg named KERNEL_NAME
Ah, nice. Thank you. I'll keep that in mind in the future
I wasn't sure if it got passed to the build scripts or not
wHAT
i just ran this locally, why is it behaving different in CI?
So I tried this stuff this morning on my laptop and ran into this as well. It's why I assumed the arg needed to be there
I just wasn't sure if I made some mistake
hmmm
this is funny 🙂
Indeed lol
letting it run longer locally to test
The ridiculous thing is - to change it I instantiate a new variable when it get set in RUN. You can't actually set it directly or the build fails saying it doesn't exist
ah, i think it was still installing kernels nd i thought it was past that
Oh, that makes sense
yet, you CAN check the values set by args
Backwards stuff
ok, i see
the
ARG KERNEL_NAME was setting a global variable for the whole Containerfile context... then, in the RUN directive, setting a local variable... and i'm not sure if it was modifying the gobal one or not... i think probably not actually, which means surface-modules were probably still getting built with the stock kernel
but... the solution is easyIt had to be or surface kernel kmods wouldn't have been built
I verified they are surface modules
@bsherman
well, i just pushed the "easy fix" 🙂
i think its more clear what is happening now, even if it did work before
we don't want someone to think they can override that KERNEL_NAME as a build-arg
I like this more :). Nice catch
random: but this annoys me
yes!
our job titles are too long...
This is awesome
we're going to have this custom kernel thing licked!
So many custom kernels, we'll need a case statement instead of an if statement to install them
Honestly though... I kinda don't like bash case statements
Simply because
;; at the end of each case looks horrible
I think we're good to merge @bshermanmake it so!
will do 🙂
Beat me to it lol