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

add BUILD_EXCLUSIVE_CONFIG #269

Merged
merged 16 commits into from
Apr 20, 2023
Merged

Conversation

anbe42
Copy link
Contributor

@anbe42 anbe42 commented Oct 26, 2022

support build exclusion depending on kernel features being present/absent, e.g. BUILD_EXCLUSIVE_CONFIG="CONFIG_FOO !CONFIG_BAR"

dkms.8.in Show resolved Hide resolved
dkms.8.in Show resolved Hide resolved
dkms.in Show resolved Hide resolved
@evelikov
Copy link
Collaborator

With the few things it's be perfect. Please double-check with other Debian/Ubuntu patches and send them all upstream.

Thanks in advance o/

dkms.in Outdated Show resolved Hide resolved
dkms.in Outdated Show resolved Hide resolved
dkms.in Outdated
@@ -1670,7 +1670,7 @@ do_status_weak()
# interested in, but that the DKMS internals do not usually care about.
module_status_built_extra() (
set_module_suffix "$3"
read_conf "$3" "$4" "$dkms_tree/$1/$2/source/dkms.conf"
read_conf "$3" "$4" "$dkms_tree/$1/$2/source/dkms.conf" 2>/dev/null
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we masking only this one and not the other dozen+ instances of read_conf/read_conf_or_die?

dkms.in Outdated Show resolved Hide resolved
@evelikov
Copy link
Collaborator

@anbe42 the Ubuntu tests are failing - it seems like the "update-secureboot-policy" change is causing some additional changes/output. Can you fix those up?

With that we should be good to land.

@xuzhen
Copy link
Collaborator

xuzhen commented Oct 27, 2022

If we enroll the created key on Ubuntu, why we not do the same on other distributions?
And there is no need to enroll the key on computers that do not have SecureBoot enabled.

@anbe42 anbe42 force-pushed the BUILD_EXCLUSIVE_CONFIG branch 2 times, most recently from bd51788 to a205e55 Compare March 23, 2023 13:59
@anbe42
Copy link
Contributor Author

anbe42 commented Apr 3, 2023

rebased onto additional fixes arising from discussion on the recently merged PRs, now all green ;-)

@anbe42
Copy link
Contributor Author

anbe42 commented Apr 3, 2023

with that merged, all my open bugs and PRs should be addressed and most Debian issues should be resolved and upstreamed ;-)

run_test.sh Show resolved Hide resolved
run_test.sh Outdated Show resolved Hide resolved
dkms.in Show resolved Hide resolved
dkms.in Outdated Show resolved Hide resolved
there is no point in special casing a mismatching return code of 0
the bash idiom <(foo) relies on the /dev/fd -> /proc/self/fd symlink

if /proc is not mounted, dkms status (and other actions) may fail with

   /usr/sbin/dkms: line ####: /dev/fd/##: No such file or directory

https://bugs.debian.org/810665
skip building the module if kernelver does not satisfy
BUILD_EXCLUSIVE_KERNEL_MIN <= kernelver <= BUILD_EXCLUSIVE_KERNEL_MAX

Closes dell#300
support build exclusion depending on kernel features being present/absent,
e.g. BUILD_EXCLUSIVE_CONFIG="CONFIG_FOO !CONFIG_BAR"

Closes dell#219
this is a purely cosmetic change to get a "natural" processing order in
case multiple kernels versions are installed, e.g. 4.1 < 4.2 < 4.10 < 5.0
instead of alphanumeric order, e.g. 4.1 < 4.10 < 4.2 < 5.0
@evelikov
Copy link
Collaborator

evelikov commented Apr 6, 2023

Fixing shellcheck warnings/issues gets a huge 💯 👍 from me. Although let's keep those a separate follow-up PR.

It's very easy to follow the suggestions and break thing in unexpected ways - it has happened (both to me and friends) a number of times.

With the shellcheck commit dropped, this is ready to merge.

@anbe42
Copy link
Contributor Author

anbe42 commented Apr 7, 2023

I've split the fixes for obvious errors from the shellcheck commit into separate verbose commits and left the quoting style changes for later.

