dell / dkms

Dynamic Kernel Module Support
GNU General Public License v2.0
627 stars 144 forks source link

Stop handling dkms.conf as a bash/shell script #414

Open evelikov opened 2 months ago

evelikov commented 2 months ago

Today dkms sources the dkms.conf, effectively treating it as a shell script instead of data/configuration file.

That in itself isn't great since:

Combining the above, we end up with a substantial surface for misuse/abuse and co.

One rather benign example is https://github.com/dell/dkms/issues/413 where dos<>unix line endings result in fatal breakage.

@xuzhen has already done a ton of great work here - see https://github.com/dell/dkms/pull/265.

IMHO to unwrap/finish this completely we should:

@xuzhen @scaronni @anbe42 what do you think of this idea? If you know of any projects/packages where this will be a problem, can you share some details - name, distro/upstream URL, etc.

Thanks o/

scaronni commented 2 months ago

Sorry for the late reply, got busy at work.

It's fine for me but I think it's a bit risky. I believe ZFS will not be the only case where copious amounts of bash scripts end up in the DKMS configuration file. For those cases, we would need to constantly extend to meet their demand where it makes sense, or they will keep on rolling out their own DKMS along with the rest, like AMD was doing.

evelikov commented 2 months ago

No need to apologise - IRL get in the way.

I think it's beneficial to first get some rough numbers and if the number are favourable work from there. I've done a list for Arch below - @scaronni if you can do one (as time permits) for distros you care about that'll be appreciated.

Let me start with Arch proper, only. The AUR (user repos) have 400 of varying quality.

From the above - there are two maybes (nvidia & nvidia-open) and the rest are easily fixable. Will chip through as time permits.

Aside: some packages seem somewhat borked (acpi-call, bbswitch, etc) aka PACKAGE_VERSION="#MODULE_VERSION#". Not sure if an Arch issue or upstream - will check as well.

anbe42 commented 2 months ago

Aside: some packages seem somewhat borked (acpi-call, bbswitch, etc) aka PACKAGE_VERSION="#MODULE_VERSION#". Not sure if an Arch issue or upstream - will check as well.

That sounds like a Debian thing ;-) That placeholder is substituted by dh_dkms at package build time while generating/installing dkms.conf into the package.

anbe42 commented 2 months ago
  • define transitionary period

my gut feeling says that must be rather two than one major release of all major distributions, the second phase with more annoying deprecation warnings

I expect that there are many third-party out-of-tree modules that are not part of any Linux distribution and that need (minor) adjustments for this proposed change ...

evelikov commented 1 month ago

my gut feeling says that must be rather two than one major release of all major distributions, the second phase with more annoying deprecation warnings

Not sure I follow... Are you saying that dkms transition should be defined by Linux distribution releases? If so, I don't fully agree - each one has their own schedule which is outside of our control.

Although before we even remotely get there, let's try and address the known problematic cases. IRL got in the way, let me see if I can open a PR or two.

scaronni commented 1 week ago

Some out of tree kernel modules also add conditions based on which kernel you are building against, for example: https://github.com/intel/ipu6-drivers/blob/master/dkms.conf

This is very handy as you can have the same source tree usable for multiple kernel versions.

This is going to change further now for IPU6 drivers as in kernel 6.10 some more modules have been merged, and some even with differences (so the out of tree kernel module still takes precedence). I don't think it's a good idea to consider the dkms.conf configuration file as a static configuration at this point.