[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Baruch Siach]

Don't special case $(XZCAT) when constructing DL_TOOLS_DEPENDENCIES. The next
commit will introduce another extractor that automatically builds when not
installed. Introduce EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS that lists
archive extensions for which the extractor is already checked in
support/dependencies/check-host-foo.mk. Use this in the newly introduced
extractor-dependency to populate DL_TOOLS_DEPENDENCIES.

Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v3: new patch; split from the next as preparatory cleanup (Thomas DS)
---
 package/pkg-generic.mk                   | 8 +-------
 package/pkg-utils.mk                     | 7 +++++++
 support/dependencies/check-host-xzcat.mk | 1 +
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3ca71b03b9df..e8a8021e3c37 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -926,13 +926,7 @@ else ifeq ($$($(2)_SITE_METHOD),cvs)
 DL_TOOLS_DEPENDENCIES += cvs
 endif # SITE_METHOD
 
-# $(firstword) is used here because the extractor can have arguments, like
-# ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
-# 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 += $$(firstword $$(call suitable-extractor,$$($(2)_SOURCE)))
-endif
+DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 
 # Ensure all virtual targets are PHONY. Listed alphabetically.
 .PHONY:	$(1) \
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index c5d4080c72f4..4821456da5b0 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -45,6 +45,13 @@ INFLATE.tar  = cat
 # suitable-extractor(filename): returns extractor based on suffix
 suitable-extractor = $(INFLATE$(suffix $(1)))
 
+# extractor-dependency(filename): returns extractor for 'filename' if the
+# extractor is a dependency. If we build the extractor return nothing.
+# $(firstword) is used here because the extractor can have arguments, like
+# ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
+extractor-dependency = $(firstword$(INFLATE$(filter-out \
+	$(EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS),$(suffix $(1))))
+
 # check-deprecated-variable -- throw an error on deprecated variables
 # example:
 #   $(eval $(call check-deprecated-variable,FOO_MAKE_OPT,FOO_MAKE_OPTS))
diff --git a/support/dependencies/check-host-xzcat.mk b/support/dependencies/check-host-xzcat.mk
index 5e08b6e8866e..c6d9eebe4d2c 100644
--- a/support/dependencies/check-host-xzcat.mk
+++ b/support/dependencies/check-host-xzcat.mk
@@ -3,5 +3,6 @@
 
 ifeq (,$(call suitable-host-package,xzcat,$(XZCAT)))
 DEPENDENCIES_HOST_PREREQ += host-xz
+EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .xz .lzma
 XZCAT = $(HOST_DIR)/usr/bin/xzcat
 endif
-- 
2.11.0

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Baruch Siach]

This commit teaches the generic package handling code how to extract .tar.lz
archives. When lzip is not installed on the host, host-lzip gets built
automatically.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v3: split extractor-dependency to a separate patch (Thomas DS)
v2: add host-extractor to fix host lzip check
---
 Config.in                               |  7 +++++++
 Makefile                                |  1 +
 package/pkg-utils.mk                    |  1 +
 support/dependencies/check-host-lzip.mk |  5 +++++
 support/dependencies/check-host-lzip.sh | 14 ++++++++++++++
 5 files changed, 28 insertions(+)
 create mode 100644 support/dependencies/check-host-lzip.mk
 create mode 100755 support/dependencies/check-host-lzip.sh

diff --git a/Config.in b/Config.in
index ccd777e8bb00..bd8f0d1a10ee 100644
--- a/Config.in
+++ b/Config.in
@@ -158,6 +158,13 @@ config BR2_XZCAT
 	  Command to be used to extract a xz'ed file to stdout.
 	  Default is "xzcat"
 
+config BR2_LZCAT
+	string "lzcat command"
+	default "lzip -d -c"
+	help
+	  Command to be used to extract a lzip'ed file to stdout.
+	  Default is "lzip -d -c"
+
 config BR2_TAR_OPTIONS
 	string "Tar options"
 	default ""
