All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/7] support/graph-depends: ensure all packages get graphed
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 2/7] support/graph-depends: also cut on host-skeleton Yann E. MORIN
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

When a package:
  - has no dependency,
  - is not a first-level target,
  - is a cut-off point,

then this package is omitted from the dependency graph.

We currently have no such package, but the next commit will make
host-skeleton, which already matches the first two conditions, a cut-off
point too, and thus host-skeleton would not appear on the graph.

To ensure that all packages, whatever their conditions, are displayed,
we add them as a dependency of 'all'.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/graph-depends | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
index 621e603278..6ce90d872c 100755
--- a/support/scripts/graph-depends
+++ b/support/scripts/graph-depends
@@ -391,6 +391,15 @@ def main():
         deps = get_all_depends(filtered_targets, get_depends_func)
         if deps is not None:
             dependencies += deps
+            # 'all' depends on everything, so add any newly discovered
+            # package. d[0] was a target, above, or a d[1] of another
+            # tuple, so it's already in the list.
+            for d in deps:
+                if d[1] in TARGET_EXCEPTIONS:
+                    continue
+                if ('all', d[1]) in dependencies:
+                    continue
+                dependencies.append(('all', d[1]))
         rootpkg = 'all'
 
     # In pkg mode, start directly with get_all_depends() on the requested
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 2/7] support/graph-depends: also cut on host-skeleton
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 1/7] support/graph-depends: ensure all packages get graphed Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 3/7] support/graph-depends: also cut on host-tar Yann E. MORIN
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

host-skeleton is a dependency of almost all packages, except a very few.
As such, it clutters the dependency graph uselessly.

Do with it as we do for the skeleton: cut the dependency chains.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/graph-depends | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
index 6ce90d872c..40a8eee5e9 100755
--- a/support/scripts/graph-depends
+++ b/support/scripts/graph-depends
@@ -175,10 +175,17 @@ def remove_transitive_deps(pkg, deps):
     return new_d
 
 
+CUT_ON_PACKAGES = [
+    'toolchain',
+    'skeleton',
+    'host-skeleton',
+]
+
+
 # This function removes the dependency on some 'mandatory' package, like the
 # 'toolchain' package, or the 'skeleton' package
 def remove_mandatory_deps(pkg, deps):
-    return [p for p in deps[pkg] if p not in ['toolchain', 'skeleton']]
+    return [p for p in deps[pkg] if p not in CUT_ON_PACKAGES]
 
 
 # This function will check that there is no loop in the dependency chain
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 3/7] support/graph-depends: also cut on host-tar
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 1/7] support/graph-depends: ensure all packages get graphed Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 2/7] support/graph-depends: also cut on host-skeleton Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions Yann E. MORIN
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

When host-tar is needed, it is a mandatory dependency of all packages.
As such, drawing the dependency lines toward host-tar would uselessly
clutter the graph.

So, like for the skeleton and host-skeleton, we cut the dependency chains
toward host-tar.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/graph-depends | 1 +
 1 file changed, 1 insertion(+)

diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends
index 40a8eee5e9..46ff164262 100755
--- a/support/scripts/graph-depends
+++ b/support/scripts/graph-depends
@@ -179,6 +179,7 @@ CUT_ON_PACKAGES = [
     'toolchain',
     'skeleton',
     'host-skeleton',
+    'host-tar',
 ]
 
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs
@ 2018-04-21  9:10 Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 1/7] support/graph-depends: ensure all packages get graphed Yann E. MORIN
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

Hello All!

This series brings two major changes:

 1- consistency cleanups in the dependency graph

 2- ensure that host-tar is available when we need it

The first part, covering the first three patches, is mostly a cleanup
pass, to ensure the graph is not too cluttered, but yet does not miss
out any package. This is pretty trivial, except for the first patch,
which has a detailed commit log: in some conditions, we may have missed
packages from the dependency graph. We did not have that situation with
our internal packages so far, but the host-skeleton cut-off really
uncovered it: when a package with no dependency is also a cut-off
point (to avoid cluterring), and is not a top-level dependency, then
that package is not displayed in the dependency graph, not even as a
dependency of 'ALL'. Fixing is pretty trivial: add any newly-discovered
package asa dependency of 'ALL'; transitivity removal will clanup the
mess afterward.

