[Buildroot] [PATCH 0/2] linux: do not always depend on host-lzop

[Buildroot] [PATCH 0/2] linux: do not always depend on host-lzop

[Yann E. MORIN]

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

Hello All!

Here is a small series that makes Linux not always depend on host-lzop.

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (2):
      packages infra: add function to get a Kconfig option
      linux: only depend on host-lzop if needed

 linux/linux.mk       | 6 +++++-
 package/pkg-utils.mk | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 1/2] packages infra: add function to get a Kconfig option

[Yann E. MORIN]

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

We so far have no mean to get the value from a Kconfig option from the
.config file of a package (eg. linux, busybox...).

Add a new function that returns the unmangled value of an option.
It expect two arguments:
  - the Kconfig option name (complete, with leading CONFIG if necessary)
  - the .config file to get it from

Note that, if the Kconfig option is a string, the returned value will
contain the leading and trailing double-quotes.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-utils.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 851575c..2f70acc 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT
 	echo "# $(1) is not set" >> $(2)
 endef
 
+# Note: we do not indent this, since we want to avoid any leading
+# space or tabs when calling this function
+define KCONFIG_GET_OPT
+$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2))
+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 *TARGETS macros to
-- 
1.8.1.2

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Yann E. MORIN]

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

There is no reason to always depend on host-lzop, even when the kernel
compression is not LZO.

Since LZO is not the default compression option in the kernel (and there
is not sign that will change in the foreseeable future), it will always
appear in a condif file, whether it is a complete config file or it is
only a defconfig.

So, only depend on host-lzop if the LZO compression is enabled in the
kernel config file (either the defconfig or the custom config file).

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 linux/linux.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/linux/linux.mk b/linux/linux.mk
index ab25fe9..f34bea1 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -38,7 +38,7 @@ endif
 LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 
 LINUX_INSTALL_IMAGES = YES
-LINUX_DEPENDENCIES  += host-kmod host-lzop
+LINUX_DEPENDENCIES  += host-kmod
 
 ifeq ($(BR2_LINUX_KERNEL_UBOOT_IMAGE),y)
 	LINUX_DEPENDENCIES += host-uboot-tools
@@ -163,6 +163,10 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
 KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
 endif
 
+ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)
+LINUX_DEPENDENCIES += host-lzop
+endif
+
 define LINUX_CONFIGURE_CMDS
 	cp $(KERNEL_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig
 	$(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) buildroot_defconfig
-- 
1.8.1.2

[Buildroot] [PATCH 1/2] packages infra: add function to get a Kconfig option

[Peter Korsgaard]

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

 > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > We so far have no mean to get the value from a Kconfig option from the
 > .config file of a package (eg. linux, busybox...).

 > Add a new function that returns the unmangled value of an option.
 > It expect two arguments:
 >   - the Kconfig option name (complete, with leading CONFIG if necessary)
 >   - the .config file to get it from

 > Note that, if the Kconfig option is a string, the returned value will
 > contain the leading and trailing double-quotes.

 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > ---
 >  package/pkg-utils.mk | 6 ++++++
 >  1 file changed, 6 insertions(+)

 > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
 > index 851575c..2f70acc 100644
 > --- a/package/pkg-utils.mk
 > +++ b/package/pkg-utils.mk
 > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT
 >  	echo "# $(1) is not set" >> $(2)
 >  endef
 
 > +# Note: we do not indent this, since we want to avoid any leading
 > +# space or tabs when calling this function
 > +define KCONFIG_GET_OPT
 > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2))

Should this perhaps use $(SED)?

Sorry, I'm probably missing something, but I don't right away see why we
don't just use:

$(SED) -n 's/^$(1)=//p' $(2)

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Peter Korsgaard]

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

 > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > There is no reason to always depend on host-lzop, even when the kernel
 > compression is not LZO.

 > Since LZO is not the default compression option in the kernel (and there
 > is not sign that will change in the foreseeable future), it will always
 > appear in a condif file, whether it is a complete config file or it is
 > only a defconfig.

 > So, only depend on host-lzop if the LZO compression is enabled in the
 > kernel config file (either the defconfig or the custom config file).

