br2-external: support multiple trees at once (branch yem/multi-br2-external-ID-7)

pkg-utils.mk contains various definitions that are used in the package infrastructures and packages themselves.

However, those definitions can be useful in other parts of Buildroot, and are already used in a few places that are not related to the package infrastructure. Also, $(sep) will be needed early in the Makefile when we eventually support multiple br2-external trees.

Since this file only contains definitions, we can include it anytime.

So, consider that file to no longer be specific to the package infras:
- move it to support and rename it,
- move a few similar definitions from the main Makefile to that file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
 Makefile              |  10 +----
 package/pkg-utils.mk  |  96 -------------------------------------------
 support/misc/utils.mk | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 104 deletions(-)
 create mode 100644 support/misc/utils.mk

Makefile +2 -8

index 027f21c..af2d982 100644
@@ -106,14 +106,8 @@ else ifneq ($(filter-out $(nobuild_targets),$(MAKECMDGOALS)),)
 BR_BUILDING = y
 endif
 
-# Strip quotes and then whitespaces
-qstrip = $(strip $(subst ",,$(1)))
-#"))
-
-# Variables for use in Make constructs
-comma := ,
-empty :=
-space := $(empty) $(empty)
+# Include some helper macros and variables
+include support/misc/utils.mk
 
 ifneq ("$(origin O)", "command line")
 O := output

package/pkg-utils.mk +0 -96

index c61b3b6..28db481 100644
@@ -5,43 +5,6 @@
 #
 ################################################################################
 
-# Case conversion macros. This is inspired by the 'up' macro from gmsl
-# (http://gmsl.sf.net). It is optimised very heavily because these macros
-# are used a lot. It is about 5 times faster than forking a shell and tr.
-#
-# The caseconvert-helper creates a definition of the case conversion macro.
-# After expansion by the outer $(eval ), the UPPERCASE macro is defined as:
-# $(strip $(eval __tmp := $(1))  $(eval __tmp := $(subst a,A,$(__tmp))) ... )
-# In other words, every letter is substituted one by one.
-#
-# The caseconvert-helper allows us to create this definition out of the
-# [FROM] and [TO] lists, so we don't need to write down every substition
-# manually. The uses of $ and $$ quoting are chosen in order to do as
-# much expansion as possible up-front.
-#
-# Note that it would be possible to conceive a slightly more optimal
-# implementation that avoids the use of __tmp, but that would be even
-# more unreadable and is not worth the effort.
-
-[FROM] := a b c d e f g h i j k l m n o p q r s t u v w x y z - .
-[TO]   := A B C D E F G H I J K L M N O P Q R S T U V W X Y Z _ _
-
-define caseconvert-helper
-$(1) = $$(strip \
-	$$(eval __tmp := $$(1))\
-	$(foreach c, $(2),\
-		$$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
-	$$(__tmp))
-endef
-
-$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
-$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
-
-# Sanitize macro cleans up generic strings so it can be used as a filename
-# and in rules. Particularly useful for VCS version strings, that can contain
-# slashes, colons (OK in filenames but not in rules), and spaces.
-sanitize = $(subst $(space),_,$(subst :,_,$(subst /,_,$(strip $(1)))))
-
 #
 # Manipulation of .config files based on the Kconfig
 # infrastructure. Used by the BusyBox package, the Linux kernel

@@ -82,65 +45,6 @@ INFLATE.tar  = cat
 # suitable-extractor(filename): returns extractor based on suffix
 suitable-extractor = $(INFLATE$(suffix $(1)))
 
-# MESSAGE Macro -- display a message in bold type
-MESSAGE = echo "$(TERM_BOLD)>>> $($(PKG)_NAME) $($(PKG)_VERSION) $(call qstrip,$(1))$(TERM_RESET)"
-TERM_BOLD := $(shell tput smso 2>/dev/null)
-TERM_RESET := $(shell tput rmso 2>/dev/null)
-
-# Utility functions for 'find'
-# findfileclauses(filelist) => -name 'X' -o -name 'Y'
-findfileclauses = $(call notfirstword,$(patsubst %,-o -name '%',$(1)))
-# finddirclauses(base, dirlist) => -path 'base/dirX' -o -path 'base/dirY'
-finddirclauses = $(call notfirstword,$(patsubst %,-o -path '$(1)/%',$(2)))
-
-# Miscellaneous utility functions
-# notfirstword(wordlist): returns all but the first word in wordlist
-notfirstword = $(wordlist 2,$(words $(1)),$(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
-
-
-endef
-
-PERCENT = %
-QUOTE = '
-# ' # Meh... syntax-highlighting
-
-# This macro properly escapes a command string, then prints it with printf:
-#
-#   - first, backslash '\' are self-escaped, so that they do not escape
-#     the following char and so that printf properly outputs a backslash;
-#
-#   - next, single quotes are escaped by closing an existing one, adding
-#     an escaped one, and re-openning a new one (see below for the reason);
-#
-#   - then '%' signs are self-escaped so that the printf does not interpret
-#     them as a format specifier, in case the variable contains an actual
-#     printf with a format;
-#
-#   - finally, $(sep) is replaced with the literal '\n' so that make does
-#     not break on the so-expanded variable, but so that the printf does
-#     correctly output an LF.
-#
-# Note: this must be escaped in this order to avoid over-escaping the
-# previously escaped elements.
-#
-# Once everything has been escaped, it is passed between single quotes
-# (that's why the single-quotes are escaped they way they are, above,
-# and why the dollar sign is not escaped) to printf(1). A trailing
-# newline is apended, too.
-#
-# Note: leading or trailing spaces are *not* stripped.
-#
-define PRINTF
-	printf '$(subst $(sep),\n,\
-			$(subst $(PERCENT),$(PERCENT)$(PERCENT),\
-				$(subst $(QUOTE),$(QUOTE)\$(QUOTE)$(QUOTE),\
-					$(subst \,\\,$(1)))))\n'
-endef
-
 # check-deprecated-variable -- throw an error on deprecated variables
 # example:
 #   $(eval $(call check-deprecated-variable,FOO_MAKE_OPT,FOO_MAKE_OPTS))

support/misc/utils.mk new file +111 -0

new file mode 100644
index 0000000..990a3d1
@@ -0,0 +1,111 @@
+################################################################################
+#
+# This file contains various utility macros and variables used about
+# everywhere in make constructs.
+#
+################################################################################
+
+# Strip quotes and then whitespaces
+qstrip = $(strip $(subst ",,$(1)))
+#"))
+
+# Variables for use in Make constructs
+comma := ,
+empty :=
+space := $(empty) $(empty)
+
+# Case conversion macros. This is inspired by the 'up' macro from gmsl
+# (http://gmsl.sf.net). It is optimised very heavily because these macros
+# are used a lot. It is about 5 times faster than forking a shell and tr.
+#
+# The caseconvert-helper creates a definition of the case conversion macro.
+# After expansion by the outer $(eval ), the UPPERCASE macro is defined as:
+# $(strip $(eval __tmp := $(1))  $(eval __tmp := $(subst a,A,$(__tmp))) ... )
+# In other words, every letter is substituted one by one.
+#
+# The caseconvert-helper allows us to create this definition out of the
+# [FROM] and [TO] lists, so we don't need to write down every substition
+# manually. The uses of $ and $$ quoting are chosen in order to do as
+# much expansion as possible up-front.
+#
+# Note that it would be possible to conceive a slightly more optimal
+# implementation that avoids the use of __tmp, but that would be even
+# more unreadable and is not worth the effort.
+
+[FROM] := a b c d e f g h i j k l m n o p q r s t u v w x y z - .
+[TO]   := A B C D E F G H I J K L M N O P Q R S T U V W X Y Z _ _
+
+define caseconvert-helper
+$(1) = $$(strip \
+	$$(eval __tmp := $$(1))\
+	$(foreach c, $(2),\
+		$$(eval __tmp := $$(subst $(word 1,$(subst :, ,$c)),$(word 2,$(subst :, ,$c)),$$(__tmp))))\
+	$$(__tmp))
+endef
+
+$(eval $(call caseconvert-helper,UPPERCASE,$(join $(addsuffix :,$([FROM])),$([TO]))))
+$(eval $(call caseconvert-helper,LOWERCASE,$(join $(addsuffix :,$([TO])),$([FROM]))))
+
+# Sanitize macro cleans up generic strings so it can be used as a filename
+# and in rules. Particularly useful for VCS version strings, that can contain
+# slashes, colons (OK in filenames but not in rules), and spaces.
+sanitize = $(subst $(space),_,$(subst :,_,$(subst /,_,$(strip $(1)))))
+
+# MESSAGE Macro -- display a message in bold type
+MESSAGE = echo "$(TERM_BOLD)>>> $($(PKG)_NAME) $($(PKG)_VERSION) $(call qstrip,$(1))$(TERM_RESET)"
+TERM_BOLD := $(shell tput smso 2>/dev/null)
+TERM_RESET := $(shell tput rmso 2>/dev/null)
+
+# Utility functions for 'find'
+# findfileclauses(filelist) => -name 'X' -o -name 'Y'
+findfileclauses = $(call notfirstword,$(patsubst %,-o -name '%',$(1)))
+# finddirclauses(base, dirlist) => -path 'base/dirX' -o -path 'base/dirY'
+finddirclauses = $(call notfirstword,$(patsubst %,-o -path '$(1)/%',$(2)))
+
+# Miscellaneous utility functions
+# notfirstword(wordlist): returns all but the first word in wordlist
+notfirstword = $(wordlist 2,$(words $(1)),$(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
+
+
+endef
+
+PERCENT = %
+QUOTE = '
+# ' # Meh... syntax-highlighting
+
+# This macro properly escapes a command string, then prints it with printf:
+#
+#   - first, backslash '\' are self-escaped, so that they do not escape
+#     the following char and so that printf properly outputs a backslash;
+#
+#   - next, single quotes are escaped by closing an existing one, adding
+#     an escaped one, and re-openning a new one (see below for the reason);
+#
+#   - then '%' signs are self-escaped so that the printf does not interpret
+#     them as a format specifier, in case the variable contains an actual
+#     printf with a format;
+#
+#   - finally, $(sep) is replaced with the literal '\n' so that make does
+#     not break on the so-expanded variable, but so that the printf does
+#     correctly output an LF.
+#
+# Note: this must be escaped in this order to avoid over-escaping the
+# previously escaped elements.
+#
+# Once everything has been escaped, it is passed between single quotes
+# (that's why the single-quotes are escaped they way they are, above,
+# and why the dollar sign is not escaped) to printf(1). A trailing
+# newline is apended, too.
+#
+# Note: leading or trailing spaces are *not* stripped.
+#
+define PRINTF
+	printf '$(subst $(sep),\n,\
+		$(subst $(PERCENT),$(PERCENT)$(PERCENT),\
+			$(subst $(QUOTE),$(QUOTE)\$(QUOTE)$(QUOTE),\
+				$(subst \,\\,$(1)))))\n'
+endef

pkg-utils.mk contains various definitions that are used in the package infrastructures and packages themselves.

However, those definitions can be useful in other parts of Buildroot, and are already used in a few places that are not related to the package infrastructure. Also, $(sep) will be needed early in the Makefile when we eventually support multiple br2-external trees.

Since this file only contains definitions, we can include it anytime.

So, consider that file to no longer be specific to the package infras:
- move it to support and rename it,
- move a few similar definitions from the main Makefile to that file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
 Makefile              |  10 +----
 package/pkg-utils.mk  |  96 -------------------------------------------
 support/misc/utils.mk | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 104 deletions(-)
 create mode 100644 support/misc/utils.mk

The code for both cases is exactly the same, and only differs in the location where defconfig files are looked for.

We use an intermediate macro to generate the corresponding rules, because directly generating the rules is ugly and needs lots of escaping and double-dollar-ing for the $(eval ...) and $(foreach ...) calls to play nicely together.

Furthermore, that will be tremendously useful when we support multiple br2-external trees.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v2 -> v3:
- use lower-case macro (Arnout)
- move comment in the generated rule (Arnout)

---
 Makefile | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

Makefile +7 -8

index af2d982..7c0dcb1 100644
@@ -847,14 +847,13 @@ olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
 
-# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(TOPDIR)/configs/%_defconfig outputmakefile
-	@$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(TOPDIR)/configs/$@ \
-		$< --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN)
-
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(BR2_EXTERNAL)/configs/%_defconfig outputmakefile
-	@$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(BR2_EXTERNAL)/configs/$@ \
-		$< --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN)
+define percent_defconfig
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
+	# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
+	@$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
+		$$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
+endef
+$(eval $(foreach d,$(TOPDIR) $(BR2_EXTERNAL),$(call percent_defconfig,$(d))$(sep)))
 
 savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@$(COMMON_CONFIG_ENV) $< \

Makefile olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile · line 852

Quoted 4 lines

Makefile +3 -0

+define percent_defconfig
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
+	# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig

This comment in here was a bit annoying, because it appears on stdout when you run "make <foo>_defconfig". So I've moved it outside of the target definition instead, in order to preserve the existing behavior in terms of what appears on stdout when loading a defconfig.

Thomas, All,

On 2016-08-27 16:12 +0200, Thomas Petazzoni spake thusly:

Quoted 10 lines · Yann E. MORIN

Makefile olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile · line 852

Quoted 4 lines

Makefile +3 -0

+define percent_defconfig
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
+	# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig

This comment in here was a bit annoying, because it appears on stdout when you run "make <foo>_defconfig". So I've moved it outside of the target definition instead, in order to preserve the existing behavior in terms of what appears on stdout when loading a defconfig.

Well, that's how I did it first, but Arnout contended that the comment should be moved like I did (or at least that's what I understood from his review).

I'm happy you moved it. ;-)

Thanks!

Regards,
Yann E. MORIN.

Currently, a document can not have dependencies, except for the purely internal ones (like checking asciidoc version, and presence of dblatex).

For our own manual, this will come in handy when we introduce a generated kconfig snippet, so we can actually make the manual depend on that snippet being generated first.

For external documents, it can be used to depend on host-packages if need be (e.g. a custom host packages that generates specific media files included in the manual).

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 docs/manual/adding-packages-asciidoc.txt | 3 +++
 package/doc-asciidoc.mk                  | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

docs/manual/adding-packages-asciidoc.txt +3 -0

index 6e21786..0c10fd1 100644
@@ -63,6 +63,9 @@ information is (assuming the document name is +foo+) :
   to one or more directories containing so-called resources (like CSS or
   images). By default, empty.
 
+* +FOO_DEPENDENCIES+, optional, the list of packages (most probably,
+  host-packages) that must be built before building this document.
+
 There are also additional hooks (see xref:hooks[] for general information
 on hooks), that a document may set to define extra actions to be done at
 various steps:

package/doc-asciidoc.mk +1 -1

index 6ab5ad2..ede6ed1 100644
@@ -140,7 +140,7 @@ endef
 ################################################################################
 define ASCIIDOC
 # Single line, because splitting a foreach is not easy...
-$(1)-check-dependencies: asciidoc-check-dependencies
+$(1)-check-dependencies: asciidoc-check-dependencies $$($(2)_DEPENDENCIES)
 	$$(Q)$$(foreach hook,$$($(2)_CHECK_DEPENDENCIES_HOOKS),$$(call $$(hook))$$(sep))
 
 # Single line, because splitting a foreach is not easy...

docs/manual/adding-packages-asciidoc.txt +3 -0

index 6e21786..0c10fd1 100644
@@ -63,6 +63,9 @@ information is (assuming the document name is +foo+) :
   to one or more directories containing so-called resources (like CSS or
   images). By default, empty.
 
+* +FOO_DEPENDENCIES+, optional, the list of packages (most probably,
+  host-packages) that must be built before building this document.
+
 There are also additional hooks (see xref:hooks[] for general information
 on hooks), that a document may set to define extra actions to be done at
 various steps:

package/doc-asciidoc.mk +0 -0

index 6ab5ad2..ede6ed1 100644

Quoted 21 lines

docs/manual/adding-packages-asciidoc.txt +3 -0

index 6e21786..0c10fd1 100644
@@ -63,6 +63,9 @@ information is (assuming the document name is +foo+) :
   to one or more directories containing so-called resources (like CSS or
   images). By default, empty.
 
+* +FOO_DEPENDENCIES+, optional, the list of packages (most probably,
+  host-packages) that must be built before building this document.
+
 There are also additional hooks (see xref:hooks[] for general information
 on hooks), that a document may set to define extra actions to be done at
 various steps:

package/doc-asciidoc.mk +0 -0

index 6ab5ad2..ede6ed1 100644
@@ -140,7 +140,7 @@ endef
 ################################################################################

package/doc-asciidoc.mk endef · line 143

Quoted 4 lines

package/doc-asciidoc.mk +1 -1

 define ASCIIDOC
 # Single line, because splitting a foreach is not easy...
-$(1)-check-dependencies: asciidoc-check-dependencies
+$(1)-check-dependencies: asciidoc-check-dependencies $$($(2)_DEPENDENCIES)

My concern is that $(2)_DEPENDENCIES has nothing to do with *checking* dependencies, so it's a bit weird to see this added to a target called <something>-check-dependencies.

Could you refresh me on how this $(1)-check-dependencies target is used? I had a quick look at the code around and couldn't remember, and wanted to look at some other patches in your series.

Currently, a document can not have dependencies, except for the purely internal ones (like checking asciidoc version, and presence of dblatex).

For our own manual, this will come in handy when we introduce a generated kconfig snippet, so we can actually make the manual depend on that snippet being generated first.

For external documents, it can be used to depend on host-packages if need be (e.g. a custom host packages that generates specific media files included in the manual).

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 docs/manual/adding-packages-asciidoc.txt | 3 +++
 package/doc-asciidoc.mk                  | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

Currently, all configurators depend on generating the out-of-tree Makefile wrapper.

In an upcoming patch, we'll need to also generate a kconfig fragment, so it will have to kick in before we run the configurators.

Introduce a new intermediate "prepare-kconfig" rule, so we can commonalise the dependencies of the configurators. Move the dependency on the Makefile wrapper to that new intermediate rule.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 Makefile | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

Makefile +19 -17

index 7c0dcb1..6f947f3 100644
@@ -760,6 +760,8 @@ endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 HOSTCFLAGS = $(CFLAGS_FOR_BUILD)
 export HOSTCFLAGS
 
+prepare-kconfig: outputmakefile
+
 $(BUILD_DIR)/buildroot-config/%onf:
 	mkdir -p $(@D)/lxdialog
 	PKG_CONFIG_PATH="$(HOST_PKG_CONFIG_PATH)" $(MAKE) CC="$(HOSTCC_NOCCACHE)" HOSTCC="$(HOSTCC_NOCCACHE)" \

@@ -779,19 +781,19 @@ COMMON_CONFIG_ENV = \
 	HOST_GCC_VERSION="$(HOSTCC_VERSION)" \
 	SKIP_LEGACY=
 
-xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile
+xconfig: $(BUILD_DIR)/buildroot-config/qconf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-gconfig: $(BUILD_DIR)/buildroot-config/gconf outputmakefile
+gconfig: $(BUILD_DIR)/buildroot-config/gconf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) srctree=$(TOPDIR) $< $(CONFIG_CONFIG_IN)
 
-menuconfig: $(BUILD_DIR)/buildroot-config/mconf outputmakefile
+menuconfig: $(BUILD_DIR)/buildroot-config/mconf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-nconfig: $(BUILD_DIR)/buildroot-config/nconf outputmakefile
+nconfig: $(BUILD_DIR)/buildroot-config/nconf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
-config: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+config: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< $(CONFIG_CONFIG_IN)
 
 # For the config targets that automatically select options, we pass

@@ -799,22 +801,22 @@ config: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 # no values are set for the legacy options so a subsequent oldconfig
 # will query them. Therefore, run an additional olddefconfig.
 
-oldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+oldconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< --oldconfig $(CONFIG_CONFIG_IN)
 
-randconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+randconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y $< --randconfig $(CONFIG_CONFIG_IN)
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-allyesconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allyesconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y $< --allyesconfig $(CONFIG_CONFIG_IN)
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-allnoconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allnoconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y $< --allnoconfig $(CONFIG_CONFIG_IN)
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+randpackageconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@grep -v BR2_PACKAGE_ $(BR2_CONFIG) > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y \
 		KCONFIG_ALLCONFIG=$(CONFIG_DIR)/.config.nopkg \

@@ -822,7 +824,7 @@ randpackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@grep -v BR2_PACKAGE_ $(BR2_CONFIG) > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y \
 		KCONFIG_ALLCONFIG=$(CONFIG_DIR)/.config.nopkg \

@@ -830,7 +832,7 @@ allyespackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@grep -v BR2_PACKAGE_ $(BR2_CONFIG) > $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) SKIP_LEGACY=y \
 		KCONFIG_ALLCONFIG=$(CONFIG_DIR)/.config.nopkg \

@@ -838,24 +840,24 @@ allnopackageconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@rm -f $(CONFIG_DIR)/.config.nopkg
 	@$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN) >/dev/null
 
-silentoldconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+silentoldconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	$(COMMON_CONFIG_ENV) $< --silentoldconfig $(CONFIG_CONFIG_IN)
 
-olddefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+olddefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	$(COMMON_CONFIG_ENV) $< --olddefconfig $(CONFIG_CONFIG_IN)
 
-defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+defconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< --defconfig$(if $(DEFCONFIG),=$(DEFCONFIG)) $(CONFIG_CONFIG_IN)
 
 define percent_defconfig
-%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig outputmakefile
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(1)/configs/%_defconfig prepare-kconfig
 	# Override the BR2_DEFCONFIG from COMMON_CONFIG_ENV with the new defconfig
 	@$$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(1)/configs/$$@ \
 		$$< --defconfig=$(1)/configs/$$@ $$(CONFIG_CONFIG_IN)
 endef
 $(eval $(foreach d,$(TOPDIR) $(BR2_EXTERNAL),$(call percent_defconfig,$(d))$(sep)))
 
-savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
+savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
 	@$(COMMON_CONFIG_ENV) $< \
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 6 lines

Currently, all configurators depend on generating the out-of-tree Makefile wrapper.

In an upcoming patch, we'll need to also generate a kconfig fragment, so it will have to kick in before we run the configurators.

Quoted 3 lines

Introduce a new intermediate "prepare-kconfig" rule, so we can commonalise the dependencies of the configurators. Move the dependency on the Makefile wrapper to that new intermediate rule.

I think we need to add prepare-kconfig to PHONY target list (for consistency as you said on IRC ;-))

With that fixed:

Reviewed-by Romain Naour <[email protected]>

Currently, all configurators depend on generating the out-of-tree Makefile wrapper.

In an upcoming patch, we'll need to also generate a kconfig fragment, so it will have to kick in before we run the configurators.

Introduce a new intermediate "prepare-kconfig" rule, so we can commonalise the dependencies of the configurators. Move the dependency on the Makefile wrapper to that new intermediate rule.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 Makefile | 36 +++++++++++++++++++-----------------
 1 file changed, 19 insertions(+), 17 deletions(-)

Some of those directories will be needed even during configuration, like BUILD_DIR, where we'll store the generated kconfig snippet.

So, move the rule to create them outside the HAVE_CONFIG block.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
 Makefile | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Makefile +9 -9

index 6f947f3..d4dc1c5 100644
@@ -200,6 +200,15 @@ LEGAL_LICENSES_TXT_HOST = $(LEGAL_INFO_DIR)/host-licenses.txt
 LEGAL_WARNINGS = $(LEGAL_INFO_DIR)/.warnings
 LEGAL_REPORT = $(LEGAL_INFO_DIR)/README
 
+################################################################################
+#
+# staging and target directories do NOT list these as
+# dependencies anywhere else
+#
+################################################################################
+$(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
+	@mkdir -p $@
+
 BR2_CONFIG = $(CONFIG_DIR)/.config
 
 # Pull in the user's configuration file

@@ -496,15 +505,6 @@ world: target-post-image
 	legal-info legal-info-prepare legal-info-clean printvars help \
 	list-defconfigs target-finalize target-post-image source-check
 
-################################################################################
-#
-# staging and target directories do NOT list these as
-# dependencies anywhere else
-#
-################################################################################
-$(BUILD_DIR) $(TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
-	@mkdir -p $@
-
 # Populating the staging with the base directories is handled by the skeleton package
 $(STAGING_DIR):
 	@mkdir -p $(STAGING_DIR)

Hi Yann,

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 4 lines

Some of those directories will be needed even during configuration, like BUILD_DIR, where we'll store the generated kconfig snippet.

So, move the rule to create them outside the HAVE_CONFIG block.

You mean BR2_HAVE_DOT_CONFIG here, right ?

Quoted 6 lines

Hi Yann,

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 3 lines

Some of those directories will be needed even during configuration, like BUILD_DIR, where we'll store the generated kconfig snippet.

Quoted 3 lines

Quoted 1 line

So, move the rule to create them outside the HAVE_CONFIG block.

You mean BR2_HAVE_DOT_CONFIG here, right ?

Yann, could you reply to this comment? I think Romain is right, but I just want to be sure before fixing up myself when applying.

Some of those directories will be needed even during configuration, like BUILD_DIR, where we'll store the generated kconfig snippet.

So, move the rule to create them outside the HAVE_CONFIG block.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Quoted 8 lines

Some of those directories will be needed even during configuration, like BUILD_DIR, where we'll store the generated kconfig snippet.

So, move the rule to create them outside the HAVE_CONFIG block.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

I've applied this patch to next, after fixing the typo in the commit log noticed by Romain Naour. One question below, though.

Makefile LEGAL_LICENSES_TXT_HOST = $(LEGAL_INFO_DIR)/host-licenses.txt · line 208

Quoted 6 lines

Makefile +6 -0

+################################################################################
+#
+# staging and target directories do NOT list these as
+# dependencies anywhere else
+#
+################################################################################

Do you understand what this comment means? It seems to date back from the "major rework" commit 08782ae7d8812933ad3f932132955ea1c0fb1e52 in 2002, and I don't really understand the meaning of this comment.

Add the infrastructure for adding generated kconfig snippet in the menuconfig.

For now, the kconfig snippet is generated empty, the recipe for filling it in will be introduced in sub-sequent patches.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Samuel Martin <[email protected]>

---
 Config.in             | 6 ++++++
 Makefile              | 7 ++++++-
 docs/manual/manual.mk | 3 +++
 3 files changed, 15 insertions(+), 1 deletion(-)

Config.in +6 -0

index 7bb2a01..7f533ce 100644
@@ -18,6 +18,10 @@ config BR2_EXTERNAL
 	string
 	option env="BR2_EXTERNAL"
 
+config BR2_BUILD_DIR
+	string
+	option env="BUILD_DIR"
+
 # Hidden config symbols for packages to check system gcc version
 config BR2_HOST_GCC_VERSION
 	string

@@ -763,3 +767,5 @@ menu "User-provided options"
 source "$BR2_EXTERNAL/Config.in"
 
 endmenu
+
+source "$BR2_BUILD_DIR/.br2-external.in"

Makefile +6 -1

index d4dc1c5..39a1507 100644
@@ -760,7 +760,7 @@ endif # ifeq ($(BR2_HAVE_DOT_CONFIG),y)
 HOSTCFLAGS = $(CFLAGS_FOR_BUILD)
 export HOSTCFLAGS
 
-prepare-kconfig: outputmakefile
+prepare-kconfig: outputmakefile $(BUILD_DIR)/.br2-external.in
 
 $(BUILD_DIR)/buildroot-config/%onf:
 	mkdir -p $(@D)/lxdialog

@@ -779,6 +779,7 @@ COMMON_CONFIG_ENV = \
 	BR2_CONFIG=$(BR2_CONFIG) \
 	BR2_EXTERNAL=$(BR2_EXTERNAL) \
 	HOST_GCC_VERSION="$(HOSTCC_VERSION)" \
+	BUILD_DIR=$(BUILD_DIR) \
 	SKIP_LEGACY=
 
 xconfig: $(BUILD_DIR)/buildroot-config/qconf prepare-kconfig

@@ -879,6 +880,10 @@ ifeq ($(NEED_WRAPPER),y)
 	$(Q)$(TOPDIR)/support/scripts/mkmakefile $(TOPDIR) $(O)
 endif
 
+.PHONY: $(BUILD_DIR)/.br2-external.in
+$(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
+	@touch $@
+
 # printvars prints all the variables currently defined in our
 # Makefiles. Alternatively, if a non-empty VARS variable is passed,
 # only the variables matching the make pattern passed in VARS are

docs/manual/manual.mk +3 -0

index caf080a..b26f912 100644
@@ -7,6 +7,9 @@
 MANUAL_SOURCES = $(sort $(wildcard docs/manual/*.txt) $(wildcard docs/images/*))
 MANUAL_RESOURCES = $(TOPDIR)/docs/images
 
+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig
+
 # Our manual needs to generate lists
 define MANUAL_GEN_LISTS
 	$(Q)$(call MESSAGE,"Updating the manual lists...")

+.PHONY: $(BUILD_DIR)/.br2-external.in

Quoted 2 lines

+.PHONY: $(BUILD_DIR)/.br2-external.in

Why is a real file that exists on the filesystem a phony target?

docs/manual/manual.mk line 11

Quoted 2 lines

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Hello,

Quoted 1 line · Yann E. MORIN

+.PHONY: $(BUILD_DIR)/.br2-external.in

Why is a real file that exists on the filesystem a phony target?

docs/manual/manual.mk line 11

Quoted 2 lines

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Quoted 4 lines

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Why do we _HAVE_ to continue to support it?

If we don't find the complexity worth the added value (I personally don't, I think we could just ask people to look under package/ for the list of packages), then we should just remove it.

Quoted 4 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Why do we _HAVE_ to continue to support it?

Quoted 3 lines

If we don't find the complexity worth the added value (I personally don't, I think we could just ask people to look under package/ for the list of packages), then we should just remove it.

I wouldn't mind if we were to remove this. I think the original intent was to give some "exposure" as to what is available in Buildroot, so that people just skimming through our documentation can see the wide variety of packages we have.

The other thing that I find quite complex for no super benefit is the whole "doc-asciidoc" infrastructure, but that's another story.

Yes, but the list is really long and overwhelming. As it stands, the list of packages takes up more than half the size of the manual. I think just asking people to browse packages/ (or make menuconfig) is more sane.

Quoted 2 lines

The other thing that I find quite complex for no super benefit is the whole "doc-asciidoc" infrastructure, but that's another story.

Yeah, having this infrastructure just for the manual is maybe a bit overkill.

Hello,

Quoted 4 lines · Peter Korsgaard

Yes, but the list is really long and overwhelming. As it stands, the list of packages takes up more than half the size of the manual. I think just asking people to browse packages/ (or make menuconfig) is more sane.

Right.

Quoted 5 lines

Quoted 2 lines

The other thing that I find quite complex for no super benefit is the whole "doc-asciidoc" infrastructure, but that's another story.

Yeah, having this infrastructure just for the manual is maybe a bit overkill.

I agree, though some people wanted to build several manuals, if I remember correctly.

Quoted 9 lines

Hello,

Quoted 4 lines · Peter Korsgaard

Yes, but the list is really long and overwhelming. As it stands, the list of packages takes up more than half the size of the manual. I think just asking people to browse packages/ (or make menuconfig) is more sane.

Right.

We could replace it with a reference to http://autobuild.buildroot.net/stats/

Quoted 5 lines

Quoted 5 lines

Quoted 2 lines

The other thing that I find quite complex for no super benefit is the whole "doc-asciidoc" infrastructure, but that's another story.

Yeah, having this infrastructure just for the manual is maybe a bit overkill.

I introduced it back in the day to reduce the size of manual.mk a little. Back
then I still believed that all code duplication is evil. Now I think that the manual generation logic would be a lot easier to understand if it was all unrolled...

Quoted 2 lines

I agree, though some people wanted to build several manuals, if I remember correctly.

I never saw the use of that, TBH. But I guess it's out there now so we should
keep it?

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

docs/manual/manual.mk line 11

Quoted 2 lines · Yann E. MORIN

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Why do we _HAVE_ to continue to support it?

If we don't find the complexity worth the added value (I personally don't, I think we could just ask people to look under package/ for the list of packages), then we should just remove it.

Peter, Thomas, All,

On 2016-08-28 22:50 +0200, Peter Korsgaard spake thusly:

Quoted 10 lines

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

docs/manual/manual.mk line 11

Quoted 2 lines · Yann E. MORIN

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Quoted 5 lines

Why do we _HAVE_ to continue to support it?

If we don't find the complexity worth the added value (I personally don't, I think we could just ask people to look under package/ for the list of packages), then we should just remove it.

I'm not entirely against removing the list of out packages from our manual.

Yet, for some users of br2-external trees, they may still want to have the list of their packages in their manuals, build with the asciidoc infrastructure.

It is entirely imaginable that such users of br2-external have packages annotated with some documentation for those packages, and want to include that doc in their manual, so need to generate that list from Kconfig.

Quoted 22 lines

Peter, Thomas, All,

On 2016-08-28 22:50 +0200, Peter Korsgaard spake thusly:

Quoted 10 lines

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

docs/manual/manual.mk line 11

Quoted 2 lines · Yann E. MORIN

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Quoted 5 lines

Why do we _HAVE_ to continue to support it?

If we don't find the complexity worth the added value (I personally don't, I think we could just ask people to look under package/ for the list of packages), then we should just remove it.

I'm not entirely against removing the list of out packages from our manual.

Quoted 8 lines

Yet, for some users of br2-external trees, they may still want to have the list of their packages in their manuals, build with the asciidoc infrastructure.

It is entirely imaginable that such users of br2-external have packages annotated with some documentation for those packages, and want to include that doc in their manual, so need to generate that list from Kconfig.

I'm not really enthousiastic about keeping complexity in our Makefiles for
imaginary users that may want to have such and such feature :-P

I'm very enthousiastic when people contribute some feature that I'm never going
to use myself - but in that case, we know that there is at least one user (the contributor). But here, we really have added a feature with no known users.

At
the time, it didn't add (much) complexity, but more and more it's starting to weigh down I think.

For sure, the asciidoc support is even more difficult to understand and
maintain than the normal package infras.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Quoted 2 lines

For sure, the asciidoc support is even more difficult to understand and maintain than the normal package infras.

And I seem to have a good track-record at looking at it:

    $ git log --follow --pretty=%an package/doc-asciidoc.mk |sort |uniq -c  |sort -k1nr,2
         22 Yann E. MORIN
          8 Samuel Martin
          7 Thomas De Schampheleire
          2 Thomas Petazzoni
          1 Atul Singh
          1 J?r?me Pouiller

So, please, please. let's keep this asciidoc infra in place.

Arnout, All,

On 2016-08-30 00:46 +0200, Arnout Vandecappelle spake thusly:

Quoted 1 line · Yann E. MORIN

Quoted 8 lines

Quoted 4 lines

It is entirely imaginable that such users of br2-external have packages annotated with some documentation for those packages, and want to include that doc in their manual, so need to generate that list from Kconfig.

I'm not really enthousiastic about keeping complexity in our Makefiles for
imaginary users that may want to have such and such feature :-P

Quoted 3 lines

I'm very enthousiastic when people contribute some feature that I'm never going
to use myself - but in that case, we know that there is at least one user (the contributor). But here, we really have added a feature with no known users.

Have you read what I wrote: I *am* using it.

    http://lists.busybox.net/pipermail/buildroot/2016-August/171055.html

Quoted 7 lines

At
the time, it didn't add (much) complexity, but more and more it's starting to weigh down I think.

For sure, the asciidoc support is even more difficult to understand and
maintain than the normal package infras.

And I seem to have a good track-record at looking at it:

    $ git log --follow --pretty=%an package/doc-asciidoc.mk |sort |uniq -c  |sort -k1nr,2
         22 Yann E. MORIN
          8 Samuel Martin
          7 Thomas De Schampheleire
          2 Thomas Petazzoni
          1 Atul Singh
          1 J?r?me Pouiller

So, please, please. let's keep this asciidoc infra in place.

Quoted 9 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Hello,

Quoted 1 line · Yann E. MORIN

+.PHONY: $(BUILD_DIR)/.br2-external.in

Why is a real file that exists on the filesystem a phony target?

Quoted 7 lines

docs/manual/manual.mk line 11

Quoted 2 lines

docs/manual/manual.mk +2 -0

+# Ensure the kconfig snippet is generated (for MANUAL_GEN_LISTS, below):
+MANUAL_DEPENDENCIES += prepare-kconfig

It's a bit crazy the complexity that we have just to allow our manual to have the list of all packages in Buildroot. I'm not even sure this section of the manual is that useful, to be honest. But oh well, that's supported today, so we have to continue support it.

Why do we _HAVE_ to continue to support it?

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Quoted 1 line

Why do we _HAVE_ to continue to support it?

I was going to prepare a patchset to remove the list of packages from the manual, when I stumbled on *one* reason parts of it is interesting to have in the manual: the list of virtual packages, their symboles and their providers.

I think this is worth having in the manual, if at least for packagers of new packages that would need or provide such a virtual package.

However, I think the normal and deperacted packages should be removed, and our script made much simpler (if at all possible).

So, what's the idea:
- remove all packages (easy)
- keep as-is (even easier)
- only keep virtual packages (not easy at all)

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hi,

Quoted 6 lines

Quoted 1 line

Why do we _HAVE_ to continue to support it?

I was going to prepare a patchset to remove the list of packages from the manual, when I stumbled on *one* reason parts of it is interesting to have in the manual: the list of virtual packages, their symboles and their providers.

Quoted 2 lines

I think this is worth having in the manual, if at least for packagers of new packages that would need or provide such a virtual package.

Perhaps.

Quoted 8 lines

However, I think the normal and deperacted packages should be removed, and our script made much simpler (if at all possible).

So, what's the idea:
- remove all packages (easy)
- keep as-is (even easier)
- only keep virtual packages (not easy at all)

Personally I would go for 1, but if 3 is doable without too much work / complexity (and you feel up for it), then that is also OK with me.

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

"Arnout" == Arnout Vandecappelle <[email protected]> writes:

Quoted 5 lines

Quoted 4 lines

So, what's the idea:
- remove all packages (easy)
- keep as-is (even easier)
- only keep virtual packages (not easy at all)

Quoted 2 lines

Personally I would go for 1, but if 3 is doable without too much work / complexity (and you feel up for it), then that is also OK with me.

Well, option 2 is not the end of the world either. The reason we consider it
now is that some complexity (1 line!) has to be added in the manual.mk to continue supporting it. It is starting to violate the KISS principle, but not overly so.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 docs/manual/adding-packages-asciidoc.txt | 2 ++
 1 file changed, 2 insertions(+)

docs/manual/adding-packages-asciidoc.txt +2 -0

index 0c10fd1..a278d44 100644
@@ -65,6 +65,8 @@ information is (assuming the document name is +foo+) :
 
 * +FOO_DEPENDENCIES+, optional, the list of packages (most probably,
   host-packages) that must be built before building this document.
+  If a hook of your document needs to access the _Kconfig_ structure,
+  you may add +prepare-kconfig+ to the list of dependencies.
 
 There are also additional hooks (see xref:hooks[] for general information
 on hooks), that a document may set to define extra actions to be done at

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

---
 docs/manual/adding-packages-asciidoc.txt | 2 ++
 1 file changed, 2 insertions(+)

Move the inclusion of br2-external's Config.in to the generated kconfig snippet.

This will ultimately allow us to use more than one br2-external tree.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Config.in | 7 -------
 Makefile  | 8 +++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

Config.in +0 -7

index 7f533ce..7810bf8 100644
@@ -761,11 +761,4 @@ source "package/Config.in.host"
 
 source "Config.in.legacy"
 
-menu "User-provided options"
-	depends on BR2_EXTERNAL != "support/dummy-external"
-
-source "$BR2_EXTERNAL/Config.in"
-
-endmenu
-
 source "$BR2_BUILD_DIR/.br2-external.in"

Makefile +7 -1

index 39a1507..86c532e 100644
@@ -882,7 +882,13 @@ endif
 
 .PHONY: $(BUILD_DIR)/.br2-external.in
 $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
-	@touch $@
+	$(Q)( \
+		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
+		printf 'menu "User-provided options"\n'; \
+		printf '\tdepends on BR2_EXTERNAL != "support/dummy-external"\n\n'; \
+		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		printf 'endmenu # User-provided options\n'; \
+	) >$@
 
 # printvars prints all the variables currently defined in our
 # Makefiles. Alternatively, if a non-empty VARS variable is passed,

Makefile endif · line 891

Quoted 10 lines

.PHONY: $(BUILD_DIR)/.br2-external.in
$(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)

Makefile +7 -1

-	@touch $@
+	$(Q)( \
+		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
+		printf 'menu "User-provided options"\n'; \
+		printf '\tdepends on BR2_EXTERNAL != "support/dummy-external"\n\n'; \
+		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		printf 'endmenu # User-provided options\n'; \
+	) >$@

Shouldn't the logic that generates the .br2-external.in be moved to a shell script?

I'm generally in favor of having most of the logic in make, but here, it is really ugly. Did you try having this in a shell script?

Also, what about using readlink or realpath to convert the path to an absolute path?

Now that we generate a kconfig snippet, we can conditionally include the BR2_EXTERNAL's Config.in only when BR2_EXTERNAL is supplied by the user, which means our empty/dummy Config.in is no longer useful to us.

As for external.mk, we can also include it only when BR2_EXTERNAL is supplied by the user, which means our empty/dummy external.mk is no longer of any use to us.

Ditch both of those files, and:

- only generate actual content in the Kconfig snippet when we actually do have a BR2_EXTERNAL provided by the user (i.e. BR2_EXTERNAL is not empty);

- add a variable that contains the path to the external.mk provided by the user, or empty if none, and include the path set in that variable (make can 'include' nothing without any problem! ;-) )

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile                           | 18 +++++++-----------
 support/dummy-external/Config.in   |  0
 support/dummy-external/external.mk |  0
 3 files changed, 7 insertions(+), 11 deletions(-)
 delete mode 100644 support/dummy-external/Config.in
 delete mode 100644 support/dummy-external/external.mk

Makefile +7 -11

index 86c532e..be2f586 100644
@@ -147,16 +147,11 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # on the command line, therefore the file is re-created every time make is run.
 #
 # When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
-# line), the .br-external file is removed and we point to
-# support/dummy-external. This makes sure we can unconditionally include the
-# Config.in and external.mk from the BR2_EXTERNAL directory. In this case,
-# override is necessary so the user can clear BR2_EXTERNAL from the command
-# line, but the dummy path is still used internally.
+# line), the .br-external file is removed.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
 ifeq ($(BR2_EXTERNAL),)
-  override BR2_EXTERNAL = support/dummy-external
   $(shell rm -f $(BR2_EXTERNAL_FILE))
 else
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)

@@ -165,6 +160,7 @@ else
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
   $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+  BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 
 # To make sure that the environment variable overrides the .config option,

@@ -461,7 +457,8 @@ include boot/common.mk
 include linux/linux.mk
 include fs/common.mk
 
-include $(BR2_EXTERNAL)/external.mk
+# Nothing to include if no BR2_EXTERNAL tree in use
+include $(BR2_EXTERNAL_MK)
 
 # Now we are sure we have all the packages scanned and defined. We now
 # check for each package in the list of enabled packages, that all its

@@ -882,13 +879,12 @@ endif
 
 .PHONY: $(BUILD_DIR)/.br2-external.in
 $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
-	$(Q)( \
+	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
-		printf 'menu "User-provided options"\n'; \
-		printf '\tdepends on BR2_EXTERNAL != "support/dummy-external"\n\n'; \
+		printf 'menu "User-provided options"\n\n'; \
 		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
 		printf 'endmenu # User-provided options\n'; \
-	) >$@
+	fi >$@
 
 # printvars prints all the variables currently defined in our
 # Makefiles. Alternatively, if a non-empty VARS variable is passed,

support/dummy-external/Config.in deleted +0 -0

deleted file mode 100644
index e69de29..0000000

support/dummy-external/external.mk deleted +0 -0

deleted file mode 100644
index e69de29..0000000

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 14 lines

Now that we generate a kconfig snippet, we can conditionally include the BR2_EXTERNAL's Config.in only when BR2_EXTERNAL is supplied by the user, which means our empty/dummy Config.in is no longer useful to us.

As for external.mk, we can also include it only when BR2_EXTERNAL is supplied by the user, which means our empty/dummy external.mk is no longer of any use to us.

Ditch both of those files, and:

- only generate actual content in the Kconfig snippet when we actually do have a BR2_EXTERNAL provided by the user (i.e. BR2_EXTERNAL is not empty);

Quoted 3 lines

- add a variable that contains the path to the external.mk provided by the user, or empty if none, and include the path set in that variable (make can 'include' nothing without any problem! ;-) )

Maybe a dumb question but it's true with old make version (i.e 3.81) ?

Otherwise:

Reviewed-by Romain Naour <[email protected]>

Best regards,
Romain

Quoted 78 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile                           | 18 +++++++-----------
 support/dummy-external/Config.in   |  0
 support/dummy-external/external.mk |  0
 3 files changed, 7 insertions(+), 11 deletions(-)
 delete mode 100644 support/dummy-external/Config.in
 delete mode 100644 support/dummy-external/external.mk

Makefile +7 -11

index 86c532e..be2f586 100644
@@ -147,16 +147,11 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # on the command line, therefore the file is re-created every time make is run.
 #
 # When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
-# line), the .br-external file is removed and we point to
-# support/dummy-external. This makes sure we can unconditionally include the
-# Config.in and external.mk from the BR2_EXTERNAL directory. In this case,
-# override is necessary so the user can clear BR2_EXTERNAL from the command
-# line, but the dummy path is still used internally.
+# line), the .br-external file is removed.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
 ifeq ($(BR2_EXTERNAL),)
-  override BR2_EXTERNAL = support/dummy-external
   $(shell rm -f $(BR2_EXTERNAL_FILE))
 else
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)

@@ -165,6 +160,7 @@ else
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
   $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+  BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 
 # To make sure that the environment variable overrides the .config option,

@@ -461,7 +457,8 @@ include boot/common.mk
 include linux/linux.mk
 include fs/common.mk
 
-include $(BR2_EXTERNAL)/external.mk
+# Nothing to include if no BR2_EXTERNAL tree in use
+include $(BR2_EXTERNAL_MK)
 
 # Now we are sure we have all the packages scanned and defined. We now
 # check for each package in the list of enabled packages, that all its

@@ -882,13 +879,12 @@ endif
 
 .PHONY: $(BUILD_DIR)/.br2-external.in
 $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
-	$(Q)( \
+	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
-		printf 'menu "User-provided options"\n'; \
-		printf '\tdepends on BR2_EXTERNAL != "support/dummy-external"\n\n'; \
+		printf 'menu "User-provided options"\n\n'; \
 		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
 		printf 'endmenu # User-provided options\n'; \
-	) >$@
+	fi >$@
 
 # printvars prints all the variables currently defined in our
 # Makefiles. Alternatively, if a non-empty VARS variable is passed,

support/dummy-external/Config.in deleted +0 -0

deleted file mode 100644
index e69de29..0000000

support/dummy-external/external.mk deleted +0 -0

deleted file mode 100644
index e69de29..0000000

This unique ID is used to construct a per br2-external tree variable, BR2_EXTERNAL_$(ID), which contains the path to the br2-external tree.

This variable is available both from Kconfig (set in the Kconfig snippet) and from the .mk files.

Also, display the BR2_EXTERNAL_$(ID) and its path as a comment in the menuconfig.

This will ultimately allow us to support multiple br2-external trees at once, with that ID (and thus BR2_EXTERNAL_$(ID)) uniquely defining which br2-external tree is being used.

Note: since the variables in the Makefile and in Kconfig are named the same, the one we computed early on (second hunk) will be overridden by the one in .config when we have it. Thus, even though they are set to the same raw value, the one from .config is quoted and, being included later in the Makefile, will take precedence, so we must un-quote it before we include the br2-external's makefile (third hunk). That's unfortunate, but there is no easy way around that as we want the two variables to be named the same in Makefile and Kconfig (and we can't ask the user to un-quote that variable himself either), hence the little dirty trick.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Makefile +21 -1

index be2f586..72b55cb 100644
@@ -148,6 +148,9 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 #
 # When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
 # line), the .br-external file is removed.
+#
+# If the br2-external tree defines its ID, then export the path in the
+# BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)

@@ -160,6 +163,13 @@ else
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
   $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+  ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
+    BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
+    ifeq ($(BR2_EXTERNAL_ID),)
+      $(error $(BR2_EXTERNAL) has no ID (in file 'external.id'))
+    endif
+    BR2_EXTERNAL_$(BR2_EXTERNAL_ID) = $(BR2_EXTERNAL)
+  endif
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

@@ -457,6 +467,10 @@ include boot/common.mk
 include linux/linux.mk
 include fs/common.mk
 
+# If the br2-external tree defines its ID, then the BR2_EXTERNAL_$(ID)
+# variable is also present in .config, so it is quoted. We must unquote
+# it before feeding it to the br2-external makefile.
+BR2_EXTERNAL_$(BR2_EXTERNAL_ID) := $(call qstrip,$(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)))
 # Nothing to include if no BR2_EXTERNAL tree in use
 include $(BR2_EXTERNAL_MK)
 

@@ -882,7 +896,13 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
 		printf 'menu "User-provided options"\n\n'; \
-		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		if [ -z "$(BR2_EXTERNAL_ID)" ]; then \
+			printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		else \
+			printf 'config BR2_EXTERNAL_%s\n' $(BR2_EXTERNAL_ID); \
+			printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)); \
+			printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(BR2_EXTERNAL_ID); \
+		fi; \
 		printf 'endmenu # User-provided options\n'; \
 	fi >$@
 

This unique ID is used to construct a per br2-external tree variable, BR2_EXTERNAL_$(ID), which contains the path to the br2-external tree.

Quoted 2 lines

This unique ID is used to construct a per br2-external tree variable, BR2_EXTERNAL_$(ID), which contains the path to the br2-external tree.

I would suggest to use the terminology "name" rather than "ID". I think it's much easier to understand when using "name" that using "ID". It's a bit longer, but not much, and will make things clearer.

This unique ID is used to construct a per br2-external tree variable, BR2_EXTERNAL_$(ID), which contains the path to the br2-external tree.

This variable is available both from Kconfig (set in the Kconfig snippet) and from the .mk files.

Also, display the BR2_EXTERNAL_$(ID) and its path as a comment in the menuconfig.

This will ultimately allow us to support multiple br2-external trees at once, with that ID (and thus BR2_EXTERNAL_$(ID)) uniquely defining which br2-external tree is being used.

Note: since the variables in the Makefile and in Kconfig are named the same, the one we computed early on (second hunk) will be overridden by the one in .config when we have it. Thus, even though they are set to the same raw value, the one from .config is quoted and, being included later in the Makefile, will take precedence, so we must un-quote it before we include the br2-external's makefile (third hunk). That's unfortunate, but there is no easy way around that as we want the two variables to be named the same in Makefile and Kconfig (and we can't ask the user to un-quote that variable himself either), hence the little dirty trick.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Makefile +0 -0

index be2f586..72b55cb 100644

Makefile +9 -0

+    ifeq ($(BR2_EXTERNAL_ID),)
+      $(error $(BR2_EXTERNAL) has no ID (in file 'external.id'))
+    endif
+    BR2_EXTERNAL_$(BR2_EXTERNAL_ID) = $(BR2_EXTERNAL)
+  endif
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

@@ -457,6 +467,10 @@ include boot/common.mk
 include linux/linux.mk
 include fs/common.mk
 
+# If the br2-external tree defines its ID, then the BR2_EXTERNAL_$(ID)
+# variable is also present in .config, so it is quoted. We must unquote
+# it before feeding it to the br2-external makefile.
+BR2_EXTERNAL_$(BR2_EXTERNAL_ID) := $(call qstrip,$(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)))
 # Nothing to include if no BR2_EXTERNAL tree in use
 include $(BR2_EXTERNAL_MK)
 

Makefile +2 -0

+			printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(BR2_EXTERNAL_ID); \
+		fi; \
 		printf 'endmenu # User-provided options\n'; \
 	fi >$@
 

I guess you're about to send a v3 so I'll try to still do a quick review of the
remaining things.

Quoted 38 lines

This unique ID is used to construct a per br2-external tree variable, BR2_EXTERNAL_$(ID), which contains the path to the br2-external tree.

This variable is available both from Kconfig (set in the Kconfig snippet) and from the .mk files.

Also, display the BR2_EXTERNAL_$(ID) and its path as a comment in the menuconfig.

This will ultimately allow us to support multiple br2-external trees at once, with that ID (and thus BR2_EXTERNAL_$(ID)) uniquely defining which br2-external tree is being used.

Note: since the variables in the Makefile and in Kconfig are named the same, the one we computed early on (second hunk) will be overridden by the one in .config when we have it. Thus, even though they are set to the same raw value, the one from .config is quoted and, being included later in the Makefile, will take precedence, so we must un-quote it before we include the br2-external's makefile (third hunk). That's unfortunate, but there is no easy way around that as we want the two variables to be named the same in Makefile and Kconfig (and we can't ask the user to un-quote that variable himself either), hence the little dirty trick.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Makefile +0 -0

index be2f586..72b55cb 100644

Makefile $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist)) · line 153

Quoted 6 lines

Makefile +3 -0

@@ -148,6 +148,9 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 #
 # When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
 # line), the .br-external file is removed.
+#
+# If the br2-external tree defines its ID, then export the path in the
+# BR2_EXTERNAL_$(ID) variable.

I agree with Thomas that NAME is better than ID.

The name can be anything, including, for example, MK - which would create a
conflict with BR2_EXTERNAL_MK. Therefore I think there should be an additional prefix, e.g. BR2_EXTERNAL_DIR_$(NAME).

Quoted 4 lines

Makefile +0 -0

 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)

@@ -160,6 +163,13 @@ else
   endif

Makefile else · line 167

Quoted 4 lines

Makefile +2 -0

   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
   $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+  ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
+    BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)

BR2_EXTERNAL_ID is an internal variable, so it should be EXTERNAL_NAME I think.
Admittedly many of the currently existing BR2_EXTERNAL variables wrongly have the BR2 prefix, but let's not make it worse :-)

Quoted 19 lines

Makefile +9 -0

+    ifeq ($(BR2_EXTERNAL_ID),)
+      $(error $(BR2_EXTERNAL) has no ID (in file 'external.id'))
+    endif
+    BR2_EXTERNAL_$(BR2_EXTERNAL_ID) = $(BR2_EXTERNAL)
+  endif
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

@@ -457,6 +467,10 @@ include boot/common.mk
 include linux/linux.mk
 include fs/common.mk
 
+# If the br2-external tree defines its ID, then the BR2_EXTERNAL_$(ID)
+# variable is also present in .config, so it is quoted. We must unquote
+# it before feeding it to the br2-external makefile.
+BR2_EXTERNAL_$(BR2_EXTERNAL_ID) := $(call qstrip,$(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)))
 # Nothing to include if no BR2_EXTERNAL tree in use
 include $(BR2_EXTERNAL_MK)
 

Makefile $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR) · line 903

Quoted 10 lines

Makefile +5 -1

@@ -882,7 +896,13 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
 		printf 'menu "User-provided options"\n\n'; \
-		printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		if [ -z "$(BR2_EXTERNAL_ID)" ]; then \
+			printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
+		else \
+			printf 'config BR2_EXTERNAL_%s\n' $(BR2_EXTERNAL_ID); \
+			printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)); \

Why is this config still needed? It was there before because that's the only
way to use it in the 'source' statement, but now that we generate the external Config.in, I don't think it's needed anymore. Which also removes the need for the unquoting.

Quoted 7 lines

Makefile $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR) · line 903

Quoted 2 lines

Makefile +2 -0

+			printf 'config BR2_EXTERNAL_%s\n' $(BR2_EXTERNAL_ID); \
+			printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)); \  

Why is this config still needed? It was there before because that's the only
way to use it in the 'source' statement, but now that we generate the external Config.in, I don't think it's needed anymore. Which also removes the need for the unquoting.

Hmmm.. As far as I can see, it is still needed, because the Config.in files in the br2-external tree may want to include other Config.in files, no?

Like for example (with the variable renamed as per above):

    .../ext-tree/Config.in
        source "$BR2_EXTERNAL_DIR_FOO/package/pkg-1/Config.in"
        source "$BR2_EXTERNAL_DIR_FOO/package/pkg-2/Config.in"

So it is stil needed to have the path to each br2-external paths from Kconfig.

Unless I missed something?

Update the manual with the new external.id mandatory file.

Take the opportunity to add a section listing all mandatory files, Config.in, external.mk and the new external.id, instead of just hinting about them in the external package recipes section.

Change the example to use the ID-suffixed variable instead of the raw BR2_EXTERNAL variable. Even though it is still possible to write a br2-external tree that is not multi-aware, we only document it with an ID.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Jeremy Rosen <[email protected]>

---
Changes v2 -> v3:
- better document the content of the ID file (Jeremy)

---
 docs/manual/customize-outside-br.txt | 59 ++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 16 deletions(-)

docs/manual/customize-outside-br.txt +40 -13

index 9ad177d..839d1b1 100644
@@ -57,19 +57,50 @@ Or disable the usage of external definitions:
 buildroot/ $ make BR2_EXTERNAL= xconfig
 -----
 
-+BR2_EXTERNAL+ allows three different things:
+A +BR2_EXTERNAL+ tree must contain at least those three files:
+
++external.id+::
+   That file should contain the ID for the +BR2_EXTERNAL+ tree.
+   That ID is used to construct the +BR2_EXTERNAL_$(ID)+ variable,
+   available in +Config.in+ and +external.mk+ (see below), so that ID
+   must be a valid make and Kconfig variable. Buildroot sets
+   +BR2_EXTERNAL_$(ID)+ to the path of the +BR2_EXTERNAL+ tree.
+   The format for this file is a single line with the ID.
++
+Examples of an +external.mk+ file that declares the ID +FOO+:
++
+----
+$ cat external.mk
+FOO
+----
++
+Examples of IDs and the corresponding +BR2_EXTERNAL_$(ID)+ variable:
++
+  * +FOO+ -> +BR2_EXTERNAL_FOO+
+  * +BAR_42+ -> +BR2_EXTERNAL_BAR_42+
+  * ...
+
+
++Config.in+::
++external.mk+::
+   Those files (which may each be empty) can be used to define package
+   recipes, like for packages bundled in Buildroot itself, or other
+   custom configuration options.
+
+Using +BR2_EXTERNAL+ then allows three different things:
 
  * One can store all the board-specific configuration files there,
    such as the kernel configuration, the root filesystem overlay, or
    any other configuration file for which Buildroot allows to set its
-   location. The +BR2_EXTERNAL+ value is available within the
-   Buildroot configuration using +$(BR2_EXTERNAL)+. As an example, one
-   could set the +BR2_ROOTFS_OVERLAY+ Buildroot option to
-   +$(BR2_EXTERNAL)/board/<boardname>/overlay/+ (to specify a root
+   location. The location of the +BR2_EXTERNAL+ tree is available in the
+   Buildroot configuration using the +BR2_EXTERNAL_$(ID)+ variable. As
+   an example, (considering the ID +BAR_42+), one could set the
+   +BR2_ROOTFS_OVERLAY+ Buildroot option to
+   +$(BR2_EXTERNAL_BAR_42)/board/<boardname>/overlay/+ (to specify a root
    filesystem overlay), or the +BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE+
    Buildroot option to
-   +$(BR2_EXTERNAL)/board/<boardname>/kernel.config+ (to specify the
-   location of the kernel configuration file).
+   +$(BR2_EXTERNAL_BAR_42)/board/<boardname>/kernel.config+ (to specify
+   the location of the kernel configuration file).
 
  * One can store package recipes (i.e. +Config.in+ and
    +<packagename>.mk+), or even custom configuration options and make

@@ -77,27 +108,23 @@ buildroot/ $ make BR2_EXTERNAL= xconfig
    make it appear in the top-level configuration menu, and includes
    +$(BR2_EXTERNAL)/external.mk+ with the rest of the makefile logic.
 +
-.Note
-Providing +Config.in+ and +external.mk+ is mandatory, but they can be
-   empty.
-+
 The main usage of this is to store package recipes. The recommended
    way to do this is to write a +$(BR2_EXTERNAL)/Config.in+ file that
    looks like:
 +
 ------
-source "$BR2_EXTERNAL/package/package1/Config.in"
-source "$BR2_EXTERNAL/package/package2/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package1/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package2/Config.in"
 ------
 +
 Then, have a +$(BR2_EXTERNAL)/external.mk+ file that looks like:

docs/manual/customize-outside-br.txt +0 -0

    package recipes, as explained in xref:adding-packages[].
    If you prefer, you can also group the packages in subdirectories
    called <boardname> and adapt the above paths accordingly.

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 1 line

Update the manual with the new external.id mandatory file.

docs/manual/customize-outside-br.txt

Quoted 4 lines

docs/manual/customize-outside-br.txt +0 -0

index 9ad177d..839d1b1 100644

Quoted 12 lines

docs/manual/customize-outside-br.txt +0 -4

@@ -77,27 +108,23 @@ buildroot/ $ make BR2_EXTERNAL= xconfig
    make it appear in the top-level configuration menu, and includes
    +$(BR2_EXTERNAL)/external.mk+ with the rest of the makefile logic.
 +
-.Note
-Providing +Config.in+ and +external.mk+ is mandatory, but they can be
-   empty.
-+
 The main usage of this is to store package recipes. The recommended
    way to do this is to write a +$(BR2_EXTERNAL)/Config.in+ file that
    looks like:
 +

docs/manual/customize-outside-br.txt buildroot/ $ make BR2_EXTERNAL= xconfig · line 120

Quoted 8 lines

docs/manual/customize-outside-br.txt +2 -2

 ------
-source "$BR2_EXTERNAL/package/package1/Config.in"
-source "$BR2_EXTERNAL/package/package2/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package1/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package2/Config.in"
 ------
 +
 Then, have a +$(BR2_EXTERNAL)/external.mk+ file that looks like:

Quoted 4 lines

docs/manual/customize-outside-br.txt +1 -1

 ------
-include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk))
+include $(sort $(wildcard $(BR2_EXTERNAL_BAR_42)/package/*/*.mk))
 ------

docs/manual/customize-outside-br.txt buildroot/ $ make BR2_EXTERNAL= xconfig · line 127

Quoted 5 lines

docs/manual/customize-outside-br.txt +2 -2

 +
-And then in +$(BR2_EXTERNAL)/package/package1+ and
-   +$(BR2_EXTERNAL)/package/package2+ create normal Buildroot
+And then in +$(BR2_EXTERNAL_FOO_42)/package/package1+ and
+   +$(BR2_EXTERNAL_FOO_42)/package/package2+ create normal Buildroot

Here BR2_EXTERNAL_FOO or BR2_EXTERNAL_BAR_42 ?
BR2_EXTERNAL_BAR_42 it seems ;-)

Quoted 9 lines

docs/manual/customize-outside-br.txt +0 -0

index 9ad177d..839d1b1 100644
@@ -57,19 +57,50 @@ Or disable the usage of external definitions:
 buildroot/ $ make BR2_EXTERNAL= xconfig
 -----
 

docs/manual/customize-outside-br.txt Or disable the usage of external definitions: · line 62

Quoted 4 lines

docs/manual/customize-outside-br.txt +3 -1

-+BR2_EXTERNAL+ allows three different things:
+A +BR2_EXTERNAL+ tree must contain at least those three files:
+
++external.id+::

I thought this file was optional when there was a single external tree, but you're saying it's mandatory here. Is it because the "no external.id file" case is only supported for backward compat, and we expect that new users write an external.id, regardless of whether they are using a single or multiple external trees ?

docs/manual/customize-outside-br.txt Or disable the usage of external definitions: · line 70

Quoted 8 lines

docs/manual/customize-outside-br.txt +8 -0

+   That file should contain the ID for the +BR2_EXTERNAL+ tree.
+   That ID is used to construct the +BR2_EXTERNAL_$(ID)+ variable,
+   available in +Config.in+ and +external.mk+ (see below), so that ID
+   must be a valid make and Kconfig variable. Buildroot sets
+   +BR2_EXTERNAL_$(ID)+ to the path of the +BR2_EXTERNAL+ tree.
+   The format for this file is a single line with the ID.
++
+Examples of an +external.mk+ file that declares the ID +FOO+:

Huh? Is it really external.mk that contains the ID ?

docs/manual/customize-outside-br.txt Or disable the usage of external definitions: · line 73

Quoted 3 lines

docs/manual/customize-outside-br.txt +3 -0

++
+----
+$ cat external.mk

Ditto.

docs/manual/customize-outside-br.txt Or disable the usage of external definitions: · line 81

Quoted 8 lines

docs/manual/customize-outside-br.txt +8 -0

+FOO
+----
++
+Examples of IDs and the corresponding +BR2_EXTERNAL_$(ID)+ variable:
++
+  * +FOO+ -> +BR2_EXTERNAL_FOO+
+  * +BAR_42+ -> +BR2_EXTERNAL_BAR_42+
+  * ...

Maybe mention which characters are allowed and which ones are not allowed?

Update the manual with the new external.id mandatory file.

Take the opportunity to add a section listing all mandatory files, Config.in, external.mk and the new external.id, instead of just hinting about them in the external package recipes section.

Change the example to use the ID-suffixed variable instead of the raw BR2_EXTERNAL variable. Even though it is still possible to write a br2-external tree that is not multi-aware, we only document it with an ID.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

Cc Samuel Martin <[email protected]>

Cc Jeremy Rosen <[email protected]>

---
Changes v2 -> v3:
- better document the content of the ID file (Jeremy)

---
 docs/manual/customize-outside-br.txt | 59 ++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 16 deletions(-)

docs/manual/customize-outside-br.txt +0 -0

index 9ad177d..839d1b1 100644

docs/manual/customize-outside-br.txt +13 -0

+   The format for this file is a single line with the ID.
++
+Examples of an +external.mk+ file that declares the ID +FOO+:
++
+----
+$ cat external.mk
+FOO
+----
++
+Examples of IDs and the corresponding +BR2_EXTERNAL_$(ID)+ variable:
++
+  * +FOO+ -> +BR2_EXTERNAL_FOO+
+  * +BAR_42+ -> +BR2_EXTERNAL_BAR_42+

docs/manual/customize-outside-br.txt +5 -5

    way to do this is to write a +$(BR2_EXTERNAL)/Config.in+ file that
    looks like:
 +
 ------
-source "$BR2_EXTERNAL/package/package1/Config.in"
-source "$BR2_EXTERNAL/package/package2/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package1/Config.in"
+source "$BR2_EXTERNAL_BAR_42/package/package2/Config.in"
 ------
 +
 Then, have a +$(BR2_EXTERNAL)/external.mk+ file that looks like:
 +
 ------
-include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk))
+include $(sort $(wildcard $(BR2_EXTERNAL_BAR_42)/package/*/*.mk))
 ------
 +
-And then in +$(BR2_EXTERNAL)/package/package1+ and
-   +$(BR2_EXTERNAL)/package/package2+ create normal Buildroot
+And then in +$(BR2_EXTERNAL_FOO_42)/package/package1+ and
+   +$(BR2_EXTERNAL_FOO_42)/package/package2+ create normal Buildroot
    package recipes, as explained in xref:adding-packages[].
    If you prefer, you can also group the packages in subdirectories
    called <boardname> and adapt the above paths accordingly.

Currently, we treat the case where we have no br2-external tree (BR2_EXTERNAL is empty) differently from the case where we do have one (BR2_EXTERNAL is not empty).

There is now no reason to treat those two cases differently:

- the kconfig snippet is always generated appropriately (i.e. it would

    include the br2-external tree if set, or include nothing otherwise);

- we no longer have a dummy br-external tree either.

As such, generate the .br-external cache file in both cases.

This will make it much easier to handle when we introduce support for multiple br2-external trees.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Makefile +1 -7

index 72b55cb..67334e5 100644
@@ -146,23 +146,17 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # On subsequent invocations of make, it is read in. It can still be overridden
 # on the command line, therefore the file is re-created every time make is run.
 #
-# When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
-# line), the .br-external file is removed.
-#
 # If the br2-external tree defines its ID, then export the path in the
 # BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
-ifeq ($(BR2_EXTERNAL),)
-  $(shell rm -f $(BR2_EXTERNAL_FILE))
-else
+ifneq ($(BR2_EXTERNAL),)
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
   ifeq ($(_BR2_EXTERNAL),)
     $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
   ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
     BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
     ifeq ($(BR2_EXTERNAL_ID),)

Makefile +1 -0

+
 # To make sure that the environment variable overrides the .config option,
 # set this before including .config.
 ifneq ($(BR2_DL_DIR),)

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 58 lines

Currently, we treat the case where we have no br2-external tree (BR2_EXTERNAL is empty) differently from the case where we do have one (BR2_EXTERNAL is not empty).

There is now no reason to treat those two cases differently:

- the kconfig snippet is always generated appropriately (i.e. it would

    include the br2-external tree if set, or include nothing otherwise);

- we no longer have a dummy br-external tree either.

As such, generate the .br-external cache file in both cases.

This will make it much easier to handle when we introduce support for multiple br2-external trees.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Makefile +1 -7

index 72b55cb..67334e5 100644
@@ -146,23 +146,17 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # On subsequent invocations of make, it is read in. It can still be overridden
 # on the command line, therefore the file is re-created every time make is run.
 #
-# When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
-# line), the .br-external file is removed.
-#
 # If the br2-external tree defines its ID, then export the path in the
 # BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
-ifeq ($(BR2_EXTERNAL),)
-  $(shell rm -f $(BR2_EXTERNAL_FILE))
-else
+ifneq ($(BR2_EXTERNAL),)
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
   ifeq ($(_BR2_EXTERNAL),)
     $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
   ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
     BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
     ifeq ($(BR2_EXTERNAL_ID),)

@@ -173,6 +167,11 @@ else
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

Makefile else · line 173

Quoted 4 lines

Makefile +4 -0

+# This needs to be *after* we compute BR_EXTERNAL, above.
+.PHONY: $(BR2_EXTERNAL_FILE)
+$(BR2_EXTERNAL_FILE):
+	@echo BR2_EXTERNAL ?= $(BR_EXTERNAL) >$@

s/BR_EXTERNAL/BR2_EXTERNAL/

It's probably a rebase issue since it's fixed in the following patch ;-)

On 2016-07-24 22:05 +0200, Romain Naour spake thusly:

Quoted 59 lines

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 58 lines

Currently, we treat the case where we have no br2-external tree (BR2_EXTERNAL is empty) differently from the case where we do have one (BR2_EXTERNAL is not empty).

There is now no reason to treat those two cases differently:

- the kconfig snippet is always generated appropriately (i.e. it would

    include the br2-external tree if set, or include nothing otherwise);

- we no longer have a dummy br-external tree either.

As such, generate the .br-external cache file in both cases.

This will make it much easier to handle when we introduce support for multiple br2-external trees.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
 Makefile | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

Makefile +1 -7

index 72b55cb..67334e5 100644
@@ -146,23 +146,17 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # On subsequent invocations of make, it is read in. It can still be overridden
 # on the command line, therefore the file is re-created every time make is run.
 #
-# When BR2_EXTERNAL is set to an empty value (e.g. explicitly in command
-# line), the .br-external file is removed.
-#
 # If the br2-external tree defines its ID, then export the path in the
 # BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
-ifeq ($(BR2_EXTERNAL),)
-  $(shell rm -f $(BR2_EXTERNAL_FILE))
-else
+ifneq ($(BR2_EXTERNAL),)
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
   ifeq ($(_BR2_EXTERNAL),)
     $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
   ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
     BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
     ifeq ($(BR2_EXTERNAL_ID),)

@@ -173,6 +167,11 @@ else
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

Quoted 6 lines

Makefile else · line 173

Quoted 4 lines

Makefile +4 -0

+# This needs to be *after* we compute BR_EXTERNAL, above.
+.PHONY: $(BR2_EXTERNAL_FILE)
+$(BR2_EXTERNAL_FILE):
+	@echo BR2_EXTERNAL ?= $(BR_EXTERNAL) >$@

s/BR_EXTERNAL/BR2_EXTERNAL/

Right, but it should be: s/BR2_EXTERNAL/_BR2_EXTERNAL/ (i.e. with a leading underscore) as we want to store the full path, which is constructed with the first line in the ifdef block.

Quoted 1 line

It's probably a rebase issue since it's fixed in the following patch ;-)

Probably, probably... ;-)

Thanks for spotting.

Regards,
Yann E. MORIN.

Signature · 10 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
-include $(BR2_EXTERNAL_FILE)

Makefile +1 -4

-ifeq ($(BR2_EXTERNAL),)
-  $(shell rm -f $(BR2_EXTERNAL_FILE))
-else
+ifneq ($(BR2_EXTERNAL),)
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
   ifeq ($(_BR2_EXTERNAL),)
     $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
   ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
     BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
     ifeq ($(BR2_EXTERNAL_ID),)

Quoted 3 lines

BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
-include $(BR2_EXTERNAL_FILE)

Quoted 17 lines

Makefile +1 -4

-ifeq ($(BR2_EXTERNAL),)
-  $(shell rm -f $(BR2_EXTERNAL_FILE))
-else
+ifneq ($(BR2_EXTERNAL),)
   _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
   ifeq ($(_BR2_EXTERNAL),)
     $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
   endif
   override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  $(shell echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
   ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
     BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
     ifeq ($(BR2_EXTERNAL_ID),)

@@ -173,6 +167,11 @@ else
   BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
 endif
 

Makefile else · line 174

Quoted 5 lines

Makefile +5 -0

+# This needs to be *after* we compute BR_EXTERNAL, above.
+.PHONY: $(BR2_EXTERNAL_FILE)
+$(BR2_EXTERNAL_FILE):
+	@echo BR2_EXTERNAL ?= $(BR_EXTERNAL) >$@
+

I'm probably missing something, but where is this make target being "triggered" ?

Quoted 2 lines

I'm probably missing something, but where is this make target being "triggered" ?

When make encounters a -include directive, and the file to be included is the target of a rule, then make will trigger the rule before including the file, even if the rule is defined after the inclusion (make parses makefiles in multiple passes).

Currently, we only support at most one br2-external tree. Being able to use more than one br2-external tree can be very useful.

A use-case would be for having a br2-external to contain the basic packages, basic board defconfigs and board files, provided by one team responsible for the "board-bringup", while other teams consume that br2-external as a base, and complements it each with their own set of packages, defconfigs and extra board files.

Another use-case would be for third-parties to provide their own Buildroot packaging in a br2-external tree, along-side the archives for their stuff.

Finally, another use-case is to be able to add FLOSS packages in a br2-external tree, and proprietary packages in another. This allows to not touch the Buildroot tree at all, and still be able to get in compliance by providing only that br2-external tree(s) that contains FLOSS packages, leaving aside the br2-external tree(s) with the proprietary bits.

What we do is to treat BR2_EXTERNAL as a space-separated list of paths, which we iterate to construct:

- the list of all br2-external IDs, BR_EXTERNAL_IDS

- the per-br2-external tree BR2_EXTERNAL_$(ID) variables, which points to the actual location of the corresponding tree,

- EXTRA_ENV which now needs to contain all BR2_EXTERNAL_$(ID) variables, and thus needs to be set early.

Once we have all those variables, we replace references to BR2_EXTERNAL with either:

- a $(patsubst ...) to include the external.mk files (and docs), or

- a $(foreach ...) iteration to generate other Makefile code.

To be noted: we also use $(foreach ...) in the kconfig-snippet generating rule, because it is cleaner than doing so in the shell

Finally, we double-quote $(BR2_EXTERNAL) when assigning it to environment variables.

Now, when more than one br2-external tree is used, each gets its own sub-menu in the "User-provided options" menu. The sub-menu is labelled with that br2-external tree's ID (prefixed with 'BR2_EXTERNAL_') and the sub-menu's first item is a comment with the path to that br2-external tree.

If there's only one br2-external tree, then there is no sub-menu, but we still add the comment with the path to the br2-external tree (which we did not have previously). If the br2-external tree has an ID, it is displayed as well as a comment.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v3 -> v4:
- drop check for file existence (Thomas)
- fix check for empty ID (Thomas)
- simpler code to check number of br2-external trees

---
 Makefile            | 99 +++++++++++++++++++++++++++++++++++++----------------
 package/Makefile.in |  2 +-
 2 files changed, 70 insertions(+), 31 deletions(-)

Makefile +69 -30

index 67334e5..7b3542c 100644
@@ -145,32 +145,57 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # The value of BR2_EXTERNAL is stored in .br-external in the output directory.
 # On subsequent invocations of make, it is read in. It can still be overridden
 # on the command line, therefore the file is re-created every time make is run.
-#
-# If the br2-external tree defines its ID, then export the path in the
-# BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
-ifneq ($(BR2_EXTERNAL),)
-  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
-  ifeq ($(_BR2_EXTERNAL),)
-    $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
-  endif
-  override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
-    BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
-    ifeq ($(BR2_EXTERNAL_ID),)
-      $(error $(BR2_EXTERNAL) has no ID (in file 'external.id'))
-    endif
-    BR2_EXTERNAL_$(BR2_EXTERNAL_ID) = $(BR2_EXTERNAL)
-  endif
-  BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
-endif
 
-# This needs to be *after* we compute BR_EXTERNAL, above.
 .PHONY: $(BR2_EXTERNAL_FILE)
 $(BR2_EXTERNAL_FILE):
-	@echo BR2_EXTERNAL ?= $(BR_EXTERNAL) >$@
+	@echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) >$@
+
+# Those two variables need to be defined as simply-expanded variables, they
+# can't be recursively-expanded, because the values they are assigned change
+# with each iteration of the foreach, below.
+BR_EXTERNAL_IDS :=
+EXTRA_ENV :=
+
+# If there is no or one br2-external trees used, then we don't require (but
+# accept) an ID; otherwise (i.e. there are two or more br2-external trees)
+# we require they do all define their ID.
+ifeq ($(filter-out 0 1,$(words $(BR2_EXTERNAL))),)
+BR2_EXTERNAL_NEED_ID := loose
+else
+BR2_EXTERNAL_NEED_ID := strict
+endif
+
+# Validate the br2-external tree passed as $(1):
+# - check the directory actually exists
+# - check if we need and have a non-empty ID
+# - check the ID is not a duplicate
+# - set variables for later use
+define BR2_EXTERNAL_VALIDATE
+  _BR_EXT_DIR := $$(shell cd $(1) >/dev/null 2>&1 && pwd)
+  ifeq ($$(_BR_EXT_DIR),)
+    $$(error BR2_EXTERNAL='$(1)' does not exist, relative to $$(TOPDIR))
+  endif
+  _BR_EXT_ID := $$(shell cat $$(_BR_EXT_DIR)/external.id 2>/dev/null)
+  ifeq ($$(_BR_EXT_ID),)
+    ifeq ($(BR2_EXTERNAL_NEED_ID),strict)
+      $$(error BR2_EXTERNAL='$(1)' has no ID (in file 'external.id'),\
+               mandatory to use more than one br2-external tree at once)
+    endif # BR2_EXTERNAL_NEED_ID strict
+  endif # No ID
+  ifneq ($$(filter $$(_BR_EXT_ID),$$(BR_EXTERNAL_IDS)),)
+    $$(error Duplicate ID '$$(_BR_EXT_ID)' in '$(1)', previously defined in '$$(BR2_EXTERNAL_$$(_BR_EXT_ID))')
+  endif
+  ifneq ($$(_BR_EXT_ID),)
+    BR2_EXTERNAL_$$(_BR_EXT_ID) := $$(_BR_EXT_DIR)
+    BR_EXTERNAL_IDS += $$(_BR_EXT_ID)
+    EXTRA_ENV += BR2_EXTERNAL_$$(_BR_EXT_ID)=$$(_BR_EXT_DIR)
+  endif # _BR_EXT_ID not empty
+endef # BR2_EXTERNAL_VALIDATE
+
+$(eval $(foreach d,$(BR2_EXTERNAL),$(call BR2_EXTERNAL_VALIDATE,$(d))$(sep)))
 
 # To make sure that the environment variable overrides the .config option,
 # set this before including .config.

@@ -469,9 +494,10 @@ include fs/common.mk
 # If the br2-external tree defines its ID, then the BR2_EXTERNAL_$(ID)
 # variable is also present in .config, so it is quoted. We must unquote
 # it before feeding it to the br2-external makefile.
-BR2_EXTERNAL_$(BR2_EXTERNAL_ID) := $(call qstrip,$(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)))
+$(eval $(foreach id,$(BR_EXTERNAL_IDS),\
+	BR2_EXTERNAL_$(id) := $(call qstrip,$(BR2_EXTERNAL_$(id)))$(sep)))
 # Nothing to include if no BR2_EXTERNAL tree in use
-include $(BR2_EXTERNAL_MK)
+include $(patsubst %,%/external.mk,$(BR2_EXTERNAL))
 
 # Now we are sure we have all the packages scanned and defined. We now
 # check for each package in the list of enabled packages, that all its

@@ -787,7 +813,7 @@ COMMON_CONFIG_ENV = \
 	KCONFIG_AUTOHEADER=$(BUILD_DIR)/buildroot-config/autoconf.h \
 	KCONFIG_TRISTATE=$(BUILD_DIR)/buildroot-config/tristate.config \
 	BR2_CONFIG=$(BR2_CONFIG) \
-	BR2_EXTERNAL=$(BR2_EXTERNAL) \
+	BR2_EXTERNAL="$(BR2_EXTERNAL)" \
 	HOST_GCC_VERSION="$(HOSTCC_VERSION)" \
 	BUILD_DIR=$(BUILD_DIR) \
 	SKIP_LEGACY=

@@ -895,12 +921,25 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
 		printf 'menu "User-provided options"\n\n'; \
-		if [ -z "$(BR2_EXTERNAL_ID)" ]; then \
+		if [ -z "$(call strip,$(BR_EXTERNAL_IDS))" ]; then \
+			printf 'comment "%s"\n\n' $(BR2_EXTERNAL); \
 			printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
 		else \
-			printf 'config BR2_EXTERNAL_%s\n' $(BR2_EXTERNAL_ID); \
-			printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)); \
-			printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(BR2_EXTERNAL_ID); \
+			$(foreach id,$(BR_EXTERNAL_IDS),\
+				for i in $$(seq 1 80); do printf '#'; done; printf '\n\n'; \
+				if [ $(words $(call strip,$(BR_EXTERNAL_IDS))) -gt 1 ]; then \
+					printf 'menu "BR2_EXTERNAL_%s"\n\n' $(id); \
+				else \
+					printf 'comment "BR2_EXTERNAL_%s"\n\n' $(id); \
+				fi; \
+				printf 'comment "%s"\n\n' $(BR2_EXTERNAL_$(id)); \
+				printf 'config BR2_EXTERNAL_%s\n' $(id); \
+				printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(id)); \
+				printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(id); \
+				if [ $(words $(call strip,$(BR_EXTERNAL_IDS))) -gt 1 ]; then \
+					printf 'endmenu # BR2_EXTERNAL_%s\n\n' $(id); \
+				fi; ) \
+			for i in $$(seq 1 80); do printf '#'; done; printf '\n\n'; \
 		fi; \
 		printf 'endmenu # User-provided options\n'; \
 	fi >$@

@@ -1004,10 +1043,10 @@ list-defconfigs:
 	@echo 'Built-in configs:'
 	@$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig))), \
 	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
-ifneq ($(wildcard $(BR2_EXTERNAL)/configs/*_defconfig),)
+ifneq ($(wildcard $(patsubst %,%/configs/*_defconfig,$(BR2_EXTERNAL))),)
 	@echo
 	@echo 'User-provided configs:'
-	@$(foreach b, $(sort $(notdir $(wildcard $(BR2_EXTERNAL)/configs/*_defconfig))), \
+	@$(foreach b, $(sort $(notdir $(wildcard $(patsubst %,%/configs/*_defconfig,$(BR2_EXTERNAL))))), \
 	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
 endif
 	@echo

@@ -1029,7 +1068,7 @@ print-version:
 	@echo $(BR2_VERSION_FULL)
 
 include docs/manual/manual.mk
--include $(BR2_EXTERNAL)/docs/*/*.mk
+-include $(patsubst %,%/docs/*/*.mk,$(BR2_EXTERNAL))
 
 .PHONY: $(noconfig_targets)
 

package/Makefile.in +1 -1

index afd5d3a..fe8ee68 100644
@@ -319,7 +319,7 @@ EXTRA_ENV = \
 	PATH=$(BR_PATH) \
 	BR2_DL_DIR=$(BR2_DL_DIR) \
 	BUILD_DIR=$(BUILD_DIR) \
-	BR2_EXTERNAL=$(BR2_EXTERNAL)
+	BR2_EXTERNAL="$(BR2_EXTERNAL)"
 
 ################################################################################
 # settings we need to pass to configure

Hi Yann,

Le 17/07/2016 ? 12:34, Yann E. MORIN a ?crit :

Quoted 20 lines

Currently, we only support at most one br2-external tree. Being able to use more than one br2-external tree can be very useful.

A use-case would be for having a br2-external to contain the basic packages, basic board defconfigs and board files, provided by one team responsible for the "board-bringup", while other teams consume that br2-external as a base, and complements it each with their own set of packages, defconfigs and extra board files.

Another use-case would be for third-parties to provide their own Buildroot packaging in a br2-external tree, along-side the archives for their stuff.

Finally, another use-case is to be able to add FLOSS packages in a br2-external tree, and proprietary packages in another. This allows to not touch the Buildroot tree at all, and still be able to get in compliance by providing only that br2-external tree(s) that contains FLOSS packages, leaving aside the br2-external tree(s) with the proprietary bits.

Quoted 4 lines

What we do is to treat BR2_EXTERNAL as a space-separated list of paths, which we iterate to construct:

- the list of all br2-external IDs, BR_EXTERNAL_IDS

                                          ^

There are several occurrence of BR_EXTERNAL_IDS instead of BR2_EXTERNAL_IDS.

Did you named BR_EXTERNAL_IDS to avoid confusion between BR2_EXTERNAL_ID and BR2_EXTERNAL_IDS.

Ha ok, BR_ variables are Buildroot internal variables not saved in .config file, right ?

Best regards,
Romain

Quoted 218 lines

- the per-br2-external tree BR2_EXTERNAL_$(ID) variables, which points to the actual location of the corresponding tree,

- EXTRA_ENV which now needs to contain all BR2_EXTERNAL_$(ID) variables, and thus needs to be set early.

Once we have all those variables, we replace references to BR2_EXTERNAL with either:

- a $(patsubst ...) to include the external.mk files (and docs), or

- a $(foreach ...) iteration to generate other Makefile code.

To be noted: we also use $(foreach ...) in the kconfig-snippet generating rule, because it is cleaner than doing so in the shell

Finally, we double-quote $(BR2_EXTERNAL) when assigning it to environment variables.

Now, when more than one br2-external tree is used, each gets its own sub-menu in the "User-provided options" menu. The sub-menu is labelled with that br2-external tree's ID (prefixed with 'BR2_EXTERNAL_') and the sub-menu's first item is a comment with the path to that br2-external tree.

If there's only one br2-external tree, then there is no sub-menu, but we still add the comment with the path to the br2-external tree (which we did not have previously). If the br2-external tree has an ID, it is displayed as well as a comment.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Cc Thomas De Schampheleire <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v3 -> v4:
- drop check for file existence (Thomas)
- fix check for empty ID (Thomas)
- simpler code to check number of br2-external trees

---
 Makefile            | 99 +++++++++++++++++++++++++++++++++++++----------------
 package/Makefile.in |  2 +-
 2 files changed, 70 insertions(+), 31 deletions(-)

Makefile +69 -30

index 67334e5..7b3542c 100644
@@ -145,32 +145,57 @@ $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 # The value of BR2_EXTERNAL is stored in .br-external in the output directory.
 # On subsequent invocations of make, it is read in. It can still be overridden
 # on the command line, therefore the file is re-created every time make is run.
-#
-# If the br2-external tree defines its ID, then export the path in the
-# BR2_EXTERNAL_$(ID) variable.
 
 BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
 -include $(BR2_EXTERNAL_FILE)
-ifneq ($(BR2_EXTERNAL),)
-  _BR2_EXTERNAL = $(shell cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd)
-  ifeq ($(_BR2_EXTERNAL),)
-    $(error BR2_EXTERNAL='$(BR2_EXTERNAL)' does not exist, relative to $(TOPDIR))
-  endif
-  override BR2_EXTERNAL := $(_BR2_EXTERNAL)
-  ifneq ($(wildcard $(BR2_EXTERNAL)/external.id),)
-    BR2_EXTERNAL_ID := $(shell cat $(BR2_EXTERNAL)/external.id 2>/dev/null)
-    ifeq ($(BR2_EXTERNAL_ID),)
-      $(error $(BR2_EXTERNAL) has no ID (in file 'external.id'))
-    endif
-    BR2_EXTERNAL_$(BR2_EXTERNAL_ID) = $(BR2_EXTERNAL)
-  endif
-  BR2_EXTERNAL_MK = $(BR2_EXTERNAL)/external.mk
-endif
 
-# This needs to be *after* we compute BR_EXTERNAL, above.
 .PHONY: $(BR2_EXTERNAL_FILE)
 $(BR2_EXTERNAL_FILE):
-	@echo BR2_EXTERNAL ?= $(BR_EXTERNAL) >$@
+	@echo BR2_EXTERNAL ?= $(BR2_EXTERNAL) >$@
+
+# Those two variables need to be defined as simply-expanded variables, they
+# can't be recursively-expanded, because the values they are assigned change
+# with each iteration of the foreach, below.
+BR_EXTERNAL_IDS :=
+EXTRA_ENV :=
+
+# If there is no or one br2-external trees used, then we don't require (but
+# accept) an ID; otherwise (i.e. there are two or more br2-external trees)
+# we require they do all define their ID.
+ifeq ($(filter-out 0 1,$(words $(BR2_EXTERNAL))),)
+BR2_EXTERNAL_NEED_ID := loose
+else
+BR2_EXTERNAL_NEED_ID := strict
+endif
+
+# Validate the br2-external tree passed as $(1):
+# - check the directory actually exists
+# - check if we need and have a non-empty ID
+# - check the ID is not a duplicate
+# - set variables for later use
+define BR2_EXTERNAL_VALIDATE
+  _BR_EXT_DIR := $$(shell cd $(1) >/dev/null 2>&1 && pwd)
+  ifeq ($$(_BR_EXT_DIR),)
+    $$(error BR2_EXTERNAL='$(1)' does not exist, relative to $$(TOPDIR))
+  endif
+  _BR_EXT_ID := $$(shell cat $$(_BR_EXT_DIR)/external.id 2>/dev/null)
+  ifeq ($$(_BR_EXT_ID),)
+    ifeq ($(BR2_EXTERNAL_NEED_ID),strict)
+      $$(error BR2_EXTERNAL='$(1)' has no ID (in file 'external.id'),\
+               mandatory to use more than one br2-external tree at once)
+    endif # BR2_EXTERNAL_NEED_ID strict
+  endif # No ID
+  ifneq ($$(filter $$(_BR_EXT_ID),$$(BR_EXTERNAL_IDS)),)
+    $$(error Duplicate ID '$$(_BR_EXT_ID)' in '$(1)', previously defined in '$$(BR2_EXTERNAL_$$(_BR_EXT_ID))')
+  endif
+  ifneq ($$(_BR_EXT_ID),)
+    BR2_EXTERNAL_$$(_BR_EXT_ID) := $$(_BR_EXT_DIR)
+    BR_EXTERNAL_IDS += $$(_BR_EXT_ID)
+    EXTRA_ENV += BR2_EXTERNAL_$$(_BR_EXT_ID)=$$(_BR_EXT_DIR)
+  endif # _BR_EXT_ID not empty
+endef # BR2_EXTERNAL_VALIDATE
+
+$(eval $(foreach d,$(BR2_EXTERNAL),$(call BR2_EXTERNAL_VALIDATE,$(d))$(sep)))
 
 # To make sure that the environment variable overrides the .config option,
 # set this before including .config.

@@ -469,9 +494,10 @@ include fs/common.mk
 # If the br2-external tree defines its ID, then the BR2_EXTERNAL_$(ID)
 # variable is also present in .config, so it is quoted. We must unquote
 # it before feeding it to the br2-external makefile.
-BR2_EXTERNAL_$(BR2_EXTERNAL_ID) := $(call qstrip,$(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)))
+$(eval $(foreach id,$(BR_EXTERNAL_IDS),\
+	BR2_EXTERNAL_$(id) := $(call qstrip,$(BR2_EXTERNAL_$(id)))$(sep)))
 # Nothing to include if no BR2_EXTERNAL tree in use
-include $(BR2_EXTERNAL_MK)
+include $(patsubst %,%/external.mk,$(BR2_EXTERNAL))
 
 # Now we are sure we have all the packages scanned and defined. We now
 # check for each package in the list of enabled packages, that all its

@@ -787,7 +813,7 @@ COMMON_CONFIG_ENV = \
 	KCONFIG_AUTOHEADER=$(BUILD_DIR)/buildroot-config/autoconf.h \
 	KCONFIG_TRISTATE=$(BUILD_DIR)/buildroot-config/tristate.config \
 	BR2_CONFIG=$(BR2_CONFIG) \
-	BR2_EXTERNAL=$(BR2_EXTERNAL) \
+	BR2_EXTERNAL="$(BR2_EXTERNAL)" \
 	HOST_GCC_VERSION="$(HOSTCC_VERSION)" \
 	BUILD_DIR=$(BUILD_DIR) \
 	SKIP_LEGACY=

@@ -895,12 +921,25 @@ $(BUILD_DIR)/.br2-external.in: $(BUILD_DIR)
 	$(Q)if [ -n '$(BR2_EXTERNAL)' ]; then \
 		printf "#\n# Automatically generated file; DO NOT EDIT.\n#\n\n"; \
 		printf 'menu "User-provided options"\n\n'; \
-		if [ -z "$(BR2_EXTERNAL_ID)" ]; then \
+		if [ -z "$(call strip,$(BR_EXTERNAL_IDS))" ]; then \
+			printf 'comment "%s"\n\n' $(BR2_EXTERNAL); \
 			printf 'source "%s/Config.in"\n\n' $$(cd $(BR2_EXTERNAL) >/dev/null 2>&1 && pwd); \
 		else \
-			printf 'config BR2_EXTERNAL_%s\n' $(BR2_EXTERNAL_ID); \
-			printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(BR2_EXTERNAL_ID)); \
-			printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(BR2_EXTERNAL_ID); \
+			$(foreach id,$(BR_EXTERNAL_IDS),\
+				for i in $$(seq 1 80); do printf '#'; done; printf '\n\n'; \
+				if [ $(words $(call strip,$(BR_EXTERNAL_IDS))) -gt 1 ]; then \
+					printf 'menu "BR2_EXTERNAL_%s"\n\n' $(id); \
+				else \
+					printf 'comment "BR2_EXTERNAL_%s"\n\n' $(id); \
+				fi; \
+				printf 'comment "%s"\n\n' $(BR2_EXTERNAL_$(id)); \
+				printf 'config BR2_EXTERNAL_%s\n' $(id); \
+				printf '\tstring\n\tdefault "%s"\n\n' $(BR2_EXTERNAL_$(id)); \
+				printf 'source "$$BR2_EXTERNAL_%s/Config.in"\n\n' $(id); \
+				if [ $(words $(call strip,$(BR_EXTERNAL_IDS))) -gt 1 ]; then \
+					printf 'endmenu # BR2_EXTERNAL_%s\n\n' $(id); \
+				fi; ) \
+			for i in $$(seq 1 80); do printf '#'; done; printf '\n\n'; \
 		fi; \
 		printf 'endmenu # User-provided options\n'; \
 	fi >$@

@@ -1004,10 +1043,10 @@ list-defconfigs:
 	@echo 'Built-in configs:'
 	@$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig))), \
 	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
-ifneq ($(wildcard $(BR2_EXTERNAL)/configs/*_defconfig),)
+ifneq ($(wildcard $(patsubst %,%/configs/*_defconfig,$(BR2_EXTERNAL))),)
 	@echo
 	@echo 'User-provided configs:'
-	@$(foreach b, $(sort $(notdir $(wildcard $(BR2_EXTERNAL)/configs/*_defconfig))), \
+	@$(foreach b, $(sort $(notdir $(wildcard $(patsubst %,%/configs/*_defconfig,$(BR2_EXTERNAL))))), \
 	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
 endif
 	@echo

@@ -1029,7 +1068,7 @@ print-version:
 	@echo $(BR2_VERSION_FULL)
 
 include docs/manual/manual.mk
--include $(BR2_EXTERNAL)/docs/*/*.mk
+-include $(patsubst %,%/docs/*/*.mk,$(BR2_EXTERNAL))
 
 .PHONY: $(noconfig_targets)
 

package/Makefile.in +1 -1

index afd5d3a..fe8ee68 100644
@@ -319,7 +319,7 @@ EXTRA_ENV = \
 	PATH=$(BR_PATH) \
 	BR2_DL_DIR=$(BR2_DL_DIR) \
 	BUILD_DIR=$(BUILD_DIR) \
-	BR2_EXTERNAL=$(BR2_EXTERNAL)
+	BR2_EXTERNAL="$(BR2_EXTERNAL)"
 
 ################################################################################
 # settings we need to pass to configure