The second part, not much more complex either, ensures that host-tar is
built and installed before a download backend, that _locally_ generates
a tarball, is called. When a package downloaded from a repository is
downloaded 'early' in the build process, it may well be so-downloaded
before host-tar was built, because host-tar is only an extract
dependency (so far), so the system tar is used, which may generate
archives that do not match the hash we have for them.

As such, a new type of dependency is added, that guarantees ordering
before the download step. Then, host-tar is added as a download
dependency to packages which site method is one of the repository-based
downloads that call to tar: cvs, git and svn (bzr and hg use their
internal tarball generator).

Finally, the filesystems are made to depend on host-tar when it is
needed, too, because we do generate the intermediate tarball, which is
then extracted by all filesystems.


Regards,
Yann E. MORIN.


The following changes since commit d5d305d40c9d65e0e51f4dfe6a43b4766a250bef

  libuv: bump to version 1.20.1 (2018-04-20 16:36:04 +0200)


are available in the git repository at:

  git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to 021f6b14835ca89bc6ecb4b36aa09fd246f7d896

  fs: always depend on build host-tar if needed (2018-04-21 11:05:58 +0200)


----------------------------------------------------------------
Yann E. MORIN (7):
      support/graph-depends: ensure all packages get graphed
      support/graph-depends: also cut on host-skeleton
      support/graph-depends: also cut on host-tar
      core/package: postpone evaluation of dependency conditions
      core/package: add possibility to declare download dependencies
      core/package: add host-tar dependency for downloads from repositories
      fs: always depend on build host-tar if needed

 fs/common.mk                  |  1 +
 package/pkg-generic.mk        | 21 ++++++++++++++-------
 support/scripts/graph-depends | 19 ++++++++++++++++++-
 3 files changed, 33 insertions(+), 8 deletions(-)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
                   ` (2 preceding siblings ...)
  2018-04-21  9:10 ` [Buildroot] [PATCH 3/7] support/graph-depends: also cut on host-tar Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-05-02 22:58   ` Arnout Vandecappelle
  2018-04-21  9:10 ` [Buildroot] [PATCH 5/7] core/package: add possibility to declare download dependencies Yann E. MORIN
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

In the pkg-inner macro, all variables, but he positional arguments,
must be $$-prefixed, so that they are expanded only when the macro
is evaluated in each package, not when the macro is parsed.

It is to be noted, though, that the current code, even though incorrect
by the above rules, seemed to work. However, the upcoming addition of
download dependencies, mimicking that code, would not work unless it
was $$-prefixed.

So, for consistency sake, and for correctness sake, let's always use
the $$-prefix in the inner macro.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Arnout: you the make expert, any idea why on Earth the single-expansion
was working in thefirst place?
---
 package/pkg-generic.mk | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1c9dd1d734..12e3981752 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -560,26 +560,26 @@ ifneq ($(1),host-skeleton)
 $(2)_DEPENDENCIES += host-skeleton
 endif
 
-ifeq ($(filter host-tar host-skeleton host-fakedate,$(1)),)
+ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
 $(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
 endif
 
-ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
+ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
 $(2)_EXTRACT_DEPENDENCIES += $(BR2_XZCAT_HOST_DEPENDENCY)
 endif
 
-ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
+ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
 $(2)_EXTRACT_DEPENDENCIES += $(BR2_LZIP_HOST_DEPENDENCY)
 endif
 
-ifeq ($(BR2_CCACHE),y)
-ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
+ifeq ($$(BR2_CCACHE),y)
+ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
 $(2)_DEPENDENCIES += host-ccache
 endif
 endif
 
-ifeq ($(BR2_REPRODUCIBLE),y)
-ifeq ($(filter host-skeleton host-fakedate,$(1)),)
+ifeq ($$(BR2_REPRODUCIBLE),y)
+ifeq ($$(filter host-skeleton host-fakedate,$(1)),)
 $(2)_DEPENDENCIES += host-fakedate
 endif
 endif
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 5/7] core/package: add possibility to declare download dependencies
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
                   ` (3 preceding siblings ...)
  2018-04-21  9:10 ` [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 6/7] core/package: add host-tar dependency for downloads from repositories Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 7/7] fs: always depend on build host-tar if needed Yann E. MORIN
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

