All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] xtables-addons does not build for arm kernel 4.1.4
@ 2015-08-17 16:50 Jan Viktorin
  2015-08-17 21:52 ` Peter Korsgaard
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Viktorin @ 2015-08-17 16:50 UTC (permalink / raw)
  To: buildroot

Hello Gustavo,

I am trying to compile xtables-addons by Buildroot, however, I get a lot
of "undefined!" warnings and some errors.

I build with:
- Buildroot (master: 15a53b93a034af78)
- internal toolchain
- Linux 4.1.4.

The Buildroot is patched by http://lists.busybox.net/pipermail/buildroot/2015-August/137485.html
for the olinuxino lime2 support. However, the same issues come with olimex_a20_olinuxino_lime_defconfig.

My setup is:

$ make olimex_a20_olinuxino_lime2_defconfig
$ make menuconfig
  enable Target packages/Network Applications/xtables-addons
$ make

And I end up with:

WARNING: "param_ops_uint" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "single_release" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "seq_read" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "seq_lseek" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "__aeabi_unwind_cpp_pr1" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "xt_unregister_matches" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "unregister_pernet_subsys" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "xt_register_matches" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "register_pernet_subsys" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "printk" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "proc_set_user" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "proc_create_data" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
WARNING: "strncpy" [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_quota2.ko] undefined!
(...)
make[6]: *** [buildroot-a20/output/build/xtables-addons-2.7/extensions/xt_DELUDE.mod.o] Error 1
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:8:1: error: variable ?__this_module? has initializer but incomplete type
 __attribute__((section(".gnu.linkonce.this_module"))) = {
 ^
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:9:2: error: unknown field ?name? specified in initializer
  .name = KBUILD_MODNAME,
  ^
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:9:2: warning: excess elements in struct initializer
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:9:2: warning: (near initialization for ?__this_module?)
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:10:2: error: unknown field ?arch? specified in initializer
  .arch = MODULE_ARCH_INIT,
  ^
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:10:10: error: ?MODULE_ARCH_INIT? undeclared here (not in a function)
  .arch = MODULE_ARCH_INIT,
          ^
buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:10:2: warning: excess elements in struct initializer
  .arch = MODULE_ARCH_INIT,
(...)

I can see the commit eaa872edca27c6f375d
that should fix some kind of issues with Linux 4.1+. Is there something I am missing here?

I can build xtables-addons from git (tag 2.7, e277360) for both Linux 4.1.4 and 4.1.5 on
x86_64 by the standard xtables-addons build-system (without Buildroot).

Regards
Jan Viktorin

-- 
   Jan Viktorin                  E-mail: Viktorin at RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] xtables-addons does not build for arm kernel 4.1.4
  2015-08-17 16:50 [Buildroot] xtables-addons does not build for arm kernel 4.1.4 Jan Viktorin
@ 2015-08-17 21:52 ` Peter Korsgaard
  2015-08-17 22:35   ` Jan Viktorin
  2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
  0 siblings, 2 replies; 18+ messages in thread
From: Peter Korsgaard @ 2015-08-17 21:52 UTC (permalink / raw)
  To: buildroot

>>>>> "Jan" == Jan Viktorin <viktorin@rehivetech.com> writes:

 > Hello Gustavo,
 > I am trying to compile xtables-addons by Buildroot, however, I get a lot
 > of "undefined!" warnings and some errors.

 > I build with:
 > - Buildroot (master: 15a53b93a034af78)
 > - internal toolchain
 > - Linux 4.1.4.

 > The Buildroot is patched by http://lists.busybox.net/pipermail/buildroot/2015-August/137485.html
 > for the olinuxino lime2 support. However, the same issues come with olimex_a20_olinuxino_lime_defconfig.

 > My setup is:

 > $ make olimex_a20_olinuxino_lime2_defconfig
 > $ make menuconfig
 >   enable Target packages/Network Applications/xtables-addons
 > $ make

 > And I end up with:

> buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:9:2: error: unknown field ?name? specified in initializer
 >   .name = KBUILD_MODNAME,

The problem is that you don't have modules (CONFIG_MODULES=y) support
enabled in your kernel, as the sunxi_defconfig in the kernel until very
recently didn't enable that - But it is naturally needed to build the
xtables-addons modules.

Arguably Buildroot should automatically enable this for you when
enabling the xtables-addons package, similar to how it is done for the
netfilter options even though it is quite a "big" option to enable
behind your back. Alternatively we could simply error out with a
sensible message like it was recently done for the pkg-kernel-module
infrastructure.


 > I can build xtables-addons from git (tag 2.7, e277360) for both Linux 4.1.4 and 4.1.5 on
 > x86_64 by the standard xtables-addons build-system (without Buildroot).

You most likely have modules support in your x86-64 kernel.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] xtables-addons does not build for arm kernel 4.1.4
  2015-08-17 21:52 ` Peter Korsgaard
