build_module: do not build if result will be obsolete#122
Merged
GoPerry merged 1 commit intodkms-project:masterfrom May 19, 2020
Merged
build_module: do not build if result will be obsolete#122GoPerry merged 1 commit intodkms-project:masterfrom
GoPerry merged 1 commit intodkms-project:masterfrom
Conversation
![monocodus[bot]](https://avatars.githubusercontent.com/in/28526?s=60&v=4){kind=small}
The OBSOLETE_BY mechanism provides a nice way of skipping modules that aren't meant for particular kernels, such as the case where drivers have gone upstream, like WireGuard. It has the desirable property that it prints a reasonable and informative message to the user. This is in contrast to BUILD_EXCLUSIVE_KERNEL, which will return an "Error!" to the user if the condition isn't met. However, one drawback of OBSOLETE_BY is that it is only invoked after the build phase. But most of the time, if a particular kernel obsoletes a module, the old out-of-tree dkms module probably won't build any more. So it makes sense to move the nice text and check into the build phase as well, which is what this commit does. We ran into this issue on Debian, where we attempted to fix the current issue with [1] but are now running into bug reports like [2]. We initially wanted to use OBSOLETE_BY but couldn't due to the issue that this commmit fixes. [1] https://salsa.debian.org/debian/wireguard-linux-compat/-/blob/debian/master/debian/patches/0002-Avoid-trying-to-compile-on-debian-5.5-kernels-Closes.patch [2] https://bugs.debian.org/956893 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
|
I haven't tested this change, but i can confirm that the behavior of DKMS leaves something to be desired at the moment -- we either get an error at build time (because |
Contributor
|
LGTM |
Contributor
|
@zx2c4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The OBSOLETE_BY mechanism provides a nice way of skipping modules that
aren't meant for particular kernels, such as the case where drivers have
gone upstream, like WireGuard. It has the desirable property that it
prints a reasonable and informative message to the user. This is in
contrast to BUILD_EXCLUSIVE_KERNEL, which will return an "Error!" to the
user if the condition isn't met. However, one drawback of OBSOLETE_BY is
that it is only invoked after the build phase. But most of the time, if
a particular kernel obsoletes a module, the old out-of-tree dkms module
probably won't build any more. So it makes sense to move the nice text
and check into the build phase as well, which is what this commit does.
We ran into this issue on Debian, where we attempted to fix the current
issue with [1] but are now running into bug reports like [2]. We
initially wanted to use OBSOLETE_BY but couldn't due to the issue that
this commmit fixes.
[1] https://salsa.debian.org/debian/wireguard-linux-compat/-/blob/debian/master/debian/patches/0002-Avoid-trying-to-compile-on-debian-5.5-kernels-Closes.patch
[2] https://bugs.debian.org/956893
CC @dkg @Unit193