Nice, thanks - Just a few comments..


 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > ---
 >  linux/linux.mk | 6 +++++-
 >  1 file changed, 5 insertions(+), 1 deletion(-)

 > diff --git a/linux/linux.mk b/linux/linux.mk
 > index ab25fe9..f34bea1 100644
 > --- a/linux/linux.mk
 > +++ b/linux/linux.mk
 > @@ -38,7 +38,7 @@ endif
 >  LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 
 >  LINUX_INSTALL_IMAGES = YES
 > -LINUX_DEPENDENCIES  += host-kmod host-lzop
 > +LINUX_DEPENDENCIES  += host-kmod
 
 >  ifeq ($(BR2_LINUX_KERNEL_UBOOT_IMAGE),y)
 >  	LINUX_DEPENDENCIES += host-uboot-tools
 > @@ -163,6 +163,10 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
 >  KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
 >  endif
 
 > +ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)
 > +LINUX_DEPENDENCIES += host-lzop
 > +endif

Another user of lzop seems to be the initramfs support, so we whould
probably also check for RD_LZO (but that's auto enabled/prompt-less if
!expert, so might not be in the .config, so perhaps we should just check
for INITRAMFS_SOURCE):

config RD_LZO
        bool "Support initial ramdisks compressed using LZO" if EXPERT
        default !EXPERT
        depends on BLK_DEV_INITRD
        select DECOMPRESS_LZO
        help
          Support loading of a LZO encoded initial ramdisk or cpio buffer
          If unsure, say N.

Furthermore, this will probably give a pretty odd error message if
people mistype the config file name. Not directly related to this
change, but it would probably be a good thing if we would check and
error out early if any of the needed config files aren't present
(busybox/uclibc/uboot/kernel/..)  with something like

ifeq ($(wildcard $(BR2_PACKAGE_FOO_CONFIG)),)
$(error Configuration file '$(BR2_PACKAGE_FOO_CONFIG)' not found. Check your BR2_PACKAGE_FOO_CONFIG settings)

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] packages infra: add function to get a Kconfig option

[Yann E. MORIN]

On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > We so far have no mean to get the value from a Kconfig option from the
>  > .config file of a package (eg. linux, busybox...).
> 
>  > Add a new function that returns the unmangled value of an option.
>  > It expect two arguments:
>  >   - the Kconfig option name (complete, with leading CONFIG if necessary)
>  >   - the .config file to get it from
> 
>  > Note that, if the Kconfig option is a string, the returned value will
>  > contain the leading and trailing double-quotes.
> 
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > ---
>  >  package/pkg-utils.mk | 6 ++++++
>  >  1 file changed, 6 insertions(+)
> 
>  > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
>  > index 851575c..2f70acc 100644
>  > --- a/package/pkg-utils.mk
>  > +++ b/package/pkg-utils.mk
>  > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT
>  >  	echo "# $(1) is not set" >> $(2)
>  >  endef
>  
>  > +# Note: we do not indent this, since we want to avoid any leading
>  > +# space or tabs when calling this function
>  > +define KCONFIG_GET_OPT
>  > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2))
> 
> Should this perhaps use $(SED)?

$(SED) is 'sed -i -e'

And we don't want to replace in-place!

BTW, that $(SED) is 'sed -i -e' is really bugging me everytime I need
to use sed.

I thionk we should introduce:
  SED := sed -e
  SEDI := sed -i -e

We unfortunately have that many calls to $(SED) here and there... We'd
need to audit and replace all of them.

> Sorry, I'm probably missing something, but I don't right away see why we
> don't just use:
> 
> $(SED) -n 's/^$(1)=//p' $(2)

Oh, I just copied and adapted the commands in the lines above, from
KCONFIG_SET_OPT or KCONFIG_ENABLE_OPT.

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 1/2] packages infra: add function to get a Kconfig option

[Yann E. MORIN]

Peter, All,

On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly:
>  > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > We so far have no mean to get the value from a Kconfig option from the
>  > .config file of a package (eg. linux, busybox...).
> 
>  > Add a new function that returns the unmangled value of an option.
>  > It expect two arguments:
>  >   - the Kconfig option name (complete, with leading CONFIG if necessary)
>  >   - the .config file to get it from
> 
>  > Note that, if the Kconfig option is a string, the returned value will
>  > contain the leading and trailing double-quotes.
> 
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > ---
>  >  package/pkg-utils.mk | 6 ++++++
>  >  1 file changed, 6 insertions(+)
> 
>  > diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
>  > index 851575c..2f70acc 100644
>  > --- a/package/pkg-utils.mk
>  > +++ b/package/pkg-utils.mk
>  > @@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT
>  >  	echo "# $(1) is not set" >> $(2)
>  >  endef
>  
>  > +# Note: we do not indent this, since we want to avoid any leading
>  > +# space or tabs when calling this function
>  > +define KCONFIG_GET_OPT
>  > +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2))
> 
> Should this perhaps use $(SED)?
> 
> Sorry, I'm probably missing something, but I don't right away see why we
> don't just use:
> 
> $(SED) -n 's/^$(1)=//p' $(2)

