Message ID | 1357847559-31530-2-git-send-email-stefan.froberg@petroprogram.com |
---|---|
State | Superseded |
Headers | show |
Dear Stefan Fröberg, On Thu, 10 Jan 2013 21:52:39 +0200, Stefan Fröberg wrote: > For example: > > DIVINE_CONFIG_FIXUP = divine-config > > or for multiple files: > > IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config I personally still believe that it is wrong to give just the filename here and not the full path, i.e: IMAGEMAGICK_CONFIG_FIXUP = \ $(STAGING_DIR)/usr/bin/Magick-config \ $(SATGING_DIR)/usr/bin/Wand-config With just the filename, my impression is that it is just too much magic happening behind the scene. That said, I would not oppose to the current solution being integrated. I'm just sharing a preference, not a strong opposition here. Thanks! Thomas
Stefan, All, On Thursday 10 January 2013 Thomas Petazzoni wrote: > On Thu, 10 Jan 2013 21:52:39 +0200, Stefan Fröberg wrote: > > For example: > > > > DIVINE_CONFIG_FIXUP = divine-config > > > > or for multiple files: > > > > IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config > > I personally still believe that it is wrong to give just the filename > here and not the full path, i.e: > > IMAGEMAGICK_CONFIG_FIXUP = \ > $(STAGING_DIR)/usr/bin/Magick-config \ > $(SATGING_DIR)/usr/bin/Wand-config > > With just the filename, my impression is that it is just too much magic > happening behind the scene. Agreed. But I'd leave away the $(STAGING_DIR), and give full paths relative to the staging dir: IMAGEMAGICK_CONFIG_FIXUP = \ /usr/bin/Magick-config \ /usr/bin/Wand-config And the infrastructure automatically adds it, instead of adding $(STAGING_DIR)/usr/bin as it does in this patch. Also, I find the _FIXUP suffix to be misleading. 'fixup' conveys the meaning that the flaws are fixed, so I'd naturally expect that the *-config scripts are fixed, while this implementation removes them. With _FIXUP, the developper may incorrectly conclude that some sed/awk/.. magic is done on these scripts. I'd suggest FOO_CONFIG_SCRIPTS which is neutral, and does not say what is done with these scripts, so reading the documentation is mandatory to understand what is done. But, as Thomas, I don't have a strong opinion either. The current situation is OK, if not the "best in my eyes". ;-) Regards, Yann E. MORIN.
Hi Thomas 10.1.2013 22:19, Thomas Petazzoni kirjoitti: > Dear Stefan Fröberg, > > On Thu, 10 Jan 2013 21:52:39 +0200, Stefan Fröberg wrote: > >> For example: >> >> DIVINE_CONFIG_FIXUP = divine-config >> >> or for multiple files: >> >> IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config > I personally still believe that it is wrong to give just the filename > here and not the full path, i.e: > > IMAGEMAGICK_CONFIG_FIXUP = \ > $(STAGING_DIR)/usr/bin/Magick-config \ > $(SATGING_DIR)/usr/bin/Wand-config > > With just the filename, my impression is that it is just too much magic > happening behind the scene. > > That said, I would not oppose to the current solution being integrated. > I'm just sharing a preference, not a strong opposition here. > > Thanks! > > Thomas But this is much more less typing this way ;-) And there really is no other place for these files than in $(STAGING_DIR)/usr/bin in buildroot. Granted, maybe the variable name could be a more descriptive, like maybe <pkg>_STAGING_DIR_CONFIG_FIXUP or something like that. It's terse, it's ugly but hey at least it works! :-) there are, however, still some problems with those $(STAGING_DIR)/usr/bin/*-config files that I noticed that even this patch won't fix. I will investigate it further and report my findings later... Thank for your help Thomas! Regards Stefan
Hi Yann 10.1.2013 22:47, Yann E. MORIN kirjoitti: > Stefan, All, > > On Thursday 10 January 2013 Thomas Petazzoni wrote: >> On Thu, 10 Jan 2013 21:52:39 +0200, Stefan Fröberg wrote: >>> For example: >>> >>> DIVINE_CONFIG_FIXUP = divine-config >>> >>> or for multiple files: >>> >>> IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config >> I personally still believe that it is wrong to give just the filename >> here and not the full path, i.e: >> >> IMAGEMAGICK_CONFIG_FIXUP = \ >> $(STAGING_DIR)/usr/bin/Magick-config \ >> $(SATGING_DIR)/usr/bin/Wand-config >> >> With just the filename, my impression is that it is just too much magic >> happening behind the scene. > Agreed. But I'd leave away the $(STAGING_DIR), and give full paths relative > to the staging dir: > > IMAGEMAGICK_CONFIG_FIXUP = \ > /usr/bin/Magick-config \ > /usr/bin/Wand-config > > And the infrastructure automatically adds it, instead of adding > $(STAGING_DIR)/usr/bin as it does in this patch. > > Also, I find the _FIXUP suffix to be misleading. 'fixup' conveys the > meaning that the flaws are fixed, so I'd naturally expect that the > *-config scripts are fixed, while this implementation removes them. > With _FIXUP, the developper may incorrectly conclude that some sed/awk/.. > magic is done on these scripts. Well, uh... there *is* some sed magic done to those scripts. Almost half of the those files in my installation provide wrong prefix, wrong exec_prefix, and worst of all, sometimes just prefix and nothing else, not even includedir and libdir (but that's another story, another patch). > I'd suggest FOO_CONFIG_SCRIPTS which is neutral, and does not say what > is done with these scripts, so reading the documentation is mandatory to > understand what is done. Good, but im not doing that documentation patch. Like I said to Thomas, I really suck in this documentation department. > But, as Thomas, I don't have a strong opinion either. The current situation > is OK, if not the "best in my eyes". ;-) > > Regards, > Yann E. MORIN. > Nice, two Ok votes sofar. Can the father (Gustavo) of this <pkg>_CONFIG_FIXUP also give his vote ? Regards Stefan
Stefan, All, On Thursday 10 January 2013 Stefan Fröberg wrote: > 10.1.2013 22:47, Yann E. MORIN kirjoitti: > > Also, I find the _FIXUP suffix to be misleading. 'fixup' conveys the > > meaning that the flaws are fixed, so I'd naturally expect that the > > *-config scripts are fixed, while this implementation removes them. > > With _FIXUP, the developper may incorrectly conclude that some sed/awk/.. > > magic is done on these scripts. > > Well, uh... there *is* some sed magic done to those scripts. Oh sh!t... I mis-read another patch elsewhere for that patch. Regards, Yann E. MORIN.
On 10/01/13 20:52, Stefan Fröberg wrote: > This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. > It's purpose is to inform buildroot that the package in question > contains some $(STAGING_DIR)/usr/bin/*-config files and that we > want to automatically fix prefixes of such files. > > It is often the case that many packages call these > files during their configuration step to determine 3rd party > library package locations and any flags needed to link against them. > > For example: > Some package might try to check the existense and linking flags > of NSPR package by calling $(STAGING_DIR)/usr/bin/nspr-config --prefix. > Without this fix, NSPR would return /usr/ as it's prefix which is > wrong when cross-compiling. > Correct would be $(STAGING_DIR)/usr. > > All packages that have<pkg>_INSTALL_STAGING = YES defined and > also install some config file(s) into $(STAGING_DIR)/usr/bin must > hereafter also define<pkg>_CONFIG_FIXUP with the corresponding > filename(s). > > For example: > > DIVINE_CONFIG_FIXUP = divine-config > > or for multiple files: > > IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config > > Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> > --- > package/pkg-generic.mk | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index a570ad7..9f6ea7b 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -121,6 +121,12 @@ $(BUILD_DIR)/%/.stamp_staging_installed: > @$(call MESSAGE,"Installing to staging directory") > $($(PKG)_INSTALL_STAGING_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) > + $(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \ > + $(call MESSAGE,"Fixing package configuration files") ;\ > + $(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ > + -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ > + $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) ;\ > + fi I just discovered that you can also write: ifneq ($($(PKG)_CONFIG_FIXUP),) @$(call MESSAGE,"Fixing package configuration files") $(Q)$(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) endif Yes, you can use make conditions in rule definitions! [How I love micro-optimizing Makefiles :-)] Regards, Arnout > $(Q)touch $@ > > # Install to images dir
11.1.2013 23:33, Arnout Vandecappelle kirjoitti: > On 10/01/13 20:52, Stefan Fröberg wrote: >> This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. >> It's purpose is to inform buildroot that the package in question >> contains some $(STAGING_DIR)/usr/bin/*-config files and that we >> want to automatically fix prefixes of such files. >> >> It is often the case that many packages call these >> files during their configuration step to determine 3rd party >> library package locations and any flags needed to link against them. >> >> For example: >> Some package might try to check the existense and linking flags >> of NSPR package by calling $(STAGING_DIR)/usr/bin/nspr-config --prefix. >> Without this fix, NSPR would return /usr/ as it's prefix which is >> wrong when cross-compiling. >> Correct would be $(STAGING_DIR)/usr. >> >> All packages that have<pkg>_INSTALL_STAGING = YES defined and >> also install some config file(s) into $(STAGING_DIR)/usr/bin must >> hereafter also define<pkg>_CONFIG_FIXUP with the corresponding >> filename(s). >> >> For example: >> >> DIVINE_CONFIG_FIXUP = divine-config >> >> or for multiple files: >> >> IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config >> >> Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> >> --- >> package/pkg-generic.mk | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index a570ad7..9f6ea7b 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -121,6 +121,12 @@ $(BUILD_DIR)/%/.stamp_staging_installed: >> @$(call MESSAGE,"Installing to staging directory") >> $($(PKG)_INSTALL_STAGING_CMDS) >> $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call >> $(hook))$(sep)) >> + $(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \ >> + $(call MESSAGE,"Fixing package configuration files") ;\ >> + $(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ >> + -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ >> + $(addprefix >> $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) ;\ >> + fi > > I just discovered that you can also write: > > ifneq ($($(PKG)_CONFIG_FIXUP),) > @$(call MESSAGE,"Fixing package configuration files") > $(Q)$(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ > -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ > $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) > endif > > Yes, you can use make conditions in rule definitions! > > [How I love micro-optimizing Makefiles :-)] > Heh :-) I have a question that has been nagging in my head two days now: Let's suppose that command $(STAGING_DIR)/usr/bin/somepkg-config --prefix gives the correct prefix $(STAGING_DIR)/usr which is ok And $(STAGING_DIR)/usr/bin/somepkg-config --cflags gives empty (which is also fine) But $(STAGING_DIR)/usr/bin/somepkg-config --libs gives -L/usr/lib Isn't this horribly wrong ? I think it should give -L$(STAGING_DIR)/usr/lib I noticed that some *-config files have just prefix (and maybe exec_prefix) but not any includedir or libdir defined inside them and just give -I/usr/include for --cflags and -L/usr/lib for --libs Im beginning to suspect now that this is the very reason that my wireshark compilation borked, like you said Arnout, with that -L/usr/lib being added somehow to the final linking of wireshark binary .... This or then that *.la file problem you mentioned. Just my two cents Regards Stefan
On 12/01/13 02:38, Stefan Fröberg wrote: > 11.1.2013 23:33, Arnout Vandecappelle kirjoitti: >> On 10/01/13 20:52, Stefan Fröberg wrote: >>> This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. >>> It's purpose is to inform buildroot that the package in question >>> contains some $(STAGING_DIR)/usr/bin/*-config files and that we >>> want to automatically fix prefixes of such files. [snip] > I have a question that has been nagging in my head two days now: > > Let's suppose that command > > $(STAGING_DIR)/usr/bin/somepkg-config --prefix gives the correct prefix > $(STAGING_DIR)/usr which is ok > > And $(STAGING_DIR)/usr/bin/somepkg-config --cflags gives empty (which is > also fine) > > But $(STAGING_DIR)/usr/bin/somepkg-config --libs gives -L/usr/lib > > Isn't this horribly wrong ? I think it should give -L$(STAGING_DIR)/usr/lib --libs should not give -L/usr/lib, that's for sure! I had a quick look at some *-config scripts, and I couldn't find any that does this. Maybe we should check all of them (51 in my allpkgbuild). A generic solution could be the following: put a script in $(HOST_DIR)/usr/bin (or some other directory) that hands the known arguments to pkg-config and redirects the rest back to the original *-config script. This makes patching of the *-config script unnecessary in most cases. Something to discuss (again) at the BR developer days? > I noticed that some *-config files have just prefix (and maybe > exec_prefix) but not any includedir or libdir defined inside them > and just give -I/usr/include for --cflags and -L/usr/lib for --libs Even worse! Which one does that? > Im beginning to suspect now that this is the very reason that my > wireshark compilation borked, like you said Arnout, with that -L/usr/lib > being added somehow to the final linking of wireshark binary .... That could very well be... > This or then that *.la file problem you mentioned. Yes, but the *.la file only puts it in there if it was instructed to search for that library in /usr/lib. So the path must have been given to it somewhere, either in config.status or hard-coded in some Makefile.in. Regards, Arnout
Hi Arnout! 17.1.2013 10:32, Arnout Vandecappelle kirjoitti: > On 12/01/13 02:38, Stefan Fröberg wrote: >> 11.1.2013 23:33, Arnout Vandecappelle kirjoitti: >>> On 10/01/13 20:52, Stefan Fröberg wrote: >>>> This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. >>>> It's purpose is to inform buildroot that the package in question >>>> contains some $(STAGING_DIR)/usr/bin/*-config files and that we >>>> want to automatically fix prefixes of such files. > [snip] >> I have a question that has been nagging in my head two days now: >> >> Let's suppose that command >> >> $(STAGING_DIR)/usr/bin/somepkg-config --prefix gives the correct prefix >> $(STAGING_DIR)/usr which is ok >> >> And $(STAGING_DIR)/usr/bin/somepkg-config --cflags gives empty (which is >> also fine) >> >> But $(STAGING_DIR)/usr/bin/somepkg-config --libs gives -L/usr/lib >> >> Isn't this horribly wrong ? I think it should give >> -L$(STAGING_DIR)/usr/lib > > --libs should not give -L/usr/lib, that's for sure! > > I had a quick look at some *-config scripts, and I couldn't find any > that does this. Maybe we should check all of them (51 in my allpkgbuild). > > A generic solution could be the following: put a script in > $(HOST_DIR)/usr/bin (or some other directory) that hands the known > arguments to pkg-config and redirects the rest back to the original > *-config script. This makes patching of the *-config script > unnecessary in most cases. > Sounds good but what about that <pkg>_CONFIG_FIXUP variable that was started from Gustavo suggestion (originally from divine-config: fixup thread) ? http://lists.busybox.net/pipermail/buildroot/2013-January/064656.html Was it now totally waste of time ? :-( > Something to discuss (again) at the BR developer days? > > >> I noticed that some *-config files have just prefix (and maybe >> exec_prefix) but not any includedir or libdir defined inside them >> and just give -I/usr/include for --cflags and -L/usr/lib for --libs > > Even worse! Which one does that? > Well, at least: giblib-config --cflags gives -I/usr/include neon-config --cflags gives -I/usr/include But Im using older 2012.08 buildroot so maybe they are fixed now? >> Im beginning to suspect now that this is the very reason that my >> wireshark compilation borked, like you said Arnout, with that -L/usr/lib >> being added somehow to the final linking of wireshark binary .... > > That could very well be... > >> This or then that *.la file problem you mentioned. > > Yes, but the *.la file only puts it in there if it was instructed to > search for that library in /usr/lib. So the path must have been given > to it somewhere, either in config.status or hard-coded in some > Makefile.in. > > Regards, > Arnout > Ok. Ill try to dig deeper from those files where wireshark fetches that damn -L/usr/lib Thanks! Stefan
Dear Stefan Fröberg, On Fri, 18 Jan 2013 14:58:13 +0200, Stefan Fröberg wrote: > Sounds good but what about that <pkg>_CONFIG_FIXUP variable that was > started from Gustavo suggestion (originally from divine-config: fixup > thread) ? > > http://lists.busybox.net/pipermail/buildroot/2013-January/064656.html > > Was it now totally waste of time ? :-( No, I think it's good. We should just give it some testing, add a bit of documentation, and that's it. But in general, I'm in favor of this. Best regards, Thomas
On 18/01/13 13:58, Stefan Fröberg wrote: > Hi Arnout! > > 17.1.2013 10:32, Arnout Vandecappelle kirjoitti: [snip] >> A generic solution could be the following: put a script in >> $(HOST_DIR)/usr/bin (or some other directory) that hands the known >> arguments to pkg-config and redirects the rest back to the original >> *-config script. This makes patching of the *-config script >> unnecessary in most cases. >> > > Sounds good but what about that<pkg>_CONFIG_FIXUP variable that was > started from Gustavo suggestion (originally from divine-config: fixup > thread) ? > > http://lists.busybox.net/pipermail/buildroot/2013-January/064656.html > > Was it now totally waste of time ? :-( Not at all. First of all, it got the discussion going which may eventually lead to an acceptable solution. Second, the generic solution is probably not implementable in the short term. Your patch certainly is, because it only refactors already-existing fixups into common infrastructure. I therefore think that your patch should be committed. Will there still be a v3 or is this it? >> Something to discuss (again) at the BR developer days? >> >> >>> I noticed that some *-config files have just prefix (and maybe >>> exec_prefix) but not any includedir or libdir defined inside them >>> and just give -I/usr/include for --cflags and -L/usr/lib for --libs >> >> Even worse! Which one does that? >> > > Well, at least: > > giblib-config --cflags gives -I/usr/include > neon-config --cflags gives -I/usr/include > > But Im using older 2012.08 buildroot so maybe they are fixed now? D'oh, I did a quick check in my allpkgconfig: for i in staging/usr/bin/*-config; \ do $i --cflags | grep -e '-I/usr' && echo " --- $i"; \ done 24 of the 41 *-config scripts give the wrong cflags (26/40 for --libs) (roughly 10 scripts need some other argument than --libs/--cflags). So I'd say that your patch is sorely needed :-) Regards, Arnout [snip]
18.1.2013 17:51, Arnout Vandecappelle kirjoitti: > On 18/01/13 13:58, Stefan Fröberg wrote: >> Hi Arnout! >> >> 17.1.2013 10:32, Arnout Vandecappelle kirjoitti: > [snip] >>> A generic solution could be the following: put a script in >>> $(HOST_DIR)/usr/bin (or some other directory) that hands the known >>> arguments to pkg-config and redirects the rest back to the original >>> *-config script. This makes patching of the *-config script >>> unnecessary in most cases. >>> >> >> Sounds good but what about that<pkg>_CONFIG_FIXUP variable that was >> started from Gustavo suggestion (originally from divine-config: fixup >> thread) ? >> >> http://lists.busybox.net/pipermail/buildroot/2013-January/064656.html >> >> Was it now totally waste of time ? :-( > > Not at all. First of all, it got the discussion going which may > eventually lead to an acceptable solution. Second, the generic > solution is probably not implementable in the short term. Your patch > certainly is, because it only refactors already-existing fixups into > common infrastructure. > > I therefore think that your patch should be committed. Will there > still be a v3 or is this it? > Well, I think this is it. > >>> Something to discuss (again) at the BR developer days? >>> >>> >>>> I noticed that some *-config files have just prefix (and maybe >>>> exec_prefix) but not any includedir or libdir defined inside them >>>> and just give -I/usr/include for --cflags and -L/usr/lib for --libs >>> >>> Even worse! Which one does that? >>> >> >> Well, at least: >> >> giblib-config --cflags gives -I/usr/include >> neon-config --cflags gives -I/usr/include >> >> But Im using older 2012.08 buildroot so maybe they are fixed now? > > D'oh, I did a quick check in my allpkgconfig: > > for i in staging/usr/bin/*-config; \ > do $i --cflags | grep -e '-I/usr' && echo " --- $i"; \ > done > > 24 of the 41 *-config scripts give the wrong cflags (26/40 for > --libs) (roughly 10 scripts need some other argument than > --libs/--cflags). > > So I'd say that your patch is sorely needed :-) > > > > Regards, > Arnout > > [snip] Nicely :-) Stefan
18.1.2013 17:23, Thomas Petazzoni kirjoitti: > Dear Stefan Fröberg, > > On Fri, 18 Jan 2013 14:58:13 +0200, Stefan Fröberg wrote: > >> Sounds good but what about that <pkg>_CONFIG_FIXUP variable that was >> started from Gustavo suggestion (originally from divine-config: fixup >> thread) ? >> >> http://lists.busybox.net/pipermail/buildroot/2013-January/064656.html >> >> Was it now totally waste of time ? :-( > No, I think it's good. We should just give it some testing, add a bit > of documentation, and that's it. But in general, I'm in favor of this. > > Best regards, > > Thomas Okay. I try to make some documentation at the end of this weekend for it then. Thanks! Stefan
On 01/10/13 20:52, Stefan Fröberg wrote: > This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. > It's purpose is to inform buildroot that the package in question > contains some $(STAGING_DIR)/usr/bin/*-config files and that we > want to automatically fix prefixes of such files. > > It is often the case that many packages call these > files during their configuration step to determine 3rd party > library package locations and any flags needed to link against them. > > For example: > Some package might try to check the existense and linking flags > of NSPR package by calling $(STAGING_DIR)/usr/bin/nspr-config --prefix. > Without this fix, NSPR would return /usr/ as it's prefix which is > wrong when cross-compiling. > Correct would be $(STAGING_DIR)/usr. > > All packages that have<pkg>_INSTALL_STAGING = YES defined and > also install some config file(s) into $(STAGING_DIR)/usr/bin must > hereafter also define<pkg>_CONFIG_FIXUP with the corresponding > filename(s). > > For example: > > DIVINE_CONFIG_FIXUP = divine-config > > or for multiple files: > > IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config > > Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> > --- > package/pkg-generic.mk | 6 ++++++ > 1 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index a570ad7..9f6ea7b 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -121,6 +121,12 @@ $(BUILD_DIR)/%/.stamp_staging_installed: > @$(call MESSAGE,"Installing to staging directory") > $($(PKG)_INSTALL_STAGING_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) > + $(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \ > + $(call MESSAGE,"Fixing package configuration files") ;\ > + $(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ > + -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ Given that some *-config hard-code something like -L/usr/lib, I would add: -e "s,-I/usr/,-I$(STAGING_DIR)/usr/" \ -e "s,-L/usr/,-L$(STAGING_DIR)/usr/" \ Of course, for each package that actually uses this infrastructure, it has to be checked if it does the right thing. If it doesn't then the infra can still be fixed. Therefore, this patch gets my Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> and I hope to see some patches that use it! For volunteers that want to contribute fixes, here's a list of *-configs that currently don't do the right thing: imagemagick divine gd gsl libdnet giblib libart libcdaudio libesmtp libftdi libusb libvncserver log4c neon libnspr libpcap taglib Oh, and one of them (libnspr) doesn't have exec_prefix at the beginning of the line, so the expression should be: -e "s,^ *exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr," Regards, Arnout > + $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) ;\ > + fi > $(Q)touch $@ > > # Install to images dir
Hi all, 2013/1/20 Arnout Vandecappelle <arnout@mind.be>: > On 01/10/13 20:52, Stefan Fröberg wrote: >> >> This patch will add<pkg>_CONFIG_FIXUP variable to buildroot infra. >> It's purpose is to inform buildroot that the package in question >> contains some $(STAGING_DIR)/usr/bin/*-config files and that we >> want to automatically fix prefixes of such files. >> >> It is often the case that many packages call these >> files during their configuration step to determine 3rd party >> library package locations and any flags needed to link against them. >> >> For example: >> Some package might try to check the existense and linking flags >> of NSPR package by calling $(STAGING_DIR)/usr/bin/nspr-config --prefix. >> Without this fix, NSPR would return /usr/ as it's prefix which is >> wrong when cross-compiling. >> Correct would be $(STAGING_DIR)/usr. >> >> All packages that have<pkg>_INSTALL_STAGING = YES defined and >> also install some config file(s) into $(STAGING_DIR)/usr/bin must >> hereafter also define<pkg>_CONFIG_FIXUP with the corresponding >> filename(s). >> >> For example: >> >> DIVINE_CONFIG_FIXUP = divine-config >> >> or for multiple files: >> >> IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config >> >> Signed-off-by: Stefan Fröberg<stefan.froberg@petroprogram.com> >> --- >> package/pkg-generic.mk | 6 ++++++ >> 1 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk >> index a570ad7..9f6ea7b 100644 >> --- a/package/pkg-generic.mk >> +++ b/package/pkg-generic.mk >> @@ -121,6 +121,12 @@ $(BUILD_DIR)/%/.stamp_staging_installed: >> @$(call MESSAGE,"Installing to staging directory") >> $($(PKG)_INSTALL_STAGING_CMDS) >> $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call >> $(hook))$(sep)) >> + $(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \ >> + $(call MESSAGE,"Fixing package configuration files") ;\ >> + $(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ >> + -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ Could also be: $(SED) "s,^\(exec_\)\?prefix=.*,\1prefix=$(STAGING_DIR)/usr,g" \ > > > Given that some *-config hard-code something like -L/usr/lib, I would add: > > -e "s,-I/usr/,-I$(STAGING_DIR)/usr/" \ > -e "s,-L/usr/,-L$(STAGING_DIR)/usr/" \ What about this? -e "s,-I/usr/,-I\${prefix}/usr/" \ -e "s,-L/usr/,-L\${exec_prefix}/usr/" \ > > > Of course, for each package that actually uses this infrastructure, it has > to be checked if it does the right thing. If it doesn't then the infra can > still be fixed. Therefore, this patch gets my > > Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> > > and I hope to see some patches that use it! For volunteers that want to > contribute fixes, here's a list of *-configs that currently don't do the > right thing: > > imagemagick > divine > gd > gsl > libdnet > giblib > libart > libcdaudio > libesmtp > libftdi > libusb > libvncserver > log4c > neon > libnspr > libpcap > taglib > And a number of others already do some per-package fix: package/cups/cups.mk package/directfb/directfb.mk package/divine/divine.mk package/freetype/freetype.mk package/gd/gd.mk package/giblib/giblib.mk package/icu/icu.mk package/imlib2/imlib2.mk package/libcurl/libcurl.mk package/libdvdnav/libdvdnav.mk package/libdvdread/libdvdread.mk package/libgcrypt/libgcrypt.mk package/libmcrypt/libmcrypt.mk package/libnspr/libnspr.mk package/libpng/libpng.mk package/libusb-compat/libusb-compat.mk package/libxml2/libxml2.mk package/libxslt/libxslt.mk package/ncurses/ncurses.mk package/neon/neon.mk package/netsnmp/netsnmp.mk package/pcre/pcre.mk package/php/php.mk package/sdl/sdl.mk Time to unify (and document) all this stuff! ;-) > > Oh, and one of them (libnspr) doesn't have exec_prefix at the beginning of > the line, so the expression should be: > > -e "s,^ *exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr," Or just fix the nspr-config.in file to match "^exec_prefix=.*" ? Regards,
20.1.2013 14:35, Samuel Martin kirjoitti: >> >> Of course, for each package that actually uses this infrastructure, it has >> to be checked if it does the right thing. If it doesn't then the infra can >> still be fixed. Therefore, this patch gets my >> >> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> >> >> and I hope to see some patches that use it! For volunteers that want to >> contribute fixes, here's a list of *-configs that currently don't do the >> right thing: >> >> imagemagick >> divine >> gd >> gsl >> libdnet >> giblib >> libart >> libcdaudio >> libesmtp >> libftdi >> libusb >> libvncserver >> log4c >> neon >> libnspr >> libpcap >> taglib >> > And a number of others already do some per-package fix: > package/cups/cups.mk > package/directfb/directfb.mk > package/divine/divine.mk > package/freetype/freetype.mk > package/gd/gd.mk > package/giblib/giblib.mk > package/icu/icu.mk > package/imlib2/imlib2.mk > package/libcurl/libcurl.mk > package/libdvdnav/libdvdnav.mk > package/libdvdread/libdvdread.mk > package/libgcrypt/libgcrypt.mk > package/libmcrypt/libmcrypt.mk > package/libnspr/libnspr.mk > package/libpng/libpng.mk > package/libusb-compat/libusb-compat.mk > package/libxml2/libxml2.mk > package/libxslt/libxslt.mk > package/ncurses/ncurses.mk > package/neon/neon.mk > package/netsnmp/netsnmp.mk > package/pcre/pcre.mk > package/php/php.mk > package/sdl/sdl.mk > > Time to unify (and document) all this stuff! ;-) Well, I can do that individual package fixing and adding <pkg>_CONFIG_FIXUP to all those packages that need it according to your list and Arnout list. And also that documentation part. But that further fine tuning of what is the best sed magic to use I leave for your guys to decide ;-) Regards Stefan >> Oh, and one of them (libnspr) doesn't have exec_prefix at the beginning of >> the line, so the expression should be: >> >> -e "s,^ *exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr," > Or just fix the nspr-config.in file to match "^exec_prefix=.*" ? > > Regards, >
On 01/20/13 15:37, Stefan Fröberg wrote: > But that further fine tuning of what is the best sed magic to use > I leave for your guys to decide Anything that works is OK. Regards, Arnout
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index a570ad7..9f6ea7b 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -121,6 +121,12 @@ $(BUILD_DIR)/%/.stamp_staging_installed: @$(call MESSAGE,"Installing to staging directory") $($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) + $(Q)if test -n "$($(PKG)_CONFIG_FIXUP)" ; then \ + $(call MESSAGE,"Fixing package configuration files") ;\ + $(SED) "s,^prefix=.*,prefix=$(STAGING_DIR)/usr,g" \ + -e "s,^exec_prefix=.*,exec_prefix=$(STAGING_DIR)/usr,g" \ + $(addprefix $(STAGING_DIR)/usr/bin/,$($(PKG)_CONFIG_FIXUP)) ;\ + fi $(Q)touch $@ # Install to images dir
This patch will add <pkg>_CONFIG_FIXUP variable to buildroot infra. It's purpose is to inform buildroot that the package in question contains some $(STAGING_DIR)/usr/bin/*-config files and that we want to automatically fix prefixes of such files. It is often the case that many packages call these files during their configuration step to determine 3rd party library package locations and any flags needed to link against them. For example: Some package might try to check the existense and linking flags of NSPR package by calling $(STAGING_DIR)/usr/bin/nspr-config --prefix. Without this fix, NSPR would return /usr/ as it's prefix which is wrong when cross-compiling. Correct would be $(STAGING_DIR)/usr. All packages that have <pkg>_INSTALL_STAGING = YES defined and also install some config file(s) into $(STAGING_DIR)/usr/bin must hereafter also define <pkg>_CONFIG_FIXUP with the corresponding filename(s). For example: DIVINE_CONFIG_FIXUP = divine-config or for multiple files: IMAGEMAGICK_CONFIG_FIXUP = Magick-config Wand-config Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com> --- package/pkg-generic.mk | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)