@ 2015-08-17 22:35   ` Jan Viktorin
  2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
  1 sibling, 0 replies; 18+ messages in thread
From: Jan Viktorin @ 2015-08-17 22:35 UTC (permalink / raw)
  To: buildroot

On Mon, 17 Aug 2015 23:52:18 +0200
Peter Korsgaard <peter@korsgaard.com> wrote:

> >>>>> "Jan" == Jan Viktorin <viktorin@rehivetech.com> writes:
> 
>  > Hello Gustavo,
>  > I am trying to compile xtables-addons by Buildroot, however, I get
>  > a lot of "undefined!" warnings and some errors.
> 
>  > I build with:
>  > - Buildroot (master: 15a53b93a034af78)
>  > - internal toolchain
>  > - Linux 4.1.4.
> 
>  > The Buildroot is patched by
>  > http://lists.busybox.net/pipermail/buildroot/2015-August/137485.html
>  > for the olinuxino lime2 support. However, the same issues come
>  > with olimex_a20_olinuxino_lime_defconfig.
> 
>  > My setup is:
> 
>  > $ make olimex_a20_olinuxino_lime2_defconfig
>  > $ make menuconfig
>  >   enable Target packages/Network Applications/xtables-addons
>  > $ make
> 
>  > And I end up with:
> 
> > buildroot-a20/output/build/xtables-addons-2.7/extensions/compat_xtables.mod.c:9:2:
> > error: unknown field ?name? specified in initializer
>  >   .name = KBUILD_MODNAME,
> 
> The problem is that you don't have modules (CONFIG_MODULES=y) support
> enabled in your kernel, as the sunxi_defconfig in the kernel until
> very recently didn't enable that - But it is naturally needed to
> build the xtables-addons modules.

Thank you for this hint! I'll check it out.

> 
> Arguably Buildroot should automatically enable this for you when
> enabling the xtables-addons package, similar to how it is done for the
> netfilter options even though it is quite a "big" option to enable
> behind your back. Alternatively we could simply error out with a
> sensible message like it was recently done for the pkg-kernel-module
> infrastructure.

I agree, I would be happy to get a reasonable error message instead
of loosing few hours of looking up for such a trivial mistake.

> 
> 
>  > I can build xtables-addons from git (tag 2.7, e277360) for both
>  > Linux 4.1.4 and 4.1.5 on x86_64 by the standard xtables-addons
>  > build-system (without Buildroot).
> 
> You most likely have modules support in your x86-64 kernel.
> 

-- 
  Jan Viktorin                E-mail: Viktorin at RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech                  Phone: +420 606 201 868
  Brno, Czech Republic

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-17 21:52 ` Peter Korsgaard
  2015-08-17 22:35   ` Jan Viktorin
@ 2015-08-19 13:14   ` Jan Viktorin
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro Jan Viktorin
                       ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Jan Viktorin @ 2015-08-19 13:14 UTC (permalink / raw)
  To: buildroot

Hello all,

this short patch series introduces a way how to check whether certain
CONFIG_* entries are set to an expected value in a selected .config
file. When building a package that requires a certain CONFIG_* to be
set (as in the case of xtables-addons), Buildroot can either force
to set the config or just error out a message.

I could not find any suitable mechanism for this in Buildroot (if there
is one, please give me a reference) so I propose a simple extension of
KCONFIG_*_OPT util macros. It is then implemented for xtables-addons to
check whether CONFIG_MODULES are set in the Linux Kernel .config.

Regards
Jan Viktorin


Jan Viktorin (2):
  pkg-utils: Define KCONFIG_ASSERT_OPT macro
  xtables-addons: test CONFIG_MODULES=y in the kernel

 package/pkg-utils.mk                     | 5 +++++
 package/xtables-addons/xtables-addons.mk | 5 +++++
 2 files changed, 10 insertions(+)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro
  2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
@ 2015-08-19 13:14     ` Jan Viktorin
  2015-08-19 19:16       ` Yann E. MORIN
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 2/2] xtables-addons: test CONFIG_MODULES=y in the kernel Jan Viktorin
  2015-08-19 17:46     ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Thomas Petazzoni
  2 siblings, 1 reply; 18+ messages in thread
