[Buildroot] [PATCH 0 of 2 v2] Introduce suitable-extractor and add host-xzcat dependency

[Buildroot] [PATCH 0 of 2 v2] Introduce suitable-extractor and add host-xzcat dependency

[Thomas De Schampheleire]

v2: update based on Thomas Petazzoni's comments.

[Buildroot] [PATCH 1 of 2 v2] infra: introduce suitable-extractor helper function

[Thomas De Schampheleire]

In order to simplify determining the right extractor tool for a given
file type, this patch introduces a make function 'suitable-extractor'.
Its usage is $(call suitable-extractor,filename), and it returns the
path to the suitable extractor.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
(v2): new patch in series

Note: one remaining direct usage of the INFLATE variables is in
package/pkg-generic.mk, but it's in the inner block where dollar signs
are thrown around your ears. I'm not sure if it's possible to use the
function there...

 package/lsof/lsof.mk                     |  2 +-
 package/perl/perl.mk                     |  2 +-
 package/pkg-generic.mk                   |  2 +-
 package/pkg-utils.mk                     |  2 ++
 package/tar/tar.mk                       |  2 +-
 toolchain/toolchain-external/ext-tool.mk |  6 +++---
 6 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/package/lsof/lsof.mk b/package/lsof/lsof.mk
--- a/package/lsof/lsof.mk
+++ b/package/lsof/lsof.mk
@@ -41,7 +41,7 @@ endif
 
 # The .tar.bz2 contains another .tar, which contains the source code.
 define LSOF_EXTRACT_CMDS
-        $(INFLATE.bz2) $(DL_DIR)/$(LSOF_SOURCE) | \
+        $(call suitable-extractor,$(LSOF_SOURCE)) $(DL_DIR)/$(LSOF_SOURCE) | \
                 $(TAR) -O $(TAR_OPTIONS) - lsof_$(LSOF_VERSION)/lsof_$(LSOF_VERSION)_src.tar | \
         $(TAR) $(TAR_STRIP_COMPONENTS)=1 -C $(LSOF_DIR) $(TAR_OPTIONS) -
 endef
diff --git a/package/perl/perl.mk b/package/perl/perl.mk
--- a/package/perl/perl.mk
+++ b/package/perl/perl.mk
@@ -30,7 +30,7 @@ endef
 PERL_POST_DOWNLOAD_HOOKS += PERL_CROSS_DOWNLOAD
 
 define PERL_CROSS_EXTRACT
-	$(INFLATE$(suffix $(PERL_CROSS_SOURCE))) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
+	$(call suitable-extractor,$(PERL_CROSS_SOURCE)) $(DL_DIR)/$(PERL_CROSS_SOURCE) | \
 	$(TAR) $(TAR_STRIP_COMPONENTS)=1 -C $(@D) $(TAR_OPTIONS) -
 endef
 PERL_POST_EXTRACT_HOOKS += PERL_CROSS_EXTRACT
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -539,7 +539,7 @@ else ifeq ($$($(2)_SITE_METHOD),hg)
 DL_TOOLS_DEPENDENCIES += hg
 endif # SITE_METHOD
 
-DL_TOOLS_DEPENDENCIES += $(firstword $(INFLATE$(suffix $($(2)_SOURCE))))
+DL_TOOLS_DEPENDENCIES += $(call suitable-extractor,$($(2)_SOURCE))
 
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -62,6 +62,8 @@ INFLATE.tbz2 = $(BZCAT)
 INFLATE.tgz  = $(ZCAT)
 INFLATE.xz   = $(XZCAT)
 INFLATE.tar  = cat
+# suitable-extractor(filename): returns extractor based on suffix
+suitable-extractor = $(firstword $(INFLATE$(suffix $(1))))
 
 # MESSAGE Macro -- display a message in bold type
 MESSAGE     = echo "$(TERM_BOLD)>>> $($(PKG)_NAME) $($(PKG)_VERSION) $(1)$(TERM_RESET)"
diff --git a/package/tar/tar.mk b/package/tar/tar.mk
--- a/package/tar/tar.mk
+++ b/package/tar/tar.mk
@@ -23,7 +23,7 @@ HOST_TAR_SOURCE = tar-$(TAR_VERSION).cpi
 define HOST_TAR_EXTRACT_CMDS
 	mkdir -p $(@D)
 	cd $(@D) && \
