Skip to contentSkip to conversation

linux: do not always depend on host-lzop

PATCH Superseded Yann E. MORIN 4 participants 15 messages Jan 26, 2014 – Jan 28, 2014

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

cover linux: do not always depend on host-lzop

From: "Yann E. MORIN" <[email protected]>

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(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#1 lore raw

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

From: "Yann E. MORIN" <[email protected]>

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" <[email protected]>
---
 package/pkg-utils.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-utils.mk +6 -0
index 851575c..2f70acc 100644
@@ -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
Signature · 1 line
1.8.1.2
#2 lore raw

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

Quoted 24 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

From: "Yann E. MORIN" <[email protected]>
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" <[email protected]>
---
 package/pkg-utils.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-utils.mk +0 -0
index 851575c..2f70acc 100644

package/pkg-utils.mk define KCONFIG_DISABLE_OPT · line 58

Quoted 7 lines
package/pkg-utils.mk +4 -0
@@ -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)

Signature · 1 line
Bye, Peter Korsgaard
author
#3 lore raw

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

On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly:

Quoted 27 lines
Quoted 26 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

From: "Yann E. MORIN" <[email protected]>
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" <[email protected]>
---
 package/pkg-utils.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-utils.mk +0 -0
index 851575c..2f70acc 100644
@@ -52,6 +52,12 @@ define KCONFIG_DISABLE_OPT
 	echo "# $(1) is not set" >> $(2)
 endef
Quoted 6 lines

package/pkg-utils.mk define KCONFIG_DISABLE_OPT · line 58

Quoted 4 lines
package/pkg-utils.mk +4 -0
+# 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.

Quoted 4 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

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

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 9 lines
Quoted 1 line

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.

Quoted 6 lines

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).

Quoted 7 lines
Quoted 4 lines

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).

Signature · 1 line
Bye, Peter Korsgaard
author
#5 lore raw

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

In reply to Peter Korsgaard

Peter, All,

On 2014-01-26 21:02 +0100, Peter Korsgaard spake thusly:

Quoted 32 lines
Quoted 22 lines

From: "Yann E. MORIN" <[email protected]>
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" <[email protected]>
---
 package/pkg-utils.mk | 6 ++++++
 1 file changed, 6 insertions(+)
package/pkg-utils.mk +0 -0
index 851575c..2f70acc 100644

package/pkg-utils.mk define KCONFIG_DISABLE_OPT · line 58

Quoted 7 lines
package/pkg-utils.mk +4 -0
@@ -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)?

Quoted 4 lines

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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#6 lore raw

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

In reply to Yann E. MORIN

package/pkg-utils.mk define KCONFIG_DISABLE_OPT · line 59

Quoted 5 lines · Yann E. MORIN
package/pkg-utils.mk +5 -0
+# 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

Signature · 6 lines
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
author
#7 lore raw

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

In reply to Yann E. MORIN

From: "Yann E. MORIN" <[email protected]>

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" <[email protected]>
---
 linux/linux.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
linux/linux.mk +5 -1
index ab25fe9..f34bea1 100644
@@ -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
Signature · 1 line
1.8.1.2
#8 lore raw

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

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

From: "Yann E. MORIN" <[email protected]>
There is no reason to always depend on host-lzop, even when the kernel compression is not LZO.

Quoted 7 lines

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

Quoted 19 lines
Signed-off-by "Yann E. MORIN" <[email protected]>
---
 linux/linux.mk | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
linux/linux.mk +1 -1
index ab25fe9..f34bea1 100644
@@ -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

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) · line 168

Quoted 6 lines
linux/linux.mk +3 -0
@@ -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)

Signature · 1 line
Bye, Peter Korsgaard
#9 lore raw

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

In reply to Yann E. MORIN
Quoted 32 lines · Yann E. MORIN

From: "Yann E. MORIN" <[email protected]>

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" <[email protected]>
---
  linux/linux.mk | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
linux/linux.mk +1 -1
index ab25fe9..f34bea1 100644
@@ -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

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) · line 166

Quoted 4 lines
linux/linux.mk +1 -0
@@ -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

Quoted 7 lines
linux/linux.mk +3 -0
+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
Signature · 6 lines
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
#10 lore raw

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

In reply to Yann E. MORIN

Dear Yann E. MORIN,

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) · line 168

Quoted 3 lines · Yann E. MORIN
linux/linux.mk +3 -0
+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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#11 lore raw

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

Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

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

Quoted 11 lines

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) · line 168

Quoted 3 lines
linux/linux.mk +3 -0
+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.

Signature · 1 line
Bye, Peter Korsgaard
#12 lore raw

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

Dear Peter Korsgaard,

Quoted 8 lines · Peter Korsgaard
Quoted 7 lines

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.

Quoted 8 lines

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

Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
#13 lore raw

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

Quoted 11 lines
Quoted 3 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> 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.

Signature · 1 line
Bye, Peter Korsgaard
author
#14 lore raw

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

In reply to Thomas Petazzoni

Thomas, All,

On 2014-01-28 23:14 +0100, Thomas Petazzoni spake thusly:

Quoted 13 lines · Yann E. MORIN

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y) · line 168

Quoted 4 lines
linux/linux.mk +3 -0
+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.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'