[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Yann E. MORIN]

Hello All!

This series is a proposal to allow extracting metadata about packages,
in a way that makes it reusable for tooling outside of Buildroot, that
is both reliable and extendable, without causing too much burden on
Buildroot own infrastructure.

This series introduces a global 'show-info' rule, and a per package and
per filesystem 'show-info' rule, that dump the pacakge metadata as a
JSON blob. JSON is a key-value based serialisation format. As such, it
is easy to parse (there are implementations in virtually all languages,
and there are even tools like jq, that can be used from shell scripts).
But most importantly, it is easy to extend without breakign existing
tools (as long as they use proper JSON parsers, that is).

This series first introduces 4 cleanup preparatory patches in the
download infra, so that it gets easy to get the list of URIs to report
them. Strictly speaking, only patches 1 and 2 are required for this
series, but the cleanup was extended to the infra with patches 3 and 4,
for consistency.

Then, the series introduces a per package and per filesystem show-info
rule, as well as a recursive variant. The per filesystem rule is needed
to be able to get the info about packages that are a dependency of
filesystems.

Finally, a global show-info rule is introduced that generates a JSON
array with all the information about a build.

In the end, the output for a packages would look like (beautified manually
for ease of reading):

    $ make cracklib-show-info
    {
      "name": "cracklib",
      "type": "target",
      "virtual": false,
      "version": "2.9.7",
      "licenses": "LGPL-2.1",
      "downloads": [
        {
          "source": "cracklib-2.9.7.tar.gz",
          "URIs": [
            "https+https://github.com/cracklib/cracklib/releases/download/v2.9.7",
            "http|urlencode+http://sources.buildroot.net/cracklib",
            "http|urlencode+http://sources.buildroot.net"
          ]
        },
        {
          "source": "cracklib-words-2.9.7.gz",
          "URIs": [
            "https://github.com/cracklib/cracklib/releases/download/v2.9.7",
            "http|urlencode+http://sources.buildroot.net/cracklib",
            "http|urlencode+http://sources.buildroot.net"
          ]
        },
        null
      ],
      "depends on": [
        "host-cracklib",
        "host-skeleton",
        "skeleton",
        "toolchain"
      ],
      "dependency of": [
        "libpwquality",
        "linux-pam"
      ]
    }

While the whole show-info would look like (elipsed for readbility):

    $ make show-info
    [
    { "name": "host-gcc-final", "type": "host", "virtual": false, ... },
    { "name": "host-binutils", "type": "host", "virtual": false, ... },
    { "name": "toolchain-buildroot", "type": "target", ... },
    { "name": "gettext-tiny", "type": "target", "virtual": false, ... },
    { "name": "gettext", "type": "target", "virtual": true, ... },
    { "name": "ifupdown-scripts", "type": "target", "virtual": false, ... },
    ...
    { "name": "rootfs-common", "type": "rootfs", ... },
    { "name": "rootfs-tar", "type": "rootfs", ... }
    ]


Regards,
Yann E. MORIN.


The following changes since commit 0ca17cdc925124d1b309788518e9d4834b9b2557

  boot/syslinux: fix build with binutils note gnu property section (2019-04-07 11:54:48 +0200)


are available in the git repository at:

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

for you to fetch changes up to 9e4d027c3a353fadde428fbba702dcc9dac15d75

  infra: introduce top-level, global show-info (2019-04-07 13:27:45 +0200)


----------------------------------------------------------------
Yann E. MORIN (10):
      infra/pkg-download: return just a list of URIs
      infra/pkg-download: make the URI list a callable macro
      infra/pkg-download: get rid of the FLOCK variable
      infra/pkg-download: make the DOWNLOAD macro fully parameterised
      infra/utils: add helper to generate comma-separated lists
      infra/pkg-generic: introduce foo-show-info
      infra/pkg-generic: introduce foo-show-recursive-info
      infra/fs: introduce rootfs-foo-show-info
      infar/fs: introduce rootfs-foo-show-recursive-info
      infra: introduce top-level, global show-info

 Makefile                |  6 ++++++
 fs/common.mk            | 32 ++++++++++++++++++++++++++++++++
 package/pkg-download.mk | 41 ++++++++++++++++++++++++-----------------
 package/pkg-generic.mk  | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 support/misc/utils.mk   |  4 ++++
 5 files changed, 114 insertions(+), 18 deletions(-)

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 01/10] infra/pkg-download: return just a list of URIs

[Yann E. MORIN]

Currently, the internal DOWNLOAD_URIS variable is set to be a list of
options to pass to the download wrapper, with each URI passed as
'-u URI'.

This precludes using that variable to get just the list of URIs, in
case we need to do something else with it.

Fix the variable to really only contain the list of URIs.

Adapt the caller accordingly.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-download.mk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 7cd87c38ff..f9411213b6 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -75,17 +75,17 @@ export BR_NO_CHECK_HASH_FOR =
 
 ifneq ($(call qstrip,$(BR2_PRIMARY_SITE)),)
 DOWNLOAD_URIS += \
-	-u $(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
-	-u $(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
+	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
+	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
 endif
 
 ifeq ($(BR2_PRIMARY_SITE_ONLY),)
 DOWNLOAD_URIS += \
-	-u $(patsubst %/,%,$(dir $(call qstrip,$(1))))
+	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
 ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
 DOWNLOAD_URIS += \
-	-u $(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
-	-u $(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)),urlencode)
+	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
+	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)),urlencode)
 endif
 endif
 
@@ -101,7 +101,7 @@ define DOWNLOAD
 		-N '$($(PKG)_RAWNAME)' \
 		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
 		$(if $($(PKG)_GIT_SUBMODULES),-r) \
-		$(DOWNLOAD_URIS) \
+		$(patsubst %,-u %,$(DOWNLOAD_URIS)) \
 		$(QUIET) \
 		-- \
 		$($(PKG)_DL_OPTS)
-- 
2.14.1

[Buildroot] [PATCH 02/10] infra/pkg-download: make the URI list a callable macro

[Yann E. MORIN]

Currently, that variable is context-dependent, as it expects the PKG
variable to exist and be defined to the current package.

This is not so clean, so change the variable to a callable macro.

Adapt the caller accordingly.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-download.mk | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index f9411213b6..611df5f982 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -71,11 +71,17 @@ export BR_NO_CHECK_HASH_FOR =
 #
 # Argument 1 is the source location
 #
+#
+# DOWNLOAD_URIS - List the candidates URIs where to get the package from:
+#
+# Argument 1 is the source location
+# Argument 2 is the upper-case package name
+#
 ################################################################################
 
 ifneq ($(call qstrip,$(BR2_PRIMARY_SITE)),)
 DOWNLOAD_URIS += \
-	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
+	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)/$($(2)_DL_SUBDIR)),urlencode) \
 	$(call getschemeplusuri,$(call qstrip,$(BR2_PRIMARY_SITE)),urlencode)
 endif
 
@@ -84,7 +90,7 @@ DOWNLOAD_URIS += \
 	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
 ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
 DOWNLOAD_URIS += \
-	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(PKG)_DL_SUBDIR)),urlencode) \
+	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(2)_DL_SUBDIR)),urlencode) \
 	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)),urlencode)
 endif
 endif
@@ -101,7 +107,7 @@ define DOWNLOAD
 		-N '$($(PKG)_RAWNAME)' \
 		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
 		$(if $($(PKG)_GIT_SUBMODULES),-r) \
-		$(patsubst %,-u %,$(DOWNLOAD_URIS)) \
+		$(patsubst %,-u %,$(call DOWNLOAD_URIS,$(1),$(PKG))) \
 		$(QUIET) \
 		-- \
 		$($(PKG)_DL_OPTS)
-- 
2.14.1

[Buildroot] [PATCH 03/10] infra/pkg-download: get rid of the FLOCK variable

[Yann E. MORIN]

The FLOCK variable is context-dependent, and expects the PKG
variable to be set to the current package.

This is not so nice. Besides, it is used in a single location.

Get rid of this intermediate variable, and directly use flock
where we need it.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-download.mk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 611df5f982..b8adb0ca05 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -18,7 +18,6 @@ export SCP := $(call qstrip,$(BR2_SCP))
 export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 DL_WRAPPER = support/download/dl-wrapper
-FLOCK = flock $($(PKG)_DL_DIR)/
 
 # DL_DIR may have been set already from the environment
 ifeq ($(origin DL_DIR),undefined)
@@ -97,7 +96,7 @@ endif
 
 define DOWNLOAD
 	$(Q)mkdir -p $($(PKG)_DL_DIR)
-	$(Q)$(EXTRA_ENV) $(FLOCK) $(DL_WRAPPER) \
+	$(Q)$(EXTRA_ENV) flock $($(PKG)_DL_DIR)/ $(DL_WRAPPER) \
 		-c '$($(PKG)_DL_VERSION)' \
 		-d '$($(PKG)_DL_DIR)' \
 		-D '$(DL_DIR)' \
-- 
2.14.1

[Buildroot] [PATCH 04/10] infra/pkg-download: make the DOWNLOAD macro fully parameterised

[Yann E. MORIN]

Currently, the DOWNLOAD macro is context-dependent and expects
the PKG variable to be set to the current package.

This is not so nice.

Change the macro to expect the upper-case package name as a
parameter, rather than expect it from a variable.

Adapt the caller accordingly.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 package/pkg-download.mk | 22 ++++++++++++----------
 package/pkg-generic.mk  |  2 +-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index b8adb0ca05..8ce9f0a97c 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -69,6 +69,7 @@ export BR_NO_CHECK_HASH_FOR =
 # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
 #
 # Argument 1 is the source location
+# Argument 2 is the upper-case package name
 #
 #
 # DOWNLOAD_URIS - List the candidates URIs where to get the package from:
@@ -95,19 +96,20 @@ endif
 endif
 
 define DOWNLOAD
