All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 01/12 v2] infra/pkg-download: return just a list of URIs
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 02/12 v2] infra/pkg-download: make the URI list a callable macro Yann E. MORIN
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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>

---
Changes v1 -> v2:
  - use foreach instead of patsubst  (Thomas)
---
 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..4163333998 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) \
+		$(foreach uri,$(DOWNLOAD_URIS),-u $(uri)) \
 		$(QUIET) \
 		-- \
 		$($(PKG)_DL_OPTS)
-- 
2.14.1

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

* [Buildroot] [PATCH 02/12 v2] infra/pkg-download: make the URI list a callable macro
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 01/12 v2] infra/pkg-download: return just a list of URIs Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 03/12 v2] infra/pkg-download: get rid of the FLOCK variable Yann E. MORIN
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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>

---
Changes v1 -> v2:
  - split DOWNLOAD/DOWNLOAD_URIS comment  (Thomas DS)
---
 package/pkg-download.mk | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 4163333998..623caf9325 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -63,19 +63,19 @@ github = https://github.com/$(1)/$(2)/archive/$(3)
 export BR_NO_CHECK_HASH_FOR =
 
 ################################################################################
-# DOWNLOAD -- Download helper. Will call DL_WRAPPER which will try to download
-# source from:
+# DOWNLOAD_URIS - List the candidates URIs where to get the package from:
 # 1) BR2_PRIMARY_SITE if enabled
 # 2) Download site, unless BR2_PRIMARY_SITE_ONLY is set
 # 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
 #
 ################################################################################
 
 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,11 +84,19 @@ 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
 
+################################################################################
+# DOWNLOAD -- Download helper. Will call DL_WRAPPER which will try to download
+# source from the list returned by DOWNLOAD_URIS.
+#
+# Argument 1 is the source location
+#
+################################################################################
+
 define DOWNLOAD
 	$(Q)mkdir -p $($(PKG)_DL_DIR)
 	$(Q)$(EXTRA_ENV) $(FLOCK) $(DL_WRAPPER) \
@@ -101,7 +109,7 @@ define DOWNLOAD
 		-N '$($(PKG)_RAWNAME)' \
 		-o '$($(PKG)_DL_DIR)/$(notdir $(1))' \
 		$(if $($(PKG)_GIT_SUBMODULES),-r) \
-		$(foreach uri,$(DOWNLOAD_URIS),-u $(uri)) \
+		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
 		$(QUIET) \
 		-- \
 		$($(PKG)_DL_OPTS)
-- 
2.14.1

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

* [Buildroot] [PATCH 03/12 v2] infra/pkg-download: get rid of the FLOCK variable
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 01/12 v2] infra/pkg-download: return just a list of URIs Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 02/12 v2] infra/pkg-download: make the URI list a callable macro Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised Yann E. MORIN
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
---
 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 623caf9325..96a12b1f2e 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)
@@ -99,7 +98,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

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

* [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (2 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 03/12 v2] infra/pkg-download: get rid of the FLOCK variable Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-15  9:08   ` Arnout Vandecappelle
  2019-04-14 20:17 ` [Buildroot] [PATCH 05/12 v2] infra/utils: add helper to generate comma-separated lists Yann E. MORIN
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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>

---
changes v1 -> v2:
  - DOWNLOAD_URIS was called twice in v1, once with $(PKG) and once with
    $(1) (due to an incorrect rebase)
---
 package/pkg-download.mk | 23 ++++++++++++-----------
 package/pkg-generic.mk  |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 96a12b1f2e..de619ba90a 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -93,23 +93,24 @@ endif
 # source from the list returned by DOWNLOAD_URIS.
 #
 # Argument 1 is the source location
+# Argument 2 is the upper-case package name
 #
 ################################################################################
 
 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) \