diff --git a/Makefile b/Makefile
index df3b64eb03ec..b4550e098958 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ KERNEL_ARCH := $(shell echo "$(ARCH)" | sed -e "s/-.*//" \
 ZCAT := $(call qstrip,$(BR2_ZCAT))
 BZCAT := $(call qstrip,$(BR2_BZCAT))
 XZCAT := $(call qstrip,$(BR2_XZCAT))
+LZCAT := $(call qstrip,$(BR2_LZCAT))
 TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
 
 # packages compiled for the host go here
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 4821456da5b0..9c9b3bea9f30 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -36,6 +36,7 @@ pkgname = $(lastword $(subst /, ,$(pkgdir)))
 # Define extractors for different archive suffixes
 INFLATE.bz2  = $(BZCAT)
 INFLATE.gz   = $(ZCAT)
+INFLATE.lz   = $(LZCAT)
 INFLATE.lzma = $(XZCAT)
 INFLATE.tbz  = $(BZCAT)
 INFLATE.tbz2 = $(BZCAT)
diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
new file mode 100644
index 000000000000..708105acd892
--- /dev/null
+++ b/support/dependencies/check-host-lzip.mk
@@ -0,0 +1,5 @@
+ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
+DEPENDENCIES_HOST_PREREQ += host-lzip
++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
+LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c
+endif
diff --git a/support/dependencies/check-host-lzip.sh b/support/dependencies/check-host-lzip.sh
new file mode 100755
index 000000000000..4f8a2ba3de5b
--- /dev/null
+++ b/support/dependencies/check-host-lzip.sh
@@ -0,0 +1,14 @@
+#!/bin/sh
+
+candidate="$1"
+
+lzip=`which $candidate 2>/dev/null`
+if [ ! -x "$lzip" ]; then
+	lzip=`which lzip 2>/dev/null`
+	if [ ! -x "$lzip" ]; then
+		# echo nothing: no suitable lzip found
+		exit 1
+	fi
+fi
+
+echo $lzip
-- 
2.11.0

[Buildroot] [PATCH v3 3/5] ed: use generic extract command

[Baruch Siach]

The generic code now knows how to extract .tar.lz archives. Remove the local
extract code.

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/ed/ed.mk | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/package/ed/ed.mk b/package/ed/ed.mk
index 50adeb4ec5b4..6b9f65ec5280 100644
--- a/package/ed/ed.mk
+++ b/package/ed/ed.mk
@@ -10,15 +10,9 @@ ED_SOURCE = ed-$(ED_VERSION).tar.lz
 ED_CONF_OPTS = \
 	CC="$(TARGET_CC)" CFLAGS="$(TARGET_CFLAGS)" \
 	LDFLAGS="$(TARGET_LDFLAGS)"
-ED_DEPENDENCIES = host-lzip
 ED_LICENSE = GPLv3+
 ED_LICENSE_FILES = COPYING
 
-define ED_EXTRACT_CMDS
-	$(HOST_DIR)/usr/bin/lzip -d -c $(DL_DIR)/$(ED_SOURCE) | \
-		tar --strip-components=1 -C $(@D) $(TAR_OPTIONS) -
-endef
-
 define ED_CONFIGURE_CMDS
 	(cd $(@D); \
 		$(TARGET_MAKE_ENV) ./configure \
-- 
2.11.0

[Buildroot] [PATCH v3 4/5] ddrescue: use generic extract command

[Baruch Siach]

The generic code now knows how to extract .tar.lz archives. Remove the local
extract code

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/ddrescue/ddrescue.mk | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/package/ddrescue/ddrescue.mk b/package/ddrescue/ddrescue.mk
index 145d0520d66f..4e244e6bdaef 100644
--- a/package/ddrescue/ddrescue.mk
+++ b/package/ddrescue/ddrescue.mk
@@ -9,12 +9,6 @@ DDRESCUE_SOURCE = ddrescue-$(DDRESCUE_VERSION).tar.lz
 DDRESCUE_SITE = http://download.savannah.gnu.org/releases/ddrescue
 DDRESCUE_LICENSE = GPLv2+
 DDRESCUE_LICENSE_FILES = COPYING
-DDRESCUE_DEPENDENCIES = host-lzip
-
-define DDRESCUE_EXTRACT_CMDS
-	$(HOST_DIR)/usr/bin/lzip -d -c $(DL_DIR)/$(DDRESCUE_SOURCE) | \
-		tar --strip-components=1 -C $(@D) $(TAR_OPTIONS) -
-endef
 
 define DDRESCUE_CONFIGURE_CMDS
 	(cd $(@D); \
-- 
2.11.0

[Buildroot] [PATCH v3 5/5] ocrad: use generic extract command

[Baruch Siach]

The generic code now knows how to extract .tar.lz archives. Remove the local
extract code

Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/ocrad/ocrad.mk | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/package/ocrad/ocrad.mk b/package/ocrad/ocrad.mk
index d19e83ef4279..54b607c79193 100644
--- a/package/ocrad/ocrad.mk
+++ b/package/ocrad/ocrad.mk
@@ -10,12 +10,6 @@ OCRAD_SITE = $(BR2_GNU_MIRROR)/ocrad
 OCRAD_LICENSE = GPLv3+
 OCRAD_LICENSE_FILES = COPYING
 OCRAD_INSTALL_STAGING = YES
-OCRAD_DEPENDENCIES = host-lzip
-
-define OCRAD_EXTRACT_CMDS
-	$(HOST_DIR)/usr/bin/lzip -d -c $(DL_DIR)/$(OCRAD_SOURCE) | \
-		tar --strip-components=1 -C $(@D) $(TAR_OPTIONS) -
-endef
 
 # This is not a true autotools package.
 define OCRAD_CONFIGURE_CMDS
-- 
2.11.0

[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Thomas De Schampheleire]

On Sun, Feb 12, 2017 at 9:15 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Don't special case $(XZCAT) when constructing DL_TOOLS_DEPENDENCIES. The next
> commit will introduce another extractor that automatically builds when not
> installed. Introduce EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS that lists
> archive extensions for which the extractor is already checked in
> support/dependencies/check-host-foo.mk. Use this in the newly introduced
> extractor-dependency to populate DL_TOOLS_DEPENDENCIES.
>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v3: new patch; split from the next as preparatory cleanup (Thomas DS)
> ---
>  package/pkg-generic.mk                   | 8 +-------
>  package/pkg-utils.mk                     | 7 +++++++
>  support/dependencies/check-host-xzcat.mk | 1 +
>  3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 3ca71b03b9df..e8a8021e3c37 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -926,13 +926,7 @@ else ifeq ($$($(2)_SITE_METHOD),cvs)
>  DL_TOOLS_DEPENDENCIES += cvs
>  endif # SITE_METHOD
>
> -# $(firstword) is used here because the extractor can have arguments, like
> -# ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
> -# 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 += $$(firstword $$(call suitable-extractor,$$($(2)_SOURCE)))
> -endif
> +DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>
>  # Ensure all virtual targets are PHONY. Listed alphabetically.
>  .PHONY:        $(1) \
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index c5d4080c72f4..4821456da5b0 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -45,6 +45,13 @@ INFLATE.tar  = cat
>  # suitable-extractor(filename): returns extractor based on suffix
>  suitable-extractor = $(INFLATE$(suffix $(1)))
>
> +# extractor-dependency(filename): returns extractor for 'filename' if the
> +# extractor is a dependency. If we build the extractor return nothing.
> +# $(firstword) is used here because the extractor can have arguments, like
> +# ZCAT="gzip -d -c", and to check for the dependency we only want 'gzip'.
> +extractor-dependency = $(firstword$(INFLATE$(filter-out \

There needs to be a space after 'firstword', otherwise this gets
expanded to $(firstword.gz) for example, which is empty.

With that fixed:

Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas De Schampheleire]

On Sun, Feb 12, 2017 at 9:15 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> This commit teaches the generic package handling code how to extract .tar.lz
> archives. When lzip is not installed on the host, host-lzip gets built
> automatically.
>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v3: split extractor-dependency to a separate patch (Thomas DS)
> v2: add host-extractor to fix host lzip check
> ---
>  Config.in                               |  7 +++++++
>  Makefile                                |  1 +
>  package/pkg-utils.mk                    |  1 +
>  support/dependencies/check-host-lzip.mk |  5 +++++
>  support/dependencies/check-host-lzip.sh | 14 ++++++++++++++
>  5 files changed, 28 insertions(+)
>  create mode 100644 support/dependencies/check-host-lzip.mk
>  create mode 100755 support/dependencies/check-host-lzip.sh
>
> diff --git a/Config.in b/Config.in
> index ccd777e8bb00..bd8f0d1a10ee 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -158,6 +158,13 @@ config BR2_XZCAT
>           Command to be used to extract a xz'ed file to stdout.
>           Default is "xzcat"
>
> +config BR2_LZCAT
> +       string "lzcat command"
> +       default "lzip -d -c"
> +       help
> +         Command to be used to extract a lzip'ed file to stdout.
> +         Default is "lzip -d -c"
> +
>  config BR2_TAR_OPTIONS
>         string "Tar options"
>         default ""
> diff --git a/Makefile b/Makefile
> index df3b64eb03ec..b4550e098958 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -431,6 +431,7 @@ KERNEL_ARCH := $(shell echo "$(ARCH)" | sed -e "s/-.*//" \
>  ZCAT := $(call qstrip,$(BR2_ZCAT))
>  BZCAT := $(call qstrip,$(BR2_BZCAT))
>  XZCAT := $(call qstrip,$(BR2_XZCAT))
> +LZCAT := $(call qstrip,$(BR2_LZCAT))
>  TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
>
>  # packages compiled for the host go here
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 4821456da5b0..9c9b3bea9f30 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -36,6 +36,7 @@ pkgname = $(lastword $(subst /, ,$(pkgdir)))
>  # Define extractors for different archive suffixes
>  INFLATE.bz2  = $(BZCAT)
>  INFLATE.gz   = $(ZCAT)
> +INFLATE.lz   = $(LZCAT)
>  INFLATE.lzma = $(XZCAT)
>  INFLATE.tbz  = $(BZCAT)
>  INFLATE.tbz2 = $(BZCAT)
> diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
> new file mode 100644
> index 000000000000..708105acd892
> --- /dev/null
> +++ b/support/dependencies/check-host-lzip.mk
> @@ -0,0 +1,5 @@
> +ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
> +DEPENDENCIES_HOST_PREREQ += host-lzip
> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c
> +endif
> diff --git a/support/dependencies/check-host-lzip.sh b/support/dependencies/check-host-lzip.sh
> new file mode 100755
> index 000000000000..4f8a2ba3de5b
> --- /dev/null
> +++ b/support/dependencies/check-host-lzip.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +candidate="$1"
> +
> +lzip=`which $candidate 2>/dev/null`
> +if [ ! -x "$lzip" ]; then
> +       lzip=`which lzip 2>/dev/null`
> +       if [ ! -x "$lzip" ]; then
> +               # echo nothing: no suitable lzip found
> +               exit 1
> +       fi
> +fi
> +
> +echo $lzip

Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Thomas Petazzoni]

Hello,

On Sun, 12 Feb 2017 22:15:38 +0200, Baruch Siach wrote:
> Don't special case $(XZCAT) when constructing DL_TOOLS_DEPENDENCIES. The next
> commit will introduce another extractor that automatically builds when not
> installed. Introduce EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS that lists
> archive extensions for which the extractor is already checked in
> support/dependencies/check-host-foo.mk. Use this in the newly introduced
> extractor-dependency to populate DL_TOOLS_DEPENDENCIES.
> 
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

I thought this patch series would be material for the next branch, as
to me it was just an improvement/cleanup.

But in fact, it is actually a fix: all packages that depend on
host-lzip today are broken. Take ddrescue for example (but ed and ocrad
are doing the same):

DDRESCUE_DEPENDENCIES = host-lzip

define DDRESCUE_EXTRACT_CMDS
        $(HOST_DIR)/usr/bin/lzip -d -c $(DL_DIR)/$(DDRESCUE_SOURCE) | \
                tar --strip-components=1 -C $(@D) $(TAR_OPTIONS) -
endef

So they put host-lzip in <pkg>_DEPENDENCIES, and use
$(HOST_DIR)/usr/bin/lzip in the extract step.

The problem is that the dependencies in <pkg>_DEPENDENCIES are only
guaranteed to be available before the *configure* step of the package,
not before the extract step. So if you run:

$ make ddrescue-patch

You currently get:

/home/thomas/projets/buildroot/output/host/usr/bin/lzip -d -c /home/thomas/dl/ddrescue-1.22.tar.lz | tar --strip-components=1 -C /home/thomas/projets/buildroot/output/build/ddrescue-1.22  -xf -
/bin/bash: /home/thomas/projets/buildroot/output/host/usr/bin/lzip: No such file or directory
tar: This does not look like a tar archive
tar: Exiting with failure status due to previous errors

So I believe this patch series should be applied on master, because it
adds host-lzip to DEPENDENCIES_HOST_PREREQ, which is the only solution
to guarantee it will be available to extract packages.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Baruch Siach]

Hi Thomas,

On Tue, Feb 14, 2017 at 10:39:38PM +0100, Thomas Petazzoni wrote:
> On Sun, 12 Feb 2017 22:15:38 +0200, Baruch Siach wrote:
> > Don't special case $(XZCAT) when constructing DL_TOOLS_DEPENDENCIES. The next
> > commit will introduce another extractor that automatically builds when not
> > installed. Introduce EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS that lists
> > archive extensions for which the extractor is already checked in
> > support/dependencies/check-host-foo.mk. Use this in the newly introduced
> > extractor-dependency to populate DL_TOOLS_DEPENDENCIES.
> > 
> > Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> I thought this patch series would be material for the next branch, as
> to me it was just an improvement/cleanup.
> 
> But in fact, it is actually a fix: all packages that depend on
> host-lzip today are broken. Take ddrescue for example (but ed and ocrad
> are doing the same):
> 
> DDRESCUE_DEPENDENCIES = host-lzip
> 
> define DDRESCUE_EXTRACT_CMDS
>         $(HOST_DIR)/usr/bin/lzip -d -c $(DL_DIR)/$(DDRESCUE_SOURCE) | \
>                 tar --strip-components=1 -C $(@D) $(TAR_OPTIONS) -
> endef
> 
> So they put host-lzip in <pkg>_DEPENDENCIES, and use
> $(HOST_DIR)/usr/bin/lzip in the extract step.
> 
> The problem is that the dependencies in <pkg>_DEPENDENCIES are only
> guaranteed to be available before the *configure* step of the package,
> not before the extract step. So if you run:
> 
> $ make ddrescue-patch
> 
> You currently get:
> 
> /home/thomas/projets/buildroot/output/host/usr/bin/lzip -d -c /home/thomas/dl/ddrescue-1.22.tar.lz | tar --strip-components=1 -C /home/thomas/projets/buildroot/output/build/ddrescue-1.22  -xf -
> /bin/bash: /home/thomas/projets/buildroot/output/host/usr/bin/lzip: No such file or directory
> tar: This does not look like a tar archive
> tar: Exiting with failure status due to previous errors
> 
> So I believe this patch series should be applied on master, because it
> adds host-lzip to DEPENDENCIES_HOST_PREREQ, which is the only solution
> to guarantee it will be available to extract packages.

To be on the safe side we might replace <pkg>_DEPENDENCIES with 
DEPENDENCIES_HOST_PREREQ for master, and apply this series to next. Not sure 
it's worth it though.

Should I resend the series to fix the issue that Thomas DS spotted?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] [PATCH v3 1/5] package: refactor listing of extractor dependencies

[Thomas Petazzoni]

Hello,

On Sun, 12 Feb 2017 22:15:38 +0200, Baruch Siach wrote:
> Don't special case $(XZCAT) when constructing DL_TOOLS_DEPENDENCIES. The next
> commit will introduce another extractor that automatically builds when not
> installed. Introduce EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS that lists
> archive extensions for which the extractor is already checked in
> support/dependencies/check-host-foo.mk. Use this in the newly introduced
> extractor-dependency to populate DL_TOOLS_DEPENDENCIES.
> 
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

I've applied the entire series to master, after fixing the
$(firstword ...) function call, as noticed by Thomas DS. I'll make some
comments on PATCH 2/5 though.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas Petazzoni]

Hello,

On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:

> diff --git a/Makefile b/Makefile
> index df3b64eb03ec..b4550e098958 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -431,6 +431,7 @@ KERNEL_ARCH := $(shell echo "$(ARCH)" | sed -e "s/-.*//" \
>  ZCAT := $(call qstrip,$(BR2_ZCAT))
>  BZCAT := $(call qstrip,$(BR2_BZCAT))
>  XZCAT := $(call qstrip,$(BR2_XZCAT))
> +LZCAT := $(call qstrip,$(BR2_LZCAT))

So here, we use the value of the config option BR2_LZCAT.

> diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
> new file mode 100644
> index 000000000000..708105acd892
> --- /dev/null
> +++ b/support/dependencies/check-host-lzip.mk
> @@ -0,0 +1,5 @@
> +ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
> +DEPENDENCIES_HOST_PREREQ += host-lzip
> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c

But here in the case where we are building our own host-lzip, we
completely ignore BR2_LZCAT, and use a hardcoded
$(HOST_DIR)/usr/bin/lzip -d -c.

Since there is already the exact same pattern for XZCAT, I decided to
apply your patch anyway. I also don't really understand the use case
for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
Peter, maybe you can shed some light on why we have these?

Also Baruch: there was one too many "+" at the beginning of:

> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz

so I've fixed that before applying.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Arnout Vandecappelle]

On 15-02-17 22:15, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:
> 
>> diff --git a/Makefile b/Makefile
>> index df3b64eb03ec..b4550e098958 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -431,6 +431,7 @@ KERNEL_ARCH := $(shell echo "$(ARCH)" | sed -e "s/-.*//" \
>>  ZCAT := $(call qstrip,$(BR2_ZCAT))
>>  BZCAT := $(call qstrip,$(BR2_BZCAT))
>>  XZCAT := $(call qstrip,$(BR2_XZCAT))
>> +LZCAT := $(call qstrip,$(BR2_LZCAT))
> 
> So here, we use the value of the config option BR2_LZCAT.
> 
>> diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
>> new file mode 100644
>> index 000000000000..708105acd892
>> --- /dev/null
>> +++ b/support/dependencies/check-host-lzip.mk
>> @@ -0,0 +1,5 @@
>> +ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
>> +DEPENDENCIES_HOST_PREREQ += host-lzip
>> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
>> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c
> 
> But here in the case where we are building our own host-lzip, we
> completely ignore BR2_LZCAT, and use a hardcoded
> $(HOST_DIR)/usr/bin/lzip -d -c.

 Er, yes of course: if BR2_LZCAT doesn't work, then we use the Buildroot
internal lzcat. What else did you expect?


> Since there is already the exact same pattern for XZCAT, I decided to
> apply your patch anyway. I also don't really understand the use case
> for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
> Peter, maybe you can shed some light on why we have these?

 As I understand it, these options are provided in case you have an old build
host and have locally installed these tools in e.g. your homedir. However, I
wouldn't mind getting rid of these options completely, and instead require that
they are in PATH.

 True, they could also be used to pass alternative options to the extractors,
but I don't see much point of that. Or they could be used to call it in an
alternative form, e.g. "zcat" instead of "gzip -d -c", or "busybox gzip -d -c".
But I also don't see much point of that.

 While we're on the subject, I don't see much point of BR2_TAR_OPTIONS either.


 But of course, it's not as if keeping these things is such a burden. So I at
least won't spend time in removing them.


 Regards,
 Arnout


> 
> Also Baruch: there was one too many "+" at the beginning of:
> 
>> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
> 
> so I've fixed that before applying.
> 
> Thanks,
> 
> Thomas
> 

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas Petazzoni]

Hello,

On Wed, 15 Feb 2017 23:48:13 +0100, Arnout Vandecappelle wrote:

> >> +DEPENDENCIES_HOST_PREREQ += host-lzip
> >> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
> >> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c  
> > 
> > But here in the case where we are building our own host-lzip, we
> > completely ignore BR2_LZCAT, and use a hardcoded
> > $(HOST_DIR)/usr/bin/lzip -d -c.  
> 
>  Er, yes of course: if BR2_LZCAT doesn't work, then we use the Buildroot
> internal lzcat. What else did you expect?

BR2_LZCAT does not specify only the path to the lzcat program, but also
its options. And we completely ignore those custom options if the
lzcat built by Buildroot is used.

>  As I understand it, these options are provided in case you have an old build
> host and have locally installed these tools in e.g. your homedir. However, I
> wouldn't mind getting rid of these options completely, and instead require that
> they are in PATH.
> 
>  True, they could also be used to pass alternative options to the extractors,
> but I don't see much point of that. Or they could be used to call it in an
> alternative form, e.g. "zcat" instead of "gzip -d -c", or "busybox gzip -d -c".
> But I also don't see much point of that.
> 
>  While we're on the subject, I don't see much point of BR2_TAR_OPTIONS either.
> 
>  But of course, it's not as if keeping these things is such a burden. So I at
> least won't spend time in removing them.

See above why it doesn't make sense to me: BR2_LZCAT allows to specify
not only the path/name of the lzip program, but also its options. And
they get completely ignored when host-lzip is built by Buildroot.

Which means one person doing the build on a machine with lzip installed
locally will have BR2_LZCAT taken into account, and another person
doing the same build on a different machine that does not have lzip
installed will not have BR2_LZCAT taken into account.

This is definitely not very consistent IMO.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Arnout Vandecappelle]

On 16-02-17 08:32, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 15 Feb 2017 23:48:13 +0100, Arnout Vandecappelle wrote:
> 
>>>> +DEPENDENCIES_HOST_PREREQ += host-lzip
>>>> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
>>>> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c  
>>>
>>> But here in the case where we are building our own host-lzip, we
>>> completely ignore BR2_LZCAT, and use a hardcoded
>>> $(HOST_DIR)/usr/bin/lzip -d -c.  
>>
>>  Er, yes of course: if BR2_LZCAT doesn't work, then we use the Buildroot
>> internal lzcat. What else did you expect?
> 
> BR2_LZCAT does not specify only the path to the lzcat program, but also
> its options. And we completely ignore those custom options if the
> lzcat built by Buildroot is used.

 It is possible to provide extra options, but these are completely useless.
Well, actually, lzcat is the first one with an option that could be useful:
--threads, to decompress over several cores. Except that --threads isn't
implemented yet for decompression. So, although it is possible to add extra
options in BR2_LZCAT, it just makes no sense.

 Regards,
 Arnout

> 
>>  As I understand it, these options are provided in case you have an old build
>> host and have locally installed these tools in e.g. your homedir. However, I
>> wouldn't mind getting rid of these options completely, and instead require that
>> they are in PATH.
>>
>>  True, they could also be used to pass alternative options to the extractors,
>> but I don't see much point of that. Or they could be used to call it in an
>> alternative form, e.g. "zcat" instead of "gzip -d -c", or "busybox gzip -d -c".
>> But I also don't see much point of that.
>>
>>  While we're on the subject, I don't see much point of BR2_TAR_OPTIONS either.
>>
>>  But of course, it's not as if keeping these things is such a burden. So I at
>> least won't spend time in removing them.
> 
> See above why it doesn't make sense to me: BR2_LZCAT allows to specify
> not only the path/name of the lzip program, but also its options. And
> they get completely ignored when host-lzip is built by Buildroot.
> 
> Which means one person doing the build on a machine with lzip installed
> locally will have BR2_LZCAT taken into account, and another person
> doing the same build on a different machine that does not have lzip
> installed will not have BR2_LZCAT taken into account.
> 
> This is definitely not very consistent IMO.
> 
> Thomas
> 

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas Petazzoni]

Hello,

On Thu, 16 Feb 2017 09:25:55 +0100, Arnout Vandecappelle wrote:

> > BR2_LZCAT does not specify only the path to the lzcat program, but also
> > its options. And we completely ignore those custom options if the
> > lzcat built by Buildroot is used.  
> 
>  It is possible to provide extra options, but these are completely useless.
> Well, actually, lzcat is the first one with an option that could be useful:
> --threads, to decompress over several cores. Except that --threads isn't
> implemented yet for decompression. So, although it is possible to add extra
> options in BR2_LZCAT, it just makes no sense.

My point is not whether it makes sense or not to be able to pass
options to lzip. My point is that we provide a Config.in option to
allow the user to pass additional options, but we don't actually take
into account the value of this option when we build our own host-lzip or
host-xz.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas De Schampheleire]

