[Buildroot] [PATCHv3 0/5] Keeping customizations outside the Buildroot tree with BR2_EXTERNAL

[Buildroot] [PATCHv3 0/5] Keeping customizations outside the Buildroot tree with BR2_EXTERNAL

[Thomas Petazzoni]

Hello,

Here is the third version of the BR2_EXTERNAL patch series. Its main
purpose is to take into account the feedback received at the ELCE
Developer Days.

This third version has been prepared with the help of Ryan Barnett
(thanks!).

People interested in testing this can find a branch named
things-outside-br-v3 at
http://git.free-electrons.com/users/thomas-petazzoni/buildroot/log/?h=things-outside-br-v3.

Changes since v2
================

 * Remove the mechanism that was storing the BR2_EXTERNAL value in the
   external wrapper.

 * Add logic that stores the BR2_EXTERNAL value in a file called
   $(O)/.br-external. Whenever the user passes a BR2_EXTERNAL=<value>,
   it gets stored in this file, and Buildroot re-uses for all
   subsequent make invocations. The user can later override it again
   by passing another BR2_EXTERNAL=<value>. He can also remove it
   entirely by passing just BR2_EXTERNAL= (empty value).

 * Enforce usage of a package/ directory in the BR2_EXTERNAL
   directory. Buildroot only includes $(BR2_EXTERNAL)/Config.in and
   shows it under the "Target packages" menu, and Buildroot will only
   include $(BR2_EXTERNAL)/package/*/*.mk.

 * Clearly separate the Buildroot built-in defconfigs from the
   user-defined external defconfigs in the 'make help' output.

 * Include a fix for building the documentation, provided by Samuel
   Martin.

 * Update the documentation accordingly.

Changes since v1
================

 * Store BR2_EXTERNAL in the Makefile generated in the output
   directory, so that BR2_EXTERNAL doesn't have to be passed over and
   over again. Suggested by Ryan Barnett.

 * Patch set splitted in more fine-grained changes, as suggested by
   Arnout.

 * Instead of having the top-level Config.in file generated at
   run-time, use instead a trick proposed by Arnout: make BR2_EXTERNAL
   point to support/dummy-external/, which contains an empty
   Config.in. This way, regardless of whether BR2_EXTERNAL is
   specified by the user or not, kconfig is happy as 'source
   "$BR2_EXTERNAL/Config.in"' will always point to an existing
   file. If BR2_EXTERNAL is not used, it will point to the dummy file,
   if BR2_EXTERNAL is used, it will point to Config.in file in the
   provided BR2_EXTERNAL directory.

   With this, BR2_EXTERNAL always has a value, so testing whether
   BR2_EXTERNAL is empty no longer tells us if the user has provided a
   value or not. Therefore, we have another variable,
   BR2_EXTERNAL_USED, which tells if the mechanism is used by the user
   or not. This variable avoids trying to use defconfigs from the
   dummy directory, or to encode the BR2_EXTERNAL value in the
   Makefile wrapper.

 * As a consequence of the previous change, it is no longer needed to
   refactor the *config dependencies using a variable, so I've dropped
   what was previously the first patch of the series.

 * BR2_EXTERNAL is turned into an absolute path, in order to avoid any
   problem. Suggested by Arnout.

 * Fixed the usage of <some board> vs. <boardname> in the
   documentation, as noted by Arnout.

Thomas

Samuel Martin (1):
  manual: fix manual generation with BR2_EXTERNAL support

Thomas Petazzoni (4):
  core: introduce the BR2_EXTERNAL variable
  core: allow external Config.in/makefile code to be integrated
  core: allow external defconfigs to be used
  docs/manual: add explanations about BR2_EXTERNAL

 Config.in                                |   4 ++
 Makefile                                 |  61 +++++++++++++++-
 docs/manual/customize-outside-br.txt     | 116 +++++++++++++++++++++++++++++++
 docs/manual/customize.txt                |   2 +
 docs/manual/manual.mk                    |   1 +
 package/Config.in                        |   2 +
 support/dummy-external/package/Config.in |   0
 support/scripts/kconfiglib.py            |   2 +-
 8 files changed, 186 insertions(+), 2 deletions(-)
 create mode 100644 docs/manual/customize-outside-br.txt
 create mode 100644 support/dummy-external/package/Config.in

-- 
1.8.1.2

[Buildroot] [PATCHv3 1/5] core: introduce the BR2_EXTERNAL variable

[Thomas Petazzoni]

This commit introduces the BR2_EXTERNAL environment variable, which
will allow to keep Buildroot customization (board-specific
configuration files or root filesystem overlays, package Config.in and
makefiles, as well as defconfigs) outside of the Buildroot tree.

This commit only introduces the variable itself, and ensures that it
is available within Config.in options, so that string options used to
specify paths to directories or files can use $BR2_EXTERNAL as a
reference. For example, one can use
$BR2_EXTERNAL/board/<someboard>/kernel.config as the
BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE value.

Following patches extend the usage of BR2_EXTERNAL to other areas
(packages and defconfigs).

In details, this commit:

 * Introduces the BR2_EXTERNAL Kconfig option. This option has no
   prompt, and is therefore not visible to the user and also not
   stored in the .config file. It is automatically set to the value of
   the BR2_EXTERNAL environment variable. The only purpose of this
   BR2_EXTERNAL Kconfig option is to allow $BR2_EXTERNAL to be
   properly expanded when used inside Kconfig option values.

 * Calculates the BR2_EXTERNAL value to use. If passed on the command
   line, then this value is taken in priority, and saved to a
   .br-external hidden file in the output directory. If not passed on
   the command line, then we read the .br-external file from the
   output directory. This allows the user to not pass the BR2_EXTERNAL
   value at each make invocation. If no BR2_EXTERNAL value is passed,
   we define it to support/dummy-external/, so that the kconfig code
   finds an existing $(BR2_EXTERNAL)/package/Config.in file to
   include.

 * Passes the BR2_EXTERNAL into the *config environment, so that its
   value is found when parsing/evaluating Config.in files and .config
   values.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Config.in |  4 ++++
 Makefile  | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Config.in b/Config.in
index d87e0f0..98726ab 100644
--- a/Config.in
+++ b/Config.in
@@ -14,6 +14,10 @@ config BR2_HOSTARCH
 	string
 	option env="HOSTARCH"
 
+config BR2_EXTERNAL
+	string
+	option env="BR2_EXTERNAL"
+
 # Hidden boolean selected by pre-built packages for x86, when they
 # need to run on x86-64 machines (example: pre-built external
 # toolchains, binary tools like SAM-BA, etc.).
diff --git a/Makefile b/Makefile
index b5368a3..a46418e 100644
--- a/Makefile
+++ b/Makefile
@@ -99,6 +99,45 @@ export CDPATH:=
 BASE_DIR := $(shell mkdir -p $(O) && cd $(O) >/dev/null && pwd)
 $(if $(BASE_DIR),, $(error output directory "$(O)" does not exist))
 
+
+# Handling of BR2_EXTERNAL. We are handling three cases here:
+#
+#  (Case 1) BR2_EXTERNAL is defined in the command line, but has an
+#           empty value. That's an indication that the user wants to
+#           remove the BR2_EXTERNAL value. So we use the
+#           dummy-external directory as BR2_EXTERNAL and remove the
+#           .br-external file.
+#  (Case 2) BR2_EXTERNAL is defined in the command line, and has a
+#           non-empty value. That's an indication that the user wants
+#           to use the provided location as the BR2_EXTERNAL. We
+#           verify that the location exists, and if it's the case,
+#           store it in .br-external.
+#  (Case 3) BR2_EXTERNAL isn't defined in the command line. We load
+#           the value from .br-external, verify that it exists and
+#           then use it.
+
+BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
+
+ifeq ($(origin BR2_EXTERNAL),command line)
+ifeq ($(BR2_EXTERNAL),) # Case 1
+override BR2_EXTERNAL := $(TOPDIR)/support/dummy-external/
+$(shell rm -f $(BR2_EXTERNAL_FILE))
+else # Case 2
+ifeq ($(wildcard $(BR2_EXTERNAL)),)
+$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")
+endif
+override BR2_EXTERNAL := $(realpath $(BR2_EXTERNAL))
+BR2_EXTERNAL_USED = y
+$(shell echo $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
+endif
+else # Case 3
+override BR2_EXTERNAL := $(shell test -f $(BR2_EXTERNAL_FILE) && cat $(BR2_EXTERNAL_FILE))
+ifeq ($(wildcard $(BR2_EXTERNAL)),)
+$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")
+endif
+BR2_EXTERNAL_USED = y
+endif
+
 BUILD_DIR:=$(BASE_DIR)/build
 STAMP_DIR:=$(BASE_DIR)/stamps
 BINARIES_DIR:=$(BASE_DIR)/images
@@ -619,7 +658,8 @@ COMMON_CONFIG_ENV = \
 	KCONFIG_AUTOCONFIG=$(BUILD_DIR)/buildroot-config/auto.conf \
 	KCONFIG_AUTOHEADER=$(BUILD_DIR)/buildroot-config/autoconf.h \
 	KCONFIG_TRISTATE=$(BUILD_DIR)/buildroot-config/tristate.config \
-	BUILDROOT_CONFIG=$(BUILDROOT_CONFIG)
+	BUILDROOT_CONFIG=$(BUILDROOT_CONFIG) \
+	BR2_EXTERNAL=$(BR2_EXTERNAL)
 
 xconfig: $(BUILD_DIR)/buildroot-config/qconf outputmakefile
 	@mkdir -p $(BUILD_DIR)/buildroot-config
-- 
1.8.1.2

[Buildroot] [PATCHv3 1/5] core: introduce the BR2_EXTERNAL variable

[Yann E. MORIN]

Thomas, All,

On 2013-11-27 23:31 +0100, Thomas Petazzoni spake thusly:
> This commit introduces the BR2_EXTERNAL environment variable, which
> will allow to keep Buildroot customization (board-specific
> configuration files or root filesystem overlays, package Config.in and
> makefiles, as well as defconfigs) outside of the Buildroot tree.
> 
> This commit only introduces the variable itself, and ensures that it
> is available within Config.in options, so that string options used to
> specify paths to directories or files can use $BR2_EXTERNAL as a
> reference. For example, one can use
> $BR2_EXTERNAL/board/<someboard>/kernel.config as the
> BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE value.
[--SNIP--]

With only this commit applied, and a clean tree, I get:

    $ make menuconfig
    Makefile:136: *** "The specified BR2_EXTERNAL '' location doesn't
    exist".  Stop.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[--SNIP--]
> diff --git a/Makefile b/Makefile
> index b5368a3..a46418e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -99,6 +99,45 @@ export CDPATH:=
[--SNIP--]
> +BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
> +
> +ifeq ($(origin BR2_EXTERNAL),command line)
> +ifeq ($(BR2_EXTERNAL),) # Case 1
> +override BR2_EXTERNAL := $(TOPDIR)/support/dummy-external/
> +$(shell rm -f $(BR2_EXTERNAL_FILE))
> +else # Case 2
> +ifeq ($(wildcard $(BR2_EXTERNAL)),)
> +$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")
> +endif
> +override BR2_EXTERNAL := $(realpath $(BR2_EXTERNAL))
> +BR2_EXTERNAL_USED = y
> +$(shell echo $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
> +endif
> +else # Case 3
> +override BR2_EXTERNAL := $(shell test -f $(BR2_EXTERNAL_FILE) && cat $(BR2_EXTERNAL_FILE))
> +ifeq ($(wildcard $(BR2_EXTERNAL)),)
> +$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")

The error occurs here.

That's becasue you test if BR2_EXTERNAL is comming from the command
line, but that last case should also test if BR2_EXTERNAL_FILE exists
before scanning it.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCHv3 1/5] core: introduce the BR2_EXTERNAL variable

[Yann E. MORIN]

Thomas, All,

On 2013-11-28 22:50 +0100, Yann E. MORIN spake thusly:
> On 2013-11-27 23:31 +0100, Thomas Petazzoni spake thusly:
> > This commit introduces the BR2_EXTERNAL environment variable, which
> > will allow to keep Buildroot customization (board-specific
> > configuration files or root filesystem overlays, package Config.in and
> > makefiles, as well as defconfigs) outside of the Buildroot tree.
> > 
> > This commit only introduces the variable itself, and ensures that it
> > is available within Config.in options, so that string options used to
> > specify paths to directories or files can use $BR2_EXTERNAL as a
> > reference. For example, one can use
> > $BR2_EXTERNAL/board/<someboard>/kernel.config as the
> > BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE value.
> [--SNIP--]
> 
> With only this commit applied, and a clean tree, I get:
> 
>     $ make menuconfig
>     Makefile:136: *** "The specified BR2_EXTERNAL '' location doesn't
>     exist".  Stop.
> 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> [--SNIP--]
> > diff --git a/Makefile b/Makefile
> > index b5368a3..a46418e 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -99,6 +99,45 @@ export CDPATH:=
> [--SNIP--]
> > +BR2_EXTERNAL_FILE = $(BASE_DIR)/.br-external
> > +
> > +ifeq ($(origin BR2_EXTERNAL),command line)
> > +ifeq ($(BR2_EXTERNAL),) # Case 1
> > +override BR2_EXTERNAL := $(TOPDIR)/support/dummy-external/
> > +$(shell rm -f $(BR2_EXTERNAL_FILE))
> > +else # Case 2
> > +ifeq ($(wildcard $(BR2_EXTERNAL)),)
> > +$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")
> > +endif
> > +override BR2_EXTERNAL := $(realpath $(BR2_EXTERNAL))
> > +BR2_EXTERNAL_USED = y
> > +$(shell echo $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
> > +endif
> > +else # Case 3
> > +override BR2_EXTERNAL := $(shell test -f $(BR2_EXTERNAL_FILE) && cat $(BR2_EXTERNAL_FILE))
> > +ifeq ($(wildcard $(BR2_EXTERNAL)),)
> > +$(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't exist")
> 
> The error occurs here.
> 
> That's becasue you test if BR2_EXTERNAL is comming from the command
> line, but that last case should also test if BR2_EXTERNAL_FILE exists
> before scanning it.

Doh, I forgot to paste the patch:

diff --git a/Makefile b/Makefile
index 83a5c06..038294b 100644
--- a/Makefile
+++ b/Makefile
@@ -131,12 +131,14 @@ BR2_EXTERNAL_USED = y
 $(shell echo $(BR2_EXTERNAL) > $(BR2_EXTERNAL_FILE))
 endif
 else # Case 3
+ifneq ($(wildcard $(BR2_EXTERNAL_FILE)),)
 override BR2_EXTERNAL := $(shell test -f $(BR2_EXTERNAL_FILE) && cat
$(BR2_EXTERNAL_FILE))
 ifeq ($(wildcard $(BR2_EXTERNAL)),)
 $(error "The specified BR2_EXTERNAL '$(BR2_EXTERNAL)' location doesn't
exist")
 endif
 BR2_EXTERNAL_USED = y
 endif
+endif
 
 BUILD_DIR:=$(BASE_DIR)/build
 STAMP_DIR:=$(BASE_DIR)/stamps

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCHv3 1/5] core: introduce the BR2_EXTERNAL variable

[Yann E. MORIN]

Thomas, All,

On 2013-11-27 23:31 +0100, Thomas Petazzoni spake thusly:
[--SNIP--]
> +# Handling of BR2_EXTERNAL. We are handling three cases here:
> +#
> +#  (Case 1) BR2_EXTERNAL is defined in the command line, but has an
> +#           empty value. That's an indication that the user wants to
> +#           remove the BR2_EXTERNAL value. So we use the
> +#           dummy-external directory as BR2_EXTERNAL and remove the
> +#           .br-external file.

Should be:
    [...] we use the dummy-external directory as BR2_EXTERNAL and store
    it in the .br-external file.

> +#  (Case 2) BR2_EXTERNAL is defined in the command line, and has a
> +#           non-empty value. That's an indication that the user wants
> +#           to use the provided location as the BR2_EXTERNAL. We
> +#           verify that the location exists, and if it's the case,
> +#           store it in .br-external.
> +#  (Case 3) BR2_EXTERNAL isn't defined in the command line. We load
> +#           the value from .br-external, verify that it exists and
> +#           then use it.

You're missing one case:

(Case 4) BR2_EXTERNAL is not defined on the command line, and
         BR2_EXTERNAL_FILE does not exist (first run). We set
         BR2_EXTERNAL to point to our dummy-external directory,
         and store it in the .br-external file.

This missing case is breaking the standard (no-BR2_EXTERNAL) build.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

This commit allows the BR2_EXTERNAL directory to contain additional
Buildroot packages:

 - Buildroot automatically includes the
   $BR2_EXTERNAL/package/Config.in in the "Target packages"
   configuration sub-menu.

 - Buildroot automatically includes the BR2_EXTERNAL/package/*/*.mk
   files in the build logic.