For some packages, we may need to have a certain set of host-tools built
before the download of said packages are attempted. For example, when
the system tar is not suitable, we will want to build our own tar before
we attempt a git download (because we generate a tarball in the git
backend).

Mimick the _EXTRACT_DEPENDENCIES, and introduce _DOWNLOAD_DEPENDENCIES.
As for _EXTRACT_DEPENDENCIES, we do not document _DOWNLOAD_DEPENDENCIES,
on the assumption that it is mostly for internal use.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 package/pkg-generic.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 12e3981752..6222d238c6 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -586,11 +586,13 @@ endif
 
 # Eliminate duplicates in dependencies
 $(2)_FINAL_DEPENDENCIES = $$(sort $$($(2)_DEPENDENCIES))
+$(2)_FINAL_DOWNLOAD_DEPENDENCIES = $$(sort $$($(2)_DOWNLOAD_DEPENDENCIES))
 $(2)_FINAL_EXTRACT_DEPENDENCIES = $$(sort $$($(2)_EXTRACT_DEPENDENCIES))
 $(2)_FINAL_PATCH_DEPENDENCIES = $$(sort $$($(2)_PATCH_DEPENDENCIES))
 $(2)_FINAL_ALL_DEPENDENCIES = \
 	$$(sort \
 		$$($(2)_FINAL_DEPENDENCIES) \
+		$$($(2)_FINAL_DOWNLOAD_DEPENDENCIES) \
 		$$($(2)_FINAL_EXTRACT_DEPENDENCIES) \
 		$$($(2)_FINAL_PATCH_DEPENDENCIES))
 
@@ -717,6 +719,7 @@ $$($(2)_TARGET_EXTRACT): | $$($(2)_FINAL_EXTRACT_DEPENDENCIES)
 $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
+$$($(2)_TARGET_SOURCE): | $$($(2)_FINAL_DOWNLOAD_DEPENDENCIES)
 
 $(1)-all-source:	$(1)-legal-source
 $(1)-legal-info:	$(1)-legal-source
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 6/7] core/package: add host-tar dependency for downloads from repositories
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
                   ` (4 preceding siblings ...)
  2018-04-21  9:10 ` [Buildroot] [PATCH 5/7] core/package: add possibility to declare download dependencies Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  2018-04-21  9:10 ` [Buildroot] [PATCH 7/7] fs: always depend on build host-tar if needed Yann E. MORIN
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

Three of our download backends need a host tar that can generate
reproducible archives: cvs, git, and svn. The other two, hbzr and hg,
use their internal implementation.

So, for those three that need it, and a dependency on host-tar when the
system tar is not appropriate.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Peter Korsgaard <peter@korsgaard.com>
---
 package/pkg-generic.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 6222d238c6..aece46ff0c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -560,6 +560,10 @@ ifneq ($(1),host-skeleton)
 $(2)_DEPENDENCIES += host-skeleton
 endif
 
+ifneq ($$(filter cvs git svn,$$($(2)_SITE_METHOD)),)
+$(2)_DOWNLOAD_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
+endif
+
 ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
 $(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
 endif
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 7/7] fs: always depend on build host-tar if needed
  2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
                   ` (5 preceding siblings ...)
  2018-04-21  9:10 ` [Buildroot] [PATCH 6/7] core/package: add host-tar dependency for downloads from repositories Yann E. MORIN
@ 2018-04-21  9:10 ` Yann E. MORIN
  6 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-04-21  9:10 UTC (permalink / raw)
  To: buildroot

Currently, the filesystems do not depend on building host-tar when it is
needed, even though all of them have to extract the intermediate tarball.

However, in degenerate (but legally valid) configurations with no
user-selectable package selected, host-tar would not be built, so the
rootfs images would use whatever improper tar the system has.

Add the conditional dependency to host-tar to the rootfs-common
intermediate image. Since this is the internal step that all real rootfs
generators depend on, they now properly depend on host-tar when needed.

In practice, when host-tar is needed, it will always be built before the
rootfs inages, because it is a dependency of all packages (except a very
few, like the skeleton), of which host-fakeroot, which is a mandatory
dependency of rootfs-comon anyway. But for consistency sake, let's
explicitly add host-tar as a dependency to rootfs-common too.

Note that rootfs-tar already had that dependency, and we leave it as-is
because it is semantically correct, even if superfluous.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