Oh, I also forgot: we use $(shell ...) because we want to call this from
inside make conditionals, like I did in the followup patch to check if
CONFIG_KERNEL_LZO was set or not.

If we do not use $(shell ...), then the expansion of KCONFIG_GET_OPT is
used, and we'd get something like:

ifeq (sed 'blabla' config-file,y)
...
endif

which is not really of any help, is it? ;-)

(Not usre if your question was about use of $(shell ...) or about the
sed expression itself. I can certinly use your alternate sed expression
if you prefer, of course.)

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 1/2] packages infra: add function to get a Kconfig option

[Peter Korsgaard]

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

Hi,

 >> Should this perhaps use $(SED)?

 > $(SED) is 'sed -i -e'

 > And we don't want to replace in-place!

 > BTW, that $(SED) is 'sed -i -e' is really bugging me everytime I need
 > to use sed.

 > I thionk we should introduce:
 >   SED := sed -e
 >   SEDI := sed -i -e

 > We unfortunately have that many calls to $(SED) here and there... We'd
 > need to audit and replace all of them.

Hmm, yes - Agreed (even though I don't think we really need a SEDI). I
don't really know why we added -i to SED, but I agree that it isn't
really nice (it dates back to the 2d523c23 mega commit).


 >> Sorry, I'm probably missing something, but I don't right away see why we
 >> don't just use:
 >> 
 >> $(SED) -n 's/^$(1)=//p' $(2)

 > Oh, I just copied and adapted the commands in the lines above, from
 > KCONFIG_SET_OPT or KCONFIG_ENABLE_OPT.

Ahh, ok - Then I prefer the simple variant then - I believe it behaves
identical but is directly obvious (to me).

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] packages infra: add function to get a Kconfig option

[Arnout Vandecappelle]

On 26/01/14 14:56, Yann E. MORIN wrote:
> +# Note: we do not indent this, since we want to avoid any leading
> +# space or tabs when calling this function
> +define KCONFIG_GET_OPT
> +$(shell sed -e "/\\<$(1)\\>=\\(.*\\)$$/!d; s//\\1/" $(2))
> +endef

  There's no need to define this with 'define', you can do with a normal 
assignment, like we do for the UPPERCASE macro.

KCONFIG_GET_OPT = $(shell sed -e ...)

  Regards,
  Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Arnout Vandecappelle]

On 26/01/14 14:56, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> There is no reason to always depend on host-lzop, even when the kernel
> compression is not LZO.
>
> Since LZO is not the default compression option in the kernel (and there
> is not sign that will change in the foreseeable future), it will always
> appear in a condif file, whether it is a complete config file or it is
> only a defconfig.
>
> So, only depend on host-lzop if the LZO compression is enabled in the
> kernel config file (either the defconfig or the custom config file).
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>   linux/linux.mk | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/linux/linux.mk b/linux/linux.mk
> index ab25fe9..f34bea1 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -38,7 +38,7 @@ endif
>   LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>
>   LINUX_INSTALL_IMAGES = YES
> -LINUX_DEPENDENCIES  += host-kmod host-lzop
> +LINUX_DEPENDENCIES  += host-kmod
>
>   ifeq ($(BR2_LINUX_KERNEL_UBOOT_IMAGE),y)
>   	LINUX_DEPENDENCIES += host-uboot-tools
> @@ -163,6 +163,10 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
>   KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
>   endif
>
> +ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)

  I would be nicer if the qstrip would move to the definition of 
KERNEL_SOURCE_CONFIG a few lines higher. It's already there in one 
condition, but not in the other.

  Regards,
  Arnout

> +LINUX_DEPENDENCIES += host-lzop
> +endif
> +
>   define LINUX_CONFIGURE_CMDS
>   	cp $(KERNEL_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig
>   	$(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) buildroot_defconfig
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun, 26 Jan 2014 14:56:27 +0100, Yann E. MORIN wrote:

> +ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)
> +LINUX_DEPENDENCIES += host-lzop
> +endif