dkms.in Show resolved Hide resolved
SC2070 (error): -n doesn't work with unquoted arguments. Quote or use [[ ]].
SC2157 (error): Argument to implicit -n is always true due to literal strings.
SC2071 (error): > is for string comparisons. Use -gt instead.
this is used in multiple
  case "$running_distribution" in ...
statements that are independent of $addon_modules_dir
@@ -2361,7 +2362,6 @@ rm -f "$tmpfile"
# These can come from the environment or the config file
[[ ! ${ADDON_MODULES_DIR} && -e /etc/sysconfig/module-init-tools ]] && . /etc/sysconfig/module-init-tools
addon_modules_dir="${ADDON_MODULES_DIR}"
[[ ! ${addon_modules_dir} ]] && running_distribution="$(distro_version)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks right, but has a high risk of causing regressions. Let's merge it here and it can be reverted independently if needed.

@evelikov evelikov merged commit 8606438 into dell:master Apr 20, 2023
@anbe42 anbe42 deleted the BUILD_EXCLUSIVE_CONFIG branch April 24, 2023 10:50
@anbe42
Copy link
Contributor Author

anbe42 commented Apr 24, 2023

With all my patches merged, what do are your plans for making a new release?

@evelikov
Copy link
Collaborator

No objections on my end although I have no idea what to do 😅

@scaronni can you share some pointers - be that here or throw them in a RELEASE.md file within git? Thanks o/

@scaronni
Copy link
Collaborator

Well, there is no rule, I got the repository as is 😄

I'll make a release now, but in general just bump the release in the Makefile and make a release whenever you think it's a good moment. I'm open for improvements!

kevinoid added a commit to kevinoid/xpadneo that referenced this pull request Jan 29, 2024
Since version 3.0.11 <dell/dkms#269>, dkms
supports the [`BUILD_EXCLUSIVE_CONFIG`] option in dkms.conf to specify
kernel configuration options which must be enabled in the targeted
kernel.  Add this option with `CONFIG_HID`, `CONFIG_INPUT_FF_MEMLESS`,
and `CONFIG_POWER_SUPPLY`, since xpadneo will fail to modpost if any of
these are disabled.  If a user installs a kernel where these options are
disabled, the module will not be built and a warning similar to the
following is printed:

    Warning: The /var/lib/dkms/hid-xpadneo/v0.9-144-g9b3b696-dirty/6.8.0-rc2/x86_64/dkms.conf
    for module hid-xpadneo includes a BUILD_EXCLUSIVE directive
    which does not match this kernel/arch/config.
    This indicates that it should not be built.
    dkms autoinstall on 6.8.0-rc2/x86_64 was skipped for hid-xpadneo

Without this option, the module build would fail and the kernel package
would fail to install, leaving the user with a partially installed
kernel package (on Debian, at least).

[`BUILD_EXCLUSIVE_CONFIG`]: https://manpages.debian.org/unstable/dkms/dkms.8.en.html#BUILD_EXCLUSIVE_CONFIG=

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid added a commit to kevinoid/xpadneo that referenced this pull request Jan 29, 2024
Since version 3.0.11 <dell/dkms#269>, dkms
supports the [`BUILD_EXCLUSIVE_CONFIG`] option in dkms.conf to specify
kernel configuration options which must be enabled in the targeted
kernel.  Add this option with `CONFIG_HID`, `CONFIG_INPUT_FF_MEMLESS`,
and `CONFIG_POWER_SUPPLY`, since xpadneo will fail to modpost if any of
these are disabled.  If a user installs a kernel where these options are
disabled, the module will not be built and a warning similar to the
following is printed:

    Warning: The /var/lib/dkms/hid-xpadneo/v0.9-144-g9b3b696-dirty/6.8.0-rc2/x86_64/dkms.conf
    for module hid-xpadneo includes a BUILD_EXCLUSIVE directive
    which does not match this kernel/arch/config.
    This indicates that it should not be built.
    dkms autoinstall on 6.8.0-rc2/x86_64 was skipped for hid-xpadneo

Without this option, the module build would fail and the kernel package
would fail to install, leaving the user with a partially installed
kernel package (on Debian, at least).