On Wed, Feb 15, 2017 at 10:15 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:
>
>> diff --git a/Makefile b/Makefile
>> index df3b64eb03ec..b4550e098958 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -431,6 +431,7 @@ KERNEL_ARCH := $(shell echo "$(ARCH)" | sed -e "s/-.*//" \
>>  ZCAT := $(call qstrip,$(BR2_ZCAT))
>>  BZCAT := $(call qstrip,$(BR2_BZCAT))
>>  XZCAT := $(call qstrip,$(BR2_XZCAT))
>> +LZCAT := $(call qstrip,$(BR2_LZCAT))
>
> So here, we use the value of the config option BR2_LZCAT.
>
>> diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
>> new file mode 100644
>> index 000000000000..708105acd892
>> --- /dev/null
>> +++ b/support/dependencies/check-host-lzip.mk
>> @@ -0,0 +1,5 @@
>> +ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
>> +DEPENDENCIES_HOST_PREREQ += host-lzip
>> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
>> +LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c
>
> But here in the case where we are building our own host-lzip, we
> completely ignore BR2_LZCAT, and use a hardcoded
> $(HOST_DIR)/usr/bin/lzip -d -c.
>
> Since there is already the exact same pattern for XZCAT, I decided to
> apply your patch anyway. I also don't really understand the use case
> for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
> Peter, maybe you can shed some light on why we have these?

The background for the xzcat case is **drumroll** old RHEL machines
which do not have xz.
If the host does have xzcat, then the definition of XZCAT from the
config is used.
If the host does NOT have xzcat, then buildroot is instructed to build
it for us, as a host package, and the result will be set in
$(HOST_DIR)/usr/bin/. This is the reason for the 'hardcoded' path. In
this scenario, the variable XZCAT from Makefile does not point to
something valid.

The reason why in general we have config options for some tools like
git, zcat, etc. is to let the user decide what they should be. This is
not only to be able to add certain options, but also because the user
may want to point to a differently named tool with the same behavior.
We can argue that this is too much flexibility and the user could just
as well use a custom wrapper added in PATH.

I personally don't use the flexibility of the config options, but I do
rely on the host-xzcat handling.

>
> Also Baruch: there was one too many "+" at the beginning of:
>
>> ++EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
>
> so I've fixed that before applying.

Sorry, I also missed that in my review...

/Thomas

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas Petazzoni]