We also add a dummy Config.in file in support/dummy-external/ to
ensure that the source "$BR2_EXTERNAL/Config.in" line will point to an
existing file even when BR2_EXTERNAL is not used by the user.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile                                 | 4 ++++
 package/Config.in                        | 2 ++
 support/dummy-external/package/Config.in | 0
 3 files changed, 6 insertions(+)
 create mode 100644 support/dummy-external/package/Config.in

diff --git a/Makefile b/Makefile
index a46418e..013b646 100644
--- a/Makefile
+++ b/Makefile
@@ -368,6 +368,10 @@ include boot/common.mk
 include linux/linux.mk
 include system/system.mk
 
+ifeq ($(BR2_EXTERNAL_USED),y)
+include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk))
+endif
+
 TARGETS+=target-finalize
 
 ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
diff --git a/package/Config.in b/package/Config.in
index 311cc6c..fafaa2b 100644
--- a/package/Config.in
+++ b/package/Config.in
@@ -997,4 +997,6 @@ source "package/vim/Config.in"
 endif
 endmenu
 
+source "$BR2_EXTERNAL/package/Config.in"
+
 endmenu
diff --git a/support/dummy-external/package/Config.in b/support/dummy-external/package/Config.in
new file mode 100644
index 0000000..e69de29
-- 
1.8.1.2

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Jeremy Rosen]