[`BUILD_EXCLUSIVE_CONFIG`]: https://github.com/dell/dkms/blob/782bd07641b957012f9b8d621bf9b51f82bfc728/dkms.8.in#L688-L696

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kakra pushed a commit to atar-axis/xpadneo that referenced this pull request Jan 29, 2024
Since version 3.0.11 <dell/dkms#269>, dkms
supports the [`BUILD_EXCLUSIVE_CONFIG`] option in dkms.conf to specify
kernel configuration options which must be enabled in the targeted
kernel.  Add this option with `CONFIG_HID`, `CONFIG_INPUT_FF_MEMLESS`,
and `CONFIG_POWER_SUPPLY`, since xpadneo will fail to modpost if any of
these are disabled.  If a user installs a kernel where these options are
disabled, the module will not be built and a warning similar to the
following is printed:

    Warning: The /var/lib/dkms/hid-xpadneo/v0.9-144-g9b3b696-dirty/6.8.0-rc2/x86_64/dkms.conf
    for module hid-xpadneo includes a BUILD_EXCLUSIVE directive
    which does not match this kernel/arch/config.
    This indicates that it should not be built.
    dkms autoinstall on 6.8.0-rc2/x86_64 was skipped for hid-xpadneo

Without this option, the module build would fail and the kernel package
would fail to install, leaving the user with a partially installed
kernel package (on Debian, at least).

[`BUILD_EXCLUSIVE_CONFIG`]: https://github.com/dell/dkms/blob/782bd07641b957012f9b8d621bf9b51f82bfc728/dkms.8.in#L688-L696

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kakra pushed a commit to kakra/xpadneo that referenced this pull request Feb 12, 2024
Since version 3.0.11 <dell/dkms#269>, dkms
supports the [`BUILD_EXCLUSIVE_CONFIG`] option in dkms.conf to specify
kernel configuration options which must be enabled in the targeted
kernel.  Add this option with `CONFIG_HID`, `CONFIG_INPUT_FF_MEMLESS`,
and `CONFIG_POWER_SUPPLY`, since xpadneo will fail to modpost if any of
these are disabled.  If a user installs a kernel where these options are
disabled, the module will not be built and a warning similar to the
following is printed:

    Warning: The /var/lib/dkms/hid-xpadneo/v0.9-144-g9b3b696-dirty/6.8.0-rc2/x86_64/dkms.conf
    for module hid-xpadneo includes a BUILD_EXCLUSIVE directive
    which does not match this kernel/arch/config.
    This indicates that it should not be built.
    dkms autoinstall on 6.8.0-rc2/x86_64 was skipped for hid-xpadneo

Without this option, the module build would fail and the kernel package
would fail to install, leaving the user with a partially installed
kernel package (on Debian, at least).

[`BUILD_EXCLUSIVE_CONFIG`]: https://github.com/dell/dkms/blob/782bd07641b957012f9b8d621bf9b51f82bfc728/dkms.8.in#L688-L696

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kakra pushed a commit to atar-axis/xpadneo that referenced this pull request Feb 12, 2024
Since version 3.0.11 <dell/dkms#269>, dkms
supports the [`BUILD_EXCLUSIVE_CONFIG`] option in dkms.conf to specify
kernel configuration options which must be enabled in the targeted
kernel.  Add this option with `CONFIG_HID`, `CONFIG_INPUT_FF_MEMLESS`,
and `CONFIG_POWER_SUPPLY`, since xpadneo will fail to modpost if any of
these are disabled.  If a user installs a kernel where these options are
disabled, the module will not be built and a warning similar to the
following is printed:

    Warning: The /var/lib/dkms/hid-xpadneo/v0.9-144-g9b3b696-dirty/6.8.0-rc2/x86_64/dkms.conf
    for module hid-xpadneo includes a BUILD_EXCLUSIVE directive
    which does not match this kernel/arch/config.
    This indicates that it should not be built.
    dkms autoinstall on 6.8.0-rc2/x86_64 was skipped for hid-xpadneo

Without this option, the module build would fail and the kernel package
would fail to install, leaving the user with a partially installed
kernel package (on Debian, at least).

[`BUILD_EXCLUSIVE_CONFIG`]: https://github.com/dell/dkms/blob/782bd07641b957012f9b8d621bf9b51f82bfc728/dkms.8.in#L688-L696

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
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.

4 participants