From: Jan Viktorin @ 2015-08-19 13:14 UTC (permalink / raw)
  To: buildroot

The macro can be used (eg. in packages) to assure that
a certain option in the Linux/BusyBox/...'s .config is
set to the given value.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 package/pkg-utils.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 44bd2c9..1e34864 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -63,6 +63,11 @@ define KCONFIG_DISABLE_OPT # (option, file)
 	echo '# $(1) is not set' >> $(2)
 endef
 
+define KCONFIG_ASSERT_OPT # (option, value, file)
+	grep "\\<$(1)\\>=$(2)" $(3) > /dev/null \
+		|| (echo "KCONFIG_ASSERT_OPT($(1),$(2),$(3)) has failed" >&2; exit 1)
+endef
+
 # Helper functions to determine the name of a package and its
 # directory from its makefile directory, using the $(MAKEFILE_LIST)
 # variable provided by make. This is used by the *-package macros to
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 2/2] xtables-addons: test CONFIG_MODULES=y in the kernel
  2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro Jan Viktorin
@ 2015-08-19 13:14     ` Jan Viktorin
  2015-08-19 17:46     ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Thomas Petazzoni
  2 siblings, 0 replies; 18+ messages in thread
From: Jan Viktorin @ 2015-08-19 13:14 UTC (permalink / raw)
  To: buildroot

The package builds several loadable modules for the Linux
kernel. There are in-kernel defconfigs that does not enable
the CONFIG_MODULES by default. Thus, a build of xtables-addons
should fail for such incompatible kernel configuration.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 package/xtables-addons/xtables-addons.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/xtables-addons/xtables-addons.mk b/package/xtables-addons/xtables-addons.mk
index 75e2a5a..9450ebb 100644
--- a/package/xtables-addons/xtables-addons.mk
+++ b/package/xtables-addons/xtables-addons.mk
@@ -22,6 +22,11 @@ define XTABLES_DISABLE_GEOIP_HELPERS
 endef
 XTABLES_ADDONS_POST_PATCH_HOOKS += XTABLES_DISABLE_GEOIP_HELPERS
 
+define XTABLES_ASSERT_CONFIG_MODULES
+	$(call KCONFIG_ASSERT_OPT,CONFIG_MODULES,y,$(LINUX_DIR)/.config)
+endef
+XTABLES_ADDONS_PRE_CONFIGURE_HOOKS += XTABLES_ASSERT_CONFIG_MODULES
+
 define XTABLES_ADDONS_BUILD_CMDS
 	$(MAKE) -C $(@D) $(LINUX_MAKE_FLAGS)
 endef
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro Jan Viktorin
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 2/2] xtables-addons: test CONFIG_MODULES=y in the kernel Jan Viktorin
@ 2015-08-19 17:46     ` Thomas Petazzoni
  2015-08-19 19:05       ` Yann E. MORIN
  2 siblings, 1 reply; 18+ messages in thread
From: Thomas Petazzoni @ 2015-08-19 17:46 UTC (permalink / raw)
  To: buildroot

Dear Jan Viktorin,

On Wed, 19 Aug 2015 15:14:34 +0200, Jan Viktorin wrote:

> this short patch series introduces a way how to check whether certain
> CONFIG_* entries are set to an expected value in a selected .config
> file. When building a package that requires a certain CONFIG_* to be
> set (as in the case of xtables-addons), Buildroot can either force
> to set the config or just error out a message.
> 
> I could not find any suitable mechanism for this in Buildroot (if there
> is one, please give me a reference) so I propose a simple extension of
> KCONFIG_*_OPT util macros. It is then implemented for xtables-addons to
> check whether CONFIG_MODULES are set in the Linux Kernel .config.

There is some discussion already going on on this topic, around the
pkg-kernel-module infrastructure. We recently committed a patch that
makes the infrastructure verify that the kernel has been built with
module support, and if not, bail out with a clear error message:

   http://git.buildroot.net/buildroot/commit/?id=8df95d926e963601c727defeb4ab90ce2368da70

However, Peter Korsgaard raised the concern that we should instead just
forcefully enable CONFIG_MODULES=y in the configuration in such a case.
There has then been some discussion with Yann on how to achieve that
properly. Please see the thread at
http://lists.busybox.net/pipermail/buildroot/2015-August/137511.html.

Best regards,

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 17:46     ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Thomas Petazzoni
@ 2015-08-19 19:05       ` Yann E. MORIN
  2015-08-19 19:08         ` Thomas Petazzoni
  2015-08-19 21:05         ` Peter Korsgaard
  0 siblings, 2 replies; 18+ messages in thread