Just an overall comment, this patch serie is great, it will help us 
a lot

by allowing us to clearly show to our customers what is specific 
to their board and what is changes to the buildroot unfrastructure
it will make it much easier for us to upstream all the non-specific 
parts


on this particular one, if I remember correctly the kconfig infrastructure
will not work correctly if one of the include is missing... so shouldn't
your infrastructure check if $(BR2_EXTERNAL)/package/Config.in 
exists, and create one if it doesn't ? 

Cordialement 

J?r?my Rosen 
+33 (0)1 42 68 28 04

fight key loggers : write some perl using vim 


Open Wide Ingenierie

23, rue Daviel
75012 Paris - France
www.openwide.fr





----- Mail original -----
> This commit allows the BR2_EXTERNAL directory to contain additional
> Buildroot packages:
> 
>  - Buildroot automatically includes the
>    $BR2_EXTERNAL/package/Config.in in the "Target packages"
>    configuration sub-menu.
> 
>  - Buildroot automatically includes the BR2_EXTERNAL/package/*/*.mk
>    files in the build logic.
> 
> We also add a dummy Config.in file in support/dummy-external/ to
> ensure that the source "$BR2_EXTERNAL/Config.in" line will point to
> an
> existing file even when BR2_EXTERNAL is not used by the user.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Makefile                                 | 4 ++++
>  package/Config.in                        | 2 ++
>  support/dummy-external/package/Config.in | 0
>  3 files changed, 6 insertions(+)
>  create mode 100644 support/dummy-external/package/Config.in
> 
> diff --git a/Makefile b/Makefile
> index a46418e..013b646 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -368,6 +368,10 @@ include boot/common.mk
>  include linux/linux.mk
>  include system/system.mk
>  
> +ifeq ($(BR2_EXTERNAL_USED),y)
> +include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk))
> +endif
> +
>  TARGETS+=target-finalize
>  
>  ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
> diff --git a/package/Config.in b/package/Config.in
> index 311cc6c..fafaa2b 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -997,4 +997,6 @@ source "package/vim/Config.in"
>  endif
>  endmenu
>  
> +source "$BR2_EXTERNAL/package/Config.in"
> +
>  endmenu
> diff --git a/support/dummy-external/package/Config.in
> b/support/dummy-external/package/Config.in
> new file mode 100644
> index 0000000..e69de29
> --
> 1.8.1.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Jeremy Rosen,

Please do not top post. This is considered bad practice on mailing
lists.

On Thu, 28 Nov 2013 09:33:50 +0100 (CET), Jeremy Rosen wrote:
> Just an overall comment, this patch serie is great, it will help us 
> a lot
> 
> by allowing us to clearly show to our customers what is specific 
> to their board and what is changes to the buildroot unfrastructure
> it will make it much easier for us to upstream all the non-specific 
> parts

Thanks for the feedback! Definitely great to see that this feature will
help companies upstream more things in Buildroot.

> on this particular one, if I remember correctly the kconfig infrastructure
> will not work correctly if one of the include is missing... so shouldn't
> your infrastructure check if $(BR2_EXTERNAL)/package/Config.in 
> exists, and create one if it doesn't ? 

The idea is that:

 * If the user is specifying a BR2_EXTERNAL location, then this
   location *must* contain a package/Config.in file. Even if it's empty.

 * If the user is not specifying a BR2_EXTERNAL location, then we are
   using support/dummy-external/ as a fake BR2_EXTERNAL, to ensure that
   package/Config.in exists.

There is no way to express in kconfig "include this file if it exists,
or ignore it if it doesn't". Yann and myself had written a patch for
kconfig that adds a new statement, similar to 'source' but that
silently ignores the inclusion if the given file doesn't exist. But
after implementing it, we found out that is was not necessary, as we
could simply use a fake BR2_EXTERNAL, and avoid changing kconfig
altogether.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Jeremy Rosen]

----- Mail original -----
> Dear Jeremy Rosen,
> 
> Please do not top post. This is considered bad practice on mailing
> lists.
> 
sorry, trying to adapt to the different habits of the different ML

usually it's ok to toppost when you don't comment the actual content...

i'll try to get it right next time.

> > on this particular one, if I remember correctly the kconfig
> > infrastructure
> > will not work correctly if one of the include is missing... so
> > shouldn't
> > your infrastructure check if $(BR2_EXTERNAL)/package/Config.in
> > exists, and create one if it doesn't ?
> 
> The idea is that:
> 
>  * If the user is specifying a BR2_EXTERNAL location, then this
>    location *must* contain a package/Config.in file. Even if it's
>    empty.
> 
>  * If the user is not specifying a BR2_EXTERNAL location, then we are
>    using support/dummy-external/ as a fake BR2_EXTERNAL, to ensure
>    that
>    package/Config.in exists.
> 
> There is no way to express in kconfig "include this file if it
> exists,

my idea was more along the line of "if we are setting BR2_EXTERNAL
and the file doesn't exist, then create an empty "Config.in" file
so the next call to make doesn't fail" 

this would be to avoid leaving the build system in a broken 
configuration after a call that sets up the build system for the
user.

I can see pro and cons on this idea, there is a risk in touching
a file the user is supposed to edit and the breakage itself could
be considered a good way to make sure the user go look into the 
file

but on the other hand, for simple use cases where the user doesn't
want to add new packages, but simply overlay a couple of config
files, creating a valid infrastructure from the start might be 
a good idea...

Cheers
Boucman

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Jeremy Rosen,

On Thu, 28 Nov 2013 10:37:27 +0100 (CET), Jeremy Rosen wrote:

> my idea was more along the line of "if we are setting BR2_EXTERNAL
> and the file doesn't exist, then create an empty "Config.in" file
> so the next call to make doesn't fail" 
> 
> this would be to avoid leaving the build system in a broken 
> configuration after a call that sets up the build system for the
> user.
> 
> I can see pro and cons on this idea, there is a risk in touching
> a file the user is supposed to edit and the breakage itself could
> be considered a good way to make sure the user go look into the 
> file
> 
> but on the other hand, for simple use cases where the user doesn't
> want to add new packages, but simply overlay a couple of config
> files, creating a valid infrastructure from the start might be 
> a good idea...

Ok, I think I see what you mean: creating
$(BR2_EXTERNAL)/package/Config.in automatically if it doesn't exist. I
don't have a really strong feeling about this, but my initial reaction
is that it's better to let the user do that by himself. BR2_EXTERNAL is
part of the user source tree, and I hate when tools mess up with my
source tree by creating files here and there automatically.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Ryan Barnett]

Thomas, Jeremy, 

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 11/28/2013 
05:33:59 AM:

> Dear Jeremy Rosen,
> 
> On Thu, 28 Nov 2013 10:37:27 +0100 (CET), Jeremy Rosen wrote:
> 
> > my idea was more along the line of "if we are setting BR2_EXTERNAL
> > and the file doesn't exist, then create an empty "Config.in" file
> > so the next call to make doesn't fail" 
> > 
> > this would be to avoid leaving the build system in a broken 
> > configuration after a call that sets up the build system for the
> > user.
> > 
> > I can see pro and cons on this idea, there is a risk in touching
> > a file the user is supposed to edit and the breakage itself could
> > be considered a good way to make sure the user go look into the 
> > file
> > 
> > but on the other hand, for simple use cases where the user doesn't
> > want to add new packages, but simply overlay a couple of config
> > files, creating a valid infrastructure from the start might be 
> > a good idea...
> 
> Ok, I think I see what you mean: creating
> $(BR2_EXTERNAL)/package/Config.in automatically if it doesn't exist. I
> don't have a really strong feeling about this, but my initial reaction
> is that it's better to let the user do that by himself. BR2_EXTERNAL is
> part of the user source tree, and I hate when tools mess up with my
> source tree by creating files here and there automatically.

I'm in agreement with Thomas on this issue. I would prefer it if the user 
has to create the file (even with it being blank). However, maybe we could 
a test within the main makefile that if BR2_EXTERNAL is used and there is 
no "package/Config.in" present we through a make error prompt the user to 
create this file. I prefer this method since it would be teaching the user 
how to correctly use this feature instead of doing some hidden 
"auto-magic".

Thanks,
-Ryan
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131128/0eabd6b0/attachment.html>

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Ryan Barnett,

On Thu, 28 Nov 2013 06:09:33 -0600, Ryan Barnett wrote:

> > Ok, I think I see what you mean: creating
> > $(BR2_EXTERNAL)/package/Config.in automatically if it doesn't
> > exist. I don't have a really strong feeling about this, but my
> > initial reaction is that it's better to let the user do that by
> > himself. BR2_EXTERNAL is part of the user source tree, and I hate
> > when tools mess up with my source tree by creating files here and
> > there automatically.
> 
> I'm in agreement with Thomas on this issue. I would prefer it if the
> user has to create the file (even with it being blank). However,
> maybe we could a test within the main makefile that if BR2_EXTERNAL
> is used and there is no "package/Config.in" present we through a make
> error prompt the user to create this file. I prefer this method since
> it would be teaching the user how to correctly use this feature
> instead of doing some hidden "auto-magic".

I don't think anything special is needed. With the current patch set
(v3), if the package/Config.in file is missing in the BR2_EXTERNAL
directory, the error is quite obvious:

$ make BR2_EXTERNAL=/tmp/koin menuconfig
package/Config.in:1000: can't open file "/tmp/koin/package/Config.in"
make: *** [menuconfig] Error 1
$

(The /tmp/koin directory is empty)

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Ryan Barnett]

Thomas,

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 11/28/2013 
06:29:14 AM:

> I don't think anything special is needed. With the current patch set
> (v3), if the package/Config.in file is missing in the BR2_EXTERNAL
> directory, the error is quite obvious:
> 
> $ make BR2_EXTERNAL=/tmp/koin menuconfig
> package/Config.in:1000: can't open file "/tmp/koin/package/Config.in"
> make: *** [menuconfig] Error 1
> $
> 
> (The /tmp/koin directory is empty)

Ah - then I agree that the current form is sufficient enough.

Thanks,
-Ryan

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Samuel Martin]

Hi Thomas, all,


2013/11/27 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

> This commit allows the BR2_EXTERNAL directory to contain additional
> Buildroot packages:
>
>  - Buildroot automatically includes the
>    $BR2_EXTERNAL/package/Config.in in the "Target packages"
>    configuration sub-menu.
>
>  - Buildroot automatically includes the BR2_EXTERNAL/package/*/*.mk
>    files in the build logic.
>
> We also add a dummy Config.in file in support/dummy-external/ to
> ensure that the source "$BR2_EXTERNAL/Config.in" line will point to an
> existing file even when BR2_EXTERNAL is not used by the user.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Makefile                                 | 4 ++++
>  package/Config.in                        | 2 ++
>  support/dummy-external/package/Config.in | 0
>  3 files changed, 6 insertions(+)
>  create mode 100644 support/dummy-external/package/Config.in
>
> diff --git a/Makefile b/Makefile
> index a46418e..013b646 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -368,6 +368,10 @@ include boot/common.mk
>  include linux/linux.mk
>  include system/system.mk
>
> +ifeq ($(BR2_EXTERNAL_USED),y)
> +include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk))
> +endif
> +
>  TARGETS+=target-finalize
>
>  ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
> diff --git a/package/Config.in b/package/Config.in
> index 311cc6c..fafaa2b 100644
> --- a/package/Config.in
> +++ b/package/Config.in
> @@ -997,4 +997,6 @@ source "package/vim/Config.in"
>  endif
>  endmenu
>
> +source "$BR2_EXTERNAL/package/Config.in"
> +
>
In this new version of this patch set, the all target and host packages
entries end up under the "Target packages" menu.
In my use of this feature, I also have a couple of host tools to integrate.
So, how about having adding the following source line in
package/Config.in.host:
---
source "$BR2_EXTERNAL/package/Config.in.host"
---
Of course, this requires to add an
empty support/dummy-external/package/Config.in.host file.

