Patchwork linux: CONFIG_KERNEL_LZO option requires host-lzop

login
register
mail settings
Submitter Fabio Porcedda
Date May 13, 2013, 7:58 a.m.
Message ID <1368431920-22020-1-git-send-email-fabio.porcedda@gmail.com>
Download mbox | patch
Permalink /patch/243318/
State Accepted
Commit 9b273ea860504666cf204eb475421e21cbcac639
Headers show

Comments

Fabio Porcedda - May 13, 2013, 7:58 a.m.
When the LZO compression mode is chosen the linux kernel requires
lzop host utility at build time.

Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 linux/linux.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Thomas Petazzoni - May 13, 2013, 8:04 a.m.
Dear Fabio Porcedda,

On Mon, 13 May 2013 09:58:40 +0200, Fabio Porcedda wrote:
> When the LZO compression mode is chosen the linux kernel requires
> lzop host utility at build time.
> 
> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>  linux/linux.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index d375cf0..3877c35 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -34,7 +34,7 @@ endif
>  LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>  
>  LINUX_INSTALL_IMAGES = YES
> -LINUX_DEPENDENCIES  += host-module-init-tools
> +LINUX_DEPENDENCIES  += host-module-init-tools host-lzop

Yeah, I know about this problem, but I'm not entirely happy with this
solution. Since we can't have dependencies that depend on the kernel
configuration (because at the time make parses linux.mk, the kernel
configuration hasn't been created yet), we have to make host-lzop a
mandatory dependency. Then, later on, someone will came up with the
same problem for the LZMA compression. And then the kernel, in some
special case will need yet another tool installed on the host machine.

I agree that host-lzop is not huge, but it's a pain to have things
being built that will not be used.

The only alternative that I see is that the kernel compression should
be selected in Buildroot, which will then enforce this selection into
the kernel configuration. Therefore, Buildroot will have the knowledge
of which compression tools will be needed.

Now, whether this alternative really scales if the kernel starts to
have more and more host dependencies, I don't know.

Thoughts? Ideas? Maybe I'm just nitpicking here?

Thomas
Fabio Porcedda - May 14, 2013, 9:02 a.m.
On Mon, May 13, 2013 at 10:04 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Fabio Porcedda,
>
> On Mon, 13 May 2013 09:58:40 +0200, Fabio Porcedda wrote:
>> When the LZO compression mode is chosen the linux kernel requires
>> lzop host utility at build time.
>>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda@gmail.com>
>> ---
>>  linux/linux.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux/linux.mk b/linux/linux.mk
>> index d375cf0..3877c35 100644
>> --- a/linux/linux.mk
>> +++ b/linux/linux.mk
>> @@ -34,7 +34,7 @@ endif
>>  LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>>
>>  LINUX_INSTALL_IMAGES = YES
>> -LINUX_DEPENDENCIES  += host-module-init-tools
>> +LINUX_DEPENDENCIES  += host-module-init-tools host-lzop
>
> Yeah, I know about this problem, but I'm not entirely happy with this
> solution. Since we can't have dependencies that depend on the kernel
> configuration (because at the time make parses linux.mk, the kernel
> configuration hasn't been created yet), we have to make host-lzop a
> mandatory dependency. Then, later on, someone will came up with the
> same problem for the LZMA compression. And then the kernel, in some
> special case will need yet another tool installed on the host machine.
>
> I agree that host-lzop is not huge, but it's a pain to have things
> being built that will not be used.
>
> The only alternative that I see is that the kernel compression should
> be selected in Buildroot, which will then enforce this selection into
> the kernel configuration. Therefore, Buildroot will have the knowledge
> of which compression tools will be needed.
>
> Now, whether this alternative really scales if the kernel starts to
> have more and more host dependencies, I don't know.
>
> Thoughts? Ideas? Maybe I'm just nitpicking here?

Hi Thomas,
thanks for reviewing.

I know that the best solution is to depend on the linux kernel
configuration, but I don't know a way to do that cleanly.
Maybe some clever idea?

Adding mandatory dependency is the simplest and always works but as
you pointed it always builds host-lzop even when is not used.

Choosing the compression within buildroot doesn't wok when using a
defconfig or linux-xconfig target but at least does build host-lzop
only when is used.

Nevertheless I'm fine with both solutions, i just want to fix this problem.

Best regards
--
Fabio Porcedda
Thomas Petazzoni - May 14, 2013, 9:16 a.m.
Dear Fabio Porcedda,

On Tue, 14 May 2013 11:02:32 +0200, Fabio Porcedda wrote:

> I know that the best solution is to depend on the linux kernel
> configuration, but I don't know a way to do that cleanly.
> Maybe some clever idea?
> 
> Adding mandatory dependency is the simplest and always works but as
> you pointed it always builds host-lzop even when is not used.
> 
> Choosing the compression within buildroot doesn't wok when using a
> defconfig or linux-xconfig target but at least does build host-lzop
> only when is used.

It does work, as long as the compression type is enforced by Buildroot,
regardless of what was selected by linux-xconfig or the defconfig.
That's what we do today for DEVTMPFS/DEVTMPFS_MOUNT/INITRAMFS_SOURCE
for example.

However, before going ahead and implementing that, I'd like to have the
opinion of Peter and maybe others.

Thomas
Peter Korsgaard - May 14, 2013, 3:30 p.m.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 Thomas> Dear Fabio Porcedda,
 Thomas> On Tue, 14 May 2013 11:02:32 +0200, Fabio Porcedda wrote:

 >> I know that the best solution is to depend on the linux kernel
 >> configuration, but I don't know a way to do that cleanly.
 >> Maybe some clever idea?
 >> 
 >> Adding mandatory dependency is the simplest and always works but as
 >> you pointed it always builds host-lzop even when is not used.
 >> 
 >> Choosing the compression within buildroot doesn't wok when using a
 >> defconfig or linux-xconfig target but at least does build host-lzop
 >> only when is used.

 Thomas> It does work, as long as the compression type is enforced by
 Thomas> Buildroot, regardless of what was selected by linux-xconfig or
 Thomas> the defconfig.  That's what we do today for
 Thomas> DEVTMPFS/DEVTMPFS_MOUNT/INITRAMFS_SOURCE for example.

 Thomas> However, before going ahead and implementing that, I'd like to
 Thomas> have the opinion of Peter and maybe others.

I agree that it isn't really nice to needlessly build host-lzop/xz if
not needed, but I also dislike extra configuration options (that might
not be in sync with the defconfig).

The difference between the other options we have and this is that either
we closely interact with the kernel build (for initramfs) or the system
fails in nonobvious ways at runtime if not set (devtmpfs).

Nothing else in buildroot really cares about the kernel compression
option (besides that the kernel needs specific build tools).

A quick test on my not-especially-beefy laptop shows:

make host-lzop  29.83s user 3.73s system 107% cpu 31.312 total
make host-xz  21.03s user 2.66s system 126% cpu 18.759 total

So ~50s extra build time if you build a Linux kernel, which isn't too
bad.

Perhaps we can come up with a better solution later, but I'll apply the
patch as it is for 2013.05.
Peter Korsgaard - May 14, 2013, 3:31 p.m.
>>>>> "Fabio" == Fabio Porcedda <fabio.porcedda@gmail.com> writes:

 Fabio> When the LZO compression mode is chosen the linux kernel requires
 Fabio> lzop host utility at build time.

Committed, thanks.

Patch

diff --git a/linux/linux.mk b/linux/linux.mk
index d375cf0..3877c35 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -34,7 +34,7 @@  endif
 LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 
 LINUX_INSTALL_IMAGES = YES
-LINUX_DEPENDENCIES  += host-module-init-tools
+LINUX_DEPENDENCIES  += host-module-init-tools host-lzop
 
 ifeq ($(BR2_LINUX_KERNEL_UBOOT_IMAGE),y)
 	LINUX_DEPENDENCIES += host-uboot-tools