core: enhance printvars for variables with newlines

If the variable content has newlines in it then the currently dumped content can't be fed again to GNU make. Add the option DEFINE_VARS which causes the variables to be dumped using

   define VAR
   ... line 1 ...
   ... line 2 ...
   ...
   endef

Updated the manual accordingly.

Signed-off-by Stefan Becker <[email protected]>

---
 Makefile                  | 10 +++++++---
 docs/manual/make-tips.txt | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

Makefile +7 -3

index 0724f28f45..3e27195de0 100644
@@ -974,9 +974,13 @@ printvars:
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
 		$(if $(filter-out environment% default automatic, \
 				$(origin $V)), \
-		$(if $(QUOTED_VARS),\
-			$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
-			$(info $V=$(if $(RAW_VARS),$(value $V),$($V))))))
+		$(if $(DEFINE_VARS), \
+			$(info define $V) \
+			$(info $(if $(RAW_VARS),$(value $V),$($V))) \
+			$(info endef), \
+			$(if $(QUOTED_VARS),\
+				$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
+				$(info $V=$(if $(RAW_VARS),$(value $V),$($V)))))))
 # ' Syntax colouring...
 
 .PHONY: clean

docs/manual/make-tips.txt +20 -0

index ea1d825bef..ba87e5d873 100644
@@ -92,6 +92,8 @@ It is possible to tweak the output using some variables:
 
 - +VARS+ will limit the listing to variables which names match the
   specified make-pattern
+- +DEFINE_VARS+, if set to +YES+, will use define...endef to preserve
+  newlines in the value
 - +QUOTED_VARS+, if set to +YES+, will single-quote the value
 - +RAW_VARS+, if set to +YES+, will print the unexpanded value
 

@@ -106,6 +108,24 @@ For example:
  BUSYBOX_RDEPENDENCIES=ncurses util-linux
 ----
 
+----
+ $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES DEFINE_VARS=YES
+ define BUSYBOX_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_ALL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_PATCH_DEPENDENCIES
+ endef
+ define BUSYBOX_RDEPENDENCIES
+ ncurses util-linux'linux-pam skeleton toolchain host-skeleton host-ccache
+ endef
+----
+
 ----
  $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES QUOTED_VARS=YES
  BUSYBOX_DEPENDENCIES='skeleton toolchain'

Signature · 1 line

2.14.3

If the variable content has newlines in it then the currently dumped content can't be fed again to GNU make. Add the option DEFINE_VARS which causes the variables to be dumped using

   define VAR
   ... line 1 ...
   ... line 2 ...
   ...
   endef

Updated the manual accordingly.

Signed-off-by Stefan Becker <[email protected]>

---
 Makefile                  | 10 +++++++---
 docs/manual/make-tips.txt | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

Makefile +7 -3

index 0724f28f45..3e27195de0 100644
@@ -974,9 +974,13 @@ printvars:
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
 		$(if $(filter-out environment% default automatic, \
 				$(origin $V)), \
-		$(if $(QUOTED_VARS),\
-			$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
-			$(info $V=$(if $(RAW_VARS),$(value $V),$($V))))))
+		$(if $(DEFINE_VARS), \
+			$(info define $V) \
+			$(info $(if $(RAW_VARS),$(value $V),$($V))) \
+			$(info endef), \
+			$(if $(QUOTED_VARS),\
+				$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
+				$(info $V=$(if $(RAW_VARS),$(value $V),$($V)))))))
 # ' Syntax colouring...
 
 .PHONY: clean

docs/manual/make-tips.txt +20 -0

index ea1d825bef..ba87e5d873 100644
@@ -92,6 +92,8 @@ It is possible to tweak the output using some variables:
 
 - +VARS+ will limit the listing to variables which names match the
   specified make-pattern
+- +DEFINE_VARS+, if set to +YES+, will use define...endef to preserve
+  newlines in the value
 - +QUOTED_VARS+, if set to +YES+, will single-quote the value
 - +RAW_VARS+, if set to +YES+, will print the unexpanded value
 

@@ -106,6 +108,24 @@ For example:
  BUSYBOX_RDEPENDENCIES=ncurses util-linux
 ----
 