-	$(Q)mkdir -p $($(PKG)_DL_DIR)
-	$(Q)$(EXTRA_ENV) flock $($(PKG)_DL_DIR)/ $(DL_WRAPPER) \
-		-c '$($(PKG)_DL_VERSION)' \
-		-d '$($(PKG)_DL_DIR)' \
+	$(Q)mkdir -p $($(2)_DL_DIR)
+	$(Q)$(EXTRA_ENV) flock $($(2)_DL_DIR)/ $(DL_WRAPPER) \
+		-c '$($(2)_DL_VERSION)' \
+		-d '$($(2)_DL_DIR)' \
 		-D '$(DL_DIR)' \
 		-f '$(notdir $(1))' \
-		-H '$($(PKG)_HASH_FILE)' \
-		-n '$($(PKG)_BASENAME_RAW)' \
-		-N '$($(PKG)_RAWNAME)' \
-		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
-		$(if $($(PKG)_GIT_SUBMODULES),-r) \
+		-H '$($(2)_HASH_FILE)' \
+		-n '$($(2)_BASENAME_RAW)' \
+		-N '$($(2)_RAWNAME)' \
+		-o '$($(2)_DL_DIR)/$(notdir $(1))' \
+		$(if $($(2)_GIT_SUBMODULES),-r) \
 		$(patsubst %,-u %,$(call DOWNLOAD_URIS,$(1),$(PKG))) \
+		$(patsubst %,-u %,$(call DOWNLOAD_URIS,$(1),$(2))) \
 		$(QUIET) \
 		-- \
-		$($(PKG)_DL_OPTS)
+		$($(2)_DL_OPTS)
 endef
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index a83813e28d..f78ff2e665 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -149,7 +149,7 @@ $(BUILD_DIR)/%/.stamp_downloaded:
 			break ; \
 		fi ; \
 	done
-	$(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call DOWNLOAD,$(p))$(sep))
+	$(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call DOWNLOAD,$(p),$(PKG))$(sep))
 	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
 	$(Q)mkdir -p $(@D)
 	@$(call step_end,download)
-- 
2.14.1

[Buildroot] [PATCH 05/10] infra/utils: add helper to generate comma-separated lists

[Yann E. MORIN]

Add a helper macro that, from a space-separated list of items, returns a
comma-separated list of the quoted items.

This will be usefull when we need to generate lists in JSON, later...

Code suggested by Thomas P.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 support/misc/utils.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/support/misc/utils.mk b/support/misc/utils.mk
index c44319338e..0903679e62 100644
--- a/support/misc/utils.mk
+++ b/support/misc/utils.mk
@@ -70,6 +70,10 @@ finddirclauses = $(call notfirstword,$(patsubst %,-o -path '$(1)/%',$(2)))
 # notfirstword(wordlist): returns all but the first word in wordlist
 notfirstword = $(wordlist 2,$(words $(1)),$(1))
 
+# build a comma-separated list of quoted items, from a space-separated
+# list of unquoted items:   a b c d  -->  "a", "b", "c", "d"
+make-comma-list = $(subst $(space),$(comma) ,$(patsubst %,"%",$(strip $(1))))
+
 # Needed for the foreach loops to loop over the list of hooks, so that
 # each hook call is properly separated by a newline.
 define sep
-- 
2.14.1

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Yann E. MORIN]

Users are increasingly trying to extract information about packages.
For example, they might need to get the list of URIs, or the
dependencies of a package.

Although we do have a bunch of rules to generate some of that, this is
done in ad-hic way, with most of the output formats just as-hoc, raw,
unformatted blurbs, mostly internal data dumped as-is.

Introduce a new rule, foo-show-info, that provides a properly formatted
output of all the meta-information about a package: name, type, version,
licenses, dependencies...

We choose to use JSON as the output format, because it is pretty
veratile, has parsers in virtually all languages, has tols to parse from
the shell (jq). It also closely matches Python data structure, which
makes it easy to use with our own internal tools as well. Finally, JSON
being a key-value store, allows for easy expanding the output without
requiring existing consumers to be updated; new, unknown keys are simply
ignored by those (as long as they are true JSON parsers).

The complex part of this change was the conditional output of parts of
the data: virtual packages have no source, version, license or
downloads, unlike non-virtual packages. The basic idea was to
intersperse those conditions directly inside the $(info ...) block.
However, becasue key-value pairs in JSON are separated with commas, and
that the comma is also the parameters separator in Makefile, we'd have
had to use $(comma) to output an actual comma when inside an $(if ...)
statement, like so:

    $(if $$($(2)_IS_VIRTUAL), \
        "virtual": true$(comma), \
        "virtual": false$(comma) \
        "version": "$$($(2)_DL_VERSION)"$(comma) \
        [...]
    )

This was, with the trailing '\'i and the indentation, a bit cumbersome
to handle, and would be difficult to maintain.

Instead, an intermediate variable is used, which is set conditionally.
And to make the rest of the information look similar, it is also stored
in an intermediate variable. This makes it easier to read, and thus to
maintain (hopefully so).

Note that we can't set part of the variable conditionally, like so:

    SOMETHING = YES
    define FOO
    ifeq ($(SOMETHING),YES)
        Meh $(SOMETHING)
    else
        Meh --$(SOMETHING)--
    endif
    endef
    $(info $(FOO))

as that would yield:

    ifeq (YES,YES)
        Meh YES
    else
        Meh --YES--
    endif

So, we use intermediate variables.

Finally, the variable is $(strip...)ed before being displayed, so as to
make it fit on a single line, one per package info. This is just JSON,
so it does not matter, but at least it should be parallel-safe in the
future.

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f78ff2e665..72b1a87f5f 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -843,6 +843,49 @@ $(1)-external-deps:
 	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
 endif
 
+ifeq ($$($(2)_IS_VIRTUAL),YES)
+define $(2)_SHOW_INFO_VIRTUAL
+	"virtual": true,
+endef
+else
+define $(2)_SHOW_INFO_VIRTUAL
+	"virtual": false,
+	"version": "$$($(2)_DL_VERSION)",
+	"licenses": "$$($(2)_LICENSE)",
+	"downloads": [
+	$$(foreach dl,$$($(2)_ALL_DOWNLOADS),
+		{
+			"source": "$$(notdir $$(dl))",
+			"URIs": [
+			$$(call make-comma-list,
+				$$(subst \|,|,
+					$$(call DOWNLOAD_URIS,$$(dl),$(2))
+				)
+			)
+			]
+		},
+	)
+	null
+	],
+endef
+endif
+
+define $(2)_SHOW_INFO
+	"name": "$(1)",
+	"type": "$(4)",
+	$$($(2)_SHOW_INFO_VIRTUAL)
+	"depends on": [
+		$$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
+	],
+	"dependency of": [
+		$$(call make-comma-list,$$($(2)_RDEPENDENCIES))
+	]
+endef
+
+$(1)-show-info:
+	@:
+	$$(info { $$(strip $$($(2)_SHOW_INFO)) })
+
 $(1)-show-version:
 			@echo $$($(2)_VERSION)
 
@@ -1099,6 +1142,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-rsync \
 	$(1)-show-dependency-tree \
 	$(1)-show-depends \
+	$(1)-show-info \
 	$(1)-show-version \
 	$(1)-source
 
-- 
2.14.1

[Buildroot] [PATCH 07/10] infra/pkg-generic: introduce foo-show-recursive-info

[Yann E. MORIN]

Recusrively display the information for a package and its complete
dependency tree.

Note that this does not generate a valid JSON blurb, as records are not
comma-separated. It is not trivial to do, so we leave this for later (or
as an exrecise to the user).

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 72b1a87f5f..f230292603 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -882,6 +882,8 @@ define $(2)_SHOW_INFO
 	]
 endef
 
+$(1)-show-recursive-info: $(1)-show-info
+$(1)-show-recursive-info: $$(patsubst %,%-show-recursive-info,$$($(2)_FINAL_ALL_DEPENDENCIES))
 $(1)-show-info:
 	@:
 	$$(info { $$(strip $$($(2)_SHOW_INFO)) })
@@ -1143,6 +1145,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-show-dependency-tree \
 	$(1)-show-depends \
 	$(1)-show-info \
+	$(1)-show-recursive-info \
 	$(1)-show-version \
 	$(1)-source
 
-- 
2.14.1

[Buildroot] [PATCH 08/10] infra/fs: introduce rootfs-foo-show-info

[Yann E. MORIN]

Similarly to what we introduced for packages, do the same for
filesystems.

This will be usefull later, when we are able to dump the complete
information for a configuration, by also reporting information
about packages that are dependencies for filesystems.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 fs/common.mk | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 4ad51fdd0a..2ec27e8e63 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -47,6 +47,19 @@ ROOTFS_COMMON_DEPENDENCIES = \
 	$(BR2_TAR_HOST_DEPENDENCY) \
 	$(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd)
 
+define ROOTFS_COMMON_SHOW_INFO
+	"name": "rootfs-common", \
+	"type": "rootfs", \
+	"depends on": [ \
+		$(call make-comma-list,$(ROOTFS_COMMON_DEPENDENCIES)) \
+	]
+endef
+
+.PHONY: rootfs-common-show-info
+rootfs-common-show-info:
+	@:
+	$(info { $(strip $(ROOTFS_COMMON_SHOW_INFO)) })
+
 rootfs-common-show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(ROOTFS_COMMON_DEPENDENCIES))
 	$(info rootfs-common: host)
 	$(info rootfs-common -> $(foreach d,$(ROOTFS_COMMON_DEPENDENCIES),$(d)))
@@ -84,6 +97,19 @@ ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
 
 ROOTFS_$(2)_DEPENDENCIES += rootfs-common
 