Hello,

On Thu, 16 Feb 2017 09:36:28 +0100, Thomas De Schampheleire wrote:

> > Since there is already the exact same pattern for XZCAT, I decided to
> > apply your patch anyway. I also don't really understand the use case
> > for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
> > Peter, maybe you can shed some light on why we have these?  
> 
> The background for the xzcat case is **drumroll** old RHEL machines
> which do not have xz.
> If the host does have xzcat, then the definition of XZCAT from the
> config is used.
> If the host does NOT have xzcat, then buildroot is instructed to build
> it for us, as a host package, and the result will be set in
> $(HOST_DIR)/usr/bin/. This is the reason for the 'hardcoded' path. In
> this scenario, the variable XZCAT from Makefile does not point to
> something valid.

Thanks for the reminder, but I believe I already knew all of this, and
it's not really the point of my concern/question.

> The reason why in general we have config options for some tools like
> git, zcat, etc. is to let the user decide what they should be. This is
> not only to be able to add certain options, but also because the user
> may want to point to a differently named tool with the same behavior.

Right but the various BR2_{GZIP,XZCAT,LZCAT,...} options also encode
options passed to these programs, not only the path to them. If it was
just the path to them, I wouldn't have a problem at all with this being
ignored if Buildroot builds its own host-xz or host-lzip.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Thomas De Schampheleire]