Do this works reliably? KERNEL_SOURCE_CONFIG typically points to a
minimal defconfig file (say arch/arm/configs/mvebu_defconfig). So if
CONFIG_KERNEL_LZO is the default choice for the kernel, it will not
appear in the defconfig, and therefore the piece of code above will not
realize that we need host-lzop, because the test is done before "make
mvebu_defconfig" is executed and turns it back into a full
configuration file.

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

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Peter Korsgaard]

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

 > Dear Yann E. MORIN,
 > On Sun, 26 Jan 2014 14:56:27 +0100, Yann E. MORIN wrote:

 >> +ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)
 >> +LINUX_DEPENDENCIES += host-lzop
 >> +endif

 > Do this works reliably? KERNEL_SOURCE_CONFIG typically points to a
 > minimal defconfig file (say arch/arm/configs/mvebu_defconfig). So if
 > CONFIG_KERNEL_LZO is the default choice for the kernel, it will not
 > appear in the defconfig, and therefore the piece of code above will not
 > realize that we need host-lzop, because the test is done before "make
 > mvebu_defconfig" is executed and turns it back into a full
 > configuration file.

The commit message contained:

Since LZO is not the default compression option in the kernel (and there
is not sign that will change in the foreseeable future), it will always
appear in a condif file, whether it is a complete config file or it is
only a defconfig.

So I believe the answer is yes, it will work.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Thomas Petazzoni]

Dear Peter Korsgaard,

On Tue, 28 Jan 2014 23:17:32 +0100, Peter Korsgaard wrote:

>  > Do this works reliably? KERNEL_SOURCE_CONFIG typically points to a
>  > minimal defconfig file (say arch/arm/configs/mvebu_defconfig). So if
>  > CONFIG_KERNEL_LZO is the default choice for the kernel, it will not
>  > appear in the defconfig, and therefore the piece of code above will not
>  > realize that we need host-lzop, because the test is done before "make
>  > mvebu_defconfig" is executed and turns it back into a full
>  > configuration file.
> 
> The commit message contained:
> 
> Since LZO is not the default compression option in the kernel (and there
> is not sign that will change in the foreseeable future), it will always
> appear in a condif file, whether it is a complete config file or it is
> only a defconfig.
> 
> So I believe the answer is yes, it will work.

Ok, thanks, I missed that. I believe it's a little bit fragile to rely
on the assumption that it will not become the default, but since the
breakage will be very clear if this ever changes, I think Yann's
proposal is a good compromise. I was also annoyed by host-lzop always
being built, even if useless. Seemed like Buildroot was turning into
this other build system that builds gazillions of useless
dependencies :)

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

[Buildroot] [PATCH 2/2] linux: only depend on host-lzop if needed

[Yann E. MORIN]

Thomas, All,

On 2014-01-28 23:14 +0100, Thomas Petazzoni spake thusly:
> On Sun, 26 Jan 2014 14:56:27 +0100, Yann E. MORIN wrote:
> 
> > +ifeq ($(call KCONFIG_GET_OPT,CONFIG_KERNEL_LZO,$(call qstrip,$(KERNEL_SOURCE_CONFIG))),y)
> > +LINUX_DEPENDENCIES += host-lzop
> > +endif
> 
> Do this works reliably? KERNEL_SOURCE_CONFIG typically points to a
> minimal defconfig file (say arch/arm/configs/mvebu_defconfig). So if
> CONFIG_KERNEL_LZO is the default choice for the kernel, it will not
> appear in the defconfig, and therefore the piece of code above will not
> realize that we need host-lzop, because the test is done before "make
> mvebu_defconfig" is executed and turns it back into a full
> configuration file.

I believe I've addresed your comments in the commit log itself:

    ---8<---
    Since LZO is not the default compression option in the kernel (and there
    is not sign that will change in the foreseeable future), it will always
    appear in a condif file, whether it is a complete config file or it is
    only a defconfig.
    ---8<---

Of course, re-reading myself now, I can now spot a typo:
    s/condif/config/

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 2/2] linux: only depend on host-lzop if needed

[Peter Korsgaard]

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

 >> So I believe the answer is yes, it will work.

 > Ok, thanks, I missed that. I believe it's a little bit fragile to rely
 > on the assumption that it will not become the default, but since the
 > breakage will be very clear if this ever changes, I think Yann's
 > proposal is a good compromise. I was also annoyed by host-lzop always
 > being built, even if useless. Seemed like Buildroot was turning into
 > this other build system that builds gazillions of useless
 > dependencies :)

Indeed. We do have a problem with the initramfs stuff though as I
pointed out.

-- 
Bye, Peter Korsgaard