+define ROOTFS_$(2)_SHOW_INFO
+	"name": "rootfs-$(1)", \
+	"type": "rootfs", \
+	"depends on": [ \
+		$$(call make-comma-list,$$(ROOTFS_$(2)_DEPENDENCIES)) \
+	]
+endef
+
+.PHONY: rootfs-$(1)-show-info
+rootfs-$(1)-show-info:
+	@:
+	$$(info { $$(strip $$(ROOTFS_$(2)_SHOW_INFO)) })
+
 rootfs-$(1)-show-dependency-tree: $$(patsubst %,%-show-dependency-tree,$$(ROOTFS_$(2)_DEPENDENCIES))
 	$$(info rootfs-$(1): host)
 	$$(info rootfs-$(1) -> $$(foreach d,$$(ROOTFS_$(2)_DEPENDENCIES),$$(d)))
-- 
2.14.1

[Buildroot] [PATCH 09/10] infar/fs: introduce rootfs-foo-show-recursive-info

[Yann E. MORIN]

As for packages, recursively display the information for a filesystem
and its complete dependency tree.

Note that this does not generate a valid JSON blurb, as records are not
comma-separated. It is not trivial to do, so we leave this for later (or
as an exrecise to the user).

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 fs/common.mk | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 2ec27e8e63..5b4e04e6bf 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -56,6 +56,9 @@ define ROOTFS_COMMON_SHOW_INFO
 endef
 
 .PHONY: rootfs-common-show-info
+.PHONY: rootfs-common-show-info rootfs-common-show-recursive-info
+rootfs-common-show-recursive-info: rootfs-common-show-info
+rootfs-common-show-recursive-info: $(patsubst %,%-show-recursive-info,$(ROOTFS_COMMON_DEPENDENCIES))
 rootfs-common-show-info:
 	@:
 	$(info { $(strip $(ROOTFS_COMMON_SHOW_INFO)) })
@@ -106,6 +109,9 @@ define ROOTFS_$(2)_SHOW_INFO
 endef
 
 .PHONY: rootfs-$(1)-show-info
+.PHONY: rootfs-$(1)-show-info rootfs-$(1)-show-recursive-info
+rootfs-$(1)-show-recursive-info: rootfs-$(1)-show-info
+rootfs-$(1)-show-recursive-info: $$(patsubst %,%-show-recursive-info,$$(ROOTFS_$(2)_DEPENDENCIES))
 rootfs-$(1)-show-info:
 	@:
 	$$(info { $$(strip $$(ROOTFS_$(2)_SHOW_INFO)) })
-- 
2.14.1

[Buildroot] [PATCH 10/10] infra: introduce top-level, global show-info

[Yann E. MORIN]

Now that packages and filesystems can report their recursive info, use
that to dump the whole info as a valid JSOn array.

As it is rather complex to ensure soemthing is run before every rules
(to display the eldin opening '[' so as to have a valid JSON array),
re-enter the Makefile and mangle the output with a sed expression, that:

  - 0,/^/s//[\n/;   inserts a '[' before the first line,
  - $!s/$/,/;       appends a ',' at the end of every lines but the last,
  - $s/$/\n]/       appends a ']' after the last line.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 Makefile | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile b/Makefile
index 60bf7d7d08..13c4390ae4 100644
--- a/Makefile
+++ b/Makefile
@@ -876,6 +876,12 @@ graph-depends-requirements:
 	@dot -? >/dev/null 2>&1 || \
 		{ echo "ERROR: The 'dot' program from Graphviz is needed for graph-depends" >&2; exit 1; }
 
+.PHONY: show-info _show-info-private
+show-info:
+	@$(MAKE) -C $(CONFIG_DIR) _show-info-private |sed -r -e '0,/^/s//[\n/; $$!s/$$/,/; $$s/$$/\n]/'
+_show-info-private: $(patsubst %,%-show-recursive-info,$(PACKAGES) $(TARGETS_ROOTFS))
+	@:
+
 .PHONY: show-dependency-tree
 show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(PACKAGES) $(TARGETS_ROOTFS))
 	@:
-- 
2.14.1

[Buildroot] [PATCH 01/10] infra/pkg-download: return just a list of URIs

[Thomas Petazzoni]

Hello Yann,

On Sun,  7 Apr 2019 13:51:16 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> @@ -101,7 +101,7 @@ define DOWNLOAD
>  		-N '$($(PKG)_RAWNAME)' \
>  		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
>  		$(if $($(PKG)_GIT_SUBMODULES),-r) \
> -		$(DOWNLOAD_URIS) \
> +		$(patsubst %,-u %,$(DOWNLOAD_URIS)) \

A patsubst here feels a bit awkward. What about a foreach instead, i.e:

		$(foreach uri,$(DOWNLOAD_URIS),-u $(uri))

 ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 01/10] infra/pkg-download: return just a list of URIs

[Yann E. MORIN]

Thomas, All,

On 2019-04-07 14:20 +0200, Thomas Petazzoni spake thusly:
> On Sun,  7 Apr 2019 13:51:16 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
> > @@ -101,7 +101,7 @@ define DOWNLOAD
> >  		-N '$($(PKG)_RAWNAME)' \
> >  		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
> >  		$(if $($(PKG)_GIT_SUBMODULES),-r) \
> > -		$(DOWNLOAD_URIS) \
> > +		$(patsubst %,-u %,$(DOWNLOAD_URIS)) \
> 
> A patsubst here feels a bit awkward. What about a foreach instead, i.e:
> 
> 		$(foreach uri,$(DOWNLOAD_URIS),-u $(uri))
> 
>  ?

OK, I remember having the choice, and going with patsubst instead of
foreach. But I don't remember what made me make up my mind that way...

Maybe it was just because using a patsubst was shorter? ;-)

I'm OK with a foreach, of course (fixed locally in a moment, so will be
ready for a v2 if there is a need).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 04/10] infra/pkg-download: make the DOWNLOAD macro fully parameterised

[Yann E. MORIN]

All,

On 2019-04-07 13:51 +0200, Yann E. MORIN spake thusly:
> Change the macro to expect the upper-case package name as a
> parameter, rather than expect it from a variable.
[--SNIP--]
>  		$(patsubst %,-u %,$(call DOWNLOAD_URIS,$(1),$(PKG))) \
> +		$(patsubst %,-u %,$(call DOWNLOAD_URIS,$(1),$(2))) \

Of course, there was an incorrect rebase on that one. Fixed locally.

Regards,
Yann E. MORIN.