From: Yann E. MORIN @ 2015-08-19 19:05 UTC (permalink / raw)
  To: buildroot

Thomas, Jan, All,

On 2015-08-19 19:46 +0200, Thomas Petazzoni spake thusly:
> On Wed, 19 Aug 2015 15:14:34 +0200, Jan Viktorin wrote:
> 
> > this short patch series introduces a way how to check whether certain
> > CONFIG_* entries are set to an expected value in a selected .config
> > file. When building a package that requires a certain CONFIG_* to be
> > set (as in the case of xtables-addons), Buildroot can either force
> > to set the config or just error out a message.

Arguably, xtables-addons should be (if possible) converted over to using
the kernel-modules infra, which should (will) take care of that (in the
future). I'll try to see if I can convert it.

> > I could not find any suitable mechanism for this in Buildroot (if there
> > is one, please give me a reference) so I propose a simple extension of
> > KCONFIG_*_OPT util macros. It is then implemented for xtables-addons to
> > check whether CONFIG_MODULES are set in the Linux Kernel .config.
> 
> There is some discussion already going on on this topic, around the
> pkg-kernel-module infrastructure. We recently committed a patch that
> makes the infrastructure verify that the kernel has been built with
> module support, and if not, bail out with a clear error message:
> 
>    http://git.buildroot.net/buildroot/commit/?id=8df95d926e963601c727defeb4ab90ce2368da70
> 
> However, Peter Korsgaard raised the concern that we should instead just
> forcefully enable CONFIG_MODULES=y in the configuration in such a case.
> There has then been some discussion with Yann on how to achieve that
> properly. Please see the thread at
> http://lists.busybox.net/pipermail/buildroot/2015-August/137511.html.

Yes, I'm thinking about it...

I think the plan I'll be goign with is to add a hidden Kconfig knob that
packages that want to build a kernel module will have to select.

Then, if that option is set, we can force-on support for modules in
linux.mk. Note: if the option is not set, we would *not* disable support
for modules, and let it as set in the user-provided (def)config file.

So, all in all, that would solve the xtables-addons issue, because we
would be handling CONFIG_MODULE explicitly.

Now, there are two questions:

  - will we need to _check_ for any arbitrary kernel option to be set,
    and offer packages a simple mean to do so?

  - will we need to _force_ (on or off) any other kernel option, and
    offer packages a simple mean to do so?

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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 19:05       ` Yann E. MORIN
@ 2015-08-19 19:08         ` Thomas Petazzoni
  2015-08-19 19:21           ` Yann E. MORIN
  2015-08-19 21:05         ` Peter Korsgaard
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Petazzoni @ 2015-08-19 19:08 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Wed, 19 Aug 2015 21:05:34 +0200, Yann E. MORIN wrote:

> I think the plan I'll be goign with is to add a hidden Kconfig knob that
> packages that want to build a kernel module will have to select.
> 
> Then, if that option is set, we can force-on support for modules in
> linux.mk. Note: if the option is not set, we would *not* disable support
> for modules, and let it as set in the user-provided (def)config file.
> 
> So, all in all, that would solve the xtables-addons issue, because we
> would be handling CONFIG_MODULE explicitly.

Sounds good to me.

> Now, there are two questions:
> 
>   - will we need to _check_ for any arbitrary kernel option to be set,
>     and offer packages a simple mean to do so?
> 
>   - will we need to _force_ (on or off) any other kernel option, and
>     offer packages a simple mean to do so?

We already do your second item.

In general, I'd like to keep the handling of the kernel configuration
to be as minimal as possible. Otherwise, we'll quickly enter a
situation where we need to enable different kernel options depending on
the kernel version. It can quickly become a real nightmare. Buildroot
has always been about the user knowing what (s)he is doing, including
in terms of making sure that the kernel configuration matches what is
done in userspace.

Thanks,

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro
  2015-08-19 13:14     ` [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro Jan Viktorin
@ 2015-08-19 19:16       ` Yann E. MORIN
  0 siblings, 0 replies; 18+ messages in thread
From: Yann E. MORIN @ 2015-08-19 19:16 UTC (permalink / raw)
  To: buildroot

Jan, All,