-		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(PKG)),-u $(uri)) \
+		-H '$($(2)_HASH_FILE)' \
+		-n '$($(2)_BASENAME_RAW)' \
+		-N '$($(2)_RAWNAME)' \
+		-o '$($(2)_DL_DIR)/$(notdir $(1))' \
+		$(if $($(2)_GIT_SUBMODULES),-r) \
+		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \
 		$(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

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

* [Buildroot] [PATCH 05/12 v2] infra/utils: add helper to generate comma-separated lists
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (3 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 06/12 v2] fs: introduce variables with name and type Yann E. MORIN
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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

This will be useful 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>

---
Changes v1 -> v2:
  - don't rely on implicit space in replacement text, instead use
    explicit $(space)  (Thmas DS)
---
 support/misc/utils.mk | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/support/misc/utils.mk b/support/misc/utils.mk
index c44319338e..b422c67d76 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)$(space),$(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

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

* [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2)
@ 2019-04-14 20:17 Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 01/12 v2] infra/pkg-download: return just a list of URIs Yann E. MORIN
                   ` (11 more replies)
  0 siblings, 12 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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 package 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. 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:

    $ make cracklib-show-info |jq . -
    {
      "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"
            ]
          }
        ],
        "dependencies": [
          "host-cracklib",
          "host-skeleton",
          "skeleton",
          "toolchain"
        ],
        "reverse_dependencies": [
          "libpwquality",
          "linux-pam"
        ]
      }
    }

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

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

Changes v1 -> v2:
  - DOWNLOAD_URIS uses foreach instead of patsubst  (Thomas P.)
  - rename dependencies keys  (Thomas DS)
  - split DOWNLOAD/DOWNLOAD_URIS comments  (Thomas DS)
  - make-comma-list uses an explicit $(space)  (Thomas DS)
  - top-level show-info is a dictionary, not an array  (Arnout)
  - use a macro to generate a JSON element for an item  (Arnout)
  - drop the 'null' object in download list  (Arnout, Thomas DS)
  - change graph-depends to use that instead of show-dependency-graph
  - drop show-dependency-graph
  - fix double list of URIs
  - fix typoes


Regards,
Yann E. MORIN.


The following changes since commit 46cd7061228720ddb93f540bc733359304d8118f

  package/dacapo: fix indentation (2019-04-14 17:20:26 +0200)


are available in the git repository at:

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

for you to fetch changes up to 3f4a75c33a5f817dcd7a3576086ffca4c18e522b

  core: remove show-depednency-tree (2019-04-14 17:56:08 +0200)


----------------------------------------------------------------
Yann E. MORIN (12):
      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
      fs: introduce variables with name and type
      fs: introduce variable with all recursive dependencies
      fs: add all recursive dependencies to packages list
      core: introduce new global show-info
      core: add per-package and per-filesystem show-info
      support/scripts: use show-info to extract dependency graph
      core: remove show-depednency-tree

 Makefile                     | 19 ++++++++++---
 fs/common.mk                 | 45 +++++++++++++++++++++++------
 package/pkg-download.mk      | 46 +++++++++++++++++-------------
 package/pkg-generic.mk       |  9 +++---
 package/pkg-utils.mk         | 68 ++++++++++++++++++++++++++++++++++++++++++++
 support/misc/utils.mk        |  4 +++
 support/scripts/brpkgutil.py | 31 +++++++++-----------
 7 files changed, 169 insertions(+), 53 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.  |
'------------------------------^-------^------------------^--------------------'

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

* [Buildroot] [PATCH 06/12 v2] fs: introduce variables with name and type
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (4 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 05/12 v2] infra/utils: add helper to generate comma-separated lists Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 07/12 v2] fs: introduce variable with all recursive dependencies Yann E. MORIN
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

This makes the filesystems resemble packages yet a bit more, and will
allow sorting "items" on their type and names, when indexed from the
upper-case names.

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

diff --git a/fs/common.mk b/fs/common.mk
index 4ad51fdd0a..286d671d06 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -42,6 +42,8 @@ define ROOTFS_REPRODUCIBLE
 endef
 endif
 
+ROOTFS_COMMON_NAME = rootfs-common
+ROOTFS_COMMON_TYPE = rootfs
 ROOTFS_COMMON_DEPENDENCIES = \
 	host-fakeroot host-makedevs \
 	$(BR2_TAR_HOST_DEPENDENCY) \
@@ -77,6 +79,8 @@ rootfs-common-show-depends:
 # all variable references except the arguments must be $$-quoted.
 define inner-rootfs
 
+ROOTFS_$(2)_NAME = rootfs-$(1)
+ROOTFS_$(2)_TYPE = rootfs
 ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1)
 ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(strip $$(ROOTFS_$(2)_IMAGE_NAME))
 ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1)
-- 
2.14.1

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

* [Buildroot] [PATCH 07/12 v2] fs: introduce variable with all recursive dependencies
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (5 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 06/12 v2] fs: introduce variables with name and type Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list Yann E. MORIN
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

This allows getting all the recursive dependencies of filesystems,
ike we have for packages, and allows us to treat both in a similar
fashion.

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

diff --git a/fs/common.mk b/fs/common.mk
index 286d671d06..5ec28ca183 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -49,6 +49,18 @@ ROOTFS_COMMON_DEPENDENCIES = \
 	$(BR2_TAR_HOST_DEPENDENCY) \
 	$(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd)
 
+ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES = $(sort \
+	$(if $(filter undefined,$(origin ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES__X)), \
+		$(eval ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES__X := \
+			$(foreach p, \
+				$(ROOTFS_COMMON_DEPENDENCIES), \
+				$(p) \
+				$($(call UPPERCASE,$(p))_FINAL_RECURSIVE_DEPENDENCIES) \
+			) \
+		) \
+	) \
+	$(ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES__X))
+
 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)))
@@ -88,6 +100,18 @@ ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
 
 ROOTFS_$(2)_DEPENDENCIES += rootfs-common
 
+ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES = $$(sort \
+	$$(if $$(filter undefined,$$(origin ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES__X)), \
+		$$(eval ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES__X := \
+			$$(foreach p, \
+				$$(ROOTFS_$(2)_DEPENDENCIES), \
+				$$(p) \
+				$$($$(call UPPERCASE,$$(p))_FINAL_RECURSIVE_DEPENDENCIES) \
+			) \
+		) \
+	) \
+	$$(ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES__X))
+
 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

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

* [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (6 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 07/12 v2] fs: introduce variable with all recursive dependencies Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-15  9:48   ` Arnout Vandecappelle
  2019-04-14 20:17 ` [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info Yann E. MORIN
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

Currently, only first-level dependencies of a filesystem are added to
the global list of packages, thus missing all recursive dependencies.

Use the newly introduced recursive variable instead, which already
contains the rootfs-common dependencies too.

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

diff --git a/fs/common.mk b/fs/common.mk
index 5ec28ca183..f7989eac57 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -190,7 +190,7 @@ rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME)
 
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
-PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES) $$(ROOTFS_COMMON_DEPENDENCIES))
+PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES))
 endif
 
 # Check for legacy POST_TARGETS rules
-- 
2.14.1

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (7 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-15 12:17   ` Arnout Vandecappelle
  2019-04-14 20:17 ` [Buildroot] [PATCH 10/12 v2] core: add per-package and per-filesystem show-info Yann E. MORIN
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

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-hoc way, with most of the output formats just ad-hoc, raw,
unformatted blurbs, mostly internal data dumped as-is.

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

We choose to use JSON as the output format, because it is pretty
versatile, has parsers in virtually all languages, has tools 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. Same goes for filesystems. We
use a wrapper macro, show-info, that de-multiplexes unto either the
package-related- or filesystem-related macros, and for packages, we also
use a detailed macro for non-virtual packages.

It is non-trivial to properly output correct JSON blurbs, especially
when trying to output an array of objects, like so, where the last item
shall not be followed by a comma:  [ { ... }, { ... } ]

So, we use a trick (as sugegsted by Arnout), to $(subst) any pair of
",}" or ", }" or ",]" or ", ]" with only the respective closing symbol,
"}" or "]".

The whole stuff is $(strip)ed to make it a somewhat-minified JSON blurb
that fits on a single line with all spaces squashed (but still with
spaces, as it is not possible to differentiate spaces between JSON
elements from spaces inside JSON strings).

Reported-by: Thomas De Schampheleire <patrickdepinguin@gmail.com>
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>

---
Changes v1 -> v2:
  - make it a macro to be called  (Arnout)
  - make it a top-level rule  (Arnout)
---
 Makefile             | 15 ++++++++++++
 package/pkg-utils.mk | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/Makefile b/Makefile
index 60bf7d7d08..33215f91e5 100644
--- a/Makefile
+++ b/Makefile
@@ -903,6 +903,21 @@ check-dependencies:
 	@cd "$(CONFIG_DIR)"; \
 	$(TOPDIR)/support/scripts/graph-depends -C
 
+.PHONY: show-info
+show-info:
+	@:
+	$(info $(call clean-json, \
+			$(foreach p, \
+				$(sort $(foreach i,$(PACKAGES) $(TARGETS_ROOTFS), \
+						$(i) \
+						$($(call UPPERCASE,$(i))_FINAL_RECURSIVE_DEPENDENCIES) \
+					) \
+				), \
+				$(call show-info,$(call UPPERCASE,$(p)))$(comma) \
+			) \
+		) \
+	)
+
 else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 
 # Some subdirectories are also package names. To avoid that "make linux"
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index bffd79dfb0..00d1d6bcac 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -62,6 +62,74 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+# show-info -- return package or filesystem metadata formatted as an entry
+#              of a JSON dictionnary
+# $(1): upper-case package or filesystem name
+define show-info
+	"$($(1)_NAME)": {
+		"type": "$($(1)_TYPE)",
+		$(if $(filter rootfs,$($(1)_TYPE)), \
+			$(call _show-info-fs,$(1)), \
+			$(call _show-info-pkg,$(1)), \
+		)
+	}
+endef
+
+# _show-info-pkg, _show-info-pkg-details, _show-info-fs: private helpers
+# for show-info, above
+define _show-info-pkg
+	$(if $($(1)_IS_VIRTUAL), \
+		"virtual": true$(comma),
+		$(call _show-info-pkg-details,$(1)) \
+	)
+	"dependencies": [
+		$(call make-comma-list,$(sort $($(1)_FINAL_ALL_DEPENDENCIES)))
+	],
+	"reverse_dependencies": [
+		$(call make-comma-list,$(sort $($(1)_RDEPENDENCIES)))
+	]
+endef
+define _show-info-pkg-details
+	"virtual": false,
+	"version": "$($(1)_DL_VERSION)",
+	"licenses": "$($(1)_LICENSE)",
+	"downloads": [
+	$(foreach dl,$(sort $($(1)_ALL_DOWNLOADS)),
+		{
+			"source": "$(notdir $(dl))",
+			"URIs": [
+				$(call make-comma-list,
+					$(subst \|,|,
+						$(call DOWNLOAD_URIS,$(dl),$(1))
+					)
+				)
+			]
+		},
+	)
+	],
+endef
+define _show-info-fs
+	"dependencies": [
+		$(call make-comma-list,$(sort $($(1)_DEPENDENCIES)))
+	]
+endef
+
+# clean-json -- cleanup pseudo-json into clean json:
+#  - adds opening { and closing }
+#  - remove commas before closing ] and }
+#  - minify with $(strip)
+clean-json = $(strip \
+	$(subst $(comma)},, \
+		$(subst $(comma)$(space)},$(space)}, \
+			$(subst $(comma)],, \
+				$(subst $(comma)$(space)],$(space)], \
+					{ $(strip $(1)) } \
+				) \
+			) \
+		) \
+	) \
+)
+
 #
 # legal-info helper functions
 #
-- 
2.14.1

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

* [Buildroot] [PATCH 10/12 v2] core: add per-package and per-filesystem show-info
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (8 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 11/12 v2] support/scripts: use show-info to extract dependency graph Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 12/12 v2] core: remove show-depednency-tree Yann E. MORIN
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

Sometimes, it is need to quickly get the metadata of a subset of
packages, without resorting to a full-blown JSON query.

Introduce a new per-package (and per-filesystem) foo-show-info rule,
that otputs a per-entity valid JSON blob.

Note that calling it for multiple packages and.or filesystems at once
will not generate a valid JSON blob, as there would be no separator
between the JSON elements:

    $ make {foo,bar}-show-info
    { "foo": { foo stuff } }
    { "bar": { bar stuff } }

However, jq is able to absorb this, with its slurping ability, which
generates an array (ellipsed and manualy reformated for readability):

    $ make {foo,bar}-show-info |jq -s . -
    [
      { "foo": { foo stuff } },
      { "bar": { bar stuff } }
    ]

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

diff --git a/fs/common.mk b/fs/common.mk
index f7989eac57..ce17c35c59 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -87,6 +87,11 @@ endif
 rootfs-common-show-depends:
 	@echo $(ROOTFS_COMMON_DEPENDENCIES)
 
+.PHONY: rootfs-common-show-info
+rootfs-common-show-info:
+	@:
+	$(info $(call clean-json,$(call show-info,ROOTFS_COMMON)))
+
 # Since this function will be called from within an $(eval ...)
 # all variable references except the arguments must be $$-quoted.
 define inner-rootfs
@@ -184,9 +189,13 @@ endif
 rootfs-$(1)-show-depends:
 	@echo $$(ROOTFS_$(2)_DEPENDENCIES)
 
+rootfs-$(1)-show-info:
+	@:
+	$$(info $$(call clean-json,$$(call show-info,ROOTFS_$(2))))
+
 rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME)
 
-.PHONY: rootfs-$(1) rootfs-$(1)-show-depends
+.PHONY: rootfs-$(1) rootfs-$(1)-show-depends rootfs-$(1)-show-info
 
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
 TARGETS_ROOTFS += rootfs-$(1)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f78ff2e665..25bc2e527a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -867,6 +867,10 @@ $(1)-show-dependency-tree: $$(patsubst %,%-show-dependency-tree,$$($(2)_FINAL_AL
 	$$(info $(1): $(4) $$(if $$($(2)_IS_VIRTUAL),virtual,$$($(2)_DL_VERSION)))
 	$$(info $(1) -> $$($(2)_FINAL_ALL_DEPENDENCIES))
 
+$(1)-show-info:
+	@:
+	$$(info $$(call clean-json,$$(call show-info,$(2))))
+
 $(1)-graph-depends: graph-depends-requirements
 	$(call pkg-graph-depends,$(1),--direct)
 
@@ -1099,6 +1103,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

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

* [Buildroot] [PATCH 11/12 v2] support/scripts: use show-info to extract dependency graph
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (9 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 10/12 v2] core: add per-package and per-filesystem show-info Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  2019-04-14 20:17 ` [Buildroot] [PATCH 12/12 v2] core: remove show-depednency-tree Yann E. MORIN
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