>  		$(QUIET) \
>  		-- \
> -		$($(PKG)_DL_OPTS)
> +		$($(2)_DL_OPTS)
>  endef
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index a83813e28d..f78ff2e665 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -149,7 +149,7 @@ $(BUILD_DIR)/%/.stamp_downloaded:
>  			break ; \
>  		fi ; \
>  	done
> -	$(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call DOWNLOAD,$(p))$(sep))
> +	$(foreach p,$($(PKG)_ALL_DOWNLOADS),$(call DOWNLOAD,$(p),$(PKG))$(sep))
>  	$(foreach hook,$($(PKG)_POST_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
>  	$(Q)mkdir -p $(@D)
>  	@$(call step_end,download)
> -- 
> 2.14.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 02/10] infra/pkg-download: make the URI list a callable macro

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Currently, that variable is context-dependent, as it expects the PKG
> variable to exist and be defined to the current package.
>
> This is not so clean, so change the variable to a callable macro.
>
> Adapt the caller accordingly.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-download.mk | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index f9411213b6..611df5f982 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -71,11 +71,17 @@ export BR_NO_CHECK_HASH_FOR =
>  #
>  # Argument 1 is the source location
>  #
> +#
> +# DOWNLOAD_URIS - List the candidates URIs where to get the package from:
> +#
> +# Argument 1 is the source location
> +# Argument 2 is the upper-case package name
> +#
>  ################################################################################

Here I think it makes sense to split the comment. The part about
'DOWNLOAD' can move below to just above the 'DOWNLOAD' macro, and
DOWNLOAD_URIS remains here. The text listing the three types of
download locations could perhaps move to the DOWNLOAD_URIS part.

[Buildroot] [PATCH 05/10] infra/utils: add helper to generate comma-separated lists

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Add a helper macro that, from a space-separated list of items, returns a
> comma-separated list of the quoted items.
>
> This will be usefull when we need to generate lists in JSON, later...

useful

>
> Code suggested by Thomas P.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  support/misc/utils.mk | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/support/misc/utils.mk b/support/misc/utils.mk
> index c44319338e..0903679e62 100644
> --- a/support/misc/utils.mk
> +++ b/support/misc/utils.mk
> @@ -70,6 +70,10 @@ finddirclauses = $(call notfirstword,$(patsubst %,-o -path '$(1)/%',$(2)))
>  # notfirstword(wordlist): returns all but the first word in wordlist
>  notfirstword = $(wordlist 2,$(words $(1)),$(1))
>
> +# build a comma-separated list of quoted items, from a space-separated
> +# list of unquoted items:   a b c d  -->  "a", "b", "c", "d"
> +make-comma-list = $(subst $(space),$(comma) ,$(patsubst %,"%",$(strip $(1))))

I think it would be more obvious here to replace with $(subst
$(space),$(comma)$(space),...)
rather than relying on the space added after '$(comma)'.


> +
>  # Needed for the foreach loops to loop over the list of hooks, so that
>  # each hook call is properly separated by a newline.
>  define sep
> --
> 2.14.1
>

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Users are increasingly trying to extract information about packages.
> For example, they might need to get the list of URIs, or the
> dependencies of a package.
>
> Although we do have a bunch of rules to generate some of that, this is
> done in ad-hic way, with most of the output formats just as-hoc, raw,
> unformatted blurbs, mostly internal data dumped as-is.

ad-hic -> ad-hoc
as-hoc -> ad-hoc


>
> Introduce a new rule, foo-show-info, that provides a properly formatted
> output of all the meta-information about a package: name, type, version,
> licenses, dependencies...
>
> We choose to use JSON as the output format, because it is pretty
> veratile, has parsers in virtually all languages, has tols to parse from

versatile
tools

> the shell (jq). It also closely matches Python data structure, which
> makes it easy to use with our own internal tools as well. Finally, JSON
> being a key-value store, allows for easy expanding the output without
> requiring existing consumers to be updated; new, unknown keys are simply
> ignored by those (as long as they are true JSON parsers).
>
> The complex part of this change was the conditional output of parts of
> the data: virtual packages have no source, version, license or
> downloads, unlike non-virtual packages. The basic idea was to
> intersperse those conditions directly inside the $(info ...) block.
> However, becasue key-value pairs in JSON are separated with commas, and

because

> that the comma is also the parameters separator in Makefile, we'd have
> had to use $(comma) to output an actual comma when inside an $(if ...)
> statement, like so:
>
>     $(if $$($(2)_IS_VIRTUAL), \
>         "virtual": true$(comma), \
>         "virtual": false$(comma) \
>         "version": "$$($(2)_DL_VERSION)"$(comma) \
>         [...]
>     )
>
> This was, with the trailing '\'i and the indentation, a bit cumbersome
> to handle, and would be difficult to maintain.
>
> Instead, an intermediate variable is used, which is set conditionally.
> And to make the rest of the information look similar, it is also stored
> in an intermediate variable. This makes it easier to read, and thus to
> maintain (hopefully so).
>
> Note that we can't set part of the variable conditionally, like so:
>
>     SOMETHING = YES
>     define FOO
>     ifeq ($(SOMETHING),YES)
>         Meh $(SOMETHING)
>     else
>         Meh --$(SOMETHING)--
>     endif
>     endef
>     $(info $(FOO))
>
> as that would yield:
>
>     ifeq (YES,YES)
>         Meh YES
>     else
>         Meh --YES--
>     endif
>
> So, we use intermediate variables.
>
> Finally, the variable is $(strip...)ed before being displayed, so as to
> make it fit on a single line, one per package info. This is just JSON,
> so it does not matter, but at least it should be parallel-safe in the
> future.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-generic.mk | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f78ff2e665..72b1a87f5f 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -843,6 +843,49 @@ $(1)-external-deps:
>         @echo "file://$$($(2)_OVERRIDE_SRCDIR)"
>  endif
>
> +ifeq ($$($(2)_IS_VIRTUAL),YES)
> +define $(2)_SHOW_INFO_VIRTUAL
> +       "virtual": true,
> +endef
> +else
> +define $(2)_SHOW_INFO_VIRTUAL
> +       "virtual": false,
> +       "version": "$$($(2)_DL_VERSION)",
> +       "licenses": "$$($(2)_LICENSE)",
> +       "downloads": [
> +       $$(foreach dl,$$($(2)_ALL_DOWNLOADS),
> +               {
> +                       "source": "$$(notdir $$(dl))",
> +                       "URIs": [
> +                       $$(call make-comma-list,
> +                               $$(subst \|,|,
> +                                       $$(call DOWNLOAD_URIS,$$(dl),$(2))
> +                               )
> +                       )
> +                       ]
> +               },
> +       )
> +       null
> +       ],

So the 'null' serves as last element in the array and is only needed
because JSON cannot handle the trailing comma, right?

> +endef
> +endif
> +
> +define $(2)_SHOW_INFO
> +       "name": "$(1)",
> +       "type": "$(4)",
> +       $$($(2)_SHOW_INFO_VIRTUAL)
> +       "depends on": [
> +               $$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
> +       ],
> +       "dependency of": [
> +               $$(call make-comma-list,$$($(2)_RDEPENDENCIES))
> +       ]

It feels odd to me to not use single-word keys here. Why not use
'depends/rdepends', 'dependencies'/'rdependencies' or if you want
dependencies/reverse_dependencies ? The naming 'rdepends' is already
exposed to users 'make show-rdepends' so it makes sense to me to use
the same naming.

> +endef
> +
> +$(1)-show-info:
> +       @:
> +       $$(info { $$(strip $$($(2)_SHOW_INFO)) })
> +
>  $(1)-show-version:
>                         @echo $$($(2)_VERSION)
>
> @@ -1099,6 +1142,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>         $(1)-rsync \
>         $(1)-show-dependency-tree \
>         $(1)-show-depends \
> +       $(1)-show-info \
>         $(1)-show-version \
>         $(1)-source
>
> --
> 2.14.1
>

[Buildroot] [PATCH 08/10] infra/fs: introduce rootfs-foo-show-info

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Similarly to what we introduced for packages, do the same for
> filesystems.
>
> This will be usefull later, when we are able to dump the complete

useful

> information for a configuration, by also reporting information
> about packages that are dependencies for filesystems.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  fs/common.mk | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>
> diff --git a/fs/common.mk b/fs/common.mk
> index 4ad51fdd0a..2ec27e8e63 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -47,6 +47,19 @@ ROOTFS_COMMON_DEPENDENCIES = \
>         $(BR2_TAR_HOST_DEPENDENCY) \
>         $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd)
>
> +define ROOTFS_COMMON_SHOW_INFO
> +       "name": "rootfs-common", \
> +       "type": "rootfs", \
> +       "depends on": [ \
> +               $(call make-comma-list,$(ROOTFS_COMMON_DEPENDENCIES)) \
> +       ]
> +endef
> +
> +.PHONY: rootfs-common-show-info
> +rootfs-common-show-info:
> +       @:
> +       $(info { $(strip $(ROOTFS_COMMON_SHOW_INFO)) })
> +
>  rootfs-common-show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(ROOTFS_COMMON_DEPENDENCIES))
>         $(info rootfs-common: host)
>         $(info rootfs-common -> $(foreach d,$(ROOTFS_COMMON_DEPENDENCIES),$(d)))
> @@ -84,6 +97,19 @@ ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
>
>  ROOTFS_$(2)_DEPENDENCIES += rootfs-common
>
> +define ROOTFS_$(2)_SHOW_INFO
> +       "name": "rootfs-$(1)", \
> +       "type": "rootfs", \
> +       "depends on": [ \
> +               $$(call make-comma-list,$$(ROOTFS_$(2)_DEPENDENCIES)) \
> +       ]
> +endef
> +
> +.PHONY: rootfs-$(1)-show-info
> +rootfs-$(1)-show-info:
> +       @:
> +       $$(info { $$(strip $$(ROOTFS_$(2)_SHOW_INFO)) })
> +
>  rootfs-$(1)-show-dependency-tree: $$(patsubst %,%-show-dependency-tree,$$(ROOTFS_$(2)_DEPENDENCIES))
>         $$(info rootfs-$(1): host)
>         $$(info rootfs-$(1) -> $$(foreach d,$$(ROOTFS_$(2)_DEPENDENCIES),$$(d)))
> --
> 2.14.1
>

[Buildroot] [PATCH 09/10] infar/fs: introduce rootfs-foo-show-recursive-info

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> As for packages, recursively display the information for a filesystem
> and its complete dependency tree.
>
> Note that this does not generate a valid JSON blurb, as records are not
> comma-separated. It is not trivial to do, so we leave this for later (or
> as an exrecise to the user).

exercise

>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  fs/common.mk | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/common.mk b/fs/common.mk
> index 2ec27e8e63..5b4e04e6bf 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -56,6 +56,9 @@ define ROOTFS_COMMON_SHOW_INFO
>  endef
>
>  .PHONY: rootfs-common-show-info
> +.PHONY: rootfs-common-show-info rootfs-common-show-recursive-info
> +rootfs-common-show-recursive-info: rootfs-common-show-info
> +rootfs-common-show-recursive-info: $(patsubst %,%-show-recursive-info,$(ROOTFS_COMMON_DEPENDENCIES))
>  rootfs-common-show-info:
>         @:
>         $(info { $(strip $(ROOTFS_COMMON_SHOW_INFO)) })
> @@ -106,6 +109,9 @@ define ROOTFS_$(2)_SHOW_INFO
>  endef
>
>  .PHONY: rootfs-$(1)-show-info
> +.PHONY: rootfs-$(1)-show-info rootfs-$(1)-show-recursive-info
> +rootfs-$(1)-show-recursive-info: rootfs-$(1)-show-info
> +rootfs-$(1)-show-recursive-info: $$(patsubst %,%-show-recursive-info,$$(ROOTFS_$(2)_DEPENDENCIES))
>  rootfs-$(1)-show-info:
>         @:
>         $$(info { $$(strip $$(ROOTFS_$(2)_SHOW_INFO)) })
> --
> 2.14.1
>

[Buildroot] [PATCH 07/10] infra/pkg-generic: introduce foo-show-recursive-info

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Recusrively display the information for a package and its complete

Recursively

> dependency tree.
>
> Note that this does not generate a valid JSON blurb, as records are not
> comma-separated. It is not trivial to do, so we leave this for later (or
> as an exrecise to the user).

exercise

>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-generic.mk | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 72b1a87f5f..f230292603 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -882,6 +882,8 @@ define $(2)_SHOW_INFO
>         ]
>  endef
>
> +$(1)-show-recursive-info: $(1)-show-info
> +$(1)-show-recursive-info: $$(patsubst %,%-show-recursive-info,$$($(2)_FINAL_ALL_DEPENDENCIES))
>  $(1)-show-info:
>         @:
>         $$(info { $$(strip $$($(2)_SHOW_INFO)) })
> @@ -1143,6 +1145,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>         $(1)-show-dependency-tree \
>         $(1)-show-depends \
>         $(1)-show-info \
> +       $(1)-show-recursive-info \
>         $(1)-show-version \
>         $(1)-source
>
> --
> 2.14.1
>

[Buildroot] [PATCH 10/10] infra: introduce top-level, global show-info

[Thomas De Schampheleire]

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Now that packages and filesystems can report their recursive info, use
> that to dump the whole info as a valid JSOn array.

JSON

>
> As it is rather complex to ensure soemthing is run before every rules

something

> (to display the eldin opening '[' so as to have a valid JSON array),

I don't know what you wanted to say for 'eldin'.