Regards,

-- 
Samuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131128/e7213f9a/attachment-0001.html>

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Simon Dawson]

On 28 November 2013 13:24, Samuel Martin <s.martin49@gmail.com> wrote:
> In this new version of this patch set, the all target and host packages
> entries end up under the "Target packages" menu.
> In my use of this feature, I also have a couple of host tools to integrate.
> So, how about having adding the following source line in
> package/Config.in.host:
> ---
> source "$BR2_EXTERNAL/package/Config.in.host"
> ---
> Of course, this requires to add an empty
> support/dummy-external/package/Config.in.host file.

That would also apply for my expected use of the feature.

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Samuel Martin,

On Thu, 28 Nov 2013 14:24:35 +0100, Samuel Martin wrote:

> In this new version of this patch set, the all target and host
> packages entries end up under the "Target packages" menu.
> In my use of this feature, I also have a couple of host tools to
> integrate. So, how about having adding the following source line in
> package/Config.in.host:
> ---
> source "$BR2_EXTERNAL/package/Config.in.host"
> ---
> Of course, this requires to add an
> empty support/dummy-external/package/Config.in.host file.

This means the user would always have to create both package/Config.in
and package/Config.in.host, which gets annoying. That's why in the v2 I
did include $(BR2_EXTERNAL)/Config.in, without enforcing a
$(BR2_EXTERNAL)/package/ subdirectory, and leave more freedom to the
user. However, several people at the ELCE meeting said that we should
enforce the package/ directory, and therefore only include
$(BR2_EXTERNAL)/package/Config.in.

