| Message ID | 1427884257-20151-1-git-send-email-sagaert.johan@proximus.be |
|---|---|
| State | Superseded |
| Headers | show |
Dear Sagaert Johan, On Wed, 1 Apr 2015 12:30:57 +0200, Sagaert Johan wrote: > diff --git a/linux/linux.mk b/linux/linux.mk > index 22fce35..137c64d 100644 > --- a/linux/linux.mk > +++ b/linux/linux.mk > @@ -49,7 +49,7 @@ LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH)) > LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) > > LINUX_INSTALL_IMAGES = YES > -LINUX_DEPENDENCIES += host-kmod host-lzop > +LINUX_DEPENDENCIES += host-kmod host-lzop host-lz4 I think it's time to add some Config.in options in linux/Config.in to select the compression tools to be made available, and enforce their usage/availability in the Linux .config file. It is annoying to bvuild host-lzop and host-lz4 every time you're building a kernel if you don't care about those compressions algos. > diff --git a/package/lz4/host-lz4.mk b/package/lz4/host-lz4.mk > new file mode 100644 > index 0000000..03138f9 > --- /dev/null > +++ b/package/lz4/host-lz4.mk > @@ -0,0 +1,21 @@ > +################################################################################ > +# > +# host-lz4 > +# > +################################################################################ > + > +HOST_LZ4_VERSION = r123 > +HOST_LZ4_SITE = $(call github,Cyan4973,lz4,$(HOST_LZ4_VERSION)) > +HOST_LZ4_LICENSE = BSD-2c > +HOST_LZ4_LICENSE_FILES = LICENSE > + > +define HOST_LZ4_BUILD_CMDS > + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) > +endef > + > +define HOST_LZ4_INSTALL_CMDS > + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) > +endef > + > +$(eval $(host-generic-package)) > + > diff --git a/package/lz4/lz4.mk b/package/lz4/lz4.mk > index 38e10d8..6f8af03 100644 > --- a/package/lz4/lz4.mk > +++ b/package/lz4/lz4.mk > @@ -17,14 +17,6 @@ endef > LZ4_POST_PATCH_HOOKS += LZ4_DISABLE_SHARED > endif > > -define HOST_LZ4_BUILD_CMDS > - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) > -endef > - > -define HOST_LZ4_INSTALL_CMDS > - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) > -endef > - > define LZ4_BUILD_CMDS > $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) liblz4 > endef > @@ -38,4 +30,3 @@ define LZ4_INSTALL_TARGET_CMDS > endef > > $(eval $(generic-package)) > -$(eval $(host-generic-package)) This entire change seems completely unnecessary, and not only that, but it is completely incompatible with the Buildroot best practices to write host packages. Why did you do this change in the first place? Thanks, Thomas
Thomas Petazzoni schreef op 1/04/2015 om 13:37: > Dear Sagaert Johan, > > On Wed, 1 Apr 2015 12:30:57 +0200, Sagaert Johan wrote: > >> diff --git a/linux/linux.mk b/linux/linux.mk >> index 22fce35..137c64d 100644 >> --- a/linux/linux.mk >> +++ b/linux/linux.mk >> @@ -49,7 +49,7 @@ LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH)) >> LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) >> >> LINUX_INSTALL_IMAGES = YES >> -LINUX_DEPENDENCIES += host-kmod host-lzop >> +LINUX_DEPENDENCIES += host-kmod host-lzop host-lz4 > I think it's time to add some Config.in options in linux/Config.in to > select the compression tools to be made available, and enforce their > usage/availability in the Linux .config file. It is annoying to bvuild > host-lzop and host-lz4 every time you're building a kernel if you don't > care about those compressions algos. I am not sure if all kernel releases have the lz4 compression option, so this seemed the most simple solution, lz4 is not taking large resources in terms of build time This works fine for me now. Not sure about this : could host-lz4 be build in a kernel_pre_build hook ? checking the kernels .config before the kernel build starts. >> diff --git a/package/lz4/host-lz4.mk b/package/lz4/host-lz4.mk >> new file mode 100644 >> index 0000000..03138f9 >> --- /dev/null >> +++ b/package/lz4/host-lz4.mk >> @@ -0,0 +1,21 @@ >> +################################################################################ >> +# >> +# host-lz4 >> +# >> +################################################################################ >> + >> +HOST_LZ4_VERSION = r123 >> +HOST_LZ4_SITE = $(call github,Cyan4973,lz4,$(HOST_LZ4_VERSION)) >> +HOST_LZ4_LICENSE = BSD-2c >> +HOST_LZ4_LICENSE_FILES = LICENSE >> + >> +define HOST_LZ4_BUILD_CMDS >> + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) >> +endef >> + >> +define HOST_LZ4_INSTALL_CMDS >> + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) >> +endef >> + >> +$(eval $(host-generic-package)) >> + >> diff --git a/package/lz4/lz4.mk b/package/lz4/lz4.mk >> index 38e10d8..6f8af03 100644 >> --- a/package/lz4/lz4.mk >> +++ b/package/lz4/lz4.mk >> @@ -17,14 +17,6 @@ endef >> LZ4_POST_PATCH_HOOKS += LZ4_DISABLE_SHARED >> endif >> >> -define HOST_LZ4_BUILD_CMDS >> - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) >> -endef >> - >> -define HOST_LZ4_INSTALL_CMDS >> - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) >> -endef >> - >> define LZ4_BUILD_CMDS >> $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) liblz4 >> endef >> @@ -38,4 +30,3 @@ define LZ4_INSTALL_TARGET_CMDS >> endef >> >> $(eval $(generic-package)) >> -$(eval $(host-generic-package)) > This entire change seems completely unnecessary, and not only that, but > it is completely incompatible with the Buildroot best practices to > write host packages. Why did you do this change in the first place? > > Thanks, > > Thomas I don't want lz4 on the target , that is why i have done it this way, lz4 that can be selected via the menu if needed on the target , and a host-lz4 that is always build when building a kernel. Sent me your suggestion, and i will try to fix this patch. (have to do all buildroot patchwork in free-time ) Regards , Johan
Dear Johan Sagaert, On Wed, 01 Apr 2015 14:25:38 +0200, Johan Sagaert wrote: > > I think it's time to add some Config.in options in linux/Config.in to > > select the compression tools to be made available, and enforce their > > usage/availability in the Linux .config file. It is annoying to bvuild > > host-lzop and host-lz4 every time you're building a kernel if you don't > > care about those compressions algos. > I am not sure if all kernel releases have the lz4 compression option, so > this > seemed the most simple solution, lz4 is not taking large resources in > terms of build time > This works fine for me now. Yes, it works. But everyone now needs to build host-lzop and host-lz4 before building a kernel even if they don't work. > Not sure about this : could host-lz4 be build in a kernel_pre_build hook ? > checking the kernels .config before the kernel build starts. Unfortunately, no. The dependencies have to be known before the kernel configuration is generated. > I don't want lz4 on the target , that is why i have done it this way, > lz4 that can be selected via the menu > if needed on the target , and a host-lz4 that is always build when > building a kernel. > Sent me your suggestion, and i will try to fix this patch. (have to do > all buildroot patchwork in free-time ) Just don't do *any* change to the lz4 package. Add your host-lz4 dependency to LINUX_DEPENDENCIES, and this is enough. It will only build the host variant of lz4. The target variant will only be built if BR2_PACKAGE_LZ4 is enabled in your Buildroot .config. You seem to believe that because the target and host variant of lz4 are implemented in the same .mk file, both of them are always built together. But this is not the case. Even with both defined in the same .mk file, it perfectly supports building only the host variant, only the target variant, or both. So your change to lz4.mk and creating a host-lz4.mk is completely unnecessary. Best regards, Thomas
Hello, On Wed, 1 Apr 2015 15:02:37 +0200, Thomas Petazzoni wrote: > Yes, it works. But everyone now needs to build host-lzop and host-lz4 > before building a kernel even if they don't work. s/work/need them/. I should _really_ stop doing 3 things at the same time. My context switching logic is so broken that I should do only one thing at once, not more. Thomas
Dear Thomas , Posted v2 patch minutes ago. Thomas Petazzoni schreef op 1/04/2015 om 15:02: > Dear Johan Sagaert, > > On Wed, 01 Apr 2015 14:25:38 +0200, Johan Sagaert wrote: > >>> I think it's time to add some Config.in options in linux/Config.in to >>> select the compression tools to be made available, and enforce their >>> usage/availability in the Linux .config file. It is annoying to bvuild >>> host-lzop and host-lz4 every time you're building a kernel if you don't >>> care about those compressions algos. >> I am not sure if all kernel releases have the lz4 compression option, so >> this >> seemed the most simple solution, lz4 is not taking large resources in >> terms of build time >> This works fine for me now. > Yes, it works. But everyone now needs to build host-lzop and host-lz4 > before building a kernel even if they don't work. > >> Not sure about this : could host-lz4 be build in a kernel_pre_build hook ? >> checking the kernels .config before the kernel build starts. > Unfortunately, no. The dependencies have to be known before the kernel > configuration is generated. Changed the patch so that host-lz4 is only build when the target is ARM . That should reduce clutter. If some make expert could show me how to use the LINUX_VERSION_PROBED variable to check for >=3.11 it would further reduce unneeded builds. >> I don't want lz4 on the target , that is why i have done it this way, >> lz4 that can be selected via the menu >> if needed on the target , and a host-lz4 that is always build when >> building a kernel. >> Sent me your suggestion, and i will try to fix this patch. (have to do >> all buildroot patchwork in free-time ) > Just don't do *any* change to the lz4 package. Add your host-lz4 > dependency to LINUX_DEPENDENCIES, and this is enough. It will only > build the host variant of lz4. The target variant will only be built if > BR2_PACKAGE_LZ4 is enabled in your Buildroot .config. > > You seem to believe that because the target and host variant of lz4 are > implemented in the same .mk file, both of them are always built > together. But this is not the case. Even with both defined in the > same .mk file, it perfectly supports building only the host variant, > only the target variant, or both. > > So your change to lz4.mk and creating a host-lz4.mk is completely > unnecessary. Yes, i was in this misconception. > Best regards, > > Thomas Best Regards , Johan
diff --git a/linux/linux.mk b/linux/linux.mk index 22fce35..137c64d 100644 --- a/linux/linux.mk +++ b/linux/linux.mk @@ -49,7 +49,7 @@ LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH)) LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)) LINUX_INSTALL_IMAGES = YES -LINUX_DEPENDENCIES += host-kmod host-lzop +LINUX_DEPENDENCIES += host-kmod host-lzop host-lz4 ifeq ($(BR2_LINUX_KERNEL_UBOOT_IMAGE),y) LINUX_DEPENDENCIES += host-uboot-tools diff --git a/package/lz4/host-lz4.mk b/package/lz4/host-lz4.mk new file mode 100644 index 0000000..03138f9 --- /dev/null +++ b/package/lz4/host-lz4.mk @@ -0,0 +1,21 @@ +################################################################################ +# +# host-lz4 +# +################################################################################ + +HOST_LZ4_VERSION = r123 +HOST_LZ4_SITE = $(call github,Cyan4973,lz4,$(HOST_LZ4_VERSION)) +HOST_LZ4_LICENSE = BSD-2c +HOST_LZ4_LICENSE_FILES = LICENSE + +define HOST_LZ4_BUILD_CMDS + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) +endef + +define HOST_LZ4_INSTALL_CMDS + $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) +endef + +$(eval $(host-generic-package)) + diff --git a/package/lz4/lz4.mk b/package/lz4/lz4.mk index 38e10d8..6f8af03 100644 --- a/package/lz4/lz4.mk +++ b/package/lz4/lz4.mk @@ -17,14 +17,6 @@ endef LZ4_POST_PATCH_HOOKS += LZ4_DISABLE_SHARED endif -define HOST_LZ4_BUILD_CMDS - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) -endef - -define HOST_LZ4_INSTALL_CMDS - $(MAKE) $(HOST_CONFIGURE_OPTS) -C $(@D) install DESTDIR=$(HOST_DIR) -endef - define LZ4_BUILD_CMDS $(MAKE) $(TARGET_CONFIGURE_OPTS) -C $(@D) liblz4 endef @@ -38,4 +30,3 @@ define LZ4_INSTALL_TARGET_CMDS endef $(eval $(generic-package)) -$(eval $(host-generic-package))
Selecting lz4 compression in the kernel configuration yields a significant improvement in boot time. Therefore we need lz4 as dependency for the kernel. Signed-off-by: Sagaert Johan <sagaert.johan@proximus.be> --- linux/linux.mk | 2 +- package/lz4/host-lz4.mk | 21 +++++++++++++++++++++ package/lz4/lz4.mk | 9 --------- 3 files changed, 22 insertions(+), 10 deletions(-) create mode 100644 package/lz4/host-lz4.mk