[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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 pckages 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 existence 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

Changes v1 -> v2:
- remove if-statement for file checking (Thomas Petazzoni)
- remove redundant '\ from sed line (Arnout Vandecappelle)
- use $(addprefix ... ) in sed line (Arnout Vandecappelle)
- add $(Q) to front of variable cheking if-statement (Stefan Fr?berg)

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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
 	$(Q)touch $@
 
 # Install to images dir
-- 
1.7.7.6

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Yann E. MORIN]

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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Arnout Vandecappelle]

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


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Arnout Vandecappelle]

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

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Thomas Petazzoni]

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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Arnout Vandecappelle]

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]
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Arnout Vandecappelle]

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


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Samuel Martin]

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,

-- 
Samuel

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Stefan Fröberg]

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,
>

[Buildroot] [PATCH v2] pkg-infra: add <pkg>_CONFIG_FIXUP to fix *-config files

[Arnout Vandecappelle]

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

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F