Just curious, why can't you simply add your host packages also in
$(BR2_EXTERNAL)/package/Config.in ? It's not because the main Buildroot
doesn't do it this way that you can't do it in your BR2_EXTERNAL
directory.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Samuel Martin]

2013/11/28 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

> Dear Samuel Martin,
>
> On Thu, 28 Nov 2013 14:24:35 +0100, Samuel Martin wrote:
>
> > In this new version of this patch set, the all target and host
> > packages entries end up under the "Target packages" menu.
> > In my use of this feature, I also have a couple of host tools to
> > integrate. So, how about having adding the following source line in
> > package/Config.in.host:
> > ---
> > source "$BR2_EXTERNAL/package/Config.in.host"
> > ---
> > Of course, this requires to add an
> > empty support/dummy-external/package/Config.in.host file.
>
> This means the user would always have to create both package/Config.in
> and package/Config.in.host, which gets annoying.

Yes, that's the only drawback of this.


> That's why in the v2 I
> did include $(BR2_EXTERNAL)/Config.in, without enforcing a
> $(BR2_EXTERNAL)/package/ subdirectory, and leave more freedom to the
> user. However, several people at the ELCE meeting said that we should
> enforce the package/ directory, and therefore only include
> $(BR2_EXTERNAL)/package/Config.in.
>
> Just curious, why can't you simply add your host packages also in
> $(BR2_EXTERNAL)/package/Config.in ? It's not because the main Buildroot
> doesn't do it this way that you can't do it in your BR2_EXTERNAL
> directory.
>
Well, right now, most of the packages I put in my BR2_EXTERNAL tree are host
tools to generate images.

With the previous version of this serie, the new menu entry was added at
the top level
of menuconfig, in which I added 2 submenus: "Target packages" and "Host
packages".
It was rather clear that all "company" stuff goes into this menu.

With the v2, I was a bit lost at first, the "company" menu got moved under
"Target packages". So, now I have a menu tree like this:

---
Main menu
  ...
  Target packages --->
    ...
    Company --->
      Target packages --->
      Host packages --->
  ...
  Host packages --->
---

It does not hurt that much, but it's not really nice IMHO.

I agree that maybe most of the users may not need to add host packages;
that does not mean there is no room for improvement ;)

BTW, to generate this/these Config.in{,host} files in the BR2_EXTERNAL tree,
I already have a script scanning the tree and updating these files, it
could be
included as a support script as well.


Regards,

-- 
Samuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131128/bf3f9c48/attachment-0001.html>

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Samuel Martin,

On Thu, 28 Nov 2013 18:53:57 +0100, Samuel Martin wrote:

> Well, right now, most of the packages I put in my BR2_EXTERNAL tree
> are host tools to generate images.
> 
> With the previous version of this serie, the new menu entry was added
> at the top level
> of menuconfig, in which I added 2 submenus: "Target packages" and
> "Host packages".
> It was rather clear that all "company" stuff goes into this menu.
> 
> With the v2, I was a bit lost at first, the "company" menu got moved

I guess you meant "v3" here, because the "v2" included
$(BR2_EXTERNAL)/Config.in in the top-level menu of menuconfig.

> under "Target packages". So, now I have a menu tree like this:
> 
> ---
> Main menu
>   ...
>   Target packages --->
>     ...
>     Company --->
>       Target packages --->
>       Host packages --->
>   ...
>   Host packages --->
> ---
> 
> It does not hurt that much, but it's not really nice IMHO.

Then please talk to the people who asked for enforcing
$(BR2_EXTERNAL)/package/Config.in usage during the Buildroot Developers
Days in Edinburgh. This decision/choice is written very clearly in
http://elinux.org/Buildroot:DeveloperDaysELCE2013#BR2_EXTERNAL :

"""
Regarding the directory hierarchy in the external tree, it was agreed
that it is a good idea to force three subdirectories: package, board,
configs. Buildroot's package/Config.in will source
$BR2_EXTERNAL/package/Config.in.
"""

(It's even in bold in the report).

I believe the most vocal person in favor of this was Arnout, so I've
added him in Cc.

> BTW, to generate this/these Config.in{,host} files in the
> BR2_EXTERNAL tree, I already have a script scanning the tree and
> updating these files, it could be included as a support script as
> well.

Why is this needed? Just write Config.in like we do in the main
Buildroot tree. For example, people may want to have menus and submenus
in their $(BR2_EXTERNAL)/package/Config.in. Therefore, I don't think
providing more and more and more scripts that match a very specific use
case is really going to help our users, probably going to confuse them.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Samuel Martin]

Thomas,

2013/11/28 Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

> Dear Samuel Martin,
>
> On Thu, 28 Nov 2013 18:53:57 +0100, Samuel Martin wrote:
>
> > Well, right now, most of the packages I put in my BR2_EXTERNAL tree
> > are host tools to generate images.
> >
> > With the previous version of this serie, the new menu entry was added
> > at the top level
> > of menuconfig, in which I added 2 submenus: "Target packages" and
> > "Host packages".
> > It was rather clear that all "company" stuff goes into this menu.
> >
> > With the v2, I was a bit lost at first, the "company" menu got moved
>
> I guess you meant "v3" here, because the "v2" included
> $(BR2_EXTERNAL)/Config.in in the top-level menu of menuconfig.
>
Indeed. Sorry for the confusion.


