| Message ID | 1378646129-4167-3-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 09/08/2013 08:15:28 AM > This commit adds support for an environment variable named > BR2_EXTERNAL, which the user can point to a directory outside of > Buildroot that contains root filesystem overlays, kernel configuration > files, package recipes, defconfigs, etc. It allows users to keep their > specific Buildroot customizations outside of the Buildroot tree. > > BR2_EXTERNAL allows: > > (1) To use $(BR2_EXTERNAL) in all Buildroot configuration options > that take a path as argument. This allows to reference a root > filesystem overlay, a kernel configuration file, a Barebox > configuration file located in the BR2_EXTERNAL directory. > > (2) To store external package or makefile logic, since the > BR2_EXTERNAL/external.mk file is automatically included by the > Makefile logic, and the BR2_EXTERNAL/Config.in file is > automatically included, and appears in the top-level menu. The > typical usage would be to create a BR2_EXTERNAL/package/ > directory to contain package definitions. > > (3) To store defconfig files under BR2_EXTERNAL/configs/. They will > be visible in 'make help' and usable through 'make > <someboard>_defconfig'. > > In terms of implementation, it is relatively straightforward: > > * A BR2_EXTERNAL kconfig variable is added, which is set to the value > of the BR2_EXTERNAL environment variable. > > * The top-level Config.in file given to kconfig is no longer the main > Config.in in the Buildroot sources, but instead a toplevel.in file > generated in the output directory, which includes the top-level > Buildroot Config.in file and the BR2_EXTERNAL/Config.in file if > provided. Since is needed because the kconfig 'source' statement > errors out if the included file doesn't exist. I have written > patches that add support for an 'isource' statement in kconfig > (that silently ignores the inclusion if the pointed file doesn't > exist), but keeping feature patches against kconfig doesn't seem > like a good idea. Note that the "mainmenu" statement is part of > this generated file, because it must be the first statement seen in > the toplevel Config.in file passed to kconfig. > > * The BR2_EXTERNAL/external.mk makefile gets included. > > * The BR2_EXTERNAL environment variable gets passed in the > environment of kconfig and when executing the kconfiglib-based > Python script that updates the manual, so that the references to > the BR2_EXTERNAL variable within Config.in files can be resolved. > > * The 'make help' and 'make %_defconfig' targets are updated to take > into account the defconfig files stored under > BR2_EXTERNAL/configs/. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Reviewed-by: Ryan Barnett <rjbarnet@rockwellcollins.com> Ryan J Barnett / Software Engineer / Platform SW MS 137-157, 855 35th St NE, Cedar Rapids, IA, 52498-3161, US Phone: 319-263-3880 / VPN: 263-3880 rjbarnet@rockwellcollins.com www.rockwellcollins.com
Hi Thomas, It's funny how people can change their mind... I've put Tzu-Jung Lee in CC, who wrote something similar a while ago. On 08/09/13 15:15, Thomas Petazzoni wrote: > This commit adds support for an environment variable named > BR2_EXTERNAL, which the user can point to a directory outside of > Buildroot that contains root filesystem overlays, kernel configuration > files, package recipes, defconfigs, etc. It allows users to keep their > specific Buildroot customizations outside of the Buildroot tree. > > BR2_EXTERNAL allows: I would split this into three separate patches for the three separate features. I think (1) and (3) are a lot less controversial than (2). > > (1) To use $(BR2_EXTERNAL) in all Buildroot configuration options > that take a path as argument. This allows to reference a root > filesystem overlay, a kernel configuration file, a Barebox > configuration file located in the BR2_EXTERNAL directory. Note to list: this was already possible before, of course, the difference is that now this path will be stored in the .config. There is one small issue with this: I don't think that all the use cases support relative paths. Ideally, the Makefile should be smart enough to prepend $(TOPDIR) if it doesn't start with a /. > (2) To store external package or makefile logic, since the > BR2_EXTERNAL/external.mk file is automatically included by the > Makefile logic, and the BR2_EXTERNAL/Config.in file is > automatically included, and appears in the top-level menu. The > typical usage would be to create a BR2_EXTERNAL/package/ > directory to contain package definitions. I'm not really convinced by the principle. The external.mk is exactly the same as the local override .mk; the Config.in appears at the very end of the top-level menu. Instead, I think we should enforce the buildroot hierarchy in the external dir, i.e. source <path-to-external-dir>/package/Config.in at the top of package/Config.in, and -include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk)) in the top-level Makefile. Of course, that Config.in thing is a lot trickier because it's almost impossible to refer to the output directory from the Config.in. > (3) To store defconfig files under BR2_EXTERNAL/configs/. They will > be visible in 'make help' and usable through 'make > <someboard>_defconfig'. That's a great idea. > In terms of implementation, it is relatively straightforward: > > * A BR2_EXTERNAL kconfig variable is added, which is set to the value > of the BR2_EXTERNAL environment variable. > > * The top-level Config.in file given to kconfig is no longer the main > Config.in in the Buildroot sources, but instead a toplevel.in file > generated in the output directory, which includes the top-level > Buildroot Config.in file and the BR2_EXTERNAL/Config.in file if > provided. Since is needed because the kconfig 'source' statement > errors out if the included file doesn't exist. I have written > patches that add support for an 'isource' statement in kconfig > (that silently ignores the inclusion if the pointed file doesn't > exist), but keeping feature patches against kconfig doesn't seem > like a good idea. It took me a while to realize that that was _not_ the approach you had taken... > Note that the "mainmenu" statement is part of > this generated file, because it must be the first statement seen in > the toplevel Config.in file passed to kconfig. > > * The BR2_EXTERNAL/external.mk makefile gets included. > > * The BR2_EXTERNAL environment variable gets passed in the > environment of kconfig and when executing the kconfiglib-based > Python script that updates the manual, so that the references to > the BR2_EXTERNAL variable within Config.in files can be resolved. So this means that the external packages will appear in the package list in the manual? I'm not sure if that is what you typically want. > * The 'make help' and 'make %_defconfig' targets are updated to take > into account the defconfig files stored under > BR2_EXTERNAL/configs/. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > Config.in | 6 ++++-- > Makefile | 29 +++++++++++++++++++++++++---- > docs/manual/manual.mk | 2 +- > 3 files changed, 30 insertions(+), 7 deletions(-) > > diff --git a/Config.in b/Config.in > index cb246a4..7e253c6 100644 > --- a/Config.in > +++ b/Config.in > @@ -1,7 +1,5 @@ > # > > -mainmenu "Buildroot $BR2_VERSION Configuration" > - > config BR2_HAVE_DOT_CONFIG > bool > default y > @@ -14,6 +12,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 fc55b87..1014399 100644 > --- a/Makefile > +++ b/Makefile > @@ -47,7 +47,6 @@ export HOSTARCH := $(shell uname -m | \ > > # absolute path > TOPDIR:=$(shell pwd) > -CONFIG_CONFIG_IN=Config.in > CONFIG=support/kconfig > DATE:=$(shell date +%Y%m%d) > > @@ -339,6 +338,10 @@ include boot/common.mk > include linux/linux.mk > include system/system.mk > > +ifneq ($(BR2_EXTERNAL),) > +include $(BR2_EXTERNAL)/external.mk > +endif > + > TARGETS+=target-finalize > > ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > @@ -622,6 +625,18 @@ $(BUILD_DIR)/buildroot-config/%onf: > mkdir -p $(@D)/lxdialog > $(MAKE) CC="$(HOSTCC_NOCCACHE)" HOSTCC="$(HOSTCC_NOCCACHE)" obj=$(@D) -C $(CONFIG) -f Makefile.br $(@F) > > +CONFIG_CONFIG_IN=$(BUILD_DIR)/buildroot-config/toplevel.in > + > +# This is intentionally a virtual target so that the file gets > +# regenerated everytime this target is invoked. > +toplevelin-generate: I guess you mean phony target. You should also add .PHONY: toplevelin-generate for the (unlikely) case that someone creates a file with this name. BTW I don't like the target's name. generate-config-dot-in maybe? > + mkdir -p $(dir $(CONFIG_CONFIG_IN)) > + echo "mainmenu \"Buildroot $$BR2_VERSION Configuration\"" > $(CONFIG_CONFIG_IN) > + echo "source \"Config.in\"" >> $(CONFIG_CONFIG_IN) > +ifneq ($(BR2_EXTERNAL),) > + echo "source \"$$BR2_EXTERNAL/Config.in\"" >> $(CONFIG_CONFIG_IN) $(BR2_EXTERNAL) would be a lot more readable than $$BR2_EXTERNAL IMHO. Then you can also put it in single quotes and remove the \. > +endif > + > DEFCONFIG = $(call qstrip,$(BR2_DEFCONFIG)) > > # We don't want to fully expand BR2_DEFCONFIG here, so Kconfig will > @@ -631,10 +646,12 @@ 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) There is one tricky issue: cd ~/src/myproject/output make -C ~/src/buildroot O=$PWD menuconfig Hack hack hack Hm, I need an external dir... BR2_EXTERNAL=.. make menuconfig => BR2_EXTERNAL will point to ~/src instead of ~/src/myproject I.e., it doesn't work well for relative paths. I don't know if there is an elegant way to solve that. > COMMON_CONFIG_DEPS = \ > - outputmakefile > + outputmakefile \ > + toplevelin-generate > > xconfig: $(BUILD_DIR)/buildroot-config/qconf $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @@ -718,6 +735,10 @@ defconfig: $(BUILD_DIR)/buildroot-config/conf $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @$(COMMON_CONFIG_ENV) $< --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN) > > +%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(BR2_EXTERNAL)/configs/%_defconfig $(COMMON_CONFIG_DEPS) > + @mkdir -p $(BUILD_DIR)/buildroot-config > + @$(COMMON_CONFIG_ENV) $< --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN) This should be in a ifneq ($(BR2_EXTERNAL),) Regards, Arnout > + > savedefconfig: $(BUILD_DIR)/buildroot-config/conf $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @$(COMMON_CONFIG_ENV) $< \ > @@ -829,7 +850,7 @@ 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 > - @$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig))), \ > + @$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig $(BR2_EXTERNAL)/configs/*_defconfig))), \ > printf " %-35s - Build for %s\\n" $(b) $(b:_defconfig=);) > @echo > @echo 'See docs/README, or generate the Buildroot manual for further details' > diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk > index 4906bc8..8e0ab30 100644 > --- a/docs/manual/manual.mk > +++ b/docs/manual/manual.mk > @@ -1,6 +1,6 @@ > manual-update-lists: > $(Q)$(call MESSAGE,"Updating the manual lists...") > - $(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ > + $(Q)BR2_DEFCONFIG="" BR2_EXTERNAL="$(BR2_EXTERNAL)" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ > $(TOPDIR)/support/scripts/gen-manual-lists.py > > ################################################################################ >
While utilizing this patch, I found the following that should be fixed Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 09/12/2013 04:39 PM: > diff --git a/Makefile b/Makefile > index fc55b87..1014399 100644 > --- a/Makefile > +++ b/Makefile > @@ -47,7 +47,6 @@ export HOSTARCH := $(shell uname -m | \ > > # absolute path > TOPDIR:=$(shell pwd) > -CONFIG_CONFIG_IN=Config.in > CONFIG=support/kconfig > DATE:=$(shell date +%Y%m%d) > > @@ -339,6 +338,10 @@ include boot/common.mk > include linux/linux.mk > include system/system.mk > > +ifneq ($(BR2_EXTERNAL),) > +include $(BR2_EXTERNAL)/external.mk > +endif > + > TARGETS+=target-finalize > > ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) > @@ -622,6 +625,18 @@ $(BUILD_DIR)/buildroot-config/%onf: > mkdir -p $(@D)/lxdialog > $(MAKE) CC="$(HOSTCC_NOCCACHE)" HOSTCC="$(HOSTCC_NOCCACHE)" > obj=$(@D) -C $(CONFIG) -f Makefile.br $(@F) > > +CONFIG_CONFIG_IN=$(BUILD_DIR)/buildroot-config/toplevel.in > + > +# This is intentionally a virtual target so that the file gets > +# regenerated everytime this target is invoked. > +toplevelin-generate: > + mkdir -p $(dir $(CONFIG_CONFIG_IN)) > + echo "mainmenu \"Buildroot $$BR2_VERSION Configuration\"" > > $(CONFIG_CONFIG_IN) > + echo "source \"Config.in\"" >> $(CONFIG_CONFIG_IN) > +ifneq ($(BR2_EXTERNAL),) > + echo "source \"$$BR2_EXTERNAL/Config.in\"" >> $(CONFIG_CONFIG_IN) > +endif > + > DEFCONFIG = $(call qstrip,$(BR2_DEFCONFIG)) Each command above should have a @ symbol since the commands print out every time. As follows: +toplevelin-generate: + @mkdir -p $(dir $(CONFIG_CONFIG_IN)) + @echo "mainmenu \"Buildroot $$BR2_VERSION Configuration\"" > $(CONFIG_CONFIG_IN) + @echo "source \"Config.in\"" >> $(CONFIG_CONFIG_IN) +ifneq ($(BR2_EXTERNAL),) + @echo "source \"$$BR2_EXTERNAL/Config.in\"" >> $(CONFIG_CONFIG_IN) +endif + > # We don't want to fully expand BR2_DEFCONFIG here, so Kconfig will > @@ -631,10 +646,12 @@ 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) > > COMMON_CONFIG_DEPS = \ > - outputmakefile > + outputmakefile \ > + toplevelin-generate > > xconfig: $(BUILD_DIR)/buildroot-config/qconf $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @@ -718,6 +735,10 @@ defconfig: $(BUILD_DIR)/buildroot-config/conf > $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @$(COMMON_CONFIG_ENV) $< --defconfig=$(TOPDIR)/configs/$@ > $(CONFIG_CONFIG_IN) > > +%_defconfig: $(BUILD_DIR)/buildroot-config/conf > $(BR2_EXTERNAL)/configs/%_defconfig $(COMMON_CONFIG_DEPS) > + @mkdir -p $(BUILD_DIR)/buildroot-config > + @$(COMMON_CONFIG_ENV) $< > --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN) > + > savedefconfig: $(BUILD_DIR)/buildroot-config/conf $(COMMON_CONFIG_DEPS) > @mkdir -p $(BUILD_DIR)/buildroot-config > @$(COMMON_CONFIG_ENV) $< \ > @@ -829,7 +850,7 @@ 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 > - @$(foreach b, $(sort $(notdir $(wildcard > $(TOPDIR)/configs/*_defconfig))), \ > + @$(foreach b, $(sort $(notdir $(wildcard > $(TOPDIR)/configs/*_defconfig $(BR2_EXTERNAL)/configs/*_defconfig))), > \ > printf " %-35s - Build for %s\\n" $(b) $(b:_defconfig=);) > @echo > @echo 'See docs/README, or generate the Buildroot manual for > further details' > diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk > index 4906bc8..8e0ab30 100644 > --- a/docs/manual/manual.mk > +++ b/docs/manual/manual.mk > @@ -1,6 +1,6 @@ > manual-update-lists: > $(Q)$(call MESSAGE,"Updating the manual lists...") > - $(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ > + $(Q)BR2_DEFCONFIG="" BR2_EXTERNAL="$(BR2_EXTERNAL)" > TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ > $(TOPDIR)/support/scripts/gen-manual-lists.py > > > ################################################################################ > -- > 1.8.1.2 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Dear Arnout Vandecappelle, On Thu, 12 Sep 2013 23:04:13 +0200, Arnout Vandecappelle wrote: > It's funny how people can change their mind... Isn't it? In french, we have an expression that more or less says: "only idiots don't change their mind" :-) > On 08/09/13 15:15, Thomas Petazzoni wrote: > > This commit adds support for an environment variable named > > BR2_EXTERNAL, which the user can point to a directory outside of > > Buildroot that contains root filesystem overlays, kernel configuration > > files, package recipes, defconfigs, etc. It allows users to keep their > > specific Buildroot customizations outside of the Buildroot tree. > > > > BR2_EXTERNAL allows: > > I would split this into three separate patches for the three separate > features. I think (1) and (3) are a lot less controversial than (2). Hum, right, I might be splitting things, but I believe than (1) and (3) without (2) doesn't make much sense. > > (1) To use $(BR2_EXTERNAL) in all Buildroot configuration options > > that take a path as argument. This allows to reference a root > > filesystem overlay, a kernel configuration file, a Barebox > > configuration file located in the BR2_EXTERNAL directory. > > Note to list: this was already possible before, of course, the > difference is that now this path will be stored in the .config. I'm not sure why you think it's going to be stored in the .config. The 'config BR2_EXTERNAL' option is here only to "forward" the environment variable as a kconfig value, and it doesn't seem to be stored in the .config file, as far as I can see: $ make BR2_EXTERNAL=../company/ menuconfig $ grep BR2_EXTERNAL .config $ > There is one small issue with this: I don't think that all the use > cases support relative paths. Ideally, the Makefile should be smart > enough to prepend $(TOPDIR) if it doesn't start with a /. So far the cases I've tested did support relative paths, but I agree that turning it into an absolute path within the Makefile is probably safer. > > (2) To store external package or makefile logic, since the > > BR2_EXTERNAL/external.mk file is automatically included by the > > Makefile logic, and the BR2_EXTERNAL/Config.in file is > > automatically included, and appears in the top-level menu. The > > typical usage would be to create a BR2_EXTERNAL/package/ > > directory to contain package definitions. > > I'm not really convinced by the principle. The external.mk is exactly > the same as the local override .mk; the Config.in appears at the very end > of the top-level menu. Instead, I think we should enforce the buildroot > hierarchy in the external dir, i.e. > > source <path-to-external-dir>/package/Config.in > > at the top of package/Config.in, and > > -include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk)) > > in the top-level Makefile. This is what I had originally done, but it seemed to me that the external stuff may wish to add other Kconfig options than just packages. That said, I've done this BR2_EXTERNAL stuff mainly for packages, so if people think that we should restrict this to packages, I'm fine with that. > Of course, that Config.in thing is a lot trickier because it's almost > impossible to refer to the output directory from the Config.in. I'm not sure to follow you on this one. > > * The top-level Config.in file given to kconfig is no longer the main > > Config.in in the Buildroot sources, but instead a toplevel.in file > > generated in the output directory, which includes the top-level > > Buildroot Config.in file and the BR2_EXTERNAL/Config.in file if > > provided. Since is needed because the kconfig 'source' statement > > errors out if the included file doesn't exist. I have written > > patches that add support for an 'isource' statement in kconfig > > (that silently ignores the inclusion if the pointed file doesn't > > exist), but keeping feature patches against kconfig doesn't seem > > like a good idea. > > It took me a while to realize that that was _not_ the approach you had > taken... Yeah, sorry if the wording was confusing. I initially experimented by doing a kconfig modification, and then realized it could be done without changing kconfig at all, which is obviously nicer. > > * The BR2_EXTERNAL environment variable gets passed in the > > environment of kconfig and when executing the kconfiglib-based > > Python script that updates the manual, so that the references to > > the BR2_EXTERNAL variable within Config.in files can be resolved. > > So this means that the external packages will appear in the package > list in the manual? I'm not sure if that is what you typically want. I remember having an issue with the manual generation script, but it might have been caused by the earlier kconfig-based implementation. I agree with quite certainly don't want to have the BR2_EXTERNAL packages listed in the manual. > I guess you mean phony target. You should also add > .PHONY: toplevelin-generate > for the (unlikely) case that someone creates a file with this name. Ok. > BTW I don't like the target's name. generate-config-dot-in maybe? Ok. > > + mkdir -p $(dir $(CONFIG_CONFIG_IN)) > > + echo "mainmenu \"Buildroot $$BR2_VERSION Configuration\"" > $(CONFIG_CONFIG_IN) > > + echo "source \"Config.in\"" >> $(CONFIG_CONFIG_IN) > > +ifneq ($(BR2_EXTERNAL),) > > + echo "source \"$$BR2_EXTERNAL/Config.in\"" >> $(CONFIG_CONFIG_IN) > > $(BR2_EXTERNAL) would be a lot more readable than $$BR2_EXTERNAL IMHO. > Then you can also put it in single quotes and remove the \. Right. > There is one tricky issue: > > cd ~/src/myproject/output > make -C ~/src/buildroot O=$PWD menuconfig > Hack hack hack > Hm, I need an external dir... > BR2_EXTERNAL=.. make menuconfig > => BR2_EXTERNAL will point to ~/src instead of ~/src/myproject > > I.e., it doesn't work well for relative paths. > > I don't know if there is an elegant way to solve that. I'll have a look into this. > This should be in a > ifneq ($(BR2_EXTERNAL),) Right. Thanks for the review! Thomas
Hi Thomas, Arnout, and all, It's been a while... Arnout has a good memory :-) And yes we have been using similar approach since then. Thomas's patch is comprehensive, and we'd like to switch to the official one whenever possible. On Fri, Sep 13, 2013 at 11:48 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Arnout Vandecappelle, > > On Thu, 12 Sep 2013 23:04:13 +0200, Arnout Vandecappelle wrote: > >> There is one small issue with this: I don't think that all the use >> cases support relative paths. Ideally, the Makefile should be smart >> enough to prepend $(TOPDIR) if it doesn't start with a /. > > So far the cases I've tested did support relative paths, but I agree > that turning it into an absolute path within the Makefile is probably > safer. >> > (2) To store external package or makefile logic, since the >> > BR2_EXTERNAL/external.mk file is automatically included by the >> > Makefile logic, and the BR2_EXTERNAL/Config.in file is >> > automatically included, and appears in the top-level menu. The >> > typical usage would be to create a BR2_EXTERNAL/package/ >> > directory to contain package definitions. >> >> I'm not really convinced by the principle. The external.mk is exactly >> the same as the local override .mk; the Config.in appears at the very end >> of the top-level menu. Instead, I think we should enforce the buildroot >> hierarchy in the external dir, i.e. >> >> source <path-to-external-dir>/package/Config.in >> >> at the top of package/Config.in, and >> >> -include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk)) >> >> in the top-level Makefile. > > This is what I had originally done, but it seemed to me that the > external stuff may wish to add other Kconfig options than just > packages. That said, I've done this BR2_EXTERNAL stuff mainly for > packages, so if people think that we should restrict this to packages, > I'm fine with that. > We do want to support the absolute paths for both Config.in and *.mk. As a "buildroot overlay", we have similar structures in overlay/ like the buildroot/, such as configs, fs, packages, etc. This requires the buildroot to support only only the *.mk and Config.in in the top level over overlay/, but also those in their sub directories. The *.mk case may be get rid of with Arnount's suggestion. But I'm wondering could we solve the Config.in case in a less trickier way? The following is the approach I've been used locally. The overlay/Makefile plays a trick, which allows us to work under overlay/ as we work under buildroot/ Suppose we have the following configs, overlay/configs/foo_defconfig Our build sequence will be: $ cd overlay $ make O=../output/foo foo_defconfig $ make O=../output/foo With either Thomas's patch or our local patch, other targets supported by buildroot, such as menuconfig, help, etc also works under overlay/ as they do under buildroot/. ##### overlay/Makefile ####### EXTERNAL_OVERLAY ?= $(realpath $(dir $(lastword $(MAKEFILE_LIST)))) export EXTERNAL_OVERLAY BUILDROOT_TOP = ../buildroot all: ln -fs ../overlay $(BUILDROOT_TOP) make -C $(BUILDROOT_TOP) %: ln -fs ../overlay $(BUILDROOT_TOP) make -C $(BUILDROOT_TOP) $(MAKECMDGOALS) ##### overlay/local.mk ####### -include overlay/package/*/*.mk -include overlay/package/*/*/*.mk -include overlay/cust/*/*.mk -include overlay/cust/*/*/*.mk -include overlay/linux/*.mk ###### overlay/Config.in ###### source "overlay/package/Config.in" source "overlay/cust/Config.in" ###### overlay/package/Config.in ###### source "overlay/package/xxx/Config.in" Thanks. Roy
Dear Tzu-Jung Lee, On Fri, 13 Sep 2013 14:43:11 +0800, Tzu-Jung Lee wrote: > >> I'm not really convinced by the principle. The external.mk is exactly > >> the same as the local override .mk; the Config.in appears at the very end > >> of the top-level menu. Instead, I think we should enforce the buildroot > >> hierarchy in the external dir, i.e. > >> > >> source <path-to-external-dir>/package/Config.in > >> > >> at the top of package/Config.in, and > >> > >> -include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk)) > >> > >> in the top-level Makefile. > > > > This is what I had originally done, but it seemed to me that the > > external stuff may wish to add other Kconfig options than just > > packages. That said, I've done this BR2_EXTERNAL stuff mainly for > > packages, so if people think that we should restrict this to packages, > > I'm fine with that. > > > > We do want to support the absolute paths for both Config.in and *.mk. > > As a "buildroot overlay", we have similar structures in overlay/ like > the buildroot/, > such as configs, fs, packages, etc. > > This requires the buildroot to support only only the *.mk and > Config.in in the top > level over overlay/, but also those in their sub directories. > > The *.mk case may be get rid of with Arnount's suggestion. > But I'm wondering could we solve the Config.in case in a less trickier way? I am not sure what you mean here. What do you mean by "support the absolute paths for both Config.in and *.mk" ? > The overlay/Makefile plays a trick, which allows us to work under overlay/ as we > work under buildroot/ > > Suppose we have the following configs, > > overlay/configs/foo_defconfig > > Our build sequence will be: > > $ cd overlay > $ make O=../output/foo foo_defconfig > $ make O=../output/foo > > With either Thomas's patch or our local patch, other targets supported by > buildroot, such as menuconfig, help, etc also works under overlay/ as they do > under buildroot/. > > ##### overlay/Makefile ####### > EXTERNAL_OVERLAY ?= $(realpath $(dir $(lastword $(MAKEFILE_LIST)))) > export EXTERNAL_OVERLAY > > BUILDROOT_TOP = ../buildroot > > all: > ln -fs ../overlay $(BUILDROOT_TOP) > make -C $(BUILDROOT_TOP) > > %: > ln -fs ../overlay $(BUILDROOT_TOP) > make -C $(BUILDROOT_TOP) $(MAKECMDGOALS) > > ##### overlay/local.mk ####### > -include overlay/package/*/*.mk > -include overlay/package/*/*/*.mk > > -include overlay/cust/*/*.mk > -include overlay/cust/*/*/*.mk > > -include overlay/linux/*.mk > > ###### overlay/Config.in ###### > source "overlay/package/Config.in" > source "overlay/cust/Config.in" > > ###### overlay/package/Config.in ###### > source "overlay/package/xxx/Config.in" This is what BR2_EXTERNAL allows. Of course, doing a BR2_EXTERNAL/Makefile that allows to run Buildroot from the external (overlay) directory is something left for the person who creates the overlay, I believe. Best regards, Thomas
Hi Thomas, On Fri, Sep 13, 2013 at 3:10 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Tzu-Jung Lee, > > On Fri, 13 Sep 2013 14:43:11 +0800, Tzu-Jung Lee wrote: > >> >> I'm not really convinced by the principle. The external.mk is exactly >> >> the same as the local override .mk; the Config.in appears at the very end >> >> of the top-level menu. Instead, I think we should enforce the buildroot >> >> hierarchy in the external dir, i.e. >> >> >> >> source <path-to-external-dir>/package/Config.in >> >> >> >> at the top of package/Config.in, and >> >> >> >> -include $(sort $(wildcard $(BR2_EXTERNAL)/package/*/*.mk)) >> >> >> >> in the top-level Makefile. >> > >> > This is what I had originally done, but it seemed to me that the >> > external stuff may wish to add other Kconfig options than just >> > packages. That said, I've done this BR2_EXTERNAL stuff mainly for >> > packages, so if people think that we should restrict this to packages, >> > I'm fine with that. >> > >> >> We do want to support the absolute paths for both Config.in and *.mk. >> >> As a "buildroot overlay", we have similar structures in overlay/ like >> the buildroot/, >> such as configs, fs, packages, etc. >> >> This requires the buildroot to support only only the *.mk and >> Config.in in the top >> level over overlay/, but also those in their sub directories. >> >> The *.mk case may be get rid of with Arnount's suggestion. >> But I'm wondering could we solve the Config.in case in a less trickier way? > > I am not sure what you mean here. What do you mean by "support the > absolute paths for both Config.in and *.mk" ? > >> The overlay/Makefile plays a trick, which allows us to work under overlay/ as we >> work under buildroot/ >> >> Suppose we have the following configs, >> >> overlay/configs/foo_defconfig >> >> Our build sequence will be: >> >> $ cd overlay >> $ make O=../output/foo foo_defconfig >> $ make O=../output/foo >> >> With either Thomas's patch or our local patch, other targets supported by >> buildroot, such as menuconfig, help, etc also works under overlay/ as they do >> under buildroot/. >> >> ##### overlay/Makefile ####### >> EXTERNAL_OVERLAY ?= $(realpath $(dir $(lastword $(MAKEFILE_LIST)))) >> export EXTERNAL_OVERLAY >> >> BUILDROOT_TOP = ../buildroot >> >> all: >> ln -fs ../overlay $(BUILDROOT_TOP) >> make -C $(BUILDROOT_TOP) >> >> %: >> ln -fs ../overlay $(BUILDROOT_TOP) >> make -C $(BUILDROOT_TOP) $(MAKECMDGOALS) >> >> ##### overlay/local.mk ####### >> -include overlay/package/*/*.mk >> -include overlay/package/*/*/*.mk >> >> -include overlay/cust/*/*.mk >> -include overlay/cust/*/*/*.mk >> >> -include overlay/linux/*.mk >> >> ###### overlay/Config.in ###### >> source "overlay/package/Config.in" >> source "overlay/cust/Config.in" >> >> ###### overlay/package/Config.in ###### >> source "overlay/package/xxx/Config.in" > > This is what BR2_EXTERNAL allows. Of course, doing a > BR2_EXTERNAL/Makefile that allows to run Buildroot from the external > (overlay) directory is something left for the person who creates the > overlay, I believe. By the absolute path, I mean the paths of sub Config.in included by the BR2_EXTERNAL/Config.in, not the path of itself. For example, suppose we have private packages in the overlay overlay/Config.in overlay/packages/foo/Config.in overlay/packages/bar/Config.in The overlay/Config.in can't include them without support of absolute paths source "overlay/packages/foo/Config.in" source "overlay/packages/bar/Config.in" In ths case, Buildroot tries to include them from the myproj/buildroot/overlay/packages/foo/Config.in instead of myproj/overlay/packages/foo/Config.in Thanks. Roy
diff --git a/Config.in b/Config.in index cb246a4..7e253c6 100644 --- a/Config.in +++ b/Config.in @@ -1,7 +1,5 @@ # -mainmenu "Buildroot $BR2_VERSION Configuration" - config BR2_HAVE_DOT_CONFIG bool default y @@ -14,6 +12,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 fc55b87..1014399 100644 --- a/Makefile +++ b/Makefile @@ -47,7 +47,6 @@ export HOSTARCH := $(shell uname -m | \ # absolute path TOPDIR:=$(shell pwd) -CONFIG_CONFIG_IN=Config.in CONFIG=support/kconfig DATE:=$(shell date +%Y%m%d) @@ -339,6 +338,10 @@ include boot/common.mk include linux/linux.mk include system/system.mk +ifneq ($(BR2_EXTERNAL),) +include $(BR2_EXTERNAL)/external.mk +endif + TARGETS+=target-finalize ifeq ($(BR2_ENABLE_LOCALE_PURGE),y) @@ -622,6 +625,18 @@ $(BUILD_DIR)/buildroot-config/%onf: mkdir -p $(@D)/lxdialog $(MAKE) CC="$(HOSTCC_NOCCACHE)" HOSTCC="$(HOSTCC_NOCCACHE)" obj=$(@D) -C $(CONFIG) -f Makefile.br $(@F) +CONFIG_CONFIG_IN=$(BUILD_DIR)/buildroot-config/toplevel.in + +# This is intentionally a virtual target so that the file gets +# regenerated everytime this target is invoked. +toplevelin-generate: + mkdir -p $(dir $(CONFIG_CONFIG_IN)) + echo "mainmenu \"Buildroot $$BR2_VERSION Configuration\"" > $(CONFIG_CONFIG_IN) + echo "source \"Config.in\"" >> $(CONFIG_CONFIG_IN) +ifneq ($(BR2_EXTERNAL),) + echo "source \"$$BR2_EXTERNAL/Config.in\"" >> $(CONFIG_CONFIG_IN) +endif + DEFCONFIG = $(call qstrip,$(BR2_DEFCONFIG)) # We don't want to fully expand BR2_DEFCONFIG here, so Kconfig will @@ -631,10 +646,12 @@ 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) COMMON_CONFIG_DEPS = \ - outputmakefile + outputmakefile \ + toplevelin-generate xconfig: $(BUILD_DIR)/buildroot-config/qconf $(COMMON_CONFIG_DEPS) @mkdir -p $(BUILD_DIR)/buildroot-config @@ -718,6 +735,10 @@ defconfig: $(BUILD_DIR)/buildroot-config/conf $(COMMON_CONFIG_DEPS) @mkdir -p $(BUILD_DIR)/buildroot-config @$(COMMON_CONFIG_ENV) $< --defconfig=$(TOPDIR)/configs/$@ $(CONFIG_CONFIG_IN) +%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(BR2_EXTERNAL)/configs/%_defconfig $(COMMON_CONFIG_DEPS) + @mkdir -p $(BUILD_DIR)/buildroot-config + @$(COMMON_CONFIG_ENV) $< --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN) + savedefconfig: $(BUILD_DIR)/buildroot-config/conf $(COMMON_CONFIG_DEPS) @mkdir -p $(BUILD_DIR)/buildroot-config @$(COMMON_CONFIG_ENV) $< \ @@ -829,7 +850,7 @@ 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 - @$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig))), \ + @$(foreach b, $(sort $(notdir $(wildcard $(TOPDIR)/configs/*_defconfig $(BR2_EXTERNAL)/configs/*_defconfig))), \ printf " %-35s - Build for %s\\n" $(b) $(b:_defconfig=);) @echo @echo 'See docs/README, or generate the Buildroot manual for further details' diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk index 4906bc8..8e0ab30 100644 --- a/docs/manual/manual.mk +++ b/docs/manual/manual.mk @@ -1,6 +1,6 @@ manual-update-lists: $(Q)$(call MESSAGE,"Updating the manual lists...") - $(Q)BR2_DEFCONFIG="" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ + $(Q)BR2_DEFCONFIG="" BR2_EXTERNAL="$(BR2_EXTERNAL)" TOPDIR=$(TOPDIR) O=$(O)/docs/manual/.build \ $(TOPDIR)/support/scripts/gen-manual-lists.py ################################################################################
This commit adds support for an environment variable named BR2_EXTERNAL, which the user can point to a directory outside of Buildroot that contains root filesystem overlays, kernel configuration files, package recipes, defconfigs, etc. It allows users to keep their specific Buildroot customizations outside of the Buildroot tree. BR2_EXTERNAL allows: (1) To use $(BR2_EXTERNAL) in all Buildroot configuration options that take a path as argument. This allows to reference a root filesystem overlay, a kernel configuration file, a Barebox configuration file located in the BR2_EXTERNAL directory. (2) To store external package or makefile logic, since the BR2_EXTERNAL/external.mk file is automatically included by the Makefile logic, and the BR2_EXTERNAL/Config.in file is automatically included, and appears in the top-level menu. The typical usage would be to create a BR2_EXTERNAL/package/ directory to contain package definitions. (3) To store defconfig files under BR2_EXTERNAL/configs/. They will be visible in 'make help' and usable through 'make <someboard>_defconfig'. In terms of implementation, it is relatively straightforward: * A BR2_EXTERNAL kconfig variable is added, which is set to the value of the BR2_EXTERNAL environment variable. * The top-level Config.in file given to kconfig is no longer the main Config.in in the Buildroot sources, but instead a toplevel.in file generated in the output directory, which includes the top-level Buildroot Config.in file and the BR2_EXTERNAL/Config.in file if provided. Since is needed because the kconfig 'source' statement errors out if the included file doesn't exist. I have written patches that add support for an 'isource' statement in kconfig (that silently ignores the inclusion if the pointed file doesn't exist), but keeping feature patches against kconfig doesn't seem like a good idea. Note that the "mainmenu" statement is part of this generated file, because it must be the first statement seen in the toplevel Config.in file passed to kconfig. * The BR2_EXTERNAL/external.mk makefile gets included. * The BR2_EXTERNAL environment variable gets passed in the environment of kconfig and when executing the kconfiglib-based Python script that updates the manual, so that the references to the BR2_EXTERNAL variable within Config.in files can be resolved. * The 'make help' and 'make %_defconfig' targets are updated to take into account the defconfig files stored under BR2_EXTERNAL/configs/. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- Config.in | 6 ++++-- Makefile | 29 +++++++++++++++++++++++++---- docs/manual/manual.mk | 2 +- 3 files changed, 30 insertions(+), 7 deletions(-)