[Buildroot] [PATCH] core: postpone evaluation on decompressor dependencies

[Buildroot] [PATCH] core: postpone evaluation on decompressor dependencies

[Yann E. MORIN]

Currently, we build any extractor dependency as soone as the host
utility is missing, even when such utility is not needed.

That is expecially the case for host-lzip, which is lacking except in
the most recent distributions, and thus is always built, which is a bit
of a shame since only very few packages need it.

To fix that, we change the host dependecies check to set a variable
named after the extension they support, to the name of the host package
that provides the necessary decompressor, if not installed on the host.

Then, we introduce a new variable, UNCOMP_TOOLS_DEPENDENCIES, to which
packages, when they are enabled, append their decompressors dependencies,
which they get from the variable set above, to form the list of host
packages that provides a decompressor.

Finally, we add this list of host packages as a (Makefile) dependency
of our 'dependencies' rule, but very late in the Makefile, after all
packages (even from a br2-external tree) have been parsed.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile                                 | 4 ++++
 package/pkg-generic.mk                   | 1 +
 support/dependencies/check-host-lzip.mk  | 2 +-
 support/dependencies/check-host-xzcat.mk | 3 ++-
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 811158295a..6894696ef1 100644
--- a/Makefile
+++ b/Makefile
@@ -511,6 +511,10 @@ include $(BR2_EXTERNAL_FILE)
 # Nothing to include if no BR2_EXTERNAL tree in use
 include $(BR2_EXTERNAL_MKS)
 
+# Apply final core dependencies on uncompressor tools.
+# This must be done after we have parsed all the packages.
+dependencies: $(UNCOMP_TOOLS_DEPENDENCIES)
+
 # Now we are sure we have all the packages scanned and defined. We now
 # check for each package in the list of enabled packages, that all its
 # dependencies are indeed enabled.
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f474704980..5745a9cfcf 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -942,6 +942,7 @@ DL_TOOLS_DEPENDENCIES += cvs
 endif # SITE_METHOD
 
 DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
+UNCOMP_TOOLS_DEPENDENCIES += $$(UNCOMP_TOOL_DEPENDENCY$$(suffix $$($(2)_SOURCE)))
 
 # Ensure all virtual targets are PHONY. Listed alphabetically.
 .PHONY:	$(1) \
diff --git a/support/dependencies/check-host-lzip.mk b/support/dependencies/check-host-lzip.mk
index 6acfdc6dfa..fc0ec07cec 100644
--- a/support/dependencies/check-host-lzip.mk
+++ b/support/dependencies/check-host-lzip.mk
@@ -1,5 +1,5 @@
 ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
-DEPENDENCIES_HOST_PREREQ += host-lzip
+UNCOMP_TOOL_DEPENDENCY.lz = host-lzip
 EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
 LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c
 endif
diff --git a/support/dependencies/check-host-xzcat.mk b/support/dependencies/check-host-xzcat.mk
index c6d9eebe4d..544d2c7aa5 100644
--- a/support/dependencies/check-host-xzcat.mk
+++ b/support/dependencies/check-host-xzcat.mk
@@ -2,7 +2,8 @@
 # If it is not present, build our own host-xzcat
 
 ifeq (,$(call suitable-host-package,xzcat,$(XZCAT)))
-DEPENDENCIES_HOST_PREREQ += host-xz
+UNCOMP_TOOL_DEPENDENCY.xz = host-xz
+UNCOMP_TOOL_DEPENDENCY.lzma = host-xz
 EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .xz .lzma
 XZCAT = $(HOST_DIR)/usr/bin/xzcat
 endif
-- 
2.11.0

[Buildroot] [PATCH] core: postpone evaluation on decompressor dependencies

[Thomas Petazzoni]

Hello,

On Sun,  2 Jul 2017 12:30:30 +0200, Yann E. MORIN wrote:
> Currently, we build any extractor dependency as soone as the host

soone -> soon

> utility is missing, even when such utility is not needed.
> 
> That is expecially the case for host-lzip, which is lacking except in

expecially -> especially

> the most recent distributions, and thus is always built, which is a bit

I'm not sure to understand the "except in the most recent
distributions". Indeed, even in the most recent distributions, lzip may
not be installed.