On Feb 16, 2017 09:40, "Thomas Petazzoni" <
thomas.petazzoni@free-electrons.com> wrote:

Hello,

On Thu, 16 Feb 2017 09:36:28 +0100, Thomas De Schampheleire wrote:

> > Since there is already the exact same pattern for XZCAT, I decided to
> > apply your patch anyway. I also don't really understand the use case
> > for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
> > Peter, maybe you can shed some light on why we have these?
>
> The background for the xzcat case is **drumroll** old RHEL machines
> which do not have xz.
> If the host does have xzcat, then the definition of XZCAT from the
> config is used.
> If the host does NOT have xzcat, then buildroot is instructed to build
> it for us, as a host package, and the result will be set in
> $(HOST_DIR)/usr/bin/. This is the reason for the 'hardcoded' path. In
> this scenario, the variable XZCAT from Makefile does not point to
> something valid.

Thanks for the reminder, but I believe I already knew all of this, and
it's not really the point of my concern/question.

> The reason why in general we have config options for some tools like
> git, zcat, etc. is to let the user decide what they should be. This is
> not only to be able to add certain options, but also because the user
> may want to point to a differently named tool with the same behavior.

Right but the various BR2_{GZIP,XZCAT,LZCAT,...} options also encode
options passed to these programs, not only the path to them. If it was
just the path to them, I wouldn't have a problem at all with this being
ignored if Buildroot builds its own host-xz or host-lzip.


