Message ID | CAE21AQraBRp3_94Td_Gw+D9NML+-X0e+2U=cwR979ww99_aJ=w@mail.gmail.com |
---|---|
State | Rejected |
Headers | show |
Dear Charles Manning, On Mon, 18 Mar 2013 15:33:27 +1300, Charles Manning wrote: > It seems to me that the package override feature is misnamed and has > some shortcomings. > > First off, the name is, IMHO, poor. Package Override is applied before > tha packages are parsed. It does not override them. Instead it is > really overriding configurations before the packages are read. Perhaps > it should really be called CONFIG_OVERRIDE? > > There is also a deficiency in that Package Override cannot override > anything set up in the packages. Yes. The idea is that the "package override file" use special variables when it needs to override something from a package recipe. For now, the only special variable available is <pkg>_OVERRIDE_SRCDIR. It actually does more than just overriding the source directory: it also changes the behavior of the package infrastructure: instead of download/extract/patch, it switches to just use the provided source directory. > For example, I recently needed to add two more configuration lines to > building bluez_utils. > > Instead of: > > BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools > I wanted > BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools --enable-bccmd --enable-dund Yes, the current override mechanism doesn't support this. > A logical way I can see to do this would be to have a mechanism > similar to Package Override but that is applied after the packages are > parsed. > > --- Makefile (revision 20385) > +++ Makefile (working copy) Using Subversion ? :-) > @@ -319,11 +319,20 @@ > -include $(PACKAGE_OVERRIDE_FILE) > endif > > include package/*/*.mk > include hj/package/*/*.mk > include boot/common.mk > include linux/linux.mk > > +POST_PACKAGE_OVERRIDE_FILE=$(call qstrip,$(BR2_POST_PACKAGE_OVERRIDE_FILE)) > +ifneq ($(POST_PACKAGE_OVERRIDE_FILE),) > +-include $(POST_PACKAGE_OVERRIDE_FILE) > +endif > + > + > > Then have POST_PACKAGE_OVERRIDE_FILE point to a file which has > > # Add bluez_utils options we need > BLUEZ_UTILS_CONF_OPT += --enable-bccmd --enable-dund > > Does that sound useful? Does this actually work? The potential problem I see is that the package variables (<pkg>_BLABLA) are defined in the package .mk file and then used when the generic-package, autotools-package or cmake-package macros are expanded. I'm a bit lost as to whether those <pkg>_BLABLA variables are actually used when the package macros are expanded, or when the functions defined by the package macros are executed. This would have to be verified, or confirmed by someone who is more capable than me in understanding how many dollar signs are needed to make this work :) Also, will this work to override the source directory? We definitely don't want to have two different mechanisms to override stuff. Best regards, Thomas
On Mon, Mar 18, 2013 at 10:08 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Dear Charles Manning, > > On Mon, 18 Mar 2013 15:33:27 +1300, Charles Manning wrote: > >> It seems to me that the package override feature is misnamed and has >> some shortcomings. >> >> First off, the name is, IMHO, poor. Package Override is applied before >> tha packages are parsed. It does not override them. Instead it is >> really overriding configurations before the packages are read. Perhaps >> it should really be called CONFIG_OVERRIDE? >> >> There is also a deficiency in that Package Override cannot override >> anything set up in the packages. > > Yes. The idea is that the "package override file" use special variables > when it needs to override something from a package recipe. For now, the > only special variable available is <pkg>_OVERRIDE_SRCDIR. It actually > does more than just overriding the source directory: it also changes > the behavior of the package infrastructure: instead of > download/extract/patch, it switches to just use the provided source > directory. > >> For example, I recently needed to add two more configuration lines to >> building bluez_utils. >> >> Instead of: >> >> BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools >> I wanted >> BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools --enable-bccmd --enable-dund > > Yes, the current override mechanism doesn't support this. > >> A logical way I can see to do this would be to have a mechanism >> similar to Package Override but that is applied after the packages are >> parsed. >> >> --- Makefile (revision 20385) >> +++ Makefile (working copy) > > Using Subversion ? :-) > >> @@ -319,11 +319,20 @@ >> -include $(PACKAGE_OVERRIDE_FILE) >> endif >> >> include package/*/*.mk >> include boot/common.mk >> include linux/linux.mk >> >> +POST_PACKAGE_OVERRIDE_FILE=$(call qstrip,$(BR2_POST_PACKAGE_OVERRIDE_FILE)) >> +ifneq ($(POST_PACKAGE_OVERRIDE_FILE),) >> +-include $(POST_PACKAGE_OVERRIDE_FILE) >> +endif >> + >> + >> >> Then have POST_PACKAGE_OVERRIDE_FILE point to a file which has >> >> # Add bluez_utils options we need >> BLUEZ_UTILS_CONF_OPT += --enable-bccmd --enable-dund >> >> Does that sound useful? > > Does this actually work? Yes this actually works. If you set this up in PACKAGE_OVERRIDE the " BLUEZ_UTILS_CONF_OPT = ..." line in bluez_utils wipes out the settings. > > The potential problem I see is that the package variables > (<pkg>_BLABLA) are defined in the package .mk file and then used when > the generic-package, autotools-package or cmake-package macros are > expanded. I'm a bit lost as to whether those <pkg>_BLABLA variables are > actually used when the package macros are expanded, or when the > functions defined by the package macros are executed. This would have > to be verified, or confirmed by someone who is more capable than me in > understanding how many dollar signs are needed to make this work :) > > Also, will this work to override the source directory? We definitely > don't want to have two different mechanisms to override stuff. It really depends on what you're overriding. If you're overriding config stuff (before package/* gets parsed) then the current PACKAGE_OVERRIDE works file. For instance, I sent the .config to use HEAD as the revision for various source fetches but then use a revison override file to set them to the desired revisions. Unfortunately it would seem that there is not just one place where you can do all overriding, hence my proposal for adding another mechanism **after** the packages to tweak them. Even this will not do everything for all packages as make immediately uses some values and defers the evaluation of others. -- Charles
On 18/03/13 03:33, Charles Manning wrote: [snip] > For example, I recently needed to add two more configuration lines to > building bluez_utils. > > Instead of: > > BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools > I wanted > BLUEZ_UTILS_CONF_OPT = --enable-test --enable-tools --enable-bccmd --enable-dund > > > A logical way I can see to do this would be to have a mechanism > similar to Package Override but that is applied after the packages are > parsed. > > --- Makefile (revision 20385) > +++ Makefile (working copy) > @@ -319,11 +319,20 @@ > -include $(PACKAGE_OVERRIDE_FILE) > endif > > include package/*/*.mk > include hj/package/*/*.mk > include boot/common.mk > include linux/linux.mk > > +POST_PACKAGE_OVERRIDE_FILE=$(call qstrip,$(BR2_POST_PACKAGE_OVERRIDE_FILE)) > +ifneq ($(POST_PACKAGE_OVERRIDE_FILE),) > +-include $(POST_PACKAGE_OVERRIDE_FILE) > +endif > + > + > > Then have POST_PACKAGE_OVERRIDE_FILE point to a file which has > > # Add bluez_utils options we need > BLUEZ_UTILS_CONF_OPT += --enable-bccmd --enable-dund > > Does that sound useful? The problem with this approach is that it is not reliable. It _happens_ to work fine for _CONF_OPT. But it would not work for _DEPENDENCIES, for example, because the rule that consumes that variable comes before it. And probably there are some variables where it doesn't work at all because of some weird double-dollar stuff. That's why we just don't want to deal with this kind of changes. The override file is meant to be a development tool that allows you to hack the source code for a package. For other stuff, you should modify the package's .mk file directly. > Is there a better way? In this particular case, the better way is to create new Config.in options for bccmd and dund. And send us the patch, of course. Regards, Arnout
--- Makefile (revision 20385) +++ Makefile (working copy) @@ -319,11 +319,20 @@ -include $(PACKAGE_OVERRIDE_FILE) endif include package/*/*.mk include hj/package/*/*.mk include boot/common.mk include linux/linux.mk +POST_PACKAGE_OVERRIDE_FILE=$(call qstrip,$(BR2_POST_PACKAGE_OVERRIDE_FILE)) +ifneq ($(POST_PACKAGE_OVERRIDE_FILE),) +-include $(POST_PACKAGE_OVERRIDE_FILE) +endif + + Then have POST_PACKAGE_OVERRIDE_FILE point to a file which has # Add bluez_utils options we need BLUEZ_UTILS_CONF_OPT += --enable-bccmd --enable-dund