Currently, we extract the dependency graph from the aptly named but
ad-hoc show-dependency-graph rule.

We now have a better solution to report package information, with
show-info.

Since show-dependency-graph never went into a release so far, and
show-info does provide the same (and more), swith to using show-info.

Thanks to Adam for suggesting the coding style to have a readable code
that is not ugly but still pleases flake8. Thanks to Arnout for
suggesting the use of dict.get() to further simplify the code.

Reported-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Adam Duskett <aduskett@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
 support/scripts/brpkgutil.py | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/support/scripts/brpkgutil.py b/support/scripts/brpkgutil.py
index f65068d348..22ceab2b10 100644
--- a/support/scripts/brpkgutil.py
+++ b/support/scripts/brpkgutil.py
@@ -1,10 +1,10 @@
 # Copyright (C) 2010-2013 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
 # Copyright (C) 2019 Yann E. MORIN <yann.morin.1998@free.fr>
 
+import json
 import logging
 import os
 import subprocess
-from collections import defaultdict
 
 
 # This function returns a tuple of four dictionaries, all using package
@@ -19,8 +19,8 @@ from collections import defaultdict
 def get_dependency_tree():
     logging.info("Getting dependency tree...")
 
-    deps = defaultdict(list)
-    rdeps = defaultdict(list)
+    deps = {}
+    rdeps = {}
     types = {}
     versions = {}
 
@@ -29,23 +29,20 @@ def get_dependency_tree():
     types['all'] = 'target'
     versions['all'] = ''
 