I agree that there is a inconsistency. I think the ways to cover it is
either to split the config option in two: one for the path and one for
options, or use firstword to split ourselves.
However, in the case that a user specified a custom tool, then the options
may not be relevant for a buildroot-built version. For example: if I make a
'my-super-extractor' that handles all archive types, and I set this one in
the config options like 'my-super-extractor --xz', 'my-super-extractor
--gz' etc, then these options '--gz' etc will not work on the
buildroot-built 'standard' tool.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170216/e284f28a/attachment.html>

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Arnout Vandecappelle]

On 16-02-17 09:34, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu, 16 Feb 2017 09:25:55 +0100, Arnout Vandecappelle wrote:
> 
>>> BR2_LZCAT does not specify only the path to the lzcat program, but also
>>> its options. And we completely ignore those custom options if the
>>> lzcat built by Buildroot is used.  
>>
>>  It is possible to provide extra options, but these are completely useless.
>> Well, actually, lzcat is the first one with an option that could be useful:
>> --threads, to decompress over several cores. Except that --threads isn't
>> implemented yet for decompression. So, although it is possible to add extra
>> options in BR2_LZCAT, it just makes no sense.
> 
> My point is not whether it makes sense or not to be able to pass
> options to lzip. My point is that we provide a Config.in option to
> allow the user to pass additional options, but we don't actually take
> into account the value of this option when we build our own host-lzip or
> host-xz.

 My point is that we *don't* have an option to be able to pass options to lzip.
We have an option to be able to provide an alternative path to lzip, and when
you use that option you have to remember to add the correct arguments as well.

 Regards,
 Arnout

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Yann E. MORIN]

Arnout, All,

On 2017-02-15 23:48 +0100, Arnout Vandecappelle spake thusly:
> On 15-02-17 22:15, Thomas Petazzoni wrote:
> > On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:
[--SNIP--]
> > Since there is already the exact same pattern for XZCAT, I decided to
> > apply your patch anyway. I also don't really understand the use case
> > for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
> > Peter, maybe you can shed some light on why we have these?
> 
>  As I understand it, these options are provided in case you have an old build
> host and have locally installed these tools in e.g. your homedir. However, I
> wouldn't mind getting rid of these options completely, and instead require that
> they are in PATH.
> 
>  True, they could also be used to pass alternative options to the extractors,
> but I don't see much point of that. Or they could be used to call it in an
> alternative form, e.g. "zcat" instead of "gzip -d -c", or "busybox gzip -d -c".
> But I also don't see much point of that.

Actually, there is a use-case for being able to specify some of those,
at least the downloaders: git, wget, svn, scp...

I use a script that is called in lieue of each downloader:

    BR2_WGET="/path/to/wrapper wget --passive-ftp -nd -t 3"
    BR2_GIT="/path/to/wrapper git"
    and so on...

That script is responsible for memorising, for each package, whether it
was downloaded from the primary mirror, the official site, or the backup
mirror, then acts according to where the package was downloaded:

  - from the primary mirror, nothing is done,

  - from the official site: it sends the archive to the primary mirror,
    and add it to a report,

  - from the backup site: it sends the archive to the primary mirror,
    and adds it to a report.

This is done by a Jenkins job, which gets both reports as the result. If
any of the reports is non-empty, the build is marked failed.

