diff mbox series

[RFCv3,07/15] package/pkg-generic: handle host-tar as an extract dependency

Message ID 20171201205352.24287-8-thomas.petazzoni@free-electrons.com
State Superseded
Headers show
Series Per-package host/target directory support | expand

Commit Message

Thomas Petazzoni Dec. 1, 2017, 8:53 p.m. UTC
This moves the host-tar dependency handling from
DEPENDENCY_HOST_PREREQ to an extract dependency.

To achieve that, check-host-tar.mk fills in the
BR2_TAR_HOST_DEPENDENCY with host-tar if building a host-tar is
needed. The name BR2_TAR_HOST_DEPENDENCY has been chosen because it
matches the name BR2_CMAKE_HOST_DEPENDENCY already used in
check-host-cmake.mk.

The BR2_TAR_HOST_DEPENDENCY is added to all packages, except host-tar
itself (obviously) and host-skeleton, because we depend on
host-skeleton to install host-tar properly in HOST_DIR.

In addition, we modify tar.mk to explicitly build host-tar without
ccache: since ccache source code is available as a tarball, ccache
will obviously depend on host-tar if the system tar is insufficient.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v2:
 - New patch
---
 package/pkg-generic.mk                 | 4 ++++
 package/tar/tar.mk                     | 6 ++++++
 support/dependencies/check-host-tar.mk | 2 +-
 3 files changed, 11 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Dec. 2, 2017, 3:06 p.m. UTC | #1
Thomas, All,

On 2017-12-01 21:53 +0100, Thomas Petazzoni spake thusly:
> This moves the host-tar dependency handling from
> DEPENDENCY_HOST_PREREQ to an extract dependency.
> 
> To achieve that, check-host-tar.mk fills in the

s/$/ variable/

> BR2_TAR_HOST_DEPENDENCY with host-tar if building a host-tar is
> needed. The name BR2_TAR_HOST_DEPENDENCY has been chosen because it
> matches the name BR2_CMAKE_HOST_DEPENDENCY already used in
> check-host-cmake.mk.
> 
> The BR2_TAR_HOST_DEPENDENCY is added to all packages, except host-tar
> itself (obviously) and host-skeleton, because we depend on
> host-skeleton to install host-tar properly in HOST_DIR.

As we discussed on IRC, there is also the case for the tar filesystem,
which may require host-tar.

Granted, it is very very unlikely that we end up with no package that
require host-tar, but still...