+----
+ $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES DEFINE_VARS=YES
+ define BUSYBOX_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_ALL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_PATCH_DEPENDENCIES
+ endef
+ define BUSYBOX_RDEPENDENCIES
+ ncurses util-linux'linux-pam skeleton toolchain host-skeleton host-ccache
+ endef
+----
+
 ----
  $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES QUOTED_VARS=YES
  BUSYBOX_DEPENDENCIES='skeleton toolchain'

I'm still not convinced that this is such a great idea.

1. As explained in reply to your first iteration, there shouldn't be a need for including Buildroot variables in a surrounding Makefile.

2. At least equally relevant would be output that is appropriate for Python, JSON, ...

3. The implementation is not complete. In case of RAW_VARS, it's pretty hard to use in practice because also all the referenced variables would need to be retrieved. In the expanded case, any remaining $ will be interpreted by your surrounding make instead of being passed down to the shell like they should. Try e.g. CANFESTIVAL_INSTALL_TARGET_CMDS.

Note that QUOTED_VARS is not perfect either, mainly because there are some make variables that are not legal shell variables (e.g. all the 4th stuff).

The current output formats of printvars do not preserve newlines.

Add
the option MULTILINE_VARS which causes the variable content to be dumped in a format that preserves newlines:

   MULTILINE_VAR_START VAR
   ... line 1 ...
   ... line 2 ...
   ...
   MULTILINE_VAR_END VAR

Updated the manual accordingly.

Signed-off-by Stefan Becker <[email protected]>

---
 Makefile                  | 10 +++++++---
 docs/manual/make-tips.txt | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

Makefile +7 -3

index 54f940659c..bd78da8362 100644
@@ -979,9 +979,13 @@ printvars:
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
 		$(if $(filter-out environment% default automatic, \
 				$(origin $V)), \
-		$(if $(QUOTED_VARS),\
-			$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
-			$(info $V=$(if $(RAW_VARS),$(value $V),$($V))))))
+		$(if $(MULTILINE_VARS), \
+			$(info MULTILINE_VAR_START $V) \
+			$(info $(if $(RAW_VARS),$(value $V),$($V))) \
+			$(info MULTILINE_VAR_END $V), \
+			$(if $(QUOTED_VARS),\
+				$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
+				$(info $V=$(if $(RAW_VARS),$(value $V),$($V)))))))
 # ' Syntax colouring...
 
 .PHONY: clean

docs/manual/make-tips.txt +20 -0

index ea1d825bef..c796bd73e2 100644
@@ -92,6 +92,8 @@ It is possible to tweak the output using some variables:
 
 - +VARS+ will limit the listing to variables which names match the
   specified make-pattern
+- +MULTILINE_VARS+, if set to +YES+, will use a start and end marker around
+  the variable content to preserve newlines in the value
 - +QUOTED_VARS+, if set to +YES+, will single-quote the value
 - +RAW_VARS+, if set to +YES+, will print the unexpanded value
 

@@ -106,6 +108,24 @@ For example:
  BUSYBOX_RDEPENDENCIES=ncurses util-linux
 ----
 
+----
+ $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES MULTILINE_VARS=YES
+ MULTILINE_VAR_START BUSYBOX_DEPENDENCIES
+ skeleton toolchain
+ MULTILINE_VAR_END BUSYBOX_DEPENDENCIES
+ MULTILINE_VAR_START BUSYBOX_FINAL_ALL_DEPENDENCIES
+ skeleton toolchain
+ MULTILINE_VAR_END BUSYBOX_FINAL_ALL_DEPENDENCIES
+ MULTILINE_VAR_START BUSYBOX_FINAL_DEPENDENCIES
+ skeleton toolchain
+ MULTILINE_VAR_END BUSYBOX_FINAL_DEPENDENCIES
+ MULTILINE_VAR_START BUSYBOX_FINAL_PATCH_DEPENDENCIES
+ MULTILINE_VAR_END BUSYBOX_FINAL_PATCH_DEPENDENCIES
+ MULTILINE_VAR_START BUSYBOX_RDEPENDENCIES
+ ncurses util-linux'linux-pam skeleton toolchain host-skeleton host-ccache
+ MULTILINE_VAR_END BUSYBOX_RDEPENDENCIES
+----
+
 ----
  $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES QUOTED_VARS=YES
  BUSYBOX_DEPENDENCIES='skeleton toolchain'

Signature · 1 line