This allows us to:

  - automatically track new dependencies added by Joe Random Developer,
    especially transitive dependencies he might not be aware of,

  - automatically feed our local mirror so we are not dependent on
    upstream or s.b.o.

So, I would argue in favour of keeping the possibility to override at
least the downloaders.

For the (de)compressors, not so much though.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Peter Korsgaard]

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 > Actually, there is a use-case for being able to specify some of those,
 > at least the downloaders: git, wget, svn, scp...

 > I use a script that is called in lieue of each downloader:

 >     BR2_WGET="/path/to/wrapper wget --passive-ftp -nd -t 3"
 >     BR2_GIT="/path/to/wrapper git"
 >     and so on...

 > That script is responsible for memorising, for each package, whether it
 > was downloaded from the primary mirror, the official site, or the backup
 > mirror, then acts according to where the package was downloaded:

 >   - from the primary mirror, nothing is done,

 >   - from the official site: it sends the archive to the primary mirror,
 >     and add it to a report,

 >   - from the backup site: it sends the archive to the primary mirror,
 >     and adds it to a report.

 > This is done by a Jenkins job, which gets both reports as the result. If
 > any of the reports is non-empty, the build is marked failed.

 > This allows us to:

 >   - automatically track new dependencies added by Joe Random Developer,
 >     especially transitive dependencies he might not be aware of,

 >   - automatically feed our local mirror so we are not dependent on
 >     upstream or s.b.o.

Nice, but presumably you could also simply create a directory with
wget/git/svn/.. symlinks to your wrapper and prepend that to your path
before running buildroot, and then change the wrapper to do its stuff
and call further in the path for the real tool, similar to the recently
added date wrapper for reproducable builds?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

>> Since there is already the exact same pattern for XZCAT, I decided to
 >> apply your patch anyway. I also don't really understand the use case
 >> for all those BR2_ZCAT, BR2_BZCAT, BR2_XZCAT, etc. config options.
 >> Peter, maybe you can shed some light on why we have these?

 >  As I understand it, these options are provided in case you have an old build
 > host and have locally installed these tools in e.g. your homedir. However, I
 > wouldn't mind getting rid of these options completely, and instead require that
 > they are in PATH.

 >  True, they could also be used to pass alternative options to the extractors,
 > but I don't see much point of that. Or they could be used to call it in an
 > alternative form, e.g. "zcat" instead of "gzip -d -c", or "busybox gzip -d -c".
 > But I also don't see much point of that.

Like Arnout, I also don't think there's much need for these options -
But yeah, they also don't cause a lot of problems.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Yann E. MORIN]

Peter, All,

On 2017-02-16 23:03 +0100, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>  > Actually, there is a use-case for being able to specify some of those,
>  > at least the downloaders: git, wget, svn, scp...
> 
>  > I use a script that is called in lieue of each downloader:
> 
>  >     BR2_WGET="/path/to/wrapper wget --passive-ftp -nd -t 3"
>  >     BR2_GIT="/path/to/wrapper git"
>  >     and so on...
> 
>  > That script is responsible for memorising, for each package, whether it
>  > was downloaded from the primary mirror, the official site, or the backup
>  > mirror, then acts according to where the package was downloaded:
[--SNIP--]
> Nice, but presumably you could also simply create a directory with
> wget/git/svn/.. symlinks to your wrapper and prepend that to your path
> before running buildroot, and then change the wrapper to do its stuff
> and call further in the path for the real tool, similar to the recently
> added date wrapper for reproducable builds?

Indeed, that would be doable, if a little bit more involved in the
wrapper. But not something overly complex either.

So, overriding the downloaders is in fact not strictly required.

Thanks!

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Arnout Vandecappelle]

On 16-02-17 23:11, Yann E. MORIN wrote:
> Peter, All,
> 
> On 2017-02-16 23:03 +0100, Peter Korsgaard spake thusly:
>>>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
>>  > Actually, there is a use-case for being able to specify some of those,
>>  > at least the downloaders: git, wget, svn, scp...
>>
>>  > I use a script that is called in lieue of each downloader:
>>
>>  >     BR2_WGET="/path/to/wrapper wget --passive-ftp -nd -t 3"
>>  >     BR2_GIT="/path/to/wrapper git"
>>  >     and so on...
>>
>>  > That script is responsible for memorising, for each package, whether it
>>  > was downloaded from the primary mirror, the official site, or the backup
>>  > mirror, then acts according to where the package was downloaded:
> [--SNIP--]
>> Nice, but presumably you could also simply create a directory with
>> wget/git/svn/.. symlinks to your wrapper and prepend that to your path
>> before running buildroot, and then change the wrapper to do its stuff
>> and call further in the path for the real tool, similar to the recently
>> added date wrapper for reproducable builds?
> 
> Indeed, that would be doable, if a little bit more involved in the
> wrapper. But not something overly complex either.
> 
> So, overriding the downloaders is in fact not strictly required.

 Actually, for the downloaders there *are* useful options to override, e.g.
proxies, authentication, certificate stores, ... The configuration variables are
not the most convenient way to use those (especially since you may need to use
different options for different packages, e.g. internal vs. external), but it's
the only thing we have now. Apart from wrapper scripts, of course.

 Regards,
 Arnout

-- 
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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v3 2/5] package: add generic support for lz archives

[Yann E. MORIN]

Arnout, All,

On 2017-02-17 08:43 +0100, Arnout Vandecappelle spake thusly:
> On 16-02-17 23:11, Yann E. MORIN wrote:
> > Peter, All,
> > 
> > On 2017-02-16 23:03 +0100, Peter Korsgaard spake thusly:
> >>>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> >>  > Actually, there is a use-case for being able to specify some of those,
> >>  > at least the downloaders: git, wget, svn, scp...
> >>
> >>  > I use a script that is called in lieue of each downloader:
> >>
> >>  >     BR2_WGET="/path/to/wrapper wget --passive-ftp -nd -t 3"
> >>  >     BR2_GIT="/path/to/wrapper git"
> >>  >     and so on...
> >>
> >>  > That script is responsible for memorising, for each package, whether it
> >>  > was downloaded from the primary mirror, the official site, or the backup
> >>  > mirror, then acts according to where the package was downloaded:
> > [--SNIP--]
> >> Nice, but presumably you could also simply create a directory with
> >> wget/git/svn/.. symlinks to your wrapper and prepend that to your path
> >> before running buildroot, and then change the wrapper to do its stuff
> >> and call further in the path for the real tool, similar to the recently
> >> added date wrapper for reproducable builds?
> > 
> > Indeed, that would be doable, if a little bit more involved in the
> > wrapper. But not something overly complex either.
> > 
> > So, overriding the downloaders is in fact not strictly required.
> 
>  Actually, for the downloaders there *are* useful options to override, e.g.
> proxies, authentication, certificate stores, ... The configuration variables are
> not the most convenient way to use those (especially since you may need to use
> different options for different packages, e.g. internal vs. external),