On 2015-08-19 15:14 +0200, Jan Viktorin spake thusly:
> The macro can be used (eg. in packages) to assure that
> a certain option in the Linux/BusyBox/...'s .config is
> set to the given value.
> 
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
>  package/pkg-utils.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 44bd2c9..1e34864 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -63,6 +63,11 @@ define KCONFIG_DISABLE_OPT # (option, file)
>  	echo '# $(1) is not set' >> $(2)
>  endef
>  
> +define KCONFIG_ASSERT_OPT # (option, value, file)
> +	grep "\\<$(1)\\>=$(2)" $(3) > /dev/null \
> +		|| (echo "KCONFIG_ASSERT_OPT($(1),$(2),$(3)) has failed" >&2; exit 1)

This message is not really explicit. Also, the construct is dubious,
especially if the caller chains many calls separated with semi-colon,
like so:

    $(call KCONFIG_ASSERT_OPT,CONFIG_FOO,y,$(LINUX_DIR)/.config); \
    $(call KCONFIG_ASSERT_OPT,CONFIG_BAR,y,$(LINUX_DIR)/.config); \
    $(call KCONFIG_ASSERT_OPT,CONFIG_BUZ,y,$(LINUX_DIR)/.config)

The first two would not cause failure.

Also, this option can not check for unset options, and does not properly
quote the arguments which are passed to echo.

What about:

    define KCONFIG_ASSERT_OPT # (option, value, file)
        if ! grep -E "^$(1)=$(2)\$$" $(3) >/dev/null; then \
            printf "Error: option %s is not set to %s\n" '$(1)' '$(2)' '$(3}'; \
            exit 1; \
        fi
    endef # KCONFIG_ASSERT_OPT

That way:
  - the message is more explicit (I think),
  - it is possible to chain calls even with semi-colons,
  - arguments are properly quoted so the shell won't parse their
    content.

But no need to repsin for now, let the dust settle down for a moment. ;-)

Regards,
Yann E. MORIN.