-    cmd = ["make", "-s", "--no-print-directory", "show-dependency-tree"]
+    cmd = ["make", "-s", "--no-print-directory", "show-info"]
     with open(os.devnull, 'wb') as devnull:
         p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=devnull,
                              universal_newlines=True)
-        output = p.communicate()[0]
+        pkg_list = json.loads(p.communicate()[0])
 
-    for l in output.splitlines():
-        if " -> " in l:
-            pkg = l.split(" -> ")[0]
-            deps[pkg] += l.split(" -> ")[1].split()
-            for p in l.split(" -> ")[1].split():
-                rdeps[p].append(pkg)
-        else:
-            pkg, type_version = l.split(": ", 1)
-            t, v = "{} -".format(type_version).split(None, 2)[:2]
-            deps['all'].append(pkg)
-            types[pkg] = t
-            versions[pkg] = v
+    for pkg in pkg_list:
+        deps['all'].append(pkg)
+        types[pkg] = pkg_list[pkg]["type"]
+        deps[pkg] = pkg_list[pkg].get("dependencies", [])
+        rdeps[pkg] = pkg_list[pkg].get("reverse_dependencies", [])
+        versions[pkg] = \
+            None if pkg_list[pkg]["type"] == "rootfs" \
+            else "virtual" if pkg_list[pkg]["virtual"] \
+            else pkg_list[pkg]["version"]
 
     return (deps, rdeps, types, versions)
-- 
2.14.1

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

* [Buildroot] [PATCH 12/12 v2] core: remove show-depednency-tree
  2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
                   ` (10 preceding siblings ...)
  2019-04-14 20:17 ` [Buildroot] [PATCH 11/12 v2] support/scripts: use show-info to extract dependency graph Yann E. MORIN
@ 2019-04-14 20:17 ` Yann E. MORIN
  11 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-14 20:17 UTC (permalink / raw)
  To: buildroot

show-depednency-tree was introduced in this release cycle, as a way to
quickly and easily provide the dependency tree to geaph-depends.

show-dependency-tree is no longer used, no that graph-depends has been
switched over to using the more versatile show-info.

Beside, show-dependency-tree has never been part of a release.

Drop it.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
---
 Makefile               | 4 ----
 fs/common.mk           | 8 --------
 package/pkg-generic.mk | 6 ------
 3 files changed, 18 deletions(-)

diff --git a/Makefile b/Makefile
index 33215f91e5..d9f2f6e645 100644
--- a/Makefile
+++ b/Makefile
@@ -876,10 +876,6 @@ 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-dependency-tree
-show-dependency-tree: $(patsubst %,%-show-dependency-tree,$(PACKAGES) $(TARGETS_ROOTFS))
-	@:
-
 .PHONY: graph-depends
 graph-depends: graph-depends-requirements
 	@$(INSTALL) -d $(GRAPHS_DIR)
diff --git a/fs/common.mk b/fs/common.mk
index ce17c35c59..0fcd1cdbdc 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -61,10 +61,6 @@ ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES = $(sort \
 	) \
 	$(ROOTFS_COMMON_FINAL_RECURSIVE_DEPENDENCIES__X))
 
-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)))
-
 .PHONY: rootfs-common
 rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize
 	@$(call MESSAGE,"Generating root filesystems common tables")
@@ -117,10 +113,6 @@ ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES = $$(sort \
 	) \
 	$$(ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES__X))
 
-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)))
-
 ifeq ($$(BR2_TARGET_ROOTFS_$(2)_GZIP),y)
 ROOTFS_$(2)_COMPRESS_EXT = .gz
 ROOTFS_$(2)_COMPRESS_CMD = gzip -9 -c
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 25bc2e527a..e5b75bc7b6 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -862,11 +862,6 @@ $(1)-show-build-order: $$(patsubst %,%-show-build-order,$$($(2)_FINAL_ALL_DEPEND
 	@:
 	$$(info $(1))
 
-$(1)-show-dependency-tree: $$(patsubst %,%-show-dependency-tree,$$($(2)_FINAL_ALL_DEPENDENCIES))
-	@:
-	$$(info $(1): $(4) $$(if $$($(2)_IS_VIRTUAL),virtual,$$($(2)_DL_VERSION)))
-	$$(info $(1) -> $$($(2)_FINAL_ALL_DEPENDENCIES))
-
 $(1)-show-info:
 	@:
 	$$(info $$(call clean-json,$$(call show-info,$(2))))
@@ -1101,7 +1096,6 @@ DL_TOOLS_DEPENDENCIES += $$(call extractor-dependency,$$($(2)_SOURCE))
 	$(1)-reconfigure \
 	$(1)-reinstall \
 	$(1)-rsync \
-	$(1)-show-dependency-tree \
 	$(1)-show-depends \
 	$(1)-show-info \
 	$(1)-show-version \
-- 
2.14.1

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

* [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised
  2019-04-14 20:17 ` [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised Yann E. MORIN
@ 2019-04-15  9:08   ` Arnout Vandecappelle
  2019-04-15 17:18     ` Yann E. MORIN
  0 siblings, 1 reply; 24+ messages in thread
From: Arnout Vandecappelle @ 2019-04-15  9:08 UTC (permalink / raw)
  To: buildroot



On 14/04/2019 22:17, Yann E. MORIN wrote:
> 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)

 Maybe this is the thing you forgot to amend, but: the DOWNLOAD macro is also
used a couple of lines lower for the actual_source.

 Regards,
 Arnout

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

* [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list
  2019-04-14 20:17 ` [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list Yann E. MORIN
@ 2019-04-15  9:48   ` Arnout Vandecappelle
  0 siblings, 0 replies; 24+ messages in thread
From: Arnout Vandecappelle @ 2019-04-15  9:48 UTC (permalink / raw)
  To: buildroot



On 14/04/2019 22:17, Yann E. MORIN wrote:
> Currently, only first-level dependencies of a filesystem are added to
> the global list of packages, thus missing all recursive dependencies.
> 
> Use the newly introduced recursive variable instead, which already
> contains the rootfs-common dependencies too.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> ---
>  fs/common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 5ec28ca183..f7989eac57 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -190,7 +190,7 @@ rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME)
>  
>  ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
>  TARGETS_ROOTFS += rootfs-$(1)
> -PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES) $$(ROOTFS_COMMON_DEPENDENCIES))
> +PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_FINAL_RECURSIVE_DEPENDENCIES))

 I don't like that we do this for rootfs but not for packages...

 So maybe we should indeed change that in the package infra itself:

-PACKAGES += $(1)
+PACKAGES += $$($(2)_FINAL_RECURSIVE_DEPENDENCIES)

 (The $$ is as usual not really needed since it will anyway be expanded
immediately because PACKAGES is defined as immediately-expanded (i.e. := instead
of =)).

 And then of course also change the top-level Makefile to remove the use of
_FINAL_ALL_DEPENDENCIES there.

 And as a bonus, you can remove -all-source, -all-external-deps and