Besides, options valid in a location may well be invalid in another. For
example, proxies can be different, even in an enterprise network (i.e.
location- or destination-based proxies). So those should be in the
environment of the user doing the build, not in the configuration.

Sme goes for the certificate stores (just to re-use your examples): an
enterprise proxy could well be an MITM proxy, so would need its certs to
be used. Or as you said, an internal server may require a set of certs
and another server another set, so that would need different options
per-package. Which we already offer, by the way...

> but it's
> the only thing we have now. Apart from wrapper scripts, of course.

I come to believe that using wrappers in those conditions is the best
answer we can suggest.

IMHO, setting site-local options in the config is toxic.

Regards,
Yann E. MORIN.

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

[Buildroot] Bug in "package: add generic support for lz archives" ?

[Thomas Petazzoni]

Hello,

On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:
> This commit teaches the generic package handling code how to extract .tar.lz
> archives. When lzip is not installed on the host, host-lzip gets built
> automatically.
> 
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

This patch causes a regression: it seems to build host-lzip
unconditionally on systems that lack it, even if host-lzip is not
needed at all to uncompress some tarballs.

Take for example:

BR2_x86_64=y
BR2_x86_atom=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-x86-64-musl-2017.02-rc1-2-g133c5ac.tar.bz2"
BR2_TOOLCHAIN_EXTERNAL_GCC_5=y
BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_9=y
BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
BR2_TOOLCHAIN_EXTERNAL_CXX=y
BR2_INIT_NONE=y
BR2_SYSTEM_BIN_SH_NONE=y
# BR2_PACKAGE_BUSYBOX is not set
BR2_PACKAGE_TCL=y
BR2_PACKAGE_EXPECT=y
# BR2_TARGET_ROOTFS_TAR is not set

Nothing is .lz compressed in there, and still, it builds host-lzip at
the beginning of the build:

>>> host-lzip 1.18 Extracting
>>> host-lzip 1.18 Patching
>>> host-lzip 1.18 Configuring
>>> host-lzip 1.18 Building
>>> host-lzip 1.18 Installing to host directory
>>> skeleton  Extracting
>>> skeleton  Patching
>>> skeleton  Configuring
>>> skeleton  Building
>>> skeleton  Installing to staging directory
>>> skeleton  Fixing libtool files
>>> skeleton  Installing to target

Could you have a look ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] Bug in "package: add generic support for lz archives" ?

[Baruch Siach]

Hi Thomas,

On Tue, Feb 21, 2017 at 10:55:09PM +0100, Thomas Petazzoni wrote:
> On Sun, 12 Feb 2017 22:15:39 +0200, Baruch Siach wrote:
> > This commit teaches the generic package handling code how to extract .tar.lz
> > archives. When lzip is not installed on the host, host-lzip gets built
> > automatically.
> > 
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> This patch causes a regression: it seems to build host-lzip
> unconditionally on systems that lack it, even if host-lzip is not
> needed at all to uncompress some tarballs.
> 
> Take for example:
> 
> BR2_x86_64=y
> BR2_x86_atom=y
> BR2_TOOLCHAIN_EXTERNAL=y
> BR2_TOOLCHAIN_EXTERNAL_CUSTOM=y
> BR2_TOOLCHAIN_EXTERNAL_DOWNLOAD=y
> BR2_TOOLCHAIN_EXTERNAL_URL="http://autobuild.buildroot.org/toolchains/tarballs/br-x86-64-musl-2017.02-rc1-2-g133c5ac.tar.bz2"
> BR2_TOOLCHAIN_EXTERNAL_GCC_5=y
> BR2_TOOLCHAIN_EXTERNAL_HEADERS_4_9=y
> BR2_TOOLCHAIN_EXTERNAL_CUSTOM_MUSL=y
> BR2_TOOLCHAIN_EXTERNAL_CXX=y
> BR2_INIT_NONE=y
> BR2_SYSTEM_BIN_SH_NONE=y
> # BR2_PACKAGE_BUSYBOX is not set
> BR2_PACKAGE_TCL=y
> BR2_PACKAGE_EXPECT=y
> # BR2_TARGET_ROOTFS_TAR is not set
> 
> Nothing is .lz compressed in there, and still, it builds host-lzip at
> the beginning of the build:
> 
> >>> host-lzip 1.18 Extracting
> >>> host-lzip 1.18 Patching
> >>> host-lzip 1.18 Configuring
> >>> host-lzip 1.18 Building
> >>> host-lzip 1.18 Installing to host directory
> >>> skeleton  Extracting
> >>> skeleton  Patching
> >>> skeleton  Configuring
> >>> skeleton  Building
> >>> skeleton  Installing to staging directory
> >>> skeleton  Fixing libtool files
> >>> skeleton  Installing to target
> 
> Could you have a look ?

The xzcat and tar checks behave the same way. There is no logic to determine 
whether a given dependency is actually needed in this particular build.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

[Buildroot] Bug in "package: add generic support for lz archives" ?

[Thomas Petazzoni]

Hello,

On Wed, 22 Feb 2017 07:32:19 +0200, Baruch Siach wrote:

> The xzcat and tar checks behave the same way. There is no logic to determine 
> whether a given dependency is actually needed in this particular build.

Hm, OK. But then it doesn't happen that often that tar or xzcat is not
available, while it does happen a lot for lzip, and we have only a few
packages that use .lz archives.

Hence, to me, it is really against the Buildroot principle of "build
only what's necessary".

What about doing like we do for other download dependencies (like git,
svn, cvs, etc.) and only check them if they are actually needed by one
of the packages that are enabled?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com