2.14.3

Quoted 5 lines · Arnout Vandecappelle

I'm still not convinced that this is such a great idea.

1. As explained in reply to your first iteration, there shouldn't be a need for including Buildroot variables in a surrounding Makefile.

There is nothing you can say that will change the fact that a monolithic meta build system is too slow for the CI of our own components. So let's just agree to disagree on this point.

I'm jumping back to this very old thread. Could you explain (perhaps re-explain) what you're trying to do ?

If you need to just re-run a specific step of a specific package, why don't you use "make foo-build", "make foo-rebuild", etc. ?

It seems very hackish to extract the commands that Buildroot executes to execute them outside the context of Buildroot. This doesn't really help to make a case for accepting a patch like you're proposing. Could you first convince us that there is a valid and reasonable use-case, and then we can discuss how to support this use-case with Buildroot.

Quoted 2 lines

I'm jumping back to this very old thread. Could you explain (perhaps re-explain) what you're trying to do ?

Modular builds to enable usable CI builds, i.e. any change a developer pushes to one of the internal components will be available as a flash-able image in less than 5 minutes. CI builds are not possible with a monolithic mega meta build system approach like buildroot.

Quoted 2 lines

If you need to just re-run a specific step of a specific package, why don't you use "make foo-build", "make foo-rebuild", etc. ?

buildroot meta build system assumes that the *whole* source code & build tree is available. In a modular build approach that is not the case for the "combining" build, e.g. the flash image build, as it only receives the build artefacts from the module builds.

Quoted 5 lines

It seems very hackish to extract the commands that Buildroot executes to execute them outside the context of Buildroot. This doesn't really help to make a case for accepting a patch like you're proposing. Could you first convince us that there is a valid and reasonable use-case, and then we can discuss how to support this use-case with Buildroot.

Understandable from the PoV of monolithic meta build system advocates like buildroot, yocto, Android, ...

But I live in the real world and simply can''t ignore Amdahl's law. Furthermore I can't, or management doesn't allow me, to spend huge amounts of money on build machines that can run monolithic mega meta build systems within time spans acceptable for CI (which in the end is futile anyway due to Amdahl's law as SW only ever grows in size. Trust me, I've been there...). Hence I've chosen a modular build approach to address the issue.

As it became clear that this patch would not be accepted any time soon, I reverted it and implemented a different approach based on quoted var printout and sed post-processing. While not 100% fool-proof, it does work for the few build variables that are required for the modular build approach.

You did not answer my question: why didn't you use features of Buildroot such as "make <pkg>-build" or "make <pkg>-rebuild" ?

How in practice you were using those commands extracted from Buildroot variables to achieve "modular builds" ?

* buildroot: output/target/, host/ plus selected buildroot meta build system source files required for flash image build + module install script

Quoted 4 lines

Understandable from the PoV of monolithic meta build system advocates like buildroot, yocto, Android, ...

Quoted 5 lines

As it became clear that this patch would not be accepted any time soon, I reverted it and implemented a different approach based on quoted var printout and sed post-processing. While not 100% fool-proof, it does work for the few build variables that are required for the modular build approach.

You did not answer my question: why didn't you use features of Buildroot such as "make <pkg>-build" or "make <pkg>-rebuild" ?

From: "Trent Piepho" <[email protected]>
To: "Thomas Petazzoni" <[email protected]>, chemobejk at gmail.com
Cc: "Yann Morin" <[email protected]>, "buildroot" <[email protected]>
Sent: Segunda-feira, 17 de dezembro de 2018 18:08:58
Subject: Re: [Buildroot] [PATCH RESEND] core: enhance printvars for variables with newlines

Quoted 4 lines · Thomas Petazzoni

Quoted 4 lines

Understandable from the PoV of monolithic meta build system advocates like buildroot, yocto, Android, ...

Quoted 8 lines

Quoted 5 lines

As it became clear that this patch would not be accepted any time soon, I reverted it and implemented a different approach based on quoted var printout and sed post-processing. While not 100% fool-proof, it does work for the few build variables that are required for the modular build approach.

You did not answer my question: why didn't you use features of Buildroot such as "make <pkg>-build" or "make <pkg>-rebuild" ?

I am also using buildroot with modern style CI. While I find <pkg>- rebuild works great for manual developer workflow, there are issues with CI integration.