> +endef
> +
>  # Helper functions to determine the name of a package and its
>  # directory from its makefile directory, using the $(MAKEFILE_LIST)
>  # variable provided by make. This is used by the *-package macros to
> -- 
> 2.5.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 19:08         ` Thomas Petazzoni
@ 2015-08-19 19:21           ` Yann E. MORIN
  2015-08-19 20:23             ` Thomas Petazzoni
  0 siblings, 1 reply; 18+ messages in thread
From: Yann E. MORIN @ 2015-08-19 19:21 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2015-08-19 21:08 +0200, Thomas Petazzoni spake thusly:
> On Wed, 19 Aug 2015 21:05:34 +0200, Yann E. MORIN wrote:
[--SNIP--]
> > Now, there are two questions:
> > 
> >   - will we need to _check_ for any arbitrary kernel option to be set,
> >     and offer packages a simple mean to do so?
> > 
> >   - will we need to _force_ (on or off) any other kernel option, and
> >     offer packages a simple mean to do so?
> 
> We already do your second item.

No, we are not allowing any random package to set kernel options.

We do have infra for packages to tweak their *own* options, not those
of other packages (and history has proved it was not doable, I should
know, I tried and failed miserably with the lzo stuff).

> In general, I'd like to keep the handling of the kernel configuration
> to be as minimal as possible. Otherwise, we'll quickly enter a
> situation where we need to enable different kernel options depending on
> the kernel version. It can quickly become a real nightmare. Buildroot
> has always been about the user knowing what (s)he is doing, including
> in terms of making sure that the kernel configuration matches what is
> done in userspace.

I do fully agree with you on this position. Tweaking the kernel options
is a real hornet's nest...

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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 19:21           ` Yann E. MORIN
@ 2015-08-19 20:23             ` Thomas Petazzoni
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Petazzoni @ 2015-08-19 20:23 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Wed, 19 Aug 2015 21:21:57 +0200, Yann E. MORIN wrote:

> > We already do your second item.
> 
> No, we are not allowing any random package to set kernel options.

No, we do it the other way around: linux.mk tweaks the kernel options
depending on which packages are selected.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 19:05       ` Yann E. MORIN
  2015-08-19 19:08         ` Thomas Petazzoni
@ 2015-08-19 21:05         ` Peter Korsgaard
  2015-08-21 13:22           ` Yann E. MORIN
  1 sibling, 1 reply; 18+ messages in thread
From: Peter Korsgaard @ 2015-08-19 21:05 UTC (permalink / raw)
  To: buildroot

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

Hi,

 > Arguably, xtables-addons should be (if possible) converted over to using
 > the kernel-modules infra, which should (will) take care of that (in the
 > future). I'll try to see if I can convert it.

Yeah, agreed.

 > Yes, I'm thinking about it...

 > I think the plan I'll be goign with is to add a hidden Kconfig knob that
 > packages that want to build a kernel module will have to select.

Why not just like the kernel-module infrastructure handle it like I
proposed? I don't think it is very nice that people have to remember to
add the select as well (and chances are they won't notice if they
forget).

 >   - will we need to _check_ for any arbitrary kernel option to be set,
 >     and offer packages a simple mean to do so?

 >   - will we need to _force_ (on or off) any other kernel option, and
 >     offer packages a simple mean to do so?

Like Thomas says, we are already doing the 2nd option and I think it
makes sense to keep on doing so - So I don't think there's a common need
for the first.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-19 21:05         ` Peter Korsgaard
@ 2015-08-21 13:22           ` Yann E. MORIN
  2015-08-21 13:41             ` Jeremy Rosen
  2015-08-21 20:28             ` Peter Korsgaard
  0 siblings, 2 replies; 18+ messages in thread
From: Yann E. MORIN @ 2015-08-21 13:22 UTC (permalink / raw)
  To: buildroot

Peter, All,

[I forgot to send that one; it got stuck in my draft folder...]

On 2015-08-19 23:05 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  > Arguably, xtables-addons should be (if possible) converted over to using
>  > the kernel-modules infra, which should (will) take care of that (in the
>  > future). I'll try to see if I can convert it.
> Yeah, agreed.

I'll be working on it (not trivial from a first look).

>  > Yes, I'm thinking about it...
> 
>  > I think the plan I'll be goign with is to add a hidden Kconfig knob that
>  > packages that want to build a kernel module will have to select.
> 
> Why not just like the kernel-module infrastructure handle it like I
> proposed? I don't think it is very nice that people have to remember to
> add the select as well (and chances are they won't notice if they
> forget).

Because that would not work for packages in br2-external trees, as I
already explained.

And I do *not* want that we treat packages from br2-external differently
that the in-tree ones [*].

I know you are not using br2-external (and also do not really see the
point of it), but a lot of people find this to be a really important
feature. I also suspect some of our (corporate) users did choose
Buildroot (partly) because of br2-external.

[*] there already are a few use-cases that br2-external does not and
can't solve, because of the way Buildroot is designed, see below for an
example; let's not add arbitrary limitations that we can very easily
avoid.

>  >   - will we need to _check_ for any arbitrary kernel option to be set,
>  >     and offer packages a simple mean to do so?
> 
>  >   - will we need to _force_ (on or off) any other kernel option, and
>  >     offer packages a simple mean to do so?
> 
> Like Thomas says, we are already doing the 2nd option and I think it
> makes sense to keep on doing so - So I don't think there's a common need
> for the first.

Still, we are currently not allowing another package to set kernel
options _from_ that package's .mk file. All we have is the kernel
(de)activating options based on the presence of other packages, and that
excludes packages from br2-external. Unless Buildroot is modified
accordingly (but then br2-external looses its attractiveness for that
package).

Note: I never said we should provide a way for packages to enable or
disable arbitrary kernel options. I was just asking.

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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-21 13:22           ` Yann E. MORIN
@ 2015-08-21 13:41             ` Jeremy Rosen
  2015-08-21 20:28             ` Peter Korsgaard
  1 sibling, 0 replies; 18+ messages in thread
From: Jeremy Rosen @ 2015-08-21 13:41 UTC (permalink / raw)
  To: buildroot

On a related note (mainly to fuel ideas) Denis posted the following
patch a couple of weeks ago...

http://lists.busybox.net/pipermail/buildroot/2015-August/135321.html

This patch is integrated in pkg-kconfig and "transforms" all kconfig
variables into makefile variables. These variables can then be
used from any package to test for available options...


Just pointing out a different approch to the same problem


Cordialement 

J?r?my Rosen 
+33 (0)1 42 68 28 13

fight key loggers : write some perl using vim 


Open Wide Ingenierie

23, rue Daviel
75013 Paris - France
www.openwide.fr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-21 13:22           ` Yann E. MORIN
  2015-08-21 13:41             ` Jeremy Rosen
@ 2015-08-21 20:28             ` Peter Korsgaard
  2015-08-21 21:05               ` Yann E. MORIN
  1 sibling, 1 reply; 18+ messages in thread
From: Peter Korsgaard @ 2015-08-21 20:28 UTC (permalink / raw)
  To: buildroot

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