> re-enter the Makefile and mangle the output with a sed expression, that:
>
>   - 0,/^/s//[\n/;   inserts a '[' before the first line,
>   - $!s/$/,/;       appends a ',' at the end of every lines but the last,
>   - $s/$/\n]/       appends a ']' after the last line.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 60bf7d7d08..13c4390ae4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -876,6 +876,12 @@ graph-depends-requirements:
>         @dot -? >/dev/null 2>&1 || \
>                 { echo "ERROR: The 'dot' program from Graphviz is needed for graph-depends" >&2; exit 1; }
>
> +.PHONY: show-info _show-info-private
> +show-info:
> +       @$(MAKE) -C $(CONFIG_DIR) _show-info-private |sed -r -e '0,/^/s//[\n/; $$!s/$$/,/; $$s/$$/\n]/'

This should have an explicit '-s' argument, otherwise the output is:

[
make[1]: Entering directory '/home/tdescham/repo/contrib/buildroot',
...
(JSON)
...
make[1]: Leaving directory '/home/tdescham/repo/contrib/buildroot'
]

which is not valid json.

With -s, I validated that the output is valid by piping 'make
show-info' to 'jq .'

> +_show-info-private: $(patsubst %,%-show-recursive-info,$(PACKAGES) $(TARGETS_ROOTFS))
> +       @:
> +
>  .PHONY: show-dependency-tree
>  show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(PACKAGES) $(TARGETS_ROOTFS))
>         @:
> --
> 2.14.1
>

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Thomas De Schampheleire]

Hi Yann,

El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
(<yann.morin.1998@free.fr>) escribi?:
>
> Hello All!
>
> This series is a proposal to allow extracting metadata about packages,
> in a way that makes it reusable for tooling outside of Buildroot, that
> is both reliable and extendable, without causing too much burden on
> Buildroot own infrastructure.
>
> This series introduces a global 'show-info' rule, and a per package and
> per filesystem 'show-info' rule, that dump the pacakge metadata as a
> JSON blob. JSON is a key-value based serialisation format. As such, it
> is easy to parse (there are implementations in virtually all languages,
> and there are even tools like jq, that can be used from shell scripts).
> But most importantly, it is easy to extend without breakign existing
> tools (as long as they use proper JSON parsers, that is).
>
> This series first introduces 4 cleanup preparatory patches in the
> download infra, so that it gets easy to get the list of URIs to report
> them. Strictly speaking, only patches 1 and 2 are required for this
> series, but the cleanup was extended to the infra with patches 3 and 4,
> for consistency.
>
> Then, the series introduces a per package and per filesystem show-info
> rule, as well as a recursive variant. The per filesystem rule is needed
> to be able to get the info about packages that are a dependency of
> filesystems.
>
> Finally, a global show-info rule is introduced that generates a JSON
> array with all the information about a build.
>
> In the end, the output for a packages would look like (beautified manually
> for ease of reading):
>
>     $ make cracklib-show-info
>     {
>       "name": "cracklib",
>       "type": "target",
>       "virtual": false,
>       "version": "2.9.7",
>       "licenses": "LGPL-2.1",
>       "downloads": [
>         {
>           "source": "cracklib-2.9.7.tar.gz",
>           "URIs": [
>             "https+https://github.com/cracklib/cracklib/releases/download/v2.9.7",
>             "http|urlencode+http://sources.buildroot.net/cracklib",
>             "http|urlencode+http://sources.buildroot.net"
>           ]
>         },
>         {
>           "source": "cracklib-words-2.9.7.gz",
>           "URIs": [
>             "https://github.com/cracklib/cracklib/releases/download/v2.9.7",
>             "http|urlencode+http://sources.buildroot.net/cracklib",
>             "http|urlencode+http://sources.buildroot.net"
>           ]
>         },
>         null
>       ],
>       "depends on": [
>         "host-cracklib",
>         "host-skeleton",
>         "skeleton",
>         "toolchain"
>       ],
>       "dependency of": [
>         "libpwquality",
>         "linux-pam"
>       ]
>     }
>
> While the whole show-info would look like (elipsed for readbility):
>
>     $ make show-info
>     [
>     { "name": "host-gcc-final", "type": "host", "virtual": false, ... },
>     { "name": "host-binutils", "type": "host", "virtual": false, ... },
>     { "name": "toolchain-buildroot", "type": "target", ... },
>     { "name": "gettext-tiny", "type": "target", "virtual": false, ... },
>     { "name": "gettext", "type": "target", "virtual": true, ... },
>     { "name": "ifupdown-scripts", "type": "target", "virtual": false, ... },
>     ...
>     { "name": "rootfs-common", "type": "rootfs", ... },
>     { "name": "rootfs-tar", "type": "rootfs", ... }
>     ]
>


Thanks for this series.
I gave some comments on the individual patches.

I cooked together a script to implement 'source-check' externally,
based on this show-info output.
It all works fine, all info needed is there.
I first process the show-info output into something like:

<name> <version> <filename> <uri1> <uri2> ...

using following jq query:

make show-info | jq -r '.[] | .["name"] as $pkg | .["version"] as
$version | .["downloads"][]? | $pkg + " " + $version + " " +
.["source"] + " " + (.["URIs"]? | select(. != null) | join(" "))'

And then I parse each resulting line, checking that for each package
at least one of the URIs that match my primary site / hg server work
fine.

For reference, below is the script as it is now (can probably still be
improved, the purpose was mainly to check if everything I need is
there, as feedback to your series):

#!/usr/bin/env python3

import os
import subprocess
import sys

LOCAL_MIRROR = 'my.local.mirror.example.com'

def sourcecheck_scp(pkg, version, filename, uri):
    real_uri = uri.split('+', 1)[1] + '/' + filename
    if real_uri.startswith('scp://'):
        real_uri = real_uri[6:]
    domain, path = real_uri.split(':', 1)
    with open(os.devnull, 'w') as devnull:
        ret = subprocess.call(['ssh', domain, 'test', '-f', path],
stderr=devnull)
    return ret == 0

def sourcecheck_hg(pkg, version, filename, uri):
    real_uri = uri.split('+', 1)[1]
    with open(os.devnull, 'w') as devnull:
        ret = subprocess.call(['hg', 'identify', '--rev', version,
real_uri], stdout=devnull, stderr=devnull)
    return ret == 0

def sourcecheck(pkg, version, filename, uri):
    if uri.startswith('scp'):
        handler = sourcecheck_scp
    elif uri.startswith('hg'):
        handler = sourcecheck_hg
    else:
        raise Exception("Cannot handle unknown URI type: '%s' for
package '%s'" % (uri, pkg))

    return handler(pkg, version, filename, uri)

def main():
    p1 = subprocess.Popen(['make', 'show-info'], stdout=subprocess.PIPE)
    p2 = subprocess.Popen(['jq', '-r', '.[] | .["name"] as $pkg |
.["version"] as $version | .["downloads"][]? | $pkg + " " + $version +
" " + .["source"] + " " + (.["URIs"]? | select(. != null) | join("
"))'],
            stdin=p1.stdout, stdout=subprocess.PIPE)
    p1.stdout.close()  # Allow p1 to receive a SIGPIPE if p2 exits.
    pkginfo = p2.communicate()[0].decode('utf-8')
    result = {}

    for line in pkginfo.splitlines():
        pkg, version, filename, *uris = line.split()

        success = any(sourcecheck(pkg, version, filename, uri) for uri
in uris if LOCAL_MIRROR in uri)
        result[pkg] = success

    print('Failed packages: ', [pkg for pkg in result if not result[pkg]])
    return all(result.values())

if __name__ == '__main__':
    ret = main()
    if not ret:
        sys.exit(1)


Best regards,
Thomas

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Thomas Petazzoni]

Hello,

On Sun,  7 Apr 2019 13:51:06 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> This series is a proposal to allow extracting metadata about packages,
> in a way that makes it reusable for tooling outside of Buildroot, that
> is both reliable and extendable, without causing too much burden on
> Buildroot own infrastructure.

Overall, I am quite happy with the general idea (as we have already
discussed). It allows to extract a lot of information about the
packages, in a format that is easily parseable.

My small annoyance is that we start to have some overlap between the
following mechanisms:

 - The "make printvars" mechanism, which essentially already allows to
   dump the metadata of all packages. Admittedly, in a format that is a
   lot less nice. I think this overlap is OK, since "make printvars" is
   mainly intended as a debugging mechanism, not so much as a real tool
   whose output can be parsed. The new JSON stuff could presumably be
   used as a replacement to "printvars" for people who need to
   currently parse the "printvars" output.

 - The "make legal-info" mechanism, which also outputs some metadata
   about the packages.

I'm not at all saying that we shouldn't merge this JSON output, I just
wanted to point out the different ways that already exist to extract
some package metadata out of Buildroot.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Thomas Petazzoni]

Hello,

On Sun,  7 Apr 2019 13:51:21 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> +ifeq ($$($(2)_IS_VIRTUAL),YES)
> +define $(2)_SHOW_INFO_VIRTUAL
> +	"virtual": true,
> +endef
> +else
> +define $(2)_SHOW_INFO_VIRTUAL

I am not too happy with the name _SHOW_INFO_VIRTUAL, because it is not
at all used just for virtual packages. I don't really have a very good
suggestion, though. _SHOW_INFO_DETAILS ?

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Yann E. MORIN]

Thpmas DS. All,

On 2019-04-10 11:16 +0200, Thomas De Schampheleire spake thusly:
> El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
> (<yann.morin.1998@free.fr>) escribi?:
[--SNIP--]
> > Introduce a new rule, foo-show-info, that provides a properly formatted
> > output of all the meta-information about a package: name, type, version,
> > licenses, dependencies...
[--SNIP--]
> > +define $(2)_SHOW_INFO_VIRTUAL
> > +       "virtual": false,
> > +       "version": "$$($(2)_DL_VERSION)",
> > +       "licenses": "$$($(2)_LICENSE)",
> > +       "downloads": [
> > +       $$(foreach dl,$$($(2)_ALL_DOWNLOADS),
> > +               {
> > +                       "source": "$$(notdir $$(dl))",
> > +                       "URIs": [
> > +                       $$(call make-comma-list,
> > +                               $$(subst \|,|,
> > +                                       $$(call DOWNLOAD_URIS,$$(dl),$(2))
> > +                               )
> > +                       )
> > +                       ]
> > +               },
> > +       )
> > +       null
> > +       ],
> 
> So the 'null' serves as last element in the array and is only needed
> because JSON cannot handle the trailing comma, right?

Ah, right, I forgot to explain that part in the commit log, because it
does deserve some explanations, indeed. And yes, that's because JSON
does not allow for a trailing comma.

Comming up with a Makefile-based construct that did not require this
trick is not obvious, so I did resort on this null trick to generate a
valifd JSON output. If we require that jq is present, then we could
pipe that to jq to filter it out. But I'm not to keen on that...

> > +endef
> > +endif
> > +
> > +define $(2)_SHOW_INFO
> > +       "name": "$(1)",
> > +       "type": "$(4)",
> > +       $$($(2)_SHOW_INFO_VIRTUAL)
> > +       "depends on": [
> > +               $$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
> > +       ],
> > +       "dependency of": [
> > +               $$(call make-comma-list,$$($(2)_RDEPENDENCIES))
> > +       ]
> 
> It feels odd to me to not use single-word keys here. Why not use
> 'depends/rdepends', 'dependencies'/'rdependencies' or if you want
> dependencies/reverse_dependencies ? The naming 'rdepends' is already
> exposed to users 'make show-rdepends' so it makes sense to me to use
> the same naming.

OK for "dependendencies" vs. "reverse_dependencies".

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 10/10] infra: introduce top-level, global show-info