---
Note: yes, I had a randpackageconfig turn up a configuration with no
user-selectable package enabled... I should just stop coding and go
play bingo. Too bad there is no big jackpot tonight... ;-]
---
 fs/common.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/common.mk b/fs/common.mk
index 9baf367729..d106783813 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -63,6 +63,7 @@ ROOTFS_COMMON_TARGET_DIR = $(FS_DIR)/target
 
 ROOTFS_COMMON_DEPENDENCIES = \
 	host-fakeroot host-makedevs \
+	$(BR2_TAR_HOST_DEPENDENCY) \
 	$(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd)
 
 $(ROOTFS_COMMON_TAR): ROOTFS=COMMON
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions
  2018-04-21  9:10 ` [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions Yann E. MORIN
@ 2018-05-02 22:58   ` Arnout Vandecappelle
  2018-05-03 19:32     ` Yann E. MORIN
  0 siblings, 1 reply; 10+ messages in thread
From: Arnout Vandecappelle @ 2018-05-02 22:58 UTC (permalink / raw)
  To: buildroot



On 21-04-18 11:10, Yann E. MORIN wrote:
> In the pkg-inner macro, all variables, but he positional arguments,
> must be $$-prefixed, so that they are expanded only when the macro
> is evaluated in each package, not when the macro is parsed.
> 
> It is to be noted, though, that the current code, even though incorrect
> by the above rules, seemed to work. However, the upcoming addition of
> download dependencies, mimicking that code, would not work unless it
> was $$-prefixed.
> 
> So, for consistency sake, and for correctness sake, let's always use
> the $$-prefix in the inner macro.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> Arnout: you the make expert, any idea why on Earth the single-expansion
> was working in thefirst place?

[You put me in Cc of too many patches, so I don't notice when you *really* want
my response :-]

 In most cases, single-$ will work as well. In this particular case, after first
expansion you will get:

ifeq (,)
FOO_EXTRACT_DEPENDENCIES += host-tar
endif

which in the $(eval ...) will get evaluated to nothing.

 After this patch, after first expansion you get

ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,foo),)
FOO_EXTRACT_DEPENDENCIES += host-tar
endif

which the $(eval ...) still evaluates to nothing.


 The cases where $$ is really needed are fairly rare. The most obvious ones are
when you have an ifeq() using a variable that is defined in the same
inner-generic-package expansion. Less obvious but important for consistency is
to keep the "late binding" property. With single-$, the variable is expanded
immediately, so if it is still updated later in the makefiles, the new values
won't be used anymore. This would e.g. break $(1)-show-rdepends.


 Because it is very hard to remember when exactly you should use $$, we
introduced the rule: use $$ *everywhere*, except for $(1), $(2) etc. Thomas's
patches that fixed the dependencies broke that rule.

 Unfortunately, this patch still doesn't fix it...


> ---
>  package/pkg-generic.mk | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 1c9dd1d734..12e3981752 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -560,26 +560,26 @@ ifneq ($(1),host-skeleton)
>  $(2)_DEPENDENCIES += host-skeleton
>  endif
>  
> -ifeq ($(filter host-tar host-skeleton host-fakedate,$(1)),)
> +ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
>  $(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
                                ^^ This should also be $$

and the same below.

 Regards,
 Arnout

>  endif
>  
> -ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
> +ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  $(2)_EXTRACT_DEPENDENCIES += $(BR2_XZCAT_HOST_DEPENDENCY)
>  endif
>  
> -ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
> +ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate,$(1)),)
>  $(2)_EXTRACT_DEPENDENCIES += $(BR2_LZIP_HOST_DEPENDENCY)
>  endif
>  
> -ifeq ($(BR2_CCACHE),y)
> -ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
> +ifeq ($$(BR2_CCACHE),y)
> +ifeq ($$(filter host-tar host-skeleton host-xz host-lzip host-fakedate host-ccache,$(1)),)
>  $(2)_DEPENDENCIES += host-ccache
>  endif
>  endif
>  
> -ifeq ($(BR2_REPRODUCIBLE),y)
> -ifeq ($(filter host-skeleton host-fakedate,$(1)),)
> +ifeq ($$(BR2_REPRODUCIBLE),y)
> +ifeq ($$(filter host-skeleton host-fakedate,$(1)),)
>  $(2)_DEPENDENCIES += host-fakedate
>  endif
>  endif
> 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions
  2018-05-02 22:58   ` Arnout Vandecappelle