In the CI system, we have many many git branches active at any time. They come and go frequently and don't start at the same branch point. There are also many build agents (themselves VMs) which perform the actual build.

So say we have a new branch we wish to test. The agent it is dispatched to happens to have the output tree from the last successful build on that agent (this is in itself very problematic!). That last build might be from anything in the git tree. The head of a different branch. An ancestor commit but from months ago. A commit from the future w.r.t. the branch we are building now.

The chance of <pkg>-rebuild working correctly is quite small. There will undoubtedly be many other packages which have changed between the last build on this agent and the once being attempted.

In addition to the above, preserving the build tree between jobs on a agent ends up being a major obstacle in Bamboo. Trying to archive and restore the multi-gigabyte output is very slow.

And given two git hashes to a tree with a buildroot + br-extern, what packages exactly must be rebuilt with <pkg>-rebuild to go from one hash to the other? This is also not so easy to find automatically.

So, my latest attempt to solve this problem - getting back quicker CI feedback - was done another way.

What I do is build an SDK with buildroot. This is part of a nightly build process that also produces a firmware image and runs system tests against real hardware, in addition to making an SDK. Two SDKs in fact, one for the real ARM target and another for an x86_64 target.

Each internal package has a CI job which fetches the latest SDK and builds the package, pointing cmake or autoconf or whatever build system the package uses at the SDK. Since only the package is built, it can produce feedback quickly. The toolchain and libraries and so on is all from the SDK and so the same as in the actual buildroot build of the entire firmware image.

The x86_64 SDK has the same package versions as the ARM firmware, made via the same buildroot. Because the target is x86_64, the binaries it produces can be run locally on the x86_64 build agents. I do this to allow running the unit tests of a package quickly in CI, with needing to use real hardware or qemu to run an ARM unit test suite.

I make a new package infra for our packages to try to handle them all in a consistent way. A complete .mk file for one might look like this:

TRANSPORT_PROXY_DEPENDENCIES = zeromq lttng-libust $(eval $(impinj-cmake-package))

Given the SDK, there's really nothing else I need from buildroot, and the above .mk file, to build this package in CI.

Quoted 25 lines

What I do is build an SDK with buildroot. This is part of a nightly build process that also produces a firmware image and runs system tests against real hardware, in addition to making an SDK. Two SDKs in fact, one for the real ARM target and another for an x86_64 target.

Each internal package has a CI job which fetches the latest SDK and builds the package, pointing cmake or autoconf or whatever build system the package uses at the SDK. Since only the package is built, it can produce feedback quickly. The toolchain and libraries and so on is all from the SDK and so the same as in the actual buildroot build of the entire firmware image.

The x86_64 SDK has the same package versions as the ARM firmware, made via the same buildroot. Because the target is x86_64, the binaries it produces can be run locally on the x86_64 build agents. I do this to allow running the unit tests of a package quickly in CI, with needing to use real hardware or qemu to run an ARM unit test suite.

I make a new package infra for our packages to try to handle them all in a consistent way. A complete .mk file for one might look like this:

TRANSPORT_PROXY_DEPENDENCIES = zeromq lttng-libust $(eval $(impinj-cmake-package))

Quoted 2 lines

Given the SDK, there's really nothing else I need from buildroot, and the above .mk file, to build this package in CI.

If you need an SDK perhaps you should try Yocto, which in its turn also has some drawbacks. We (DATACOM) attempted to use it years ago and gave up because the development process was too complicated and slow.

The main drawback of an SDK is that as you integrate new packages and change the base system the SDK becomes outdated. It may work for a few packages but as we started adding modules (currently there are 295) it became impossible to build and publish new SDK versions at the same pace without going crazy updating our development environments every day.

Quoted 4 lines

If the variable content has newlines in it then the currently dumped content can't be fed again to GNU make. Add the option DEFINE_VARS which causes the variables to be dumped using

Quoted 5 lines

   define VAR
   ... line 1 ...
   ... line 2 ...
   ...
   endef

I'm still not convinced that this is such a great idea.

1. As explained in reply to your first iteration, there shouldn't be a need for including Buildroot variables in a surrounding Makefile.

2. At least equally relevant would be output that is appropriate for Python, JSON, ...

