| Message ID | 1396469147-21713-1-git-send-email-sojka@merica.cz |
|---|---|
| State | Superseded |
| Headers | show |
Hi Michal, On Wed, Apr 2, 2014 at 10:05 PM, Michal Sojka <sojka@merica.cz> wrote: > This patch adds dependency of busybox configure target to the > configuration file specified with BUSYBOX_CONFIG_FILE variable. This > means that the following sequence of commands rebuilds busybox after > the busybox.config is changed: > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > echo SOME_OPTION=y >> busybox.config > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > > This behaviour is handy when a per-project busybox config is > maintained in another repository and the config gets updated by > another user (e.g. after git pull). > > Without this patch, the last command above does not rebuild busybox. > > This patch also modifies bysubox-update-config target to preserve the > timestamp of "exported" config. This is to ensure, that the following > sequence of commands builds busybox only once. > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config busybox-update-config > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > The issue you're describing is not limited to busybox alone. There are other packages using config files that have the same limitation: linux and uclibc for example. In general, buildroot is mainly aimed at building a given configuration completely. It is not really targeted at development environment, where you change part of the configuration (like the busybox config file) and then can rebuild all necessary components automatically. In fact, there are many different type of situations related to this: for example, if you start from an existing built system, and then enable one package (like a library), not all of the other packages that have optional dependencies are automatically rebuilt. Buildroot does not even attempt to do this. So I'm not sure if this patch is desirable given the above. Other contributors may think otherwise, of course. Best regards, Thomas
Thomas, Michal, All, On 2014-05-01 21:46 +0200, Thomas De Schampheleire spake thusly: > On Wed, Apr 2, 2014 at 10:05 PM, Michal Sojka <sojka@merica.cz> wrote: > > This patch adds dependency of busybox configure target to the > > configuration file specified with BUSYBOX_CONFIG_FILE variable. This > > means that the following sequence of commands rebuilds busybox after > > the busybox.config is changed: > > > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > > echo SOME_OPTION=y >> busybox.config > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > > > > This behaviour is handy when a per-project busybox config is > > maintained in another repository and the config gets updated by > > another user (e.g. after git pull). > > > > Without this patch, the last command above does not rebuild busybox. > > > > This patch also modifies bysubox-update-config target to preserve the > > timestamp of "exported" config. This is to ensure, that the following > > sequence of commands builds busybox only once. > > > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config busybox-update-config > > make BUSYBOX_CONFIG_FILE=$PWD/busybox.config > > > > The issue you're describing is not limited to busybox alone. There are > other packages using config files that have the same limitation: linux > and uclibc for example. > > In general, buildroot is mainly aimed at building a given > configuration completely. It is not really targeted at development > environment, where you change part of the configuration (like the > busybox config file) and then can rebuild all necessary components > automatically. > In fact, there are many different type of situations related to this: > for example, if you start from an existing built system, and then > enable one package (like a library), not all of the other packages > that have optional dependencies are automatically rebuilt. Buildroot > does not even attempt to do this. > > So I'm not sure if this patch is desirable given the above. > Other contributors may think otherwise, of course. We've already talked about it with Thomas P on IRC a while back. Here's the transcript of our discussion: --- Log opened Sun Apr 20 10:52:25 2014 [--SNIP--] 12:02 < kos_tom> opinions about http://patchwork.ozlabs.org/patch/336465/ ? 12:04 < y_morin> kos_tom: http://patchwork.ozlabs.org/patch/336465/ <-- we have PKG-clean-for-reconfigure for this, no? 12:05 < y_morin> kos_tom: But most importantly: if we do that for busybox, we need to do that for: linux, uClibc and any other package for which the user can provide a .config 12:06 < y_morin> kos_tom: So, I'd suggest we mark is as "Rejected" and ask to modify the manual about his situation. 12:09 < kos_tom> y_morin: to me, it's part of the "we don't try to be smart and to detect what needs to be rebuilt" 12:11 < kos_tom> clean-for-* are internal targets, they are not really meant to be used, and they are not documented 12:12 < y_morin> kos_tom: Well, we do have rebuild and reconfigure. So, it looks like both Thomas and me thinks this is not really something we want. I especially like kos_tom's comment: it's part of the "we don't try to be smart and to detect what needs to be rebuilt" I still stand by this position. Regards, Yann E. MORIN.
Hi, thanks for the response. On Thu, May 01 2014, Yann E. MORIN wrote: > Thomas, Michal, All, > > On 2014-05-01 21:46 +0200, Thomas De Schampheleire spake thusly: [...] > We've already talked about it with Thomas P on IRC a while back. Here's > the transcript of our discussion: > > --- Log opened Sun Apr 20 10:52:25 2014 > [--SNIP--] > 12:02 < kos_tom> opinions about http://patchwork.ozlabs.org/patch/336465/ ? > 12:04 < y_morin> kos_tom: http://patchwork.ozlabs.org/patch/336465/ <-- > we have PKG-clean-for-reconfigure for this, no? > 12:05 < y_morin> kos_tom: But most importantly: if we do that for > busybox, we need to do that for: linux, uClibc and any > other package for which the user can provide a .config > 12:06 < y_morin> kos_tom: So, I'd suggest we mark is as "Rejected" and > ask to modify the manual about his situation. > 12:09 < kos_tom> y_morin: to me, it's part of the "we don't try to be > smart and to detect what needs to be rebuilt" > 12:11 < kos_tom> clean-for-* are internal targets, they are not really > meant to be used, and they are not documented > 12:12 < y_morin> kos_tom: Well, we do have rebuild and reconfigure. > > > So, it looks like both Thomas and me thinks this is not really something > we want. I especially like kos_tom's comment: > > it's part of the "we don't try to be smart and to detect what needs > to be rebuilt" > > I still stand by this position. I could definitely live with this conclusion, but on the other hand I don't think that adding one more dependency is being "too smart". What are the drawbacks of adding this functionality? If it is just adding the same to other packages, I'm willing to do that. Quick grep suggests that only the following packages have custom config files: at91bootstrap3.mk, barebox.mk, ubi.mk, linux.mk, busybox.mk, freetype.mk, luarocks.mk, qt.mk, uclibc.mk. Or, is there other easy way how to achieve the following? I want to have all files that influence my root filesystem in a separate repository. Currently my repo contains the following files: output/.config # buildroot config output/.gitignore output/GNUmakefile # See below output/busybox.config output/perm_table.txt output/postbuild output/ubinize.cfg buildroot # Git submodule with buildroot Cloning this repo and running make in output/ reproducibly rebuilds the root filesystem. The GNUmakefile is a modified version of automatically generated Makefile from 'make menuconfig O=../output'. The modification include changing the -C parameter of make to relative path, setting environment variables such as BUSYBOX_CONFIG_FILE and updating "my" busybox config after buildroot innovation. See the full listing below. This setup allows me the easily track what has been changed in my project and I don't have to remember running XXX-update-config after every busybox-menuconfig. export BUSYBOX_CONFIG_FILE=$(CURDIR)/busybox.config #export BR2_EXTERNAL=$(CURDIR)/../XXX lastword = $(word $(words $(1)),$(1)) makedir := $(dir $(call lastword,$(MAKEFILE_LIST))) MAKEARGS := -C ../buildroot MAKEARGS += O=$(if $(patsubst /%,,$(makedir)),$(CURDIR)/)$(patsubst %/,%,$(makedir)) MAKEFLAGS += --no-print-directory .PHONY: all $(MAKECMDGOALS) all := $(filter-out Makefile,$(MAKECMDGOALS)) _all: $(MAKE) $(MAKEARGS) $(all) -$(MAKE) $(MAKEARGS) busybox-update-config Makefile:; $(all): _all @: %/: _all @: Best regards, -Michal
On 05/02/2014 10:07 AM, Michal Sojka wrote: > I could definitely live with this conclusion, but on the other hand I > don't think that adding one more dependency is being "too smart". What > are the drawbacks of adding this functionality? If it is just adding the > same to other packages, I'm willing to do that. Quick grep suggests that > only the following packages have custom config files: at91bootstrap3.mk, > barebox.mk, ubi.mk, linux.mk, busybox.mk, freetype.mk, luarocks.mk, > qt.mk, uclibc.mk. Hi. I think i've already mentioned it in the past, but here it goes again. Because you'd need a full rebuild of packages that depend on busybox as well since we use DEPENDENCIES to make packages with superior functionality to that provided by busybox build afterwards to override it's functionality (logic = it's YES and better so do it). If you rebuild busybox without doing so for the other packages then your "reproducibly" goes down the drain since the "big and featured" binaries get overwritten by busybox (with the way busybox is installed at the moment) for a built project. Regards.
On Fri, May 2, 2014 at 9:21 AM, Gustavo Zacarias <gustavo@zacarias.com.ar> wrote: > On 05/02/2014 10:07 AM, Michal Sojka wrote: [SNIP] > If you rebuild busybox without doing so for the other packages then your > "reproducibly" goes down the drain since the "big and featured" binaries > get overwritten by busybox (with the way busybox is installed at the > moment) for a built project. Ah, but that's where the busybox install script's "no clobber" feature is useful - it won't create links on top of existing files/links. I sent a patch [1] to the bb mailing list to add an option for this in the bb config menu, but got no reply thus far. In the meanwhile, I've been using a simpler patch that just forces this feature on - it makes "make busybox-menuconfig" much safer. [1] http://lists.busybox.net/pipermail/busybox/2014-April/080786.html Danomi -
Hello Gustavo, On Fri, May 02 2014, Gustavo Zacarias wrote: > On 05/02/2014 10:07 AM, Michal Sojka wrote: > >> I could definitely live with this conclusion, but on the other hand I >> don't think that adding one more dependency is being "too smart". What >> are the drawbacks of adding this functionality? If it is just adding the >> same to other packages, I'm willing to do that. Quick grep suggests that >> only the following packages have custom config files: at91bootstrap3.mk, >> barebox.mk, ubi.mk, linux.mk, busybox.mk, freetype.mk, luarocks.mk, >> qt.mk, uclibc.mk. > > Hi. > I think i've already mentioned it in the past, but here it goes again. > Because you'd need a full rebuild of packages that depend on busybox as > well since we use DEPENDENCIES to make packages with superior > functionality to that provided by busybox build afterwards to override > it's functionality (logic = it's YES and better so do it). > If you rebuild busybox without doing so for the other packages then your > "reproducibly" goes down the drain since the "big and featured" binaries > get overwritten by busybox (with the way busybox is installed at the > moment) for a built project. I understand that, but then "make busybox-menuconfig" has the same problem, doesn't it? I agree that the word "reproducible" is a bit overstatement, but my proposed change does not make buildroot worse than it is now. On Fri, May 02 2014, Danomi Manchego wrote: > Ah, but that's where the busybox install script's "no clobber" feature > is useful - it won't create links on top of existing files/links. > > I sent a patch [1] to the bb mailing list to add an option for this in > the bb config menu, but got no reply thus far. In the meanwhile, I've > been using a simpler patch that just forces this feature on - it makes > "make busybox-menuconfig" much safer. > > [1] http://lists.busybox.net/pipermail/busybox/2014-April/080786.html When my proposal is combined with Busybox's "no clobber" feature pointed out by Danomi, this problem could go away completely. What do you think? -Michal
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk index 793ffb9..b9be330 100644 --- a/package/busybox/busybox.mk +++ b/package/busybox/busybox.mk @@ -215,6 +215,8 @@ endef $(eval $(generic-package)) +$(BUSYBOX_TARGET_CONFIGURE): $(BUSYBOX_CONFIG_FILE) + busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch $(BUSYBOX_MAKE_ENV) $(MAKE) $(BUSYBOX_MAKE_OPTS) -C $(BUSYBOX_DIR) \ $(subst busybox-,,$@) @@ -222,4 +224,4 @@ busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch rm -f $(BUSYBOX_DIR)/.stamp_target_installed busybox-update-config: busybox-configure - cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE) + cp -fa $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
This patch adds dependency of busybox configure target to the configuration file specified with BUSYBOX_CONFIG_FILE variable. This means that the following sequence of commands rebuilds busybox after the busybox.config is changed: make BUSYBOX_CONFIG_FILE=$PWD/busybox.config echo SOME_OPTION=y >> busybox.config make BUSYBOX_CONFIG_FILE=$PWD/busybox.config This behaviour is handy when a per-project busybox config is maintained in another repository and the config gets updated by another user (e.g. after git pull). Without this patch, the last command above does not rebuild busybox. This patch also modifies bysubox-update-config target to preserve the timestamp of "exported" config. This is to ensure, that the following sequence of commands builds busybox only once. make BUSYBOX_CONFIG_FILE=$PWD/busybox.config make BUSYBOX_CONFIG_FILE=$PWD/busybox.config busybox-update-config make BUSYBOX_CONFIG_FILE=$PWD/busybox.config Signed-off-by: Michal Sojka <sojka@merica.cz> --- package/busybox/busybox.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)