>
> > under "Target packages". So, now I have a menu tree like this:
> >
> > ---
> > Main menu
> >   ...
> >   Target packages --->
> >     ...
> >     Company --->
> >       Target packages --->
> >       Host packages --->
> >   ...
> >   Host packages --->
> > ---
> >
> > It does not hurt that much, but it's not really nice IMHO.
>
> Then please talk to the people who asked for enforcing
> $(BR2_EXTERNAL)/package/Config.in usage during the Buildroot Developers
> Days in Edinburgh.

I thought the ml was the place for this...


> This decision/choice is written very clearly in
> http://elinux.org/Buildroot:DeveloperDaysELCE2013#BR2_EXTERNAL :
>
> """
> Regarding the directory hierarchy in the external tree, it was agreed
> that it is a good idea to force three subdirectories: package, board,
> configs. Buildroot's package/Config.in will source
> $BR2_EXTERNAL/package/Config.in.
> """
>
Doh! I don't mean this at all! Looks like I was not clear enough.

I'm not talking about the directory hierarchy, I do follow the Buildroot
hierarchy.
It does look like this:

BR2_EXTERNAL/
|`- board/
|    `- someboard/
|       |`- linux-myversion/
|       |    `- linux-0001-fix-something.patch
|       |`- busybox-1.21.1.config
|        `- post-build-script.sh
|`- configs/
|    `- someboard_defconfig
 `- package/
    |`- Config.in
    |`- Config.in.host
    |`- foo/
    |   |`- foo.mk
    |   |`- Config.in
    |    `- Config.in.host
    |`- bar/
    |   |`- bar.mk
    |    `- Config.in.host
     `- toto/
        |`- toto.mk
         `- Config.in

With BR2_EXTERNAL/package/Config.in sourcing all Config.in files from
BR2_EXTERNAL,
and BR2_EXTERNAL/package/Config.in.host sourcing all Config.in.host files
from BR2_EXTERNAL,

My point was only about sourcing BR2_EXTERNAL/package/Config.in.host under
the
"Host utilities" menu.

This is nothing more than consistency in the menuconfig rendering.
If no one thinks it's relevant, fair enough, I can live without it.


> (It's even in bold in the report).
>

> I believe the most vocal person in favor of this was Arnout, so I've
> added him in Cc.
>
> > BTW, to generate this/these Config.in{,host} files in the
> > BR2_EXTERNAL tree, I already have a script scanning the tree and
> > updating these files, it could be included as a support script as
> > well.
>
It only generate the BR2_EXTERNAL/package/Config.in{,.host} files because
I'm lazy.


> Why is this needed? Just write Config.in like we do in the main
> Buildroot tree. For example, people may want to have menus and submenus
> in their $(BR2_EXTERNAL)/package/Config.in. Therefore, I don't think
> providing more and more and more scripts that match a very specific use
> case is really going to help our users, probably going to confuse them.
>
Fair enough.

Regards,

-- 
Samuel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131128/deee8554/attachment-0001.html>

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Samuel Martin,

On Thu, 28 Nov 2013 21:04:04 +0100, Samuel Martin wrote:

> > Then please talk to the people who asked for enforcing
> > $(BR2_EXTERNAL)/package/Config.in usage during the Buildroot Developers
> > Days in Edinburgh.
> 
> I thought the ml was the place for this...

It is indeed. But understand that it's a little bit annoying to get
some review at the Buildroot Developer Days (to which you
participated), change the patches accordingly, and then get some review
that goes in the opposite direction once you post the new version of
the patches.

> > This decision/choice is written very clearly in
> > http://elinux.org/Buildroot:DeveloperDaysELCE2013#BR2_EXTERNAL :
> >
> > """
> > Regarding the directory hierarchy in the external tree, it was agreed
> > that it is a good idea to force three subdirectories: package, board,
> > configs. Buildroot's package/Config.in will source
> > $BR2_EXTERNAL/package/Config.in.
> > """
> >
> Doh! I don't mean this at all! Looks like I was not clear enough.
> 
> I'm not talking about the directory hierarchy, I do follow the Buildroot
> hierarchy.
> It does look like this:
> 
> BR2_EXTERNAL/
> |`- board/
> |    `- someboard/
> |       |`- linux-myversion/
> |       |    `- linux-0001-fix-something.patch
> |       |`- busybox-1.21.1.config
> |        `- post-build-script.sh
> |`- configs/
> |    `- someboard_defconfig
>  `- package/
>     |`- Config.in
>     |`- Config.in.host
>     |`- foo/
>     |   |`- foo.mk
>     |   |`- Config.in
>     |    `- Config.in.host
>     |`- bar/
>     |   |`- bar.mk
>     |    `- Config.in.host
>      `- toto/
>         |`- toto.mk
>          `- Config.in
> 
> With BR2_EXTERNAL/package/Config.in sourcing all Config.in files from
> BR2_EXTERNAL,
> and BR2_EXTERNAL/package/Config.in.host sourcing all Config.in.host files
> from BR2_EXTERNAL,
> 
> My point was only about sourcing BR2_EXTERNAL/package/Config.in.host under
> the
> "Host utilities" menu.

I perfectly understand what you mean, but I don't really like the idea
of sourcing BR2_EXTERNAL/package/Config.in.host, because it means the
user has to *always* create two Config.in files in its BR2_EXTERNAL
hierarchy to just get started in using BR2_EXTERNAL.

So, the reason your comment is *entirely* related to the previous
discussion is that in my previous proposal, I was including *ONE*
top-level BR2_EXTERNAL/Config.in, and it was up to the user to then do
whatever he wanted in this top-level Config.in file. We were not
enforcing anything.

I was OK with enforcing the usage of BR2_EXTERNAL/package/Config.in,
but not if we extend that to also enforce the usage (and existence) of
BR2_EXTERNAL/package/Config.in.host.

As you can see, your comment is *completely* related to the previous
proposal.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Yann E. MORIN]

Thomas, Samuel, All,