Hi,

 >> > I think the plan I'll be goign with is to add a hidden Kconfig knob that
 >> > packages that want to build a kernel module will have to select.
 >> 
 >> Why not just like the kernel-module infrastructure handle it like I
 >> proposed? I don't think it is very nice that people have to remember to
 >> add the select as well (and chances are they won't notice if they
 >> forget).

 > Because that would not work for packages in br2-external trees, as I
 > already explained.

Are you sure? I didn't try it, but as we only *USE* the variable when
LINUX_KCONFIG_FIXUP_CMDS runs, which is after all the .mk files have
been parsed I think it should work?


> I know you are not using br2-external (and also do not really see the
 > point of it), but a lot of people find this to be a really important
 > feature. I also suspect some of our (corporate) users did choose
 > Buildroot (partly) because of br2-external.

It's true that I don't personally use br2-external and don't find it
such a killer feature, I *DO* acknowledge that some people do (which is
why I merged the support for it in the first place) and will try to keep
it working as good as possible. With that said, we have a lot of other
requirements for Buildroot (like keeping it simple), so will not bend
over backwards to improve br2-external support if it means making big
compromises elsewhere.

br2-external stuff will always been more limited than normal packages in
the tree.


 >> Like Thomas says, we are already doing the 2nd option and I think it
 >> makes sense to keep on doing so - So I don't think there's a common need
 >> for the first.

 > Still, we are currently not allowing another package to set kernel
 > options _from_ that package's .mk file. All we have is the kernel
 > (de)activating options based on the presence of other packages, and that
 > excludes packages from br2-external. Unless Buildroot is modified
 > accordingly (but then br2-external looses its attractiveness for that
 > package).

Ok, but if all of these use the kernel-module infrastructure (which
linux.mk knows about), then this should work for CONFIG_MODULES, right?
It naturally cannot work for random other config settings.

-- 
Venlig hilsen,
Peter Korsgaard 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-21 20:28             ` Peter Korsgaard
@ 2015-08-21 21:05               ` Yann E. MORIN
  2015-08-21 22:17                 ` Peter Korsgaard
  0 siblings, 1 reply; 18+ messages in thread
From: Yann E. MORIN @ 2015-08-21 21:05 UTC (permalink / raw)
  To: buildroot

Peter, All,

On 2015-08-21 22:28 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  >> > I think the plan I'll be goign with is to add a hidden Kconfig knob that
>  >> > packages that want to build a kernel module will have to select.
>  >> 
>  >> Why not just like the kernel-module infrastructure handle it like I
>  >> proposed? I don't think it is very nice that people have to remember to
>  >> add the select as well (and chances are they won't notice if they
>  >> forget).
> 
>  > Because that would not work for packages in br2-external trees, as I
>  > already explained.
> 
> Are you sure? I didn't try it, but as we only *USE* the variable when
> LINUX_KCONFIG_FIXUP_CMDS runs, which is after all the .mk files have
> been parsed I think it should work?

Ah, just checking would indeed be doable, something similar (but more
restrictive) to the patch proposed by Jan:
    https://patchwork.ozlabs.org/patch/508729/

I.e. somthing like:

    define $(2)_KERNEL_MODULES_BUILD
        if ! grep -E "^CONFIG_MODULES=y\$$" $$(LINUX_DIR)/.config >/dev/null; then \
            printf "Error: your Linux kernel does not support modules\n"; \
            exit 1; \
        fi
        [--current code--]
    endef

Yes, that would work.

But I was focused on actually enabling the CONFIG_MODULES option when a
package wants to build a kernel module, and _that_ is not possible with
the kernel-module infra.

> > I know you are not using br2-external (and also do not really see the
>  > point of it), but a lot of people find this to be a really important
>  > feature. I also suspect some of our (corporate) users did choose
>  > Buildroot (partly) because of br2-external.
> 
> It's true that I don't personally use br2-external and don't find it
> such a killer feature, I *DO* acknowledge that some people do (which is
> why I merged the support for it in the first place) and will try to keep
> it working as good as possible.

OK, thanks! :-)

> With that said, we have a lot of other
> requirements for Buildroot (like keeping it simple), so will not bend
> over backwards to improve br2-external support if it means making big
> compromises elsewhere.

I do agree with that.

> br2-external stuff will always been more limited than normal packages in
> the tree.

Yes, probably. But what I argue is that we should not introduce
_arbitrary_ restrictions that we can easily avoid. And by "easily", I do
agree that it should not require huge changes in Buildroot, be simple
code, be simple to understand and be simple to use.

>  >> Like Thomas says, we are already doing the 2nd option and I think it
>  >> makes sense to keep on doing so - So I don't think there's a common need
>  >> for the first.
> 
>  > Still, we are currently not allowing another package to set kernel
>  > options _from_ that package's .mk file. All we have is the kernel
>  > (de)activating options based on the presence of other packages, and that
>  > excludes packages from br2-external. Unless Buildroot is modified
>  > accordingly (but then br2-external looses its attractiveness for that
>  > package).
> 
> Ok, but if all of these use the kernel-module infrastructure (which
> linux.mk knows about), then this should work for CONFIG_MODULES, right?

No, linux.mk does not know of the kernel-module infra. The kernel-module
infra is entirely implemented using post-build and post-install hooks of
the package itself, not of the kernel.

Maybe you confused kernel-module with the linux extensions?

So I guess I start to see where the misunderstanding comes from... The
kernel-module infra can indeed _check_ that CONFIG_MODULES is set, but
it can not set it.

(Arguably, we could do it for in-tree packages, because their .mk are
parsed before that of the kernel, so we could set a Makefiel variable to
tell linux.mk to enable CONFIG_MODULES. But since br2-external packages
are parsed after linux.mk, then we could not for those packages).

So, I've decided to prepare two patch series, and we can apply the one
that gets consensus:
  - one that does the check in the kernel-module infra,
  - one that forces CONFIG_MODULES when required.

When both are ready, I'll push them to the list.

I hope we can get either in *before* the release: the kernel-module
infra is brand new in this cycle, and I'd like we get a stable "API"
for it before releasing it to the wild.

> It naturally cannot work for random other config settings.

ACK.

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.  |
'------------------------------^-------^------------------^--------------------'

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set
  2015-08-21 21:05               ` Yann E. MORIN