> of a shame since only very few packages need it.
> 
> To fix that, we change the host dependecies check to set a variable
> named after the extension they support, to the name of the host package
> that provides the necessary decompressor, if not installed on the host.
> 
> Then, we introduce a new variable, UNCOMP_TOOLS_DEPENDENCIES, to which

I think EXTRACT_TOOLS_DEPENDENCIES would be easier to understand. Yes,
it's not technically extraction but uncompression, but they are needed
as part of Buildroot "extract" step for each package, so I believe
EXTRACT_TOOLS_DEPENDENCIES would be better.

> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f474704980..5745a9cfcf 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -942,6 +942,7 @@ DL_TOOLS_DEPENDENCIES += cvs
>  endif # SITE_METHOD
>  
>  DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
> +UNCOMP_TOOLS_DEPENDENCIES += $$(UNCOMP_TOOL_DEPENDENCY$$(suffix $$($(2)_SOURCE)))

I know we looked at it yesterday, but I again don't remember what
extractor-dependency is for. When looking at this code, it seems a bit
weird that extractor dependencies are added to DL_TOOLS_DEPENDENCIES
and "uncompressor dependencies" are added to UNCOMP_TOOLS_DEPENDENCIES.

Can we clarify this ? Should the output of extractor-dependency be added
to UNCOMP_TOOLS_DEPENDENCIES ? Do we need both extractor-dependency and
UNCOMP_TOOL_DEPENDENCY ?

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

[Buildroot] [PATCH] core: postpone evaluation on decompressor dependencies

[Arnout Vandecappelle]

On 02-07-17 12:36, Thomas Petazzoni wrote:
>>  DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>> +UNCOMP_TOOLS_DEPENDENCIES += $$(UNCOMP_TOOL_DEPENDENCY$$(suffix $$($(2)_SOURCE)))
> I know we looked at it yesterday, but I again don't remember what
> extractor-dependency is for. When looking at this code, it seems a bit
> weird that extractor dependencies are added to DL_TOOLS_DEPENDENCIES
> and "uncompressor dependencies" are added to UNCOMP_TOOLS_DEPENDENCIES.
> 
> Can we clarify this ? Should the output of extractor-dependency be added
> to UNCOMP_TOOLS_DEPENDENCIES ? Do we need both extractor-dependency and
> UNCOMP_TOOL_DEPENDENCY ?

 It also took me a bit of browsing to find out what this was about. IIUC,
DL_TOOLS_DEPENDENCIES is not actually a list of dependencies, but rather a list
of tools that should be checked by dependencies.mk. In addition, this list mixes
download tools (e.g. git) with extractor/uncompressor tools - and it could in
fact contain anything else. So it would be better to call it something like
DEPENDENCIES_TO_CHECK.

 Also, this patch only looks at the uncompressor tools, but the same approach
could be applied to the download tools. In fact, I'd like those two to be
refactored a little.

 Ideally, we'd even have a distinct name for dependencies.mk "things" (i.e.
tools that may or may not be available on the host and, for some of them, that
we build on demand) and FOO_DEPENDENCIES "things" (i.e. packages that we need to
build before package foo).

 So perhaps:
DL_TOOLS_DEPENDENCIES -> TOOLS_TO_CHECK
UNCOMP_TOOLS_DEPENDENCIES -> TOOLS_DEPENDENCIES


 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] core: postpone evaluation on decompressor dependencies

[Thomas De Schampheleire]