3. The implementation is not complete. In case of RAW_VARS, it's pretty hard to use in practice because also all the referenced variables would need to be retrieved. In the expanded case, any remaining $ will be interpreted by your surrounding make instead of being passed down to the shell like they should. Try e.g. CANFESTIVAL_INSTALL_TARGET_CMDS.

Note that QUOTED_VARS is not perfect either, mainly because there are some make variables that are not legal shell variables (e.g. all the 4th stuff).

So, if anything changes in here, I'd rather have some output that is in some
intermediate format that can easily be converted into or parsed by shell, python, etc.

Regards,
Arnout

Quoted 69 lines

Updated the manual accordingly.

Signed-off-by Stefan Becker <[email protected]>

---
 Makefile                  | 10 +++++++---
 docs/manual/make-tips.txt | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 3 deletions(-)

Makefile +7 -3

index 0724f28f45..3e27195de0 100644
@@ -974,9 +974,13 @@ printvars:
 		$(sort $(if $(VARS),$(filter $(VARS),$(.VARIABLES)),$(.VARIABLES))), \
 		$(if $(filter-out environment% default automatic, \
 				$(origin $V)), \
-		$(if $(QUOTED_VARS),\
-			$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
-			$(info $V=$(if $(RAW_VARS),$(value $V),$($V))))))
+		$(if $(DEFINE_VARS), \
+			$(info define $V) \
+			$(info $(if $(RAW_VARS),$(value $V),$($V))) \
+			$(info endef), \
+			$(if $(QUOTED_VARS),\
+				$(info $V='$(subst ','\'',$(if $(RAW_VARS),$(value $V),$($V)))'), \
+				$(info $V=$(if $(RAW_VARS),$(value $V),$($V)))))))
 # ' Syntax colouring...
 
 .PHONY: clean

docs/manual/make-tips.txt +20 -0

index ea1d825bef..ba87e5d873 100644
@@ -92,6 +92,8 @@ It is possible to tweak the output using some variables:
 
 - +VARS+ will limit the listing to variables which names match the
   specified make-pattern
+- +DEFINE_VARS+, if set to +YES+, will use define...endef to preserve
+  newlines in the value
 - +QUOTED_VARS+, if set to +YES+, will single-quote the value
 - +RAW_VARS+, if set to +YES+, will print the unexpanded value
 

@@ -106,6 +108,24 @@ For example:
  BUSYBOX_RDEPENDENCIES=ncurses util-linux
 ----
 
+----
+ $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES DEFINE_VARS=YES
+ define BUSYBOX_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_ALL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_DEPENDENCIES
+ skeleton toolchain
+ endef
+ define BUSYBOX_FINAL_PATCH_DEPENDENCIES
+ endef
+ define BUSYBOX_RDEPENDENCIES
+ ncurses util-linux'linux-pam skeleton toolchain host-skeleton host-ccache
+ endef
+----
+
 ----
  $ make -s printvars VARS=BUSYBOX_%DEPENDENCIES QUOTED_VARS=YES
  BUSYBOX_DEPENDENCIES='skeleton toolchain'

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Compare with the non-expanded, raw value:

    $ eval make -s printvars VARS=SYSTEMD_USERS QUOTED_VARS=YES RAW_VARS=YES
    SYSTEMD_USERS=' - - input -1 * - - - Input device group
            - - systemd-journal -1 * - - - Journal
            - - render -1 * - - - DRI rendering nodes
            - - kvm -1 * - - - kvm nodes
            systemd-bus-proxy -1 systemd-bus-proxy -1 * - - - Proxy D-Bus messages to/from a bus
            systemd-journal-gateway -1 systemd-journal-gateway -1 * /var/log/journal - - Journal Gateway
            systemd-journal-remote -1 systemd-journal-remote -1 * /var/log/journal/remote - - Journal Remote
            systemd-journal-upload -1 systemd-journal-upload -1 * - - - Journal Upload
            $(SYSTEMD_COREDUMP_USER)
            $(SYSTEMD_NETWORKD_USER)
            $(SYSTEMD_RESOLVED_USER)
            $(SYSTEMD_TIMESYNCD_USER)'

So, newlines are still present, and we can especially see this is the case around the variables that are expanded to empty (the coredump, networkd, resolved, and timesyncd users).

So, the whole excuse for the change, as explained in the commit log, does not stand: newlines are preserved.