| Message ID | 1460734834-32123-1-git-send-email-patrick@stwcx.xyz |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Apr 15, 2016 at 8:40 AM, Patrick Williams <patrick@stwcx.xyz> wrote: > From: Elizabeth Liner <eliner@us.ibm.com> > > This change will add in the custom _defconfig's to the buildroot > makefile so that when a _defconfig is in the custom directory it is > recognized correctly, and is able to be used. > > Signed-off-by: Elizabeth Liner <eliner@us.ibm.com> > Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Hi, What is the usecase? Isn't a defconfig in br2_external/configs already custom? It seems the affect of this patch is just to add another subdirectory in $BR2_EXTERNAL named custom? I think the philosophy is to keep buildroot as simple as possible, but no more than that. Regards, Steve
Hello, On Fri, 15 Apr 2016 10:40:34 -0500, Patrick Williams wrote: > From: Elizabeth Liner <eliner@us.ibm.com> > > This change will add in the custom _defconfig's to the buildroot > makefile so that when a _defconfig is in the custom directory it is > recognized correctly, and is able to be used. > > Signed-off-by: Elizabeth Liner <eliner@us.ibm.com> > Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Like Steve replied, I don't see the usefulness for this patch. We already allow custom configurations to be stored in $(BR2_EXTERNAL)/configs/. Best regards, Thomas
On Sun, Apr 17, 2016 at 10:31:43PM +0200, Thomas Petazzoni wrote: > Like Steve replied, I don't see the usefulness for this patch. We > already allow custom configurations to be stored in > $(BR2_EXTERNAL)/configs/. > Thomas / Steve, Thank you for the reply. We agree this is probably not the best approach for this in the broader sense of buildroot. Let me explain what we are trying to accomplish and what we have tried to do thus far. This patchset originated from work we are doing on the OpenPower firmware. We have an existing BR2_EXTERNAL tree, (https://github.com/open-power/op-build), which has a collection of packages specific to OpenPower. You will find a openpower/configs tree there that contains the defconfigs for the publicly supported systems. Since machines are often developed under confidentiality we are attempting to create a "second level" BR2_EXTERNAL. This is the openpower/custom tree and where the 'custom' came from in this patchset. Our intention with that is to give developers a location to place machines, packages, and patches currently under NDAs while they are being developed. By having them in a separate tree, the developers can continuously rebase against our master branch and have less of a issue with merge conflicts, since we promise not to place anything in the custom tree ourselves. We already utilize BR2_GLOBAL_PATCH_DIR and include custom/*.mk to allow patches and additional packages to be placed under the custom tree. The only piece that is missing is the defconfigs; hence this first attempt at a patch. We would like to be able to do the following: 1. Have support for a defconfig location other than $(BR2_EXTERNAL)/configs/. We have the proposed patchset, which is too specific due to the use of 'custom', and we have a local hack where we put the same recipe into $(BR2_EXTERNAL)/docs/custom/custom.mk. Other than docs/*/*.mk, there is no include in buildroot/Makefile into the BR2_EXTERNAL tree when invoked without an existing .config. Would there be opposition to a new -include in buildroot/Makefile outside of the large $(if BR2_HAVE_DOT_CONFIG... block into the BR2_EXTERNAL tree? This would give us a hook point to add the %_defconfig recipe found in this patchset. Alternatively, or additionally, we could create a BR2_EXTERNAL_CONFIGS variable as a set of additional directories to search as defconfig locations. 2. (Bonus) Enhance list-defconfigs to also display the defconfigs found at this extra location. Would there be opposition to something like a $(BR2_LIST_DEFCONFIGS_CMDS) variable to extend the list-defconfigs in the custom tree?
Hi Patrick, On Mon, Apr 18, 2016 at 9:25 AM, Patrick Williams <patrick@stwcx.xyz> wrote: > Since machines are often developed under confidentiality we are > attempting to create a "second level" BR2_EXTERNAL. This is the > openpower/custom tree and where the 'custom' came from in this patchset. > Our intention with that is to give developers a location to place > machines, packages, and patches currently under NDAs while they are > being developed. By having them in a separate tree, the developers can > continuously rebase against our master branch and have less of a issue > with merge conflicts, since we promise not to place anything in the > custom tree ourselves. > > We already utilize BR2_GLOBAL_PATCH_DIR and include custom/*.mk to allow > patches and additional packages to be placed under the custom tree. The > only piece that is missing is the defconfigs; hence this first attempt > at a patch. We would like to be able to do the following: > Couldn't you get everything you want by allowing a custom user "fred" to create a symlink in your ...configs/ directory to his own custom config like: ln -s ../custom/configs/freds_defconfig Then all the buildroot infrastructure will work as designed, you can have your standard BR2_EXTERNAL tree and "fred" can have his own custom tree. Since you won't check in the symlink and only fred's development will use it, no git conflicts....? Regards, Steve
On Mon, Apr 18, 2016 at 10:07:58AM -0700, Steve Calfee wrote: > Hi Patrick, > > On Mon, Apr 18, 2016 at 9:25 AM, Patrick Williams <patrick@stwcx.xyz> wrote: > > > Since machines are often developed under confidentiality we are > > attempting to create a "second level" BR2_EXTERNAL. This is the > > openpower/custom tree and where the 'custom' came from in this patchset. > > Our intention with that is to give developers a location to place > > machines, packages, and patches currently under NDAs while they are > > being developed. By having them in a separate tree, the developers can > > continuously rebase against our master branch and have less of a issue > > with merge conflicts, since we promise not to place anything in the > > custom tree ourselves. > > > > We already utilize BR2_GLOBAL_PATCH_DIR and include custom/*.mk to allow > > patches and additional packages to be placed under the custom tree. The > > only piece that is missing is the defconfigs; hence this first attempt > > at a patch. We would like to be able to do the following: > > > > Couldn't you get everything you want by allowing a custom user "fred" > to create a symlink in your ...configs/ directory to his own custom > config like: > > ln -s ../custom/configs/freds_defconfig > > Then all the buildroot infrastructure will work as designed, you can > have your standard BR2_EXTERNAL tree and "fred" can have his own > custom tree. Since you won't check in the symlink and only fred's > development will use it, no git conflicts....? > > Regards, Steve Steve, That is a good idea. I failed to mention one aspect that would prevent that from working very well. A few of the companies we work with keep a custom tree in their own repository (one of them SVN) and then as part of their build process they copy it on top of the custom tree. They don't have a mechanism to maintain the symlinks because they don't actually make commits to our repository.
On Mon, Apr 18, 2016 at 10:14 AM, Patrick Williams <patrick@stwcx.xyz> wrote: > On Mon, Apr 18, 2016 at 10:07:58AM -0700, Steve Calfee wrote: >> Hi Patrick, >> >> >> Couldn't you get everything you want by allowing a custom user "fred" >> to create a symlink in your ...configs/ directory to his own custom >> config like: >> >> ln -s ../custom/configs/freds_defconfig >> >> Then all the buildroot infrastructure will work as designed, you can >> have your standard BR2_EXTERNAL tree and "fred" can have his own >> custom tree. Since you won't check in the symlink and only fred's >> development will use it, no git conflicts....? >> >> Regards, Steve > > Steve, > > That is a good idea. I failed to mention one aspect that would prevent > that from working very well. > > A few of the companies we work with keep a custom tree in their own > repository (one of them SVN) and then as part of their build process > they copy it on top of the custom tree. They don't have a mechanism to > maintain the symlinks because they don't actually make commits to our > repository. > Hi Patrick, I don't understand the problem, symlinks don't care if their targets are deleted and recreated. I was already assuming users were not putting stuff into your git tree and stuff in yourtree/custom was someone else's repo. Having the symlink is nice if a custom repo is actually checked out into "custom" inside your tree. If they must have an external repo and splat their repo on top of custom no problem -- the only thing that won't work as planned is save_defconfig (etc). The newly saved configs will be properly updated in the custom filesystem/tree, but if it is a copy, the changes will have to be manually updated into the original (SVN) repo. Similarly, a user could put a copy of his configs in your configs directory and just not check them into your tree. Again a manual update to an actual repo will be needed if it changes. But how would having the originally proposed second "custom" external help this problem? - if it isn't their actual repo config changes would have to be manually updated anyway. Regards, Steve
Hello, On Mon, 18 Apr 2016 11:25:28 -0500, Patrick Williams wrote: > 1. Have support for a defconfig location other than > $(BR2_EXTERNAL)/configs/. > > We have the proposed patchset, which is too specific due to the use > of 'custom', and we have a local hack where we put the same recipe > into $(BR2_EXTERNAL)/docs/custom/custom.mk. Other than docs/*/*.mk, > there is no include in buildroot/Makefile into the BR2_EXTERNAL tree > when invoked without an existing .config. > > Would there be opposition to a new -include in buildroot/Makefile > outside of the large $(if BR2_HAVE_DOT_CONFIG... block into the > BR2_EXTERNAL tree? This would give us a hook point to add the > %_defconfig recipe found in this patchset. > > Alternatively, or additionally, we could create a > BR2_EXTERNAL_CONFIGS variable as a set of additional directories to > search as defconfig locations. > > 2. (Bonus) Enhance list-defconfigs to also display the defconfigs found > at this extra location. > > Would there be opposition to something like a > $(BR2_LIST_DEFCONFIGS_CMDS) variable to extend the list-defconfigs > in the custom tree? I think the only reasonable solution to this is to really allow multiple BR2_EXTERNAL directories. A patch series doing this was proposed by Yann E. Morin a while ago, but due to the fairly significant additional complexity, it hasn't been merged so far. Maybe hearing your use case will help push the feature further, or will encourage other folks in the Buildroot community to come up with alternate solutions for your problem. One question that immediately comes to mind is: why do you maintain a BR2_EXTERNAL tree on your side? If all you do is related to publicly available machines/packages, why don't you upstream all what you have in Buildroot, and let only your downstream users that need to create private/confidential things use the BR2_EXTERNAL feature? Thanks! Thomas
On Mon, Apr 18, 2016 at 10:37:27AM -0700, Steve Calfee wrote: > On Mon, Apr 18, 2016 at 10:14 AM, Patrick Williams <patrick@stwcx.xyz> wrote: > > On Mon, Apr 18, 2016 at 10:07:58AM -0700, Steve Calfee wrote: > >> Hi Patrick, > >> > >> > >> Couldn't you get everything you want by allowing a custom user "fred" > >> to create a symlink in your ...configs/ directory to his own custom > >> config like: > >> > >> ln -s ../custom/configs/freds_defconfig > >> > >> Then all the buildroot infrastructure will work as designed, you can > >> have your standard BR2_EXTERNAL tree and "fred" can have his own > >> custom tree. Since you won't check in the symlink and only fred's > >> development will use it, no git conflicts....? > >> > >> Regards, Steve > > > > Steve, > > > > That is a good idea. I failed to mention one aspect that would prevent > > that from working very well. > > > > A few of the companies we work with keep a custom tree in their own > > repository (one of them SVN) and then as part of their build process > > they copy it on top of the custom tree. They don't have a mechanism to > > maintain the symlinks because they don't actually make commits to our > > repository. > > > > Hi Patrick, > > I don't understand the problem, symlinks don't care if their targets > are deleted and recreated. > > I was already assuming users were not putting stuff into your git tree > and stuff in yourtree/custom was someone else's repo. Having the > symlink is nice if a custom repo is actually checked out into "custom" > inside your tree. > > If they must have an external repo and splat their repo on top of > custom no problem -- the only thing that won't work as planned is > save_defconfig (etc). The newly saved configs will be properly > updated in the custom filesystem/tree, but if it is a copy, the > changes will have to be manually updated into the original (SVN) repo. > > Similarly, a user could put a copy of his configs in your configs > directory and just not check them into your tree. Again a manual > update to an actual repo will be needed if it changes. But how would > having the originally proposed second "custom" external help this > problem? - if it isn't their actual repo config changes would have to > be manually updated anyway. > > Regards, Steve Steve, I re-read your proposal and missed before where you said to not check the symlinks into git. It seems like this is workable but creates extra overhead on the part of people using the custom tree, in that they must manually symlink into $(BR2_EXTERNAL)/configs.
On Mon, Apr 18, 2016 at 09:15:58PM +0200, Thomas Petazzoni wrote: > Hello, > > On Mon, 18 Apr 2016 11:25:28 -0500, Patrick Williams wrote: > > > 1. Have support for a defconfig location other than > > $(BR2_EXTERNAL)/configs/. > > > > We have the proposed patchset, which is too specific due to the use > > of 'custom', and we have a local hack where we put the same recipe > > into $(BR2_EXTERNAL)/docs/custom/custom.mk. Other than docs/*/*.mk, > > there is no include in buildroot/Makefile into the BR2_EXTERNAL tree > > when invoked without an existing .config. > > > > Would there be opposition to a new -include in buildroot/Makefile > > outside of the large $(if BR2_HAVE_DOT_CONFIG... block into the > > BR2_EXTERNAL tree? This would give us a hook point to add the > > %_defconfig recipe found in this patchset. > > > > Alternatively, or additionally, we could create a > > BR2_EXTERNAL_CONFIGS variable as a set of additional directories to > > search as defconfig locations. > > > > 2. (Bonus) Enhance list-defconfigs to also display the defconfigs found > > at this extra location. > > > > Would there be opposition to something like a > > $(BR2_LIST_DEFCONFIGS_CMDS) variable to extend the list-defconfigs > > in the custom tree? > > I think the only reasonable solution to this is to really allow > multiple BR2_EXTERNAL directories. A patch series doing this was > proposed by Yann E. Morin a while ago, but due to the fairly > significant additional complexity, it hasn't been merged so far. Has there been any previous discussion about allowing sub-directories under 'configs'? The Linux kernel tree allows both $(ARCH)/configs/*_defconfig and $(ARCH)/configs/*/*_defconfig . Coupled with Steve's proposal for us to use symlinks, we could create $(BR2_EXTERNAL)/configs/custom as a symlink to $(BR2_EXTERNAL)/custom/configs and have a solution as well. > One question that immediately comes to mind is: why do you maintain a > BR2_EXTERNAL tree on your side? If all you do is related to publicly > available machines/packages, why don't you upstream all what you have > in Buildroot, and let only your downstream users that need to create > private/confidential things use the BR2_EXTERNAL feature? We have been upstreaming packages that seemed globally applicable, but many of the packages have a very limited set of users. The burden vs benefit on the Buildroot team to maintain the recipes for these packages did not seem justifiable to us. If we are wrong in that decision we are certainly open to larger contributions upstream. For those unfamiliar with the Power architecture, this BR2_EXTERNAL repository (http://github.com/open-power/op-build) is roughly the equivalent of a BIOS + grub for Power. There packages here fall into the following categories: 1. Early system initialization firmware (CPU, Memory, SMP interconnect): - Hostboot 2. Late system initialization firmware (PCIe) and "OPAL" runtime abstraction: - Skiboot 3. Board specific data files for #1: - <board>-xml 4. Power bootloader: - Petitboot 5. On-chip power management microcode: - OCC 6. Processor specific binary microcode blobs. The only one of these that is even remotely useful outside of building the firmware for a Power8 system is Petitboot because it was also used in PS3. The main benefit we get from keeping these in a BR2_EXTERNAL tree is that we de-couple from Buildroot's patch integration. Due to product release cycles it is sometimes useful to keep on a "stable" Buildroot but add fixes to the Power specific packages. This could obviously be done as a separately maintained tree / branch instead. We have also had a few cases where the package teams were "slow" at getting in fixes for a compiler version bump and I suspect if these were directly upstream in Buildroot there would be an expectation of faster focus on those types of issues. CC'ing a few others involved in the project.
On 04/18/16 23:44, Patrick Williams wrote: > On Mon, Apr 18, 2016 at 09:15:58PM +0200, Thomas Petazzoni wrote: >> >Hello, >> > >> >On Mon, 18 Apr 2016 11:25:28 -0500, Patrick Williams wrote: >> > >>> > >1. Have support for a defconfig location other than >>> > >$(BR2_EXTERNAL)/configs/. >>> > > >>> > > We have the proposed patchset, which is too specific due to the use >>> > > of 'custom', and we have a local hack where we put the same recipe >>> > > into $(BR2_EXTERNAL)/docs/custom/custom.mk. Other than docs/*/*.mk, >>> > > there is no include in buildroot/Makefile into the BR2_EXTERNAL tree >>> > > when invoked without an existing .config. >>> > > >>> > > Would there be opposition to a new -include in buildroot/Makefile >>> > > outside of the large $(if BR2_HAVE_DOT_CONFIG... block into the >>> > > BR2_EXTERNAL tree? This would give us a hook point to add the >>> > > %_defconfig recipe found in this patchset. >>> > > >>> > > Alternatively, or additionally, we could create a >>> > > BR2_EXTERNAL_CONFIGS variable as a set of additional directories to >>> > > search as defconfig locations. >>> > > >>> > >2. (Bonus) Enhance list-defconfigs to also display the defconfigs found >>> > >at this extra location. >>> > > >>> > > Would there be opposition to something like a >>> > > $(BR2_LIST_DEFCONFIGS_CMDS) variable to extend the list-defconfigs >>> > > in the custom tree? >> > >> >I think the only reasonable solution to this is to really allow >> >multiple BR2_EXTERNAL directories. A patch series doing this was >> >proposed by Yann E. Morin a while ago, but due to the fairly >> >significant additional complexity, it hasn't been merged so far. > Has there been any previous discussion about allowing sub-directories > under 'configs'? The Linux kernel tree allows both > $(ARCH)/configs/*_defconfig and $(ARCH)/configs/*/*_defconfig . Coupled > with Steve's proposal for us to use symlinks, we could create > $(BR2_EXTERNAL)/configs/custom as a symlink to > $(BR2_EXTERNAL)/custom/configs and have a solution as well. > If that doesn't add too much complexity (and I can't imagine it does), it could definitely be a good solution. Especially if that allows us to kill the idea of a multi-layered-without-calling-it-layers BR2_EXTERNAL :-) Regards, Arnout
Hello, On Tue, 19 Apr 2016 01:57:12 +0200, Arnout Vandecappelle wrote: > >> >I think the only reasonable solution to this is to really allow > >> >multiple BR2_EXTERNAL directories. A patch series doing this was > >> >proposed by Yann E. Morin a while ago, but due to the fairly > >> >significant additional complexity, it hasn't been merged so far. > > Has there been any previous discussion about allowing sub-directories > > under 'configs'? The Linux kernel tree allows both > > $(ARCH)/configs/*_defconfig and $(ARCH)/configs/*/*_defconfig . Coupled > > with Steve's proposal for us to use symlinks, we could create > > $(BR2_EXTERNAL)/configs/custom as a symlink to > > $(BR2_EXTERNAL)/custom/configs and have a solution as well. > > If that doesn't add too much complexity (and I can't imagine it does), it > could definitely be a good solution. Especially if that allows us to kill the > idea of a multi-layered-without-calling-it-layers BR2_EXTERNAL :-) Our configs/ directory is also growing, so we could think of using sub-directories to organize the configurations per-vendor, or to separate the minimal configs from the demo ones, or something. Or we could move the configs next to the board/ related files they correspond. Hm, wait, that's what we used to do 5+ years ago :-) Thomas
diff --git a/Makefile b/Makefile index 63502d0..15ef194 100644 --- a/Makefile +++ b/Makefile @@ -845,6 +845,10 @@ defconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile @$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(BR2_EXTERNAL)/configs/$@ \ $< --defconfig=$(BR2_EXTERNAL)/configs/$@ $(CONFIG_CONFIG_IN) +%_defconfig: $(BUILD_DIR)/buildroot-config/conf $(BR2_EXTERNAL)/custom/configs/%_defconfig outputmakefile + @$(COMMON_CONFIG_ENV) BR2_DEFCONFIG=$(BR2_EXTERNAL)/custom/configs/$@ \ + $< --defconfig=$(BR2_EXTERNAL)/custom/configs/$@ $(CONFIG_CONFIG_IN) + savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile @$(COMMON_CONFIG_ENV) $< \ --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \ @@ -984,6 +988,12 @@ ifneq ($(wildcard $(BR2_EXTERNAL)/configs/*_defconfig),) @$(foreach b, $(sort $(notdir $(wildcard $(BR2_EXTERNAL)/configs/*_defconfig))), \ printf " %-35s - Build for %s\\n" $(b) $(b:_defconfig=);) endif +ifneq ($(wildcard $(BR2_EXTERNAL)/custom/configs/*_defconfig),) + @echo + @echo 'User-provided custom configs:' + @$(foreach b, $(sort $(notdir $(wildcard $(BR2_EXTERNAL)/custom/configs/*_defconfig))), \ + printf " %-35s - Build for %s\\n" $(b) $(b:_defconfig=);) +endif @echo release: OUT = buildroot-$(BR2_VERSION)