-all-legal-info because PACKAGES is already complete.

 But this is diverging a little bit from the show-info integration :-)

 In other words, this patch is fine as is.

 Regards,
 Arnout

>  endif
>  
>  # Check for legacy POST_TARGETS rules
> 

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-14 20:17 ` [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info Yann E. MORIN
@ 2019-04-15 12:17   ` Arnout Vandecappelle
  2019-04-15 12:23     ` Thomas Petazzoni
                       ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Arnout Vandecappelle @ 2019-04-15 12:17 UTC (permalink / raw)
  To: buildroot



On 14/04/2019 22:17, 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-hoc way, with most of the output formats just ad-hoc, raw,
> unformatted blurbs, mostly internal data dumped as-is.
> 
> Introduce a new rule, show-info, that provides a properly formatted
> output of all the meta-information about packages: name, type, version,
> licenses, dependencies...
> 
> We choose to use JSON as the output format, because it is pretty
> versatile, has parsers in virtually all languages, has tools to parse
> from the shell (jq).

 Actually, OpenWrt has a couple of bash functions to convert JSON into bash
variables. If we start making extensive use of show-info, it might be worth
copying that.

> 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. Same goes for filesystems. We
> use a wrapper macro, show-info, that de-multiplexes unto either the
> package-related- or filesystem-related macros, and for packages, we also
> use a detailed macro for non-virtual packages.
> 
> It is non-trivial to properly output correct JSON blurbs, especially
> when trying to output an array of objects, like so, where the last item
> shall not be followed by a comma:  [ { ... }, { ... } ]
> 
> So, we use a trick (as sugegsted by Arnout), to $(subst) any pair of
> ",}" or ", }" or ",]" or ", ]" with only the respective closing symbol,
> "}" or "]".
> 
> The whole stuff is $(strip)ed to make it a somewhat-minified JSON blurb
> that fits on a single line with all spaces squashed (but still with
> spaces, as it is not possible to differentiate spaces between JSON
> elements from spaces inside JSON strings).

 The implicit assumption here is that show-info will never include anything
where whitespace is significant. Which I think is a valid assumption (I don't
expect any whitespace at all anywhere within the keys or values, except for the
license list where the whitespace is usually a list separator anyway).

> 
> Reported-by: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> 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>
> 
> ---
> Changes v1 -> v2:
>   - make it a macro to be called  (Arnout)
>   - make it a top-level rule  (Arnout)
> ---
>  Makefile             | 15 ++++++++++++
>  package/pkg-utils.mk | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 60bf7d7d08..33215f91e5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -903,6 +903,21 @@ check-dependencies:
>  	@cd "$(CONFIG_DIR)"; \
>  	$(TOPDIR)/support/scripts/graph-depends -C
>  
> +.PHONY: show-info
> +show-info:
> +	@:
> +	$(info $(call clean-json, \
> +			$(foreach p, \
> +				$(sort $(foreach i,$(PACKAGES) $(TARGETS_ROOTFS), \
> +						$(i) \
> +						$($(call UPPERCASE,$(i))_FINAL_RECURSIVE_DEPENDENCIES) \

 So, with the comment I made in the earlier patch, this line won't be needed
anymore because packages already contains *_FINAL_RECURSIVE_DEPENDENCIES. Boy,
this is too good to be true...

> +					) \
> +				), \
> +				$(call show-info,$(call UPPERCASE,$(p)))$(comma) \
> +			) \
> +		) \
> +	)
> +
>  else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
>  
>  # Some subdirectories are also package names. To avoid that "make linux"
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index bffd79dfb0..00d1d6bcac 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -62,6 +62,74 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
>  endif
>  endef
>  
> +# show-info -- return package or filesystem metadata formatted as an entry
> +#              of a JSON dictionnary
> +# $(1): upper-case package or filesystem name
> +define show-info

 Really nitpicking here: this function is not showing anything. So maybe
'json-info' is better. Same for the internal functions obviously.

> +	"$($(1)_NAME)": {
> +		"type": "$($(1)_TYPE)",

 I may be exaggerating here, but I am getting a bit confused between commas that
are intrepreted by make and the JSON commas. Maybe we should consistently use
$(comma) for the commas that go into the output, even if it is not needed like here?

> +		$(if $(filter rootfs,$($(1)_TYPE)), \
> +			$(call _show-info-fs,$(1)), \
> +			$(call _show-info-pkg,$(1)), \

 Very elegant!

> +		)
> +	}
> +endef
> +
> +# _show-info-pkg, _show-info-pkg-details, _show-info-fs: private helpers
> +# for show-info, above
> +define _show-info-pkg
> +	$(if $($(1)_IS_VIRTUAL), \
> +		"virtual": true$(comma),

 Again, I may be exaggerating, but I would find it more aesthetically pleasing
to put the
		"virtual": false$(comma)
here rather than in the pkg-details.

> +		$(call _show-info-pkg-details,$(1)) \
> +	)
> +	"dependencies": [
> +		$(call make-comma-list,$(sort $($(1)_FINAL_ALL_DEPENDENCIES)))
> +	],
> +	"reverse_dependencies": [
> +		$(call make-comma-list,$(sort $($(1)_RDEPENDENCIES)))
> +	]
> +endef

 Add an empty line here.

> +define _show-info-pkg-details
> +	"virtual": false,
> +	"version": "$($(1)_DL_VERSION)",
> +	"licenses": "$($(1)_LICENSE)",

 I'm torn here...

 The licenses could be converted into a list - actually, it is already a list,
just missing the quotes. But clearly, converting that into a JSON list in make
syntax is... a challenge.

 However, I'm actually dreaming of making the license statement a SPDX
expression instead of a list (i.e. conjoined with AND and OR, and with
parenthesis; unfortunately, SPDX has no syntax to express that it only applies
to a part, e.g. GPL (programs) is not valid SPDX). Then it is no longer a list.

 And we should make a decision now, we're defining "userspace ABI" here so it's
difficult to switch the string to a list or vice versa later.


> +	"downloads": [
> +	$(foreach dl,$(sort $($(1)_ALL_DOWNLOADS)),
> +		{
> +			"source": "$(notdir $(dl))",
> +			"URIs": [

 I would prefer all keys to be consistent case, i.e. lowercase.

> +				$(call make-comma-list,
> +					$(subst \|,|,
> +						$(call DOWNLOAD_URIS,$(dl),$(1))
> +					)
> +				)
> +			]
> +		},
> +	)
> +	],
> +endef

 Empty line again.

> +define _show-info-fs
> +	"dependencies": [
> +		$(call make-comma-list,$(sort $($(1)_DEPENDENCIES)))
> +	]
> +endef
> +
> +# clean-json -- cleanup pseudo-json into clean json:
> +#  - adds opening { and closing }

 I don't think it's appropriate that clean-json does that. Without it, you can
call clean-json as often as you like.

> +#  - remove commas before closing ] and }
> +#  - minify with $(strip)
> +clean-json = $(strip \

 Why strip a second time?

> +	$(subst $(comma)},, \
                          ^}

> +		$(subst $(comma)$(space)},$(space)}, \
> +			$(subst $(comma)],, \
                                          ^]

> +				$(subst $(comma)$(space)],$(space)], \
> +					{ $(strip $(1)) } \
> +				) \

 Even though incorrect, I think this would be more readable and maintainable by
not indenting the nested subst's:

	$(subst $(comma)},},
	$(subst $(comma)],],
	$(subst $(comma)$(space)},$(space)},
	$(subst $(comma)$(space)],$(space)],
		$(strip $(1))
	))))

 Regards,
 Arnout

> +			) \
> +		) \
> +	) \
> +)
> +
>  #
>  # legal-info helper functions
>  #
> 

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-15 12:17   ` Arnout Vandecappelle
@ 2019-04-15 12:23     ` Thomas Petazzoni
  2019-04-15 17:34     ` Yann E. MORIN
  2019-04-16  9:47     ` Peter Korsgaard
  2 siblings, 0 replies; 24+ messages in thread
From: Thomas Petazzoni @ 2019-04-15 12:23 UTC (permalink / raw)
  To: buildroot

On Mon, 15 Apr 2019 14:17:13 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  However, I'm actually dreaming of making the license statement a SPDX
> expression instead of a list (i.e. conjoined with AND and OR, and with
> parenthesis; unfortunately, SPDX has no syntax to express that it only applies
> to a part, e.g. GPL (programs) is not valid SPDX). Then it is no longer a list.
> 
>  And we should make a decision now, we're defining "userspace ABI" here so it's
> difficult to switch the string to a list or vice versa later.

Well, you can always have "licenses" be a string corresponding directly
to <pkg>_LICENSES as proposed by Yann, and if later on we feel like
having the same information in a different format is useful as a list,
we could introduce a separate "licenses-list" JSON property.

It is not nice, but it illustrates that even though we are defining
userspace ABI, it doesn't mean that we have to find the perfect
solution right now.

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

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

* [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised
  2019-04-15  9:08   ` Arnout Vandecappelle