-		$(INFLATE.gz) $(DL_DIR)/$(HOST_TAR_SOURCE) | cpio -i
+		$(call suitable-extractor,$(HOST_TAR_SOURCE)) $(DL_DIR)/$(HOST_TAR_SOURCE) | cpio -i
 	mv $(@D)/tar-$(TAR_VERSION)/* $(@D)
 	rmdir $(@D)/tar-$(TAR_VERSION)
 endef
diff --git a/toolchain/toolchain-external/ext-tool.mk b/toolchain/toolchain-external/ext-tool.mk
--- a/toolchain/toolchain-external/ext-tool.mk
+++ b/toolchain/toolchain-external/ext-tool.mk
@@ -337,9 +337,9 @@ ifeq ($(BR2_TOOLCHAIN_EXTERNAL_BLACKFIN_
 
 $(TOOLCHAIN_EXTERNAL_DIR)/.extracted: $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_1) $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_2)
 	mkdir -p $(@D)
-	$(INFLATE$(suffix $(TOOLCHAIN_EXTERNAL_SOURCE_1))) $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_1) | \
+	$(call suitable-extractor,$(TOOLCHAIN_EXTERNAL_SOURCE_1)) $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_1) | \
 		$(TAR) $(TAR_STRIP_COMPONENTS)=3 --hard-dereference -C $(@D) $(TAR_OPTIONS) -
-	$(INFLATE$(suffix $(TOOLCHAIN_EXTERNAL_SOURCE_2))) $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_2) | \
+	$(call suitable-extractor,$(TOOLCHAIN_EXTERNAL_SOURCE_2)) $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE_2) | \
 		$(TAR) $(TAR_STRIP_COMPONENTS)=3 --hard-dereference -C $(@D) $(TAR_OPTIONS) -
 	$(Q)touch $@
 else
@@ -349,7 +349,7 @@ else
 
 $(TOOLCHAIN_EXTERNAL_DIR)/.extracted: $(DL_DIR)/$(TOOLCHAIN_EXTERNAL_SOURCE)
 	mkdir -p $(@D)
-	$(INFLATE$(suffix $(TOOLCHAIN_EXTERNAL_SOURCE))) $^ | \
+	$(call suitable-extractor,$(TOOLCHAIN_EXTERNAL_SOURCE)) $^ | \
 		$(TAR) $(TAR_STRIP_COMPONENTS)=1 --exclude='usr/lib/locale/*' -C $(@D) $(TAR_OPTIONS) -
 	$(TOOLCHAIN_EXTERNAL_FIXUP_CMDS)
 	$(Q)touch $@

[Buildroot] [PATCH 2 of 2 v2] xzcat: treat as host prerequisite and build if needed

[Thomas De Schampheleire]

If xzcat is not present on the host system, buildroot bails out early asking
the developer to install it (xzcat is now a DL_TOOLS_DEPENDENCY)
Conversely, when BR2_TARGET_ROOTFS_CPIO_XZ is enabled, then host-xz is a
build dependency, and no manual action is required from the developer.

Because the second approach is nicer, also build host-xz when xzcat is not
available, using the host-prerequisite and suitable-host-pkg mechanisms,
already used for tar.

Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

---
v2: avoid filtering out xzcat from DL_TOOLS_DEPENDENCIES (comment ThomasP)

 package/pkg-generic.mk                   |   4 ++++
 support/dependencies/check-host-xzcat.mk |   7 +++++++
 support/dependencies/check-host-xzcat.sh |  14 ++++++++++++++
 3 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -539,7 +539,11 @@ else ifeq ($$($(2)_SITE_METHOD),hg)
 DL_TOOLS_DEPENDENCIES += hg
 endif # SITE_METHOD
 
+# Do not add xzcat to the list of required dependencies, as it gets
+# built automatically if it isn't found.
+ifneq ($(call suitable-extractor,$($(2)_SOURCE)),$(XZCAT))
 DL_TOOLS_DEPENDENCIES += $(call suitable-extractor,$($(2)_SOURCE))
+endif
 
 endif # $(2)_KCONFIG_VAR
 endef # inner-generic-package
diff --git a/support/dependencies/check-host-xzcat.mk b/support/dependencies/check-host-xzcat.mk
new file mode 100644
--- /dev/null
+++ b/support/dependencies/check-host-xzcat.mk
@@ -0,0 +1,7 @@
+# XZCAT is taken from BR2_XZCAT (defaults to 'xzcat') (see Makefile)
+# If it is not present, build our own host-xzcat
+
+ifeq (,$(call suitable-host-package,xzcat,$(XZCAT)))
+  DEPENDENCIES_HOST_PREREQ += host-xz
+  XZCAT = $(HOST_DIR)/usr/bin/xzcat
+endif
diff --git a/support/dependencies/check-host-xzcat.sh b/support/dependencies/check-host-xzcat.sh
new file mode 100755
--- /dev/null
+++ b/support/dependencies/check-host-xzcat.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+candidate="$1"
+
+xzcat=`which $candidate 2>/dev/null`
+if [ ! -x "$xzcat" ]; then
+	xzcat=`which xzcat 2>/dev/null`
+	if [ ! -x "$xzcat" ]; then
+		# echo nothing: no suitable xzcat found
+		exit 1
+	fi
+fi
+
+echo $xzcat

[Buildroot] [PATCH 1 of 2 v2] infra: introduce suitable-extractor helper function

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Thu, 01 Aug 2013 22:55:47 +0200, Thomas De Schampheleire wrote:

> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -62,6 +62,8 @@ INFLATE.tbz2 = $(BZCAT)
>  INFLATE.tgz  = $(ZCAT)
>  INFLATE.xz   = $(XZCAT)
>  INFLATE.tar  = cat
> +# suitable-extractor(filename): returns extractor based on suffix
> +suitable-extractor = $(firstword $(INFLATE$(suffix $(1))))

Do you know why we need this $(firstword ...) call here? In all places
in was using directly $(INFLATE$(...)), except in the package
infrastructure where it was doing this firstword additional call.

It was added by Peter in 2c6390a5d0c01420879e9f23bc89afb19976da4a.

Ideas?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 1 of 2 v2] infra: introduce suitable-extractor helper function

[Thomas De Schampheleire]

On Fri, Aug 2, 2013 at 10:28 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Thu, 01 Aug 2013 22:55:47 +0200, Thomas De Schampheleire wrote:
>
>> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
>> --- a/package/pkg-utils.mk
>> +++ b/package/pkg-utils.mk
>> @@ -62,6 +62,8 @@ INFLATE.tbz2 = $(BZCAT)
>>  INFLATE.tgz  = $(ZCAT)
>>  INFLATE.xz   = $(XZCAT)
>>  INFLATE.tar  = cat
>> +# suitable-extractor(filename): returns extractor based on suffix
>> +suitable-extractor = $(firstword $(INFLATE$(suffix $(1))))
>
> Do you know why we need this $(firstword ...) call here? In all places
> in was using directly $(INFLATE$(...)), except in the package
> infrastructure where it was doing this firstword additional call.
>
> It was added by Peter in 2c6390a5d0c01420879e9f23bc89afb19976da4a.

It wasn't clear to me when I sent the patch, but I figured it didn't hurt.
But now I think I know: the inflate targets originate from the
.config, and users can add extra arguments there. In fact, the default
for $(ZCAT) is:
BR2_ZCAT="gzip -d -c"

To check the dependency, we only want to check for 'gzip', but to do
the actual inflate, we shouldn't use 'firstword'. So, the patch is
wrong, and I will fix it :)

Thanks for highlighting this!

Best regards,
Thomas

[Buildroot] [PATCH 1 of 2 v2] infra: introduce suitable-extractor helper function

[Thomas Petazzoni]

Dear Thomas De Schampheleire,

On Fri, 2 Aug 2013 10:50:44 +0200, Thomas De Schampheleire wrote:

> > Do you know why we need this $(firstword ...) call here? In all places
> > in was using directly $(INFLATE$(...)), except in the package
> > infrastructure where it was doing this firstword additional call.
> >
> > It was added by Peter in 2c6390a5d0c01420879e9f23bc89afb19976da4a.
> 
> It wasn't clear to me when I sent the patch, but I figured it didn't hurt.
> But now I think I know: the inflate targets originate from the
> .config, and users can add extra arguments there. In fact, the default
> for $(ZCAT) is:
> BR2_ZCAT="gzip -d -c"
> 
> To check the dependency, we only want to check for 'gzip', but to do
> the actual inflate, we shouldn't use 'firstword'. So, the patch is
> wrong, and I will fix it :)

Ok, makes sense to me now. Indeed this makes the patch wrong :)

Are you annoyed if I integrate this in -next ? I want to release -rc1
today, and I think this kind of stuff is a little bit too sensitive to
be merged now. From your quick feedback on those patches, I have the
feeling that you wanted it in 2013.08, so I don't want to ruin your
hope, but I believe it's a bit too late now. What do you think?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH 1 of 2 v2] infra: introduce suitable-extractor helper function

[Thomas De Schampheleire]

Hi Thomas,

On Fri, Aug 2, 2013 at 10:56 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Fri, 2 Aug 2013 10:50:44 +0200, Thomas De Schampheleire wrote:
>
>> > Do you know why we need this $(firstword ...) call here? In all places
>> > in was using directly $(INFLATE$(...)), except in the package
>> > infrastructure where it was doing this firstword additional call.
>> >
>> > It was added by Peter in 2c6390a5d0c01420879e9f23bc89afb19976da4a.
>>
>> It wasn't clear to me when I sent the patch, but I figured it didn't hurt.
>> But now I think I know: the inflate targets originate from the
>> .config, and users can add extra arguments there. In fact, the default
>> for $(ZCAT) is:
>> BR2_ZCAT="gzip -d -c"
>>
>> To check the dependency, we only want to check for 'gzip', but to do
>> the actual inflate, we shouldn't use 'firstword'. So, the patch is
>> wrong, and I will fix it :)
>
> Ok, makes sense to me now. Indeed this makes the patch wrong :)
>
> Are you annoyed if I integrate this in -next ? I want to release -rc1
> today, and I think this kind of stuff is a little bit too sensitive to
> be merged now. From your quick feedback on those patches, I have the
> feeling that you wanted it in 2013.08, so I don't want to ruin your
> hope, but I believe it's a bit too late now. What do you think?
>

It's always nice if patches make it to the upcoming release and not
the one after that :)
But I understand that it's not critical and could have side-effects we
didn't think of, so no problem to put it in -next. I won't hate you
for it ;)

Best regards,
Thomas