Skip to contentSkip to conversation

Introduce suitable-extractor and add host-xzcat dependency

PATCH Superseded Thomas De Schampheleire 2 participants 7 messages Aug 1, 2013 – Aug 2, 2013

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Thomas De Schampheleire

author
#0 lore raw

PATCH v2 1/1 Introduce suitable-extractor and add host-xzcat dependency

v2: update based on Thomas Petazzoni's comments.

author
#1 lore raw

PATCH v2 1/1 infra: introduce suitable-extractor helper function

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 <[email protected]>

---
(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(-)
package/lsof/lsof.mk +1 -1
@@ -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
package/perl/perl.mk +1 -1
@@ -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
package/pkg-generic.mk +1 -1
@@ -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
package/pkg-utils.mk +2 -0
@@ -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)"
package/tar/tar.mk +1 -1
@@ -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
toolchain/toolchain-external/ext-tool.mk +3 -3
@@ -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 $@
#2 lore raw

PATCH v2 1/1 infra: introduce suitable-extractor helper function

Dear Thomas De Schampheleire,

package/pkg-utils.mk INFLATE.tbz2 = $(BZCAT) · line 66

Quoted 9 lines · Thomas De Schampheleire
package/pkg-utils.mk +2 -0
@@ -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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#3 lore raw

PATCH v2 1/1 infra: introduce suitable-extractor helper function

Quoted 18 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,

package/pkg-utils.mk INFLATE.tbz2 = $(BZCAT) · line 66

Quoted 9 lines · Thomas De Schampheleire
package/pkg-utils.mk +2 -0
@@ -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.

Quoted 1 line

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

#4 lore raw

PATCH v2 1/1 infra: introduce suitable-extractor helper function

Dear Thomas De Schampheleire,

Quoted 12 lines · Thomas De Schampheleire
Quoted 5 lines

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"

Quoted 3 lines

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

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#5 lore raw

PATCH v2 1/1 infra: introduce suitable-extractor helper function

Hi Thomas,

Quoted 20 lines · Thomas Petazzoni

Dear Thomas De Schampheleire,

Quoted 12 lines · Thomas De Schampheleire
Quoted 5 lines

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"

Quoted 3 lines

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

Quoted 8 lines

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

author
#6 lore raw

PATCH v2 1/1 xzcat: treat as host prerequisite and build if needed

In reply to 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 <[email protected]>

---
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(-)
package/pkg-generic.mk +4 -0
@@ -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
support/dependencies/check-host-xzcat.mk new file +7 -0
new file mode 100644
@@ -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
support/dependencies/check-host-xzcat.sh new file +14 -0
new file mode 100755
@@ -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