[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Noé Rubinstein]

Test the config of the kernel to see if loadable module support is
enabled, and error out otherwise. This makes the build failure less
confusing.

Signed-off-by: No? Rubinstein <nrubinstein@aldebaran.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-kernel-module.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
index 5fb19be..6fb7704 100644
--- a/package/pkg-kernel-module.mk
+++ b/package/pkg-kernel-module.mk
@@ -60,6 +60,10 @@ $(2)_MODULE_SUBDIRS ?= .
 # includes and other support files (Booo!)
 define $(2)_KERNEL_MODULES_BUILD
 	@$$(call MESSAGE,"Building kernel module(s)")
+	@if ! grep -Fqx 'CONFIG_MODULES=y' $(LINUX_DIR)/.config; then \
+		echo "ERROR: Kernel does not support loadable modules"; \
+		exit 1; \
+	fi
 	$$(foreach d,$$($(2)_MODULE_SUBDIRS), \
 		$$(LINUX_MAKE_ENV) $$($$(PKG)_MAKE) \
 			-C $$(LINUX_DIR) \
-- 
2.1.4

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Thomas Petazzoni]

Dear No? Rubinstein,

On Mon, 17 Aug 2015 09:10:44 +0200, No? Rubinstein wrote:
> Test the config of the kernel to see if loadable module support is
> enabled, and error out otherwise. This makes the build failure less
> confusing.
> 
> Signed-off-by: No? Rubinstein <nrubinstein@aldebaran.com>
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  package/pkg-kernel-module.mk | 4 ++++
>  1 file changed, 4 insertions(+)

Applied after tweaking the commit log, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Dear No? Rubinstein,
 > On Mon, 17 Aug 2015 09:10:44 +0200, No? Rubinstein wrote:
 >> Test the config of the kernel to see if loadable module support is
 >> enabled, and error out otherwise. This makes the build failure less
 >> confusing.
 >> 
 >> Signed-off-by: No? Rubinstein <nrubinstein@aldebaran.com>
 >> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 >> ---
 >> package/pkg-kernel-module.mk | 4 ++++
 >> 1 file changed, 4 insertions(+)

 > Applied after tweaking the commit log, thanks.

Couldn't we instead just force modules support on in the kernel if the
pkg-kernel-module infrastructure is used?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Thomas Petazzoni]

Hello,

On Tue, 18 Aug 2015 07:41:11 +0200, Peter Korsgaard wrote:
> >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Dear No? Rubinstein,
>  > On Mon, 17 Aug 2015 09:10:44 +0200, No? Rubinstein wrote:
>  >> Test the config of the kernel to see if loadable module support is
>  >> enabled, and error out otherwise. This makes the build failure less
>  >> confusing.
>  >> 
>  >> Signed-off-by: No? Rubinstein <nrubinstein@aldebaran.com>
>  >> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  >> ---
>  >> package/pkg-kernel-module.mk | 4 ++++
>  >> 1 file changed, 4 insertions(+)
> 
>  > Applied after tweaking the commit log, thanks.
> 
> Couldn't we instead just force modules support on in the kernel if the
> pkg-kernel-module infrastructure is used?

That's another option, yes. No??

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Yann E. MORIN]

Peter, All,

On 2015-08-18 07:41 +0200, Peter Korsgaard spake thusly:
> >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Dear No? Rubinstein,
>  > On Mon, 17 Aug 2015 09:10:44 +0200, No? Rubinstein wrote:
>  >> Test the config of the kernel to see if loadable module support is
>  >> enabled, and error out otherwise. This makes the build failure less
>  >> confusing.
>  >> 
>  >> Signed-off-by: No? Rubinstein <nrubinstein@aldebaran.com>
>  >> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  >> ---
>  >> package/pkg-kernel-module.mk | 4 ++++
>  >> 1 file changed, 4 insertions(+)
> 
>  > Applied after tweaking the commit log, thanks.
> 
> Couldn't we instead just force modules support on in the kernel if the
> pkg-kernel-module infrastructure is used?

And how do you suggest we test whether the kernel-module infra is used?

    1) it *is* used already: we do have packages that do call
        $(eval $(kernel-module)) , but I guess that's not what
        you meant. ;-)

    2) so you probably meant: if we actually need to build a kernel
       module.

But that's not so easy:

  - we need to know whether to enable moduls when parsing linux/linux.mk

  - external packages may call kernel-module

  - external packages are parsed after linux.mk

So, too late...

We can't enable kernel modules just from the .mk, we can only check for
them.

Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that
packages may select if they want to build modules. Of a visible knob
(user-selectable) that packages can depend on if they need to build
kernel modules...

Needless to say, for 2015.08, No?'s patch is good to go in, while the
alternatives are not; they can however be done post-2015.08.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