[Yann E. MORIN]

Thomas, All,

On 2019-04-10 11:28 +0200, Thomas De Schampheleire spake thusly:
> El dom., 7 abr. 2019 a las 13:51, Yann E. MORIN
> (<yann.morin.1998@free.fr>) escribi?:
> > (to display the eldin opening '[' so as to have a valid JSON array),
> I don't know what you wanted to say for 'eldin'.

'leading' that was.

> > re-enter the Makefile and mangle the output with a sed expression, that:
> >
> >   - 0,/^/s//[\n/;   inserts a '[' before the first line,
> >   - $!s/$/,/;       appends a ',' at the end of every lines but the last,
> >   - $s/$/\n]/       appends a ']' after the last line.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Arnout Vandecappelle <arnout@mind.be>
> > ---
> >  Makefile | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 60bf7d7d08..13c4390ae4 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -876,6 +876,12 @@ graph-depends-requirements:
> >         @dot -? >/dev/null 2>&1 || \
> >                 { echo "ERROR: The 'dot' program from Graphviz is needed for graph-depends" >&2; exit 1; }
> >
> > +.PHONY: show-info _show-info-private
> > +show-info:
> > +       @$(MAKE) -C $(CONFIG_DIR) _show-info-private |sed -r -e '0,/^/s//[\n/; $$!s/$$/,/; $$s/$$/\n]/'
> 
> This should have an explicit '-s' argument, otherwise the output is:
> 
> [
> make[1]: Entering directory '/home/tdescham/repo/contrib/buildroot',
> ...
> (JSON)
> ...
> make[1]: Leaving directory '/home/tdescham/repo/contrib/buildroot'
> ]
> 
> which is not valid json.

Ah yes, I always forget about in-tree builds, as I almost exclusively
only ever use out-of-tree builds...

Thanks!

Regards,
Yann E. MORIN.

> With -s, I validated that the output is valid by piping 'make
> show-info' to 'jq .'
> 
> > +_show-info-private: $(patsubst %,%-show-recursive-info,$(PACKAGES) $(TARGETS_ROOTFS))
> > +       @:
> > +
> >  .PHONY: show-dependency-tree
> >  show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(PACKAGES) $(TARGETS_ROOTFS))
> >         @:
> > --
> > 2.14.1
> >

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Yann E. MORIN]

Thomas, All,

On 2019-04-11 19:26 +0200, Thomas Petazzoni spake thusly:
> On Sun,  7 Apr 2019 13:51:06 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
> > This series is a proposal to allow extracting metadata about packages,
> > in a way that makes it reusable for tooling outside of Buildroot, that
> > is both reliable and extendable, without causing too much burden on
> > Buildroot own infrastructure.
> 
> Overall, I am quite happy with the general idea (as we have already
> discussed). It allows to extract a lot of information about the
> packages, in a format that is easily parseable.
> 
> My small annoyance is that we start to have some overlap between the
> following mechanisms:
> 
>  - The "make printvars" mechanism, which essentially already allows to
>    dump the metadata of all packages. Admittedly, in a format that is a
>    lot less nice. I think this overlap is OK, since "make printvars" is
>    mainly intended as a debugging mechanism, not so much as a real tool
>    whose output can be parsed. The new JSON stuff could presumably be
>    used as a replacement to "printvars" for people who need to
>    currently parse the "printvars" output.

I don't see printvars and show-info to compete against each other.
Instead, printvars is indeed more about debugging Buildroot, or poking
at internals. That really what it is: dumping Buildroot internals; there
is no guarantee about its stability, i.e. it is not an API of some sorts.

show-info, on the other hand, really exposes metadata so that it can be
consumed by external tooling provided by the user, and for which we
actually want to expose that data in a reliable and stable way.

>  - The "make legal-info" mechanism, which also outputs some metadata
>    about the packages.

When I wrote show-info, I realy pondered whether to expose the licensing
information or not, because of legal-info, and maybe I should have
refrained from doing so, with it maybe as an addition in the future.

legal-info does not print much information, though. It does collect that
information and makes it available for the user in the form of a
directory with source code, license files, and so on...

But I don't see putting the source code, the license texts and all the
content of legal-info/ into the JSON output, while the license list is
toally trivial.

I also pondered adding the licenses for the dependencies in
foo-show-info, but I did not do so, on the assumption that users who
want to make use of it would anyway have to write their JSON extractor,
in which case they would be able to very easily generate the
licenses-of-dependencies.

> I'm not at all saying that we shouldn't merge this JSON output, I just
> wanted to point out the different ways that already exist to extract
> some package metadata out of Buildroot.

Well, my idea wuld maybe to actually phase-out those rules, or at least
base them on this new JSON output.

Heck, even graph-depends could use that instead of the recently
introduced show-dependency-tree, and this is what I was going to do
before the release, so that show-dependency-tree never ever gets into
a release.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Arnout Vandecappelle]

On 11/04/2019 23:20, Yann E. MORIN wrote:
>> I'm not at all saying that we shouldn't merge this JSON output, I just
>> wanted to point out the different ways that already exist to extract
>> some package metadata out of Buildroot.
> Well, my idea wuld maybe to actually phase-out those rules, or at least
> base them on this new JSON output.
> 
> Heck, even graph-depends could use that instead of the recently
> introduced show-dependency-tree, and this is what I was going to do
> before the release, so that show-dependency-tree never ever gets into
> a release.

 Indeed, I think show-info can replace a lot of our special targets:
show-version, show-*depends, external-deps.

 Regards,
 Arnout

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Arnout Vandecappelle]

 Hi Yann,

 First of all, for me, patches 1-5 or good to go, with or without the changes
proposed by Thomas & Thomas.


On 07/04/2019 13:51, Yann E. MORIN wrote:
> Users are increasingly trying to extract information about packages.
> For example, they might need to get the list of URIs, or the
> dependencies of a package.
> 
> Although we do have a bunch of rules to generate some of that, this is
> done in ad-hic way, with most of the output formats just as-hoc, raw,
> unformatted blurbs, mostly internal data dumped as-is.
> 
> Introduce a new rule, foo-show-info, that provides a properly formatted
> output of all the meta-information about a package: name, type, version,
> licenses, dependencies...

 I love the concept of this patch series, because it makes it much easier to
write external scripts but also to write our own internal scripts (graph-depends
etc.). However, I have a number of gripes with the chosen implementation. Only
the first one is essential, the rest is nice-to-have.

* The null in the downloads list is really silly, and makes life difficult for
the user because it has to be filtered out again (cfr. Thomas's PoC jq script).
So it really has to go.

* I think the structure should always be completely the same. So a virtual
package should still have version: "", licenses: "", downloads: []. This again
makes it easier to process things.

* I think it would be better to output per-package minified strings (i.e.
without whitespace). To make it human-readable, you can just pipe through jq.
The advantage of the minified string is that you can just grep for some metadata
that you're interested in and get all the lines of info that you need - this is
a lot easier than writing a jq filter. It also gives a solution for the null,
because you can just do $(subst $(comma)},},...) to get rid of the redundant comma.

* I absolutely hate these per-package rules. I also don't think it would behave
correctly: I would expect 'make {foo,bar}-show-info' to output JSON, but it
doesn't (as you note for the show-recursive-info). So I think we should just
have a top-level show-info, which uses _FINAL_RECURSIVE_DEPENDENCIES to iterate
over all packages (note that that variable would have to be added for rootfs as
well). If we want to support selecting a subset of packages, we could add
something like the VARS of printvars.

* I have a vague feeling that the _SHOW_INFO variable looks ugly, but I can't
put my finger on it what makes it ugly (or how it could be better).

* Maybe it would be better if the top-level structure where a dict as well:
"pkg": { "type": ... }.

* I think it would be useful to also export either the uppercase name of the
package, or at least the kconfig symbol. In fact, I would export as much info as
we can - though that can of course be done in follow-up patches. infra would be
a nice one, but we don't have a variable for that at the moment.


 Again, except for the stupid null, none of this is important enough for me to
block the merging of this patch. And I can even back down on the null if getting
rid of it is too difficult. This feature trumps any little concerns I may have
about the beauty of it.


> We choose to use JSON as the output format, because it is pretty
> veratile, has parsers in virtually all languages, has tols to parse from
> the shell (jq). It also closely matches Python data structure, which
> makes it easy to use with our own internal tools as well. Finally, JSON
> being a key-value store, allows for easy expanding the output without
> requiring existing consumers to be updated; new, unknown keys are simply
> ignored by those (as long as they are true JSON parsers).
> 
> The complex part of this change was the conditional output of parts of
> the data: virtual packages have no source, version, license or
> downloads, unlike non-virtual packages. 

 So, as I wrote, I think we should keep the same structure, which makes this