@ 2019-04-15 17:18     ` Yann E. MORIN
  0 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-15 17:18 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2019-04-15 11:08 +0200, Arnout Vandecappelle spake thusly:
> On 14/04/2019 22:17, Yann E. MORIN wrote:
> > 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)
> 
>  Maybe this is the thing you forgot to amend, but: the DOWNLOAD macro is also
> used a couple of lines lower for the actual_source.

Good catch. And no, that was not the thing I missed...

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

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-15 12:17   ` Arnout Vandecappelle
  2019-04-15 12:23     ` Thomas Petazzoni
@ 2019-04-15 17:34     ` Yann E. MORIN
  2019-04-15 17:51       ` Arnout Vandecappelle
  2019-04-16  9:47     ` Peter Korsgaard
  2 siblings, 1 reply; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-15 17:34 UTC (permalink / raw)
  To: buildroot

Arnout, All,

(note: I won't reply to comments I agree with, so they got elided from
my reply, unless I had more to state about them)

On 2019-04-15 14:17 +0200, Arnout Vandecappelle spake thusly:
> On 14/04/2019 22:17, Yann E. MORIN wrote:
[--SNIP--]
> > The whole stuff is $(strip)ed to make it a somewhat-minified JSON blurb
> > that fits on a single line with all spaces squashed (but still with
> > spaces, as it is not possible to differentiate spaces between JSON
> > elements from spaces inside JSON strings).
>  The implicit assumption here is that show-info will never include anything
> where whitespace is significant. Which I think is a valid assumption (I don't
> expect any whitespace at all anywhere within the keys or values, except for the
> license list where the whitespace is usually a list separator anyway).

Indeed. I shall add that to the commit log, then. Thanks!

> > Reported-by: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> > 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>
> > 
> > ---
> > Changes v1 -> v2:
> >   - make it a macro to be called  (Arnout)
> >   - make it a top-level rule  (Arnout)
> > ---
> >  Makefile             | 15 ++++++++++++
> >  package/pkg-utils.mk | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 83 insertions(+)
> > 
> > diff --git a/Makefile b/Makefile
> > index 60bf7d7d08..33215f91e5 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -903,6 +903,21 @@ check-dependencies:
> >  	@cd "$(CONFIG_DIR)"; \
> >  	$(TOPDIR)/support/scripts/graph-depends -C
> >  
> > +.PHONY: show-info
> > +show-info:
> > +	@:
> > +	$(info $(call clean-json, \
> > +			$(foreach p, \
> > +				$(sort $(foreach i,$(PACKAGES) $(TARGETS_ROOTFS), \
> > +						$(i) \
> > +						$($(call UPPERCASE,$(i))_FINAL_RECURSIVE_DEPENDENCIES) \
> 
>  So, with the comment I made in the earlier patch, this line won't be needed
> anymore because packages already contains *_FINAL_RECURSIVE_DEPENDENCIES. Boy,
> this is too good to be true...

Won't do that in this series. Can be refined later. ;-)

> > +	"$($(1)_NAME)": {
> > +		"type": "$($(1)_TYPE)",
> 
>  I may be exaggerating here, but I am getting a bit confused between commas that
> are intrepreted by make and the JSON commas. Maybe we should consistently use
> $(comma) for the commas that go into the output, even if it is not needed like here?

This is a macro definition, not a macro call, so commas are not
interpreted.

> > +# _show-info-pkg, _show-info-pkg-details, _show-info-fs: private helpers
> > +# for show-info, above
> > +define _show-info-pkg
> > +	$(if $($(1)_IS_VIRTUAL), \
> > +		"virtual": true$(comma),
> 
>  Again, I may be exaggerating, but I would find it more aesthetically pleasing
> to put the
> 		"virtual": false$(comma)
> here rather than in the pkg-details.

Can doo, OK.

Also, since that would be the else-clause of the if, the folowing commas
would *not* be interpreted as separators of the if clause. E.g.:

    all:
        @: $(info $(if ,ignored,shown, too))

would print:

    shown, too

Yet, it looks hackish to do so...

> > +define _show-info-pkg-details
> > +	"virtual": false,
> > +	"version": "$($(1)_DL_VERSION)",
> > +	"licenses": "$($(1)_LICENSE)",
> 
>  I'm torn here...
> 
>  The licenses could be converted into a list - actually, it is already a list,
> just missing the quotes. But clearly, converting that into a JSON list in make
> syntax is... a challenge.
> 
>  However, I'm actually dreaming of making the license statement a SPDX
> expression instead of a list (i.e. conjoined with AND and OR, and with
> parenthesis; unfortunately, SPDX has no syntax to express that it only applies
> to a part, e.g. GPL (programs) is not valid SPDX). Then it is no longer a list.
> 
>  And we should make a decision now, we're defining "userspace ABI" here so it's
> difficult to switch the string to a list or vice versa later.

Sorry, two things:

1. I don't think we should treat the _LICENSE fields as a list, becasue
   it is *very* difficult to split:
        FOO_LICENSE = GPL (tools, utils, doc), LGPL (libs)

2. The reason to use JSON is to actually be able to expand it later. We
   can add and "spdx" object later when we are confident we can generate
   something reliable.

> > +define _show-info-fs
> > +	"dependencies": [
> > +		$(call make-comma-list,$(sort $($(1)_DEPENDENCIES)))
> > +	]
> > +endef
> > +
> > +# clean-json -- cleanup pseudo-json into clean json:
> > +#  - adds opening { and closing }
> 
>  I don't think it's appropriate that clean-json does that. Without it, you can
> call clean-json as often as you like.

On the other hand, everywhere we call clean-json, we enclose the result
between { }. I.e. the purpose of that macro *is* to output a clean json
object.

But OK, I can switch over (that was what I did previously, so I don't
have a strong feeling either...)

> > +#  - remove commas before closing ] and }
> > +#  - minify with $(strip)
> > +clean-json = $(strip \
>  Why strip a second time?

First is to eliminate new lines and duplicate spaces, second is to
remove duplicate spaces and newlines introduced by the macro itself.
Try without either, it is not pretty... ;-)

> > +	$(subst $(comma)},, \
>                           ^}

This is the part I lost during a borked rebase. :-(

> > +		$(subst $(comma)$(space)},$(space)}, \
> > +			$(subst $(comma)],, \
>                                           ^]

Ditto.

> > +				$(subst $(comma)$(space)],$(space)], \
> > +					{ $(strip $(1)) } \
> > +				) \
> 
>  Even though incorrect, I think this would be more readable and maintainable by
> not indenting the nested subst's:
> 
> 	$(subst $(comma)},},
> 	$(subst $(comma)],],
> 	$(subst $(comma)$(space)},$(space)},
> 	$(subst $(comma)$(space)],$(space)],
> 		$(strip $(1))
> 	))))