@ 2015-08-21 22:17                 ` Peter Korsgaard
  0 siblings, 0 replies; 18+ messages in thread
From: Peter Korsgaard @ 2015-08-21 22:17 UTC (permalink / raw)
  To: buildroot

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

Hi,

>> >> Why not just like the kernel-module infrastructure handle it like I
 >> >> proposed? I don't think it is very nice that people have to remember to
 >> >> add the select as well (and chances are they won't notice if they
 >> >> forget).
 >> 
 >> > Because that would not work for packages in br2-external trees, as I
 >> > already explained.
 >> 
 >> Are you sure? I didn't try it, but as we only *USE* the variable when
 >> LINUX_KCONFIG_FIXUP_CMDS runs, which is after all the .mk files have
 >> been parsed I think it should work?

 > Ah, just checking would indeed be doable, something similar (but more
 > restrictive) to the patch proposed by Jan:
 >     https://patchwork.ozlabs.org/patch/508729/

We seemed to be talking past eachother, but got it sorted out on
IRC. For others following, the change I suggested was actually the one
mentioned here:

http://lists.busybox.net/pipermail/buildroot/2015-August/137540.html

-- 
Venlig hilsen,
Peter Korsgaard 

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-08-21 22:17 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-17 16:50 [Buildroot] xtables-addons does not build for arm kernel 4.1.4 Jan Viktorin
2015-08-17 21:52 ` Peter Korsgaard
2015-08-17 22:35   ` Jan Viktorin
2015-08-19 13:14   ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Jan Viktorin
2015-08-19 13:14     ` [Buildroot] [PATCH v1 1/2] pkg-utils: Define KCONFIG_ASSERT_OPT macro Jan Viktorin
2015-08-19 19:16       ` Yann E. MORIN
2015-08-19 13:14     ` [Buildroot] [PATCH v1 2/2] xtables-addons: test CONFIG_MODULES=y in the kernel Jan Viktorin
2015-08-19 17:46     ` [Buildroot] [PATCH v1 0/2] Checking whether a certain CONFIG_* is set Thomas Petazzoni
2015-08-19 19:05       ` Yann E. MORIN
2015-08-19 19:08         ` Thomas Petazzoni
2015-08-19 19:21           ` Yann E. MORIN
2015-08-19 20:23             ` Thomas Petazzoni
2015-08-19 21:05         ` Peter Korsgaard
2015-08-21 13:22           ` Yann E. MORIN
2015-08-21 13:41             ` Jeremy Rosen
2015-08-21 20:28             ` Peter Korsgaard
2015-08-21 21:05               ` Yann E. MORIN
2015-08-21 22:17                 ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.