extremely trivial because the empty source, version, license variables will just
do the right thing.

> The basic idea was to
> intersperse those conditions directly inside the $(info ...) block.
> However, becasue key-value pairs in JSON are separated with commas, and
> that the comma is also the parameters separator in Makefile, we'd have
> had to use $(comma) to output an actual comma when inside an $(if ...)
> statement, like so:
> 
>     $(if $$($(2)_IS_VIRTUAL), \
>         "virtual": true$(comma), \
>         "virtual": false$(comma) \
>         "version": "$$($(2)_DL_VERSION)"$(comma) \
>         [...]
>     )
> 
> This was, with the trailing '\'i and the indentation, a bit cumbersome
> to handle, and would be difficult to maintain.
> 
> Instead, an intermediate variable is used, which is set conditionally.
> And to make the rest of the information look similar, it is also stored
> in an intermediate variable. This makes it easier to read, and thus to
> maintain (hopefully so).
> 
> Note that we can't set part of the variable conditionally, like so:
> 
>     SOMETHING = YES
>     define FOO
>     ifeq ($(SOMETHING),YES)
>         Meh $(SOMETHING)
>     else
>         Meh --$(SOMETHING)--
>     endif
>     endef
>     $(info $(FOO))
> 
> as that would yield:
> 
>     ifeq (YES,YES)
>         Meh YES
>     else
>         Meh --YES--
>     endif
> 
> So, we use intermediate variables.
> 
> Finally, the variable is $(strip...)ed before being displayed, so as to
> make it fit on a single line, one per package info.

 Oh, I thought strip just stripped starting and trailing whitespace, not the
whitespace in the middle... Apparently I was wrong. So the JSON is in fact
almost-minified already. I wonder, then, what is the value of still adding that
additional space after the comma.

> This is just JSON,
> so it does not matter, but at least it should be parallel-safe in the
> future.

 Well, only if the lines remain under BUFSIZ, and only if you don't unbuffer the
output (i.e. if you don't use brmake...).


> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  package/pkg-generic.mk | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f78ff2e665..72b1a87f5f 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -843,6 +843,49 @@ $(1)-external-deps:
>  	@echo "file://$$($(2)_OVERRIDE_SRCDIR)"
>  endif
>  
> +ifeq ($$($(2)_IS_VIRTUAL),YES)
> +define $(2)_SHOW_INFO_VIRTUAL
> +	"virtual": true,
> +endef
> +else
> +define $(2)_SHOW_INFO_VIRTUAL
> +	"virtual": false,
> +	"version": "$$($(2)_DL_VERSION)",
> +	"licenses": "$$($(2)_LICENSE)",
> +	"downloads": [
> +	$$(foreach dl,$$($(2)_ALL_DOWNLOADS),
> +		{
> +			"source": "$$(notdir $$(dl))",
> +			"URIs": [
> +			$$(call make-comma-list,
> +				$$(subst \|,|,

 Is this really needed? Isn't "\|" and "|" the same in JSON?

> +					$$(call DOWNLOAD_URIS,$$(dl),$(2))
> +				)
> +			)
> +			]
> +		},
> +	)

 I think you might be able to get rid of the null like this:

	$$(subst }{,}$$(comma){,
		$$(foreach dl,$$($(2)_ALL_DOWNLOADS),{
			...
		})

 Ugly, I know :-)

> +	null
> +	],
> +endef
> +endif
> +
> +define $(2)_SHOW_INFO
> +	"name": "$(1)",
> +	"type": "$(4)",
> +	$$($(2)_SHOW_INFO_VIRTUAL)
> +	"depends on": [
> +		$$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
> +	],
> +	"dependency of": [
> +		$$(call make-comma-list,$$($(2)_RDEPENDENCIES))
> +	]
> +endef

 Maybe it would be better to make this a function (defined outside of
inner-generic-package) rather than a per-package variable.

> +
> +$(1)-show-info:
> +	@:
> +	$$(info { $$(strip $$($(2)_SHOW_INFO)) })$$($(2)_ALL_DOWNLOADS),

 So here you'd use

	$$(info { $$(strip $$(call show-info,$(1),$(2))) })


 Regards,
 Arnout

> +
>  $(1)-show-version:
>  			@echo $$($(2)_VERSION)
>  
> @@ -1099,6 +1142,7 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
>  	$(1)-rsync \
>  	$(1)-show-dependency-tree \
>  	$(1)-show-depends \
> +	$(1)-show-info \
>  	$(1)-show-version \
>  	$(1)-source
>  
> 

[Buildroot] [PATCH 10/10] infra: introduce top-level, global show-info

[Arnout Vandecappelle]

On 07/04/2019 13:51, Yann E. MORIN wrote:
> Now that packages and filesystems can report their recursive info, use
> that to dump the whole info as a valid JSOn array.
> 
> As it is rather complex to ensure soemthing is run before every rules
> (to display the eldin opening '[' so as to have a valid JSON array),
> re-enter the Makefile and mangle the output with a sed expression, that:
> 
>   - 0,/^/s//[\n/;   inserts a '[' before the first line,
>   - $!s/$/,/;       appends a ',' at the end of every lines but the last,
>   - $s/$/\n]/       appends a ']' after the last line.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> ---
>  Makefile | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 60bf7d7d08..13c4390ae4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -876,6 +876,12 @@ graph-depends-requirements:
>  	@dot -? >/dev/null 2>&1 || \
>  		{ echo "ERROR: The 'dot' program from Graphviz is needed for graph-depends" >&2; exit 1; }
>  
> +.PHONY: show-info _show-info-private
> +show-info:
> +	@$(MAKE) -C $(CONFIG_DIR) _show-info-private |sed -r -e '0,/^/s//[\n/; $$!s/$$/,/; $$s/$$/\n]/'

 This would be so much simpler as $(subst }$(sep){,}$(comma)$(sep){,...) but
then of course you need to recurse through $(shell) instead of calling MAKE
directly, which makes it very ugly again...

 But of course, I would prefer this to be one big call:

ALL_PACKAGES = $(sort $(foreach pkg,$(PACKAGES) $(TARGETS_ROOTFS),\
	$($(call UPPERCASE,$(pkg))_FINAL_RECURSIVE_DEPENDENCIES)))

show-info:
	@:
	@$(info $(subst }$(sep){,}$(comma)$(sep){,\
		$(foreach pkg,$(ALL_PACKAGES),\
			$(call show-info,$(pkg),$(call UPPERCASE $(pkg))))))

> +_show-info-private: $(patsubst %,%-show-recursive-info,$(PACKAGES) $(TARGETS_ROOTFS))
> +	@:

 Why do you need an actual command for this rule? A dependency-only rule would
be OK, no?

 Regards,
 Arnout

> +
>  .PHONY: show-dependency-tree
>  show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(PACKAGES) $(TARGETS_ROOTFS))
>  	@:
> 

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Yann E. MORIN]

Arnout, All,

On 2019-04-13 10:58 +0200, Arnout Vandecappelle spake thusly:
> On 07/04/2019 13:51, Yann E. MORIN wrote:
> > Users are increasingly trying to extract information about packages.
> > For example, they might need to get the list of URIs, or the
> > dependencies of a package.
> > 
> > Although we do have a bunch of rules to generate some of that, this is
> > done in ad-hic way, with most of the output formats just as-hoc, raw,
> > unformatted blurbs, mostly internal data dumped as-is.
> > 
> > Introduce a new rule, foo-show-info, that provides a properly formatted
> > output of all the meta-information about a package: name, type, version,
> > licenses, dependencies...
> 
>  I love the concept of this patch series, because it makes it much easier to
> write external scripts but also to write our own internal scripts (graph-depends
> etc.). However, I have a number of gripes with the chosen implementation. Only
> the first one is essential, the rest is nice-to-have.
> 
> * The null in the downloads list is really silly, and makes life difficult for
> the user because it has to be filtered out again (cfr. Thomas's PoC jq script).
> So it really has to go.

I'll try to see how it can be done with your sugestion below. Not that I
find the code nice, though...

> * I think the structure should always be completely the same. So a virtual
> package should still have version: "", licenses: "", downloads: []. This again
> makes it easier to process things.

I really wondered about that one. But to me, it does not make sense that
a virtual package has a license or a version: it is virtual... Yet, there
might be a point in having that information however, because, apart from
being virtual, they actually *can* install things in the target (or host
or staging).

But then, what about filesystems? Semantically, it does not make sense
at all to have a license or a version for them...

> * I think it would be better to output per-package minified strings (i.e.
> without whitespace).

As you noticed later, this is already the case.

> To make it human-readable, you can just pipe through jq.
> The advantage of the minified string is that you can just grep for some metadata
> that you're interested in and get all the lines of info that you need - this is
> a lot easier than writing a jq filter. It also gives a solution for the null,
> because you can just do $(subst $(comma)},},...) to get rid of the redundant comma.

Hmm... That's a neat trick, which could also be applied to $(comma)] as
well, I guess...

> * I absolutely hate these per-package rules. I also don't think it would behave
> correctly: I would expect 'make {foo,bar}-show-info' to output JSON, but it
> doesn't (as you note for the show-recursive-info). So I think we should just
> have a top-level show-info, which uses _FINAL_RECURSIVE_DEPENDENCIES to iterate
> over all packages (note that that variable would have to be added for rootfs as
> well). If we want to support selecting a subset of packages, we could add
> something like the VARS of printvars.

It would certainly simplify things a little bit... But I do like to keep
the per-package rules consistent each with the others:
    foo-build
    foo-install
    foo-reconfigure
    foo-legal-info
    foo-show-info

So, I'll at least keep foo-show-info, but will ditch the recursive one.

> * I have a vague feeling that the _SHOW_INFO variable looks ugly, but I can't
> put my finger on it what makes it ugly (or how it could be better).