> In addition, we modify tar.mk to explicitly build host-tar without
> ccache: since ccache source code is available as a tarball, ccache
> will obviously depend on host-tar if the system tar is insufficient.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> Changes since v2:
>  - New patch
> ---
>  package/pkg-generic.mk                 | 4 ++++
>  package/tar/tar.mk                     | 6 ++++++
>  support/dependencies/check-host-tar.mk | 2 +-
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 824c34bec9..86dd2561d4 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -587,6 +587,10 @@ ifneq ($(1),host-skeleton)
>  $(2)_DEPENDENCIES += host-skeleton
>  endif
>  
> +ifeq ($(filter host-tar host-skeleton,$(1)),)
> +$(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
> +endif

So, even packages that are available as a zip file will get the
dependency on host-tar?

>  # Eliminate duplicates in dependencies
>  $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES))
>  $(2)_FINAL_EXTRACT_DEPENDENCIES = $$(sort $$($(2)_EXTRACT_DEPENDENCIES))
> diff --git a/package/tar/tar.mk b/package/tar/tar.mk
> index cb2ddc0eca..813eabed14 100644
> --- a/package/tar/tar.mk
> +++ b/package/tar/tar.mk
> @@ -44,4 +44,10 @@ define HOST_TAR_EXTRACT_CMDS
>  	mv $(@D)/tar-$(TAR_VERSION)/* $(@D)
>  	rmdir $(@D)/tar-$(TAR_VERSION)
>  endef
> +
> +# we are built before ccache
> +HOST_TAR_CONF_ENV = \
> +	CC="$(HOSTCC_NOCCACHE)" \
> +	CXX="$(HOSTCXX_NOCCACHE)"
> +
>  $(eval $(host-autotools-package))
> diff --git a/support/dependencies/check-host-tar.mk b/support/dependencies/check-host-tar.mk
> index ad0b32e277..d07f727c4c 100644
> --- a/support/dependencies/check-host-tar.mk
> +++ b/support/dependencies/check-host-tar.mk
> @@ -1,6 +1,6 @@
>  TAR ?= tar
>  
>  ifeq (,$(call suitable-host-package,tar,$(TAR)))
> -DEPENDENCIES_HOST_PREREQ += host-tar
>  TAR = $(HOST_DIR)/bin/tar
> +BR2_TAR_HOST_DEPENDENCY += host-tar

Why assign-append here, instead of simply assign?

Regards,
Yann E. MORIN.

>  endif
> -- 
> 2.13.6
>
Thomas Petazzoni Dec. 2, 2017, 8:19 p.m. UTC | #2
Hello,

On Sat, 2 Dec 2017 16:06:06 +0100, Yann E. MORIN wrote:

> > To achieve that, check-host-tar.mk fills in the  
> 
> s/$/ variable/

ACK, will fix.

> > The BR2_TAR_HOST_DEPENDENCY is added to all packages, except host-tar
> > itself (obviously) and host-skeleton, because we depend on
> > host-skeleton to install host-tar properly in HOST_DIR.  
> 
> As we discussed on IRC, there is also the case for the tar filesystem,
> which may require host-tar.
> 
> Granted, it is very very unlikely that we end up with no package that
> require host-tar, but still...

Yes, I agree, the tar filesystem should depend on
$(BR2_TAR_HOST_DEPENDENCY) as well. I'll fix that.

> > +ifeq ($(filter host-tar host-skeleton,$(1)),)
> > +$(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
> > +endif  
> 
> So, even packages that are available as a zip file will get the
> dependency on host-tar?

Yes. Like host-xz/host-lzip, my series does not attempt to switch to
more fine-grained dependencies, it only intends to move to proper
package dependencies. Using more fine-grained dependencies is left for
future work.

> >  ifeq (,$(call suitable-host-package,tar,$(TAR)))
> > -DEPENDENCIES_HOST_PREREQ += host-tar
> >  TAR = $(HOST_DIR)/bin/tar
> > +BR2_TAR_HOST_DEPENDENCY += host-tar  
> 
> Why assign-append here, instead of simply assign?

Good point, will fix.

Thanks for the review!

Thomas
Yann E. MORIN Dec. 29, 2017, 3:34 p.m. UTC | #3
Thomas, All,

On 2017-12-02 21:19 +0100, Thomas Petazzoni spake thusly:
> On Sat, 2 Dec 2017 16:06:06 +0100, Yann E. MORIN wrote:
> > > To achieve that, check-host-tar.mk fills in the  
> > s/$/ variable/
> ACK, will fix.
> 
> > > The BR2_TAR_HOST_DEPENDENCY is added to all packages, except host-tar
> > > itself (obviously) and host-skeleton, because we depend on
> > > host-skeleton to install host-tar properly in HOST_DIR.  
> > 
> > As we discussed on IRC, there is also the case for the tar filesystem,
> > which may require host-tar.
> > 
> > Granted, it is very very unlikely that we end up with no package that
> > require host-tar, but still...
> 
> Yes, I agree, the tar filesystem should depend on
> $(BR2_TAR_HOST_DEPENDENCY) as well. I'll fix that.
> 
> > > +ifeq ($(filter host-tar host-skeleton,$(1)),)
> > > +$(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
> > > +endif  
> > 
> > So, even packages that are available as a zip file will get the
> > dependency on host-tar?
> 
> Yes. Like host-xz/host-lzip, my series does not attempt to switch to
> more fine-grained dependencies, it only intends to move to proper
> package dependencies. Using more fine-grained dependencies is left for
> future work.
> 
> > >  ifeq (,$(call suitable-host-package,tar,$(TAR)))
> > > -DEPENDENCIES_HOST_PREREQ += host-tar
> > >  TAR = $(HOST_DIR)/bin/tar
> > > +BR2_TAR_HOST_DEPENDENCY += host-tar  
> > Why assign-append here, instead of simply assign?
> Good point, will fix.

Once the two minor issues are fixed:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.
diff mbox series

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 824c34bec9..86dd2561d4 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -587,6 +587,10 @@  ifneq ($(1),host-skeleton)
 $(2)_DEPENDENCIES += host-skeleton
 endif
 
+ifeq ($(filter host-tar host-skeleton,$(1)),)
+$(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
+endif
+
 # Eliminate duplicates in dependencies
 $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES))
 $(2)_FINAL_EXTRACT_DEPENDENCIES = $$(sort $$($(2)_EXTRACT_DEPENDENCIES))
diff --git a/package/tar/tar.mk b/package/tar/tar.mk
index cb2ddc0eca..813eabed14 100644
--- a/package/tar/tar.mk
+++ b/package/tar/tar.mk
@@ -44,4 +44,10 @@  define HOST_TAR_EXTRACT_CMDS
 	mv $(@D)/tar-$(TAR_VERSION)/* $(@D)
 	rmdir $(@D)/tar-$(TAR_VERSION)
 endef
+
+# we are built before ccache
+HOST_TAR_CONF_ENV = \
+	CC="$(HOSTCC_NOCCACHE)" \
+	CXX="$(HOSTCXX_NOCCACHE)"
+
 $(eval $(host-autotools-package))
diff --git a/support/dependencies/check-host-tar.mk b/support/dependencies/check-host-tar.mk
index ad0b32e277..d07f727c4c 100644
--- a/support/dependencies/check-host-tar.mk
+++ b/support/dependencies/check-host-tar.mk
@@ -1,6 +1,6 @@ 
 TAR ?= tar
 
 ifeq (,$(call suitable-host-package,tar,$(TAR)))
-DEPENDENCIES_HOST_PREREQ += host-tar
 TAR = $(HOST_DIR)/bin/tar
+BR2_TAR_HOST_DEPENDENCY += host-tar
 endif