Ah, I was not sure this would be appropriate to do so. I even thought of
doing:

    clean-json = $(strip \
        $(subst $(comma)},}, $(subst $(comma)$(space)},$(space)},
        $(subst $(comma)],], $(subst $(comma)$(space)],$(space)], \
            $(strip $(1)) \
        )))) \
    )

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> > +			) \
> > +		) \
> > +	) \
> > +)
> > +
> >  #
> >  # legal-info helper functions
> >  #
> > 

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

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-15 17:34     ` Yann E. MORIN
@ 2019-04-15 17:51       ` Arnout Vandecappelle
  2019-04-15 18:51         ` Yann E. MORIN
  0 siblings, 1 reply; 24+ messages in thread
From: Arnout Vandecappelle @ 2019-04-15 17:51 UTC (permalink / raw)
  To: buildroot



On 15/04/2019 19:34, Yann E. MORIN wrote:
[snip]
>>> +	"$($(1)_NAME)": {
>>> +		"type": "$($(1)_TYPE)",
>>
>>  I may be exaggerating here, but I am getting a bit confused between commas that
>> are intrepreted by make and the JSON commas. Maybe we should consistently use
>> $(comma) for the commas that go into the output, even if it is not needed like here?
> 
> This is a macro definition, not a macro call, so commas are not
> interpreted.

 I know, of course. My point is that it is not immediately apparent. My mind is
jumbling the JSON separators and the macro call separators together. My thought
was: if we use $(comma) everywhere to mark the JSON separators, things might
become more readable.

 In fact, in the part you snipped, one line lower there is a macro-separator:

		"type": "$($(1)_TYPE)",
		$(if $(filter rootfs,$($(1)_TYPE)),

 You see where I'm coming from?

 Now, I can imagine that sprinkling this code with $(comma)s is not going to
help readability one bit. But I thought I'd just float the idea.

> 
>>> +# _show-info-pkg, _show-info-pkg-details, _show-info-fs: private helpers
>>> +# for show-info, above
>>> +define _show-info-pkg
>>> +	$(if $($(1)_IS_VIRTUAL), \
>>> +		"virtual": true$(comma),
>>
>>  Again, I may be exaggerating, but I would find it more aesthetically pleasing
>> to put the
>> 		"virtual": false$(comma)
>> here rather than in the pkg-details.
> 
> Can doo, OK.
> 
> Also, since that would be the else-clause of the if, the folowing commas
> would *not* be interpreted as separators of the if clause. E.g.:
> 
>     all:
>         @: $(info $(if ,ignored,shown, too))
> 
> would print:
> 
>     shown, too
> 
> Yet, it looks hackish to do so...

 No shit.

[snip]
>>> +#  - remove commas before closing ] and }
>>> +#  - minify with $(strip)
>>> +clean-json = $(strip \
>>  Why strip a second time?
> 
> First is to eliminate new lines and duplicate spaces, second is to
> remove duplicate spaces and newlines introduced by the macro itself.
> Try without either, it is not pretty... ;-)

 Of course, the macro itself inserts additional newlines, I forgot about that.


>>> +	$(subst $(comma)},, \
>>                           ^}
> 
> This is the part I lost during a borked rebase. :-(
> 
>>> +		$(subst $(comma)$(space)},$(space)}, \
>>> +			$(subst $(comma)],, \
>>                                           ^]
> 
> Ditto.
> 
>>> +				$(subst $(comma)$(space)],$(space)], \
>>> +					{ $(strip $(1)) } \
>>> +				) \
>>
>>  Even though incorrect, I think this would be more readable and maintainable by
>> not indenting the nested subst's:
>>
>> 	$(subst $(comma)},},
>> 	$(subst $(comma)],],
>> 	$(subst $(comma)$(space)},$(space)},
>> 	$(subst $(comma)$(space)],$(space)],
>> 		$(strip $(1))
>> 	))))
> 
> Ah, I was not sure this would be appropriate to do so. I even thought of
> doing:
> 
>     clean-json = $(strip \
>         $(subst $(comma)},}, $(subst $(comma)$(space)},$(space)},
>         $(subst $(comma)],], $(subst $(comma)$(space)],$(space)], \
>             $(strip $(1)) \
>         )))) \
>     )

 It's up to you, really. Same as for the $(comma) thing: you can decide what
looks best for you and I'll accept it.

 Regards,
 Arnout

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-15 17:51       ` Arnout Vandecappelle
@ 2019-04-15 18:51         ` Yann E. MORIN
  0 siblings, 0 replies; 24+ messages in thread
From: Yann E. MORIN @ 2019-04-15 18:51 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2019-04-15 19:51 +0200, Arnout Vandecappelle spake thusly:
> On 15/04/2019 19:34, Yann E. MORIN wrote:
> [snip]
> >>> +	"$($(1)_NAME)": {
> >>> +		"type": "$($(1)_TYPE)",
> >>
> >>  I may be exaggerating here, but I am getting a bit confused between commas that
> >> are intrepreted by make and the JSON commas. Maybe we should consistently use
> >> $(comma) for the commas that go into the output, even if it is not needed like here?
> > 
> > This is a macro definition, not a macro call, so commas are not
> > interpreted.
> 
>  I know, of course. My point is that it is not immediately apparent. My mind is
> jumbling the JSON separators and the macro call separators together. My thought
> was: if we use $(comma) everywhere to mark the JSON separators, things might
> become more readable.
> 
>  In fact, in the part you snipped, one line lower there is a macro-separator:
> 
> 		"type": "$($(1)_TYPE)",
> 		$(if $(filter rootfs,$($(1)_TYPE)),
> 
>  You see where I'm coming from?

Yeah, I see now.

>  Now, I can imagine that sprinkling this code with $(comma)s is not going to
> help readability one bit. But I thought I'd just float the idea.

To be honest, I tried to use $(comma) in place of actual commas. But
seriously, that is totally unreadable... :-(

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

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-15 12:17   ` Arnout Vandecappelle
  2019-04-15 12:23     ` Thomas Petazzoni
  2019-04-15 17:34     ` Yann E. MORIN