I honestly ca'tn see uglyness in that one. _SHOW_INFO_DETAILS, which is
conditional, is indeed not nice because of the conditional block...

But really, I don't see a better solution.

> * Maybe it would be better if the top-level structure where a dict as well:
> "pkg": { "type": ... }.

Can do.

> * I think it would be useful to also export either the uppercase name of the
> package, or at least the kconfig symbol. In fact, I would export as much info as
> we can - though that can of course be done in follow-up patches. infra would be
> a nice one, but we don't have a variable for that at the moment.

This can be added later, in followup patches. That's the interesting
part of this solution: we can add more information to the output without
breaking existing consumers.

snip
> > The complex part of this change was the conditional output of parts of
> > the data: virtual packages have no source, version, license or
> > downloads, unlike non-virtual packages. 
> 
>  So, as I wrote, I think we should keep the same structure, which makes this
> extremely trivial because the empty source, version, license variables will just
> do the right thing.

Sorry, I still disagree, if at least because for filesystems it does not
even make sense. So a consumer of the JSON output will anyway have to
cope with this, so it should also cope with the rest.

I will keep the difference for now.

[--SNIP--]
> > Finally, the variable is $(strip...)ed before being displayed, so as to
> > make it fit on a single line, one per package info.
> 
>  Oh, I thought strip just stripped starting and trailing whitespace, not the
> whitespace in the middle... Apparently I was wrong. So the JSON is in fact
> almost-minified already. I wonder, then, what is the value of still adding that
> additional space after the comma.

You mean, in make-comma-list?

I still believe the output should be readable by humans too, so a space
after the comma is still nice.

> > This is just JSON,
> > so it does not matter, but at least it should be parallel-safe in the
> > future.
> 
>  Well, only if the lines remain under BUFSIZ, and only if you don't unbuffer the
> output (i.e. if you don't use brmake...).

Hmm.. I'll remove that part of the commit log.

> > +	$$(foreach dl,$$($(2)_ALL_DOWNLOADS),
> > +		{
> > +			"source": "$$(notdir $$(dl))",
> > +			"URIs": [
> > +			$$(call make-comma-list,
> > +				$$(subst \|,|,
>  Is this really needed? Isn't "\|" and "|" the same in JSON?

YUes it is required, otherwise tools liek json_pp will whine:

    $ echo '{ "a": "\|" }' |json_pp
    illegal backslash escape sequence in string, at character offset 8
    (before "\\|" }\n")

jq whines, too: parse error: Invalid escape at line 1, column 11

> > +					$$(call DOWNLOAD_URIS,$$(dl),$(2))
> > +				)
> > +			)
> > +			]
> > +		},
> > +	)
> 
>  I think you might be able to get rid of the null like this:
> 
> 	$$(subst }{,}$$(comma){,
> 		$$(foreach dl,$$($(2)_ALL_DOWNLOADS),{
> 			...
> 		})
> 
>  Ugly, I know :-)

And then you fond that the _SHOW_INFO variable is ugly, and you want me
to add more uglyness to it? Meh! ;-)

But yes, your subst trick is interesting.

> > +	null
> > +	],
> > +endef
> > +endif
> > +
> > +define $(2)_SHOW_INFO
> > +	"name": "$(1)",
> > +	"type": "$(4)",
> > +	$$($(2)_SHOW_INFO_VIRTUAL)
> > +	"depends on": [
> > +		$$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
> > +	],
> > +	"dependency of": [
> > +		$$(call make-comma-list,$$($(2)_RDEPENDENCIES))
> > +	]
> > +endef
> 
>  Maybe it would be better to make this a function (defined outside of
> inner-generic-package) rather than a per-package variable.

Good idea, indeed.

> > +$(1)-show-info:
> > +	@:
> > +	$$(info { $$(strip $$($(2)_SHOW_INFO)) })$$($(2)_ALL_DOWNLOADS),
> 
>  So here you'd use
> 
> 	$$(info { $$(strip $$(call show-info,$(1),$(2))) })

Right.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 00/10] infra: add solution to dump metadata from packages (branch yem/misc)

[Yann E. MORIN]

Arnout, All,

On 2019-04-13 10:00 +0200, Arnout Vandecappelle spake thusly:
> On 11/04/2019 23:20, Yann E. MORIN wrote:
> >> I'm not at all saying that we shouldn't merge this JSON output, I just
> >> wanted to point out the different ways that already exist to extract
> >> some package metadata out of Buildroot.
> > Well, my idea wuld maybe to actually phase-out those rules, or at least
> > base them on this new JSON output.
> > 
> > Heck, even graph-depends could use that instead of the recently
> > introduced show-dependency-tree, and this is what I was going to do
> > before the release, so that show-dependency-tree never ever gets into
> > a release.
> 
>  Indeed, I think show-info can replace a lot of our special targets:
> show-version, show-*depends, external-deps.

I already changed graph-depends to use that instead of show-dependency-graph.
That wil be in v2.

Also, I have a patch removing show-dependency-graph altogether, since it
no longer is useful, and never made it to a release.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 06/10] infra/pkg-generic: introduce foo-show-info

[Arnout Vandecappelle]

On 13/04/2019 19:17, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2019-04-13 10:58 +0200, Arnout Vandecappelle spake thusly:
[snip]
>> * The null in the downloads list is really silly, and makes life difficult for
>> the user because it has to be filtered out again (cfr. Thomas's PoC jq script).
>> So it really has to go.
> 
> I'll try to see how it can be done with your sugestion below. Not that I
> find the code nice, though...

 True. But here, I think niceness of the user-visible output trumps niceness of
the implementation.


>> * I think the structure should always be completely the same. So a virtual
>> package should still have version: "", licenses: "", downloads: []. This again
>> makes it easier to process things.
> 
> I really wondered about that one. But to me, it does not make sense that
> a virtual package has a license or a version: it is virtual... Yet, there
> might be a point in having that information however, because, apart from
> being virtual, they actually *can* install things in the target (or host
> or staging).
> 
> But then, what about filesystems? Semantically, it does not make sense
> at all to have a license or a version for them...

 So, the reason I like to have the same structure everywhere is Python, which
gives you KeyError rather than None when a key isn't present in the dict. Thus,
a simple filter like this will give an exception:

[p for p in pkginfo if "GPL" in p["licenses"]]

 But maybe I'm making too big a deal of this, since it can just be written as:

[p for p in pkginfo if "GPL" in p.get("licenses","")]


[snip]
>> * I absolutely hate these per-package rules. I also don't think it would behave
>> correctly: I would expect 'make {foo,bar}-show-info' to output JSON, but it
>> doesn't (as you note for the show-recursive-info). So I think we should just
>> have a top-level show-info, which uses _FINAL_RECURSIVE_DEPENDENCIES to iterate
>> over all packages (note that that variable would have to be added for rootfs as
>> well). If we want to support selecting a subset of packages, we could add
>> something like the VARS of printvars.
> 
> It would certainly simplify things a little bit... But I do like to keep
> the per-package rules consistent each with the others:
>     foo-build
>     foo-install
>     foo-reconfigure
>     foo-legal-info
>     foo-show-info
> 
> So, I'll at least keep foo-show-info, but will ditch the recursive one.

 As I wrote, I don't like foo-show-info since it gives different output than
show-info (a single dict rather than a list of dicts) and it doesn't output
valid JSON when multiple packages are requested simultaneously (which would be a
pretty typical use case I expect). Still, no big deal.


>> * I have a vague feeling that the _SHOW_INFO variable looks ugly, but I can't
>> put my finger on it what makes it ugly (or how it could be better).
> 
> I honestly ca'tn see uglyness in that one.

 My first reaction was "this doesn't look like Buildroot code". But I think
maybe it's just because it's JSON, and it doesn't look at all anymore like our
normal formatting.

> _SHOW_INFO_DETAILS, which is
> conditional, is indeed not nice because of the conditional block...
> 
> But really, I don't see a better solution.
[snip]
>>> +					$$(call DOWNLOAD_URIS,$$(dl),$(2))
>>> +				)
>>> +			)
>>> +			]
>>> +		},
>>> +	)
>>
>>  I think you might be able to get rid of the null like this:
>>
>> 	$$(subst }{,}$$(comma){,
>> 		$$(foreach dl,$$($(2)_ALL_DOWNLOADS),{
>> 			...
>> 		})
>>
>>  Ugly, I know :-)
> 
> And then you fond that the _SHOW_INFO variable is ugly, and you want me
> to add more uglyness to it? Meh! ;-)
> 
> But yes, your subst trick is interesting.

 Might warrant making a function for it, like make-comma-list.

> 
>>> +	null
>>> +	],
>>> +endef
>>> +endif
>>> +
>>> +define $(2)_SHOW_INFO
>>> +	"name": "$(1)",
>>> +	"type": "$(4)",
>>> +	$$($(2)_SHOW_INFO_VIRTUAL)
>>> +	"depends on": [
>>> +		$$(call make-comma-list,$$($(2)_FINAL_ALL_DEPENDENCIES))
>>> +	],
>>> +	"dependency of": [
>>> +		$$(call make-comma-list,$$($(2)_RDEPENDENCIES))
>>> +	]
>>> +endef
>>
>>  Maybe it would be better to make this a function (defined outside of
>> inner-generic-package) rather than a per-package variable.
> 
> Good idea, indeed.

 That's not going to fly with the condition on virtual, though. That one you
can't easily put in a function. And if you anyway have _SHOW_INFO_VIRTUAL, you
can just as well have _SHOW_INFO as well. Remove the special casing of virtual,
I say! :-)


 Regards,
 Arnout


>>> +$(1)-show-info:
>>> +	@:
>>> +	$$(info { $$(strip $$($(2)_SHOW_INFO)) })$$($(2)_ALL_DOWNLOADS),
>>
>>  So here you'd use
>>
>> 	$$(info { $$(strip $$(call show-info,$(1),$(2))) })
> 
> Right.
> 
> Regards,
> Yann E. MORIN.
>