On 2013-11-28 21:21 +0100, Thomas Petazzoni spake thusly:
> On Thu, 28 Nov 2013 21:04:04 +0100, Samuel Martin wrote:
> > It does look like this:
> > 
> > BR2_EXTERNAL/
> > |`- board/
> > |    `- someboard/
> > |       |`- linux-myversion/
> > |       |    `- linux-0001-fix-something.patch
> > |       |`- busybox-1.21.1.config
> > |        `- post-build-script.sh
> > |`- configs/
> > |    `- someboard_defconfig
> >  `- package/
> >     |`- Config.in
> >     |`- Config.in.host
> >     |`- foo/
> >     |   |`- foo.mk
> >     |   |`- Config.in
> >     |    `- Config.in.host
> >     |`- bar/
> >     |   |`- bar.mk
> >     |    `- Config.in.host
> >      `- toto/
> >         |`- toto.mk
> >          `- Config.in
> > 
> > With BR2_EXTERNAL/package/Config.in sourcing all Config.in files from
> > BR2_EXTERNAL,
> > and BR2_EXTERNAL/package/Config.in.host sourcing all Config.in.host files
> > from BR2_EXTERNAL,
> > 
> > My point was only about sourcing BR2_EXTERNAL/package/Config.in.host under
> > the
> > "Host utilities" menu.
> 
> I perfectly understand what you mean, but I don't really like the idea
> of sourcing BR2_EXTERNAL/package/Config.in.host, because it means the
> user has to *always* create two Config.in files in its BR2_EXTERNAL
> hierarchy to just get started in using BR2_EXTERNAL.
> 
> So, the reason your comment is *entirely* related to the previous
> discussion is that in my previous proposal, I was including *ONE*
> top-level BR2_EXTERNAL/Config.in, and it was up to the user to then do
> whatever he wanted in this top-level Config.in file. We were not
> enforcing anything.
> 
> I was OK with enforcing the usage of BR2_EXTERNAL/package/Config.in,
> but not if we extend that to also enforce the usage (and existence) of
> BR2_EXTERNAL/package/Config.in.host.

At first, I would have sided with Samuel on that one, since it might
sound a bit ugly to have host packages appear in the target packages
menu.

Yes, we want to enforce the directory layout in BR2_EXTERNAL. But do we
want to enforce the menu structure. too?

In the end I think Thomas is right, but for different reasons: if we
eventually added package/Config.in.host. then we should do the same for
the bootloaders, too. And probably other menus, too. Which means the
user would have to provide all of these files, even empty ones.

Anyway, let's keep it simple for now. We can refine it later if needed.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Thu, 28 Nov 2013 23:21:15 +0100, Yann E. MORIN wrote:

> > I perfectly understand what you mean, but I don't really like the idea
> > of sourcing BR2_EXTERNAL/package/Config.in.host, because it means the
> > user has to *always* create two Config.in files in its BR2_EXTERNAL
> > hierarchy to just get started in using BR2_EXTERNAL.
> > 
> > So, the reason your comment is *entirely* related to the previous
> > discussion is that in my previous proposal, I was including *ONE*
> > top-level BR2_EXTERNAL/Config.in, and it was up to the user to then do
> > whatever he wanted in this top-level Config.in file. We were not
> > enforcing anything.
> > 
> > I was OK with enforcing the usage of BR2_EXTERNAL/package/Config.in,
> > but not if we extend that to also enforce the usage (and existence) of
> > BR2_EXTERNAL/package/Config.in.host.
> 
> At first, I would have sided with Samuel on that one, since it might
> sound a bit ugly to have host packages appear in the target packages
> menu.
> 
> Yes, we want to enforce the directory layout in BR2_EXTERNAL. But do we
> want to enforce the menu structure. too?
> 
> In the end I think Thomas is right, but for different reasons: if we
> eventually added package/Config.in.host. then we should do the same for
> the bootloaders, too. And probably other menus, too. Which means the
> user would have to provide all of these files, even empty ones.
> 
> Anyway, let's keep it simple for now. We can refine it later if needed.

In other words, you're suggestion to revert back to the principle of
the v2 in terms of .mk file and Config.in inclusion? (I.e include a
top-level .mk file and top-level Config.in, and let the user do what he
wants?).

I personally believe this is the easiest and most flexible solution. We
can always state in the manual that we strongly recommend to keep a
directory structure similar to the one used in Buildroot. But at least
the user can organize its top-level menu as he sees fit (maybe even
using it to add board-specific config options, which we don't want in
Buildroot itself, but which a user may want for some reason).

The main improvement of v3 is the usage of the .br-external hidden
file, and this remains useful, so I'm really fine about reverting the
other part of v3 to what was done in v2 :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCHv3 2/5] core: allow external Config.in/makefile code to be integrated

[Arnout Vandecappelle]

  Hi all,

  Sorry that I'm so late to join the discussion - I've been busy, and I 
felt this was something that I needed to think about a little.

On 29/11/13 09:38, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Thu, 28 Nov 2013 23:21:15 +0100, Yann E. MORIN wrote:
>
>>> I perfectly understand what you mean, but I don't really like the idea
>>> of sourcing BR2_EXTERNAL/package/Config.in.host, because it means the
>>> user has to *always* create two Config.in files in its BR2_EXTERNAL
>>> hierarchy to just get started in using BR2_EXTERNAL.
>>>
>>> So, the reason your comment is *entirely* related to the previous
>>> discussion is that in my previous proposal, I was including *ONE*
>>> top-level BR2_EXTERNAL/Config.in, and it was up to the user to then do
>>> whatever he wanted in this top-level Config.in file. We were not
>>> enforcing anything.
>>>
>>> I was OK with enforcing the usage of BR2_EXTERNAL/package/Config.in,
>>> but not if we extend that to also enforce the usage (and existence) of
>>> BR2_EXTERNAL/package/Config.in.host.
>>
>> At first, I would have sided with Samuel on that one, since it might
>> sound a bit ugly to have host packages appear in the target packages
>> menu.
>>
>> Yes, we want to enforce the directory layout in BR2_EXTERNAL. But do we
>> want to enforce the menu structure. too?
>>
>> In the end I think Thomas is right, but for different reasons: if we
>> eventually added package/Config.in.host. then we should do the same for
>> the bootloaders, too. And probably other menus, too. Which means the
>> user would have to provide all of these files, even empty ones.
>>
>> Anyway, let's keep it simple for now. We can refine it later if needed.
>
> In other words, you're suggestion to revert back to the principle of
> the v2 in terms of .mk file and Config.in inclusion? (I.e include a
> top-level .mk file and top-level Config.in, and let the user do what he
> wants?).
>
> I personally believe this is the easiest and most flexible solution. We
> can always state in the manual that we strongly recommend to keep a
> directory structure similar to the one used in Buildroot. But at least
> the user can organize its top-level menu as he sees fit (maybe even
> using it to add board-specific config options, which we don't want in
> Buildroot itself, but which a user may want for some reason).

  With the v2/v4 solution, in the most common case (i.e. the user adds 
some target packages and wants to follow the buildroot directory 
structure), he has to add two files as well: BR2_EXTERNAL/Config.in and 
BR2_EXTERNAL/package/Config.in. That was I think the driving reason why 
we decided on this approach in Edinburgh. I think we even mentioned the 
bootloaders, but I argued that a bootloader is not really different from 
a target package anyway.

  I'll admit, though, that the host packages do make a difference. There 
is no really nice solution there.

  A possibility would be to add another BR2_EXTERNAL_HOST variable, that 
is set by the Makefile to support/dummy-external if
BR2_EXTERNAL/package/Config.in.host doesn't exist. But that solution is 
so ugly that I don't want to see it :-)

  So I agree with reverting to the v2 solution.

  Regards,
  Arnout

>
> The main improvement of v3 is the usage of the .br-external hidden
> file, and this remains useful, so I'm really fine about reverting the
> other part of v3 to what was done in v2 :-)
>
> Best regards,
>
> Thomas
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCHv3 3/5] core: allow external defconfigs to be used

[Thomas Petazzoni]

This commit allows the user to store defconfigs in
$BR2_EXTERNAL/configs/. To achieve this:

 * It adds a new %_defconfig that looks in $BR2_EXTERNAL/configs/ for
   the corresponding defconfig file.

 * Updates the help target to also list external defconfigs.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Makefile | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Makefile b/Makefile
index 013b646..3d73cf2 100644
--- a/Makefile
+++ b/Makefile
@@ -747,6 +747,12 @@ defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN)
 
+ifeq ($(BR2_EXTERNAL_USED),y)
+%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(BR2_EXTERNAL)/configs/%_defconfig outputmakefile
+	@mkdir -p $(BUILD_DIR)/buildroot-config
+	@$(COMMON_CONFIG_ENV) $< --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN)
+endif
+
 savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 	@mkdir -p $(BUILD_DIR)/buildroot-config
 	@$(COMMON_CONFIG_ENV) $< \
@@ -855,8 +861,17 @@ endif
 	@echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
 	@echo '  make O=dir             - Locate all output files in "dir", including .config'
 	@echo
+	@echo 'Built-in configs'
+	@echo
 	@$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig))), \
 	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
+ifeq ($(BR2_EXTERNAL_USED),y)
+	@echo
+	@echo 'User-provided configs'
+	@echo
+	@$(foreach b, $(sort $(notdir $(wildcard $(BR2_EXTERNAL)/configs/*_defconfig))), \
+	  printf "  %-35s - Build for %s\\n" $(b) $(b:_defconfig=);)
+endif
 	@echo
 	@echo 'See docs/README, or generate the Buildroot manual for further details'
 	@echo
-- 
1.8.1.2

[Buildroot] [PATCHv3 4/5] docs/manual: add explanations about BR2_EXTERNAL

[Thomas Petazzoni]

This commit updates the manual to add details on how to use the
BR2_EXTERNAL feature.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 docs/manual/customize-outside-br.txt | 116 +++++++++++++++++++++++++++++++++++
 docs/manual/customize.txt            |   2 +
 2 files changed, 118 insertions(+)
 create mode 100644 docs/manual/customize-outside-br.txt

diff --git a/docs/manual/customize-outside-br.txt b/docs/manual/customize-outside-br.txt
new file mode 100644
index 0000000..3928885
--- /dev/null
+++ b/docs/manual/customize-outside-br.txt
@@ -0,0 +1,116 @@
+// -*- mode:doc -*- ;
+
+[[outside-br-custom]]
+Keeping customization outside Buildroot
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+While the Buildroot community recommends and encourages upstreaming to
+the official Buildroot version the packages and boards support that
+are written by developers, it is sometimes not possible or desirable,
+due to these packages or boards being highly specific or proprietary.
+
+In this case, Buildroot users are offered two choices:
+
+ * They can add their packages, board support and configuration files
+   directly within the Buildroot tree, and maintain them by using
+   branches in a version control system.
+
+ * They can use the +BR2_EXTERNAL+ mechanism, which allows to keep
+   package recipes, board support and configuration files outside of
+   the Buildroot tree, while still having them nicely integrated in
+   the build logic. The following paragraphs give details on how to
+   use +BR2_EXTERNAL+.
+
++BR2_EXTERNAL+ is an environment variable that one can use to point to
+a directory that contains Buildroot customizations. It can be passed
+to any Buildroot +make+ invocation, and when it is passed. It is
+automatically saved in the hidden +.br-external+ file in the output
+directory, so that it is not needed to pass +BR2_EXTERNAL+ at every
++make+ invocation. It can however be changed at any time by specifying
+a new value, and can be removed by passing an empty value. The
++BR2_EXTERNAL+ path can be either an absolute or a relative path, but
+if it's passed as a relative path, it is important to note that it is
+interpreted relatively to the main Buildroot source directory, not the
+Buildroot output directory.
+
+Some examples:
+
+-----
+ buildroot/ $ make BR2_EXTERNAL=../foobar menuconfig
+-----
+
+Starting from now on, external definitions fromt he +../company+
+directory will be used:
+
+-----
+ buildroot/ $ make
+ buildroot/ $ make legal-info
+-----
+
+We can switch to another external definitions directory at any time:
+
+-----
+ buildroot/ $ make BR2_EXTERNAL=../barfoo xconfig
+-----
+
+Or disable the usage of external definitions:
+
+-----
+ buildroot/ $ make BR2_EXTERNAL= xconfig
+-----
+
+This +BR2_EXTERNAL+ then allows three different things:
+
+ * One can store all the board-specific configuration files here, 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
+   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).  + To
+   achieve this, it is recommended but not mandatory, to store those
+   details in directories called +board/<boardname>/+ under
+   +BR2_EXTERNAL+.
+
+ * One can store package recipes (i.e +Config.in+ and
+   +<packagename>.mk+), or even custom configuration options and make
+   logic. Buildroot automatically includes
+   +BR2_EXTERNAL/package/Config.in+ to make it appear in the +Target
+   packages+ configuration submenu, and include all the
+   +BR2_EXTERNAL/package/*/*.mk+ with the rest of the makefile
+   logic. This allows to create package recipes in
+   +BR2_EXTERNAL/package+ in the exact same way as normal packages
+   created in the main Buildroot tree. However, a minor difference is
+   that if the package needs to reference a file stored within its
+   package directory, it should use
+   +$(BR2_EXTERNAL)/package/<pkgname>/<filename>+ instead of just
+   +package/<pkgname>/<filename>+.
+
+ * One can store Buildroot defconfigs in the +configs+ subdirectory of
+   +BR2_EXTERNAL+. Buildroot will automatically show them in the
+   output of +make help+ and allow them to be loaded with the normal
+   +make <name>_defconfig+ command.
+
+In the end, a typical +BR2_EXTERNAL+ directory organization would
+generally be:
+
+-----
+??? board/
+?   ??? <boardname>/
+?       ??? overlay/
+?           ??? etc/
+?               ??? <some file>
+??? configs/
+?   ??? <boardname>_defconfig
+??? package/
+    ??? Config.in
+    ??? package1/
+        ??? Config.in
+        ??? package1.mk
+    ??? package2/
+        ??? Config.in
+        ??? package2.mk
+------
diff --git a/docs/manual/customize.txt b/docs/manual/customize.txt
index 0456ef1..7e46fd8 100644
--- a/docs/manual/customize.txt
+++ b/docs/manual/customize.txt
@@ -17,3 +17,5 @@ include::customize-toolchain.txt[]
 include::customize-store.txt[]
 
 include::customize-packages.txt[]
+
+include::customize-outside-br.txt[]
-- 
1.8.1.2

[Buildroot] [PATCHv3 5/5] manual: fix manual generation with BR2_EXTERNAL support

[Thomas Petazzoni]

From: Samuel Martin <s.martin49@gmail.com>

Reported-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
Signed-off-by: Samuel Martin <s.martin49@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Ryan Barnett <rjbarnet@rockwellcollins.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 docs/manual/manual.mk         | 1 +
 support/scripts/kconfiglib.py | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
index aeafd10..570318f 100644
--- a/docs/manual/manual.mk
+++ b/docs/manual/manual.mk
@@ -1,6 +1,7 @@
 manual-update-lists: manual-check-dependencies-lists
 	$(Q)$(call MESSAGE,"Updating the manual lists...")
 	$(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \
+		BR2_EXTERNAL=$(TOPDIR)/support/dummy-external \
 		$(TOPDIR)/support/scripts/gen-manual-lists.py
 
 # we can't use suitable-host-package here because that's not available in
diff --git a/support/scripts/kconfiglib.py b/support/scripts/kconfiglib.py
index 0704cc0..e40947c 100644
--- a/support/scripts/kconfiglib.py
+++ b/support/scripts/kconfiglib.py
@@ -2074,7 +2074,7 @@ set_re   = re.compile(r"CONFIG_(\w+)=(.*)")
 unset_re = re.compile(r"# CONFIG_(\w+) is not set")
 
 # Regular expression for finding $-references to symbols in strings
-sym_ref_re = re.compile(r"\$[A-Za-z_]+")
+sym_ref_re = re.compile(r"\$[A-Za-z_][0-9A-Za-z_]*")
 
 # Integers representing symbol types
 UNKNOWN, BOOL, TRISTATE, STRING, HEX, INT = range(0, 6)
-- 
1.8.1.2