@ 2019-04-16  9:47     ` Peter Korsgaard
  2019-04-16 19:49       ` Arnout Vandecappelle
  2 siblings, 1 reply; 24+ messages in thread
From: Peter Korsgaard @ 2019-04-16  9:47 UTC (permalink / raw)
  To: buildroot

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

 > On 14/04/2019 22:17, 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-hoc way, with most of the output formats just ad-hoc, raw,
 >> unformatted blurbs, mostly internal data dumped as-is.
 >> 
 >> Introduce a new rule, show-info, that provides a properly formatted
 >> output of all the meta-information about packages: name, type, version,
 >> licenses, dependencies...
 >> 
 >> We choose to use JSON as the output format, because it is pretty
 >> versatile, has parsers in virtually all languages, has tools to parse
 >> from the shell (jq).

 >  Actually, OpenWrt has a couple of bash functions to convert JSON into bash
 > variables. If we start making extensive use of show-info, it might be worth
 > copying that.

That should be pretty trivial to do with jq, E.G. something like:

https://stackoverflow.com/questions/25378013/how-to-convert-a-json-object-to-key-value-format-in-jq

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-16  9:47     ` Peter Korsgaard
@ 2019-04-16 19:49       ` Arnout Vandecappelle
  2019-04-16 20:10         ` Peter Korsgaard
  0 siblings, 1 reply; 24+ messages in thread
From: Arnout Vandecappelle @ 2019-04-16 19:49 UTC (permalink / raw)
  To: buildroot



On 16/04/2019 11:47, Peter Korsgaard wrote:
>>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:
> 
>  > On 14/04/2019 22:17, 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-hoc way, with most of the output formats just ad-hoc, raw,
>  >> unformatted blurbs, mostly internal data dumped as-is.
>  >> 
>  >> Introduce a new rule, show-info, that provides a properly formatted
>  >> output of all the meta-information about packages: name, type, version,
>  >> licenses, dependencies...
>  >> 
>  >> We choose to use JSON as the output format, because it is pretty
>  >> versatile, has parsers in virtually all languages, has tools to parse
>  >> from the shell (jq).
> 
>  >  Actually, OpenWrt has a couple of bash functions to convert JSON into bash
>  > variables. If we start making extensive use of show-info, it might be worth
>  > copying that.
> 
> That should be pretty trivial to do with jq, E.G. something like:
> 
> https://stackoverflow.com/questions/25378013/how-to-convert-a-json-object-to-key-value-format-in-jq

 Yes, but then jq needs to be installed. The point is to be able to use the
show-info output in our own shell scripts without requiring an additional
package to be installed.

 Regards,
 Arnout

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

* [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info
  2019-04-16 19:49       ` Arnout Vandecappelle
@ 2019-04-16 20:10         ` Peter Korsgaard
  0 siblings, 0 replies; 24+ messages in thread
From: Peter Korsgaard @ 2019-04-16 20:10 UTC (permalink / raw)
  To: buildroot

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

Hi,

 >> That should be pretty trivial to do with jq, E.G. something like:
 >> 
 >> https://stackoverflow.com/questions/25378013/how-to-convert-a-json-object-to-key-value-format-in-jq

 >  Yes, but then jq needs to be installed. The point is to be able to use the
 > show-info output in our own shell scripts without requiring an additional
 > package to be installed.

Ok, but we do have a host-jq package and jq has no extra dependencies.

-- 
Bye, Peter Korsgaard

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

end of thread, other threads:[~2019-04-16 20:10 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-14 20:17 [Buildroot] [PATCH 00/12 v2] infra: add solution to dump metadata from packages (branch yem/show-info-2) Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 01/12 v2] infra/pkg-download: return just a list of URIs Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 02/12 v2] infra/pkg-download: make the URI list a callable macro Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 03/12 v2] infra/pkg-download: get rid of the FLOCK variable Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 04/12 v2] infra/pkg-download: make the DOWNLOAD macro fully parameterised Yann E. MORIN
2019-04-15  9:08   ` Arnout Vandecappelle
2019-04-15 17:18     ` Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 05/12 v2] infra/utils: add helper to generate comma-separated lists Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 06/12 v2] fs: introduce variables with name and type Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 07/12 v2] fs: introduce variable with all recursive dependencies Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 08/12 v2] fs: add all recursive dependencies to packages list Yann E. MORIN
2019-04-15  9:48   ` Arnout Vandecappelle
2019-04-14 20:17 ` [Buildroot] [PATCH 09/12 v2] core: introduce new global show-info Yann E. MORIN
2019-04-15 12:17   ` Arnout Vandecappelle
2019-04-15 12:23     ` Thomas Petazzoni
2019-04-15 17:34     ` Yann E. MORIN
2019-04-15 17:51       ` Arnout Vandecappelle
2019-04-15 18:51         ` Yann E. MORIN
2019-04-16  9:47     ` Peter Korsgaard
2019-04-16 19:49       ` Arnout Vandecappelle
2019-04-16 20:10         ` Peter Korsgaard
2019-04-14 20:17 ` [Buildroot] [PATCH 10/12 v2] core: add per-package and per-filesystem show-info Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 11/12 v2] support/scripts: use show-info to extract dependency graph Yann E. MORIN
2019-04-14 20:17 ` [Buildroot] [PATCH 12/12 v2] core: remove show-depednency-tree 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.