2017-07-02 13:18 GMT+02:00 Arnout Vandecappelle <arnout@mind.be>:
>
>
> On 02-07-17 12:36, Thomas Petazzoni wrote:
>>>  DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>>> +UNCOMP_TOOLS_DEPENDENCIES += $$(UNCOMP_TOOL_DEPENDENCY$$(suffix $$($(2)_SOURCE)))
>> I know we looked at it yesterday, but I again don't remember what
>> extractor-dependency is for. When looking at this code, it seems a bit
>> weird that extractor dependencies are added to DL_TOOLS_DEPENDENCIES
>> and "uncompressor dependencies" are added to UNCOMP_TOOLS_DEPENDENCIES.
>>
>> Can we clarify this ? Should the output of extractor-dependency be added
>> to UNCOMP_TOOLS_DEPENDENCIES ? Do we need both extractor-dependency and
>> UNCOMP_TOOL_DEPENDENCY ?
>
>  It also took me a bit of browsing to find out what this was about. IIUC,
> DL_TOOLS_DEPENDENCIES is not actually a list of dependencies, but rather a list
> of tools that should be checked by dependencies.mk. In addition, this list mixes
> download tools (e.g. git) with extractor/uncompressor tools - and it could in
> fact contain anything else. So it would be better to call it something like
> DEPENDENCIES_TO_CHECK.
>
>  Also, this patch only looks at the uncompressor tools, but the same approach
> could be applied to the download tools. In fact, I'd like those two to be
> refactored a little.
>
>  Ideally, we'd even have a distinct name for dependencies.mk "things" (i.e.
> tools that may or may not be available on the host and, for some of them, that
> we build on demand) and FOO_DEPENDENCIES "things" (i.e. packages that we need to
> build before package foo).
>
>  So perhaps:
> DL_TOOLS_DEPENDENCIES -> TOOLS_TO_CHECK
> UNCOMP_TOOLS_DEPENDENCIES -> TOOLS_DEPENDENCIES
>
>

I have been looking at this in the past too. Here is some input from my side:

- host-lzip requires tar to extract. If the tar on the host is not
good enough (RHEL4/5), then host-tar is built, but host-lzip does not
express a dependency on it. As a quick fix we have currently following
patch:

diff --git a/support/dependencies/check-host-lzip.mk
b/support/dependencies/check-host-lzip.mk
--- a/support/dependencies/check-host-lzip.mk
+++ b/support/dependencies/check-host-lzip.mk
@@ -1,4 +1,7 @@
 ifeq (,$(call suitable-host-package,lzip,$(LZCAT)))
+ifeq (,$(call suitable-host-package,tar,tar))
+HOST_LZIP_DEPENDENCIES = host-tar
+endif
 DEPENDENCIES_HOST_PREREQ += host-lzip
 EXTRACTOR_DEPENDENCY_PRECHECKED_EXTENSIONS += .lz
 LZCAT = $(HOST_DIR)/usr/bin/lzip -d -c

- when ccache is enabled, we need to make sure that ccache is not used
for any of the dependencies, also for those in
DEPENDENCIES_HOST_PREREQ, as ccache is not yet necessarily built.
Today, this is happening based on the 'dependencies' target, but if
any of them are built via another path, this is not working. One such
case had to do with the above lzip/tar problem, but I don't recall all
details. Another case is that 'make clean host-ccache' does not work.
Following patch fixes that: (we didn't get around to sending that properly yet)

diff --git a/support/dependencies/dependencies.mk
b/support/dependencies/dependencies.mk
--- a/support/dependencies/dependencies.mk
+++ b/support/dependencies/dependencies.mk
@@ -25,8 +25,8 @@ core-dependencies:
         DL_TOOLS="$(sort $(DL_TOOLS_DEPENDENCIES))" \
         $(TOPDIR)/support/dependencies/dependencies.sh

-dependencies: HOSTCC=$(HOSTCC_NOCCACHE)
-dependencies: HOSTCXX=$(HOSTCXX_NOCCACHE)
+core-dependencies $(DEPENDENCIES_HOST_PREREQ): HOSTCC=$(HOSTCC_NOCCACHE)
+core-dependencies $(DEPENDENCIES_HOST_PREREQ): HOSTCXX=$(HOSTCXX_NOCCACHE)
 dependencies: core-dependencies $(DEPENDENCIES_HOST_PREREQ)

 ################################################################################

- regarding the naming: I agree with Thomas that EXTRACT_TOOLS is
better than UNCOMP_TOOLS;

- but further, like Arnout, I also think that a better split may be
needed. In my rough ideas I had written down:
  TOOLS_DEPENDENCIES_EXPECTED_ON_HOST versus
TOOLS_DEPENDENCIES_BUILT_IF_NEEDED, because that is actually the split
we need, regardless of whether it is a tool used for downloading,
extracting or something else.

Best regards,
Thomas