@ 2018-05-03 19:32     ` Yann E. MORIN
  0 siblings, 0 replies; 10+ messages in thread
From: Yann E. MORIN @ 2018-05-03 19:32 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2018-05-03 00:58 +0200, Arnout Vandecappelle spake thusly:
> On 21-04-18 11:10, Yann E. MORIN wrote:
> > In the pkg-inner macro, all variables, but he positional arguments,
> > must be $$-prefixed, so that they are expanded only when the macro
> > is evaluated in each package, not when the macro is parsed.
> > 
> > It is to be noted, though, that the current code, even though incorrect
> > by the above rules, seemed to work. However, the upcoming addition of
> > download dependencies, mimicking that code, would not work unless it
> > was $$-prefixed.
> > 
> > So, for consistency sake, and for correctness sake, let's always use
> > the $$-prefix in the inner macro.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > 
> > ---
> > Arnout: you the make expert, any idea why on Earth the single-expansion
> > was working in thefirst place?
> 
> [You put me in Cc of too many patches, so I don't notice when you *really* want
> my response :-]

Well, you were previously part of the discussions that lead to those
patches about the git backend, and you even partiually reviewed and
commented on an earlier revision of that git series, so it was only
natural that I added you in Cc of those patches...

>  In most cases, single-$ will work as well. In this particular case, after first
> expansion you will get:
> 
> ifeq (,)
> FOO_EXTRACT_DEPENDENCIES += host-tar
> endif
> 
> which in the $(eval ...) will get evaluated to nothing.

Well, I might be totally numb here, but an empty string does equal
another empty string, and thus I'd expect that host-tar be added
as a dependency...

Which is now weird, because then host-tar would be its own dependency,
which clearly isn't the case:

    $ make -s printvars VARS=HOST_TAR_FINAL_ALL_DEPENDENCIES
    HOST_TAR_FINAL_ALL_DEPENDENCIES=host-skeleton

So, an empty string is not the same as another empty string... I'm
totally clueless now... :-/

>  After this patch, after first expansion you get
> 
> ifeq ($(filter host-tar host-skeleton host-xz host-lzip host-fakedate,foo),)
> FOO_EXTRACT_DEPENDENCIES += host-tar
> endif
> 
> which the $(eval ...) still evaluates to nothing.

There, I do agree...

>  The cases where $$ is really needed are fairly rare. The most obvious ones are
> when you have an ifeq() using a variable that is defined in the same
> inner-generic-package expansion. Less obvious but important for consistency is
> to keep the "late binding" property. With single-$, the variable is expanded
> immediately, so if it is still updated later in the makefiles, the new values
> won't be used anymore. This would e.g. break $(1)-show-rdepends.
> 
>  Because it is very hard to remember when exactly you should use $$, we
> introduced the rule: use $$ *everywhere*, except for $(1), $(2) etc. Thomas's
> patches that fixed the dependencies broke that rule.

OK. I have to admit that I am still not sure what happens, though...

>  Unfortunately, this patch still doesn't fix it...

[--SNIP--]
> > -ifeq ($(filter host-tar host-skeleton host-fakedate,$(1)),)
> > +ifeq ($$(filter host-tar host-skeleton host-fakedate,$(1)),)
> >  $(2)_EXTRACT_DEPENDENCIES += $(BR2_TAR_HOST_DEPENDENCY)
>                                 ^^ This should also be $$
> 
> and the same below.

ACK, will fix.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-05-03 19:32 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-21  9:10 [Buildroot] [PATCH 0/7] dependencies: ensure availability of host-tar, cleanups in graphs Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 1/7] support/graph-depends: ensure all packages get graphed Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 2/7] support/graph-depends: also cut on host-skeleton Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 3/7] support/graph-depends: also cut on host-tar Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 4/7] core/package: postpone evaluation of dependency conditions Yann E. MORIN
2018-05-02 22:58   ` Arnout Vandecappelle
2018-05-03 19:32     ` Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 5/7] core/package: add possibility to declare download dependencies Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 6/7] core/package: add host-tar dependency for downloads from repositories Yann E. MORIN
2018-04-21  9:10 ` [Buildroot] [PATCH 7/7] fs: always depend on build host-tar if needed Yann E. MORIN

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.