>> Couldn't we instead just force modules support on in the kernel if the
 >> pkg-kernel-module infrastructure is used?

 > And how do you suggest we test whether the kernel-module infra is used?

 >     1) it *is* used already: we do have packages that do call
 >         $(eval $(kernel-module)) , but I guess that's not what
 >         you meant. ;-)

 >     2) so you probably meant: if we actually need to build a kernel
 >        module.

 > But that's not so easy:

 >   - we need to know whether to enable moduls when parsing linux/linux.mk

 >   - external packages may call kernel-module

 >   - external packages are parsed after linux.mk

 > So, too late...

If we can come up with a solution that only works for internal packages
then that is imho still better than what we have today. We can still
leave the check in the pkg-kernel-module infrastructure to atleast warn
BR2_EXTERNAL users.

 > We can't enable kernel modules just from the .mk, we can only check for
 > them.

 > Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that
 > packages may select if they want to build modules. Of a visible knob
 > (user-selectable) that packages can depend on if they need to build
 > kernel modules...

Or the pkg-kernel-module infrastructure can just set a variable that
gets checked by LINUX_KCONFIG_FIXUP_CMDS, E.G. something like:

diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
index 6fb7704..436a721 100644
--- a/package/pkg-kernel-module.mk
+++ b/package/pkg-kernel-module.mk
@@ -91,6 +91,10 @@ define $(2)_KERNEL_MODULES_INSTALL
 endef
 $(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
 
+ifeq ($(BR2_PACKAGE_$(2)),y)
+LINUX_NEEDS_MODULES = YES
+endif
+
 endef
 
 ################################################################################


And then:

diff --git a/linux/linux.mk b/linux/linux.mk
index d91dbb2..eb7ce7e 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -219,6 +219,8 @@ define LINUX_KCONFIG_FIXUP_CMDS
                $(call KCONFIG_ENABLE_OPT,CONFIG_NF_CONNTRACK_MARK,$(@D)/.config))
        $(if $(BR2_LINUX_KERNEL_APPENDED_DTB),
                $(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config))
+       $(if $(LINUX_NEEDS_MODULES),
+               $(call KCONFIG_ENABLE_OPT,CONFIG_MODULES,$(@D)/.config))
 endef
 
 ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)

 > Needless to say, for 2015.08, No?'s patch is good to go in, while the
 > alternatives are not; they can however be done post-2015.08.

Agreed, this is not a new regression.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH v2] pkg-kernel-module: Die when kernel module are disabled

[Yann E. MORIN]

Peter, All,

On 2015-08-18 12:41 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
[--SNIP--]
>  > Unless we add an hidden kconfig knob BR2)LINUX_NEEDS_MODULES that
>  > packages may select if they want to build modules. Of a visible knob
>  > (user-selectable) that packages can depend on if they need to build
>  > kernel modules...
> 
> Or the pkg-kernel-module infrastructure can just set a variable that
> gets checked by LINUX_KCONFIG_FIXUP_CMDS, E.G. something like:

OK, I don;t know why I missed that mail of yours (except it is not in
the same thread we were "battling" in).

> diff --git a/package/pkg-kernel-module.mk b/package/pkg-kernel-module.mk
> index 6fb7704..436a721 100644
> --- a/package/pkg-kernel-module.mk
> +++ b/package/pkg-kernel-module.mk
> @@ -91,6 +91,10 @@ define $(2)_KERNEL_MODULES_INSTALL
>  endef
>  $(2)_POST_INSTALL_TARGET_HOOKS += $(2)_KERNEL_MODULES_INSTALL
>  
> +ifeq ($(BR2_PACKAGE_$(2)),y)
> +LINUX_NEEDS_MODULES = YES
> +endif
> +
>  endef
>  
>  ################################################################################
> 
> 
> And then:
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index d91dbb2..eb7ce7e 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -219,6 +219,8 @@ define LINUX_KCONFIG_FIXUP_CMDS
>                 $(call KCONFIG_ENABLE_OPT,CONFIG_NF_CONNTRACK_MARK,$(@D)/.config))
>         $(if $(BR2_LINUX_KERNEL_APPENDED_DTB),
>                 $(call KCONFIG_ENABLE_OPT,CONFIG_ARM_APPENDED_DTB,$(@D)/.config))
> +       $(if $(LINUX_NEEDS_MODULES),
> +               $(call KCONFIG_ENABLE_OPT,CONFIG_MODULES,$(@D)/.config))
>  endef
>  
>  ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT),y)

Yes, that should work.

Of course, you are right, we'd only use that variable after all .mk are
parsed, being in the kconfig-fixup commands.

I'll see to implement and test that.

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'