[Buildroot] [PATCH 1/3] pkgconf: Split pkgconf command into multi-line

[Buildroot] [PATCH 1/3] pkgconf: Split pkgconf command into multi-line

[Thomas Preston]

The pkgconf command is a long and confusing line, which is about to get
longer. Split it up into logical stages so that it is easier to
visualise changes.

Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
---
 package/pkgconf/pkg-config.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
index 99c0add8fb..8795f64b68 100644
--- a/package/pkgconf/pkg-config.in
+++ b/package/pkgconf/pkg-config.in
@@ -2,4 +2,7 @@
 PKGCONFDIR=$(dirname $0)
 DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
 DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
-PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
+
+PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
+	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
+	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
-- 
2.20.1

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Thomas Preston]

Buildroot does not reconfigure pkgconf system library and system include
dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
library and system include dirs instead of letting the compiler handle
the logical sysroot. This breaks the -isystem compiler flag, as it
increases the priority of the system library and system include
directories. For example:

	$ output/host/bin/pkg-config --cflags glib-2.0
	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include

A header in `.../sysroot/usr/include` will be included before a header
in any directory specified with -isystem flags. Specifically, this
breaks the Chromium build system, which expects a C++ math.h in a
bundled LLVM C++ library, and gets a GNU C math.h instead.

Fix this by telling pkgconf about the sysroot's system library and
system include directories, so that it doesn't accidentally print them.

Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
---
 package/pkgconf/pkg-config.in | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
index 8795f64b68..894069c492 100644
--- a/package/pkgconf/pkg-config.in
+++ b/package/pkgconf/pkg-config.in
@@ -1,8 +1,12 @@
 #!/bin/sh
 PKGCONFDIR=$(dirname $0)
+DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
+DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
 DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
 DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
 
-PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
+PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
+	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
+	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
 	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
 	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
-- 
2.20.1

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Thomas Preston]

The correct way to configure pkgconf when cross-compiling is to use
pkgconf-personality, rather than using environment variables. The
personality is selected with a symbolic link mechanism, which we now use
in the pkg-config wrapper script.

Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
---
 package/pkgconf/pkg-config.in      | 11 +----------
 package/pkgconf/pkgconf.mk         | 12 +++++++++++-
 package/pkgconf/target.personality |  5 +++++
 3 files changed, 17 insertions(+), 11 deletions(-)
 create mode 100644 package/pkgconf/target.personality

diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
index 894069c492..51db4d87e1 100644
--- a/package/pkgconf/pkg-config.in
+++ b/package/pkgconf/pkg-config.in
@@ -1,12 +1,3 @@
 #!/bin/sh
 PKGCONFDIR=$(dirname $0)
-DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
-DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
-DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
-DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
-
-PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
-	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
-	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
-	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
-	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
+exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"
diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
index 1851ecfca4..66bc61d797 100644
--- a/package/pkgconf/pkgconf.mk
+++ b/package/pkgconf/pkgconf.mk
@@ -11,15 +11,25 @@ PKGCONF_LICENSE = pkgconf license
 PKGCONF_LICENSE_FILES = COPYING
 
 PKG_CONFIG_HOST_BINARY = $(HOST_DIR)/bin/pkg-config
+PKG_CONFIG_HOST_PERSONALITYD = $(HOST_DIR)/usr/share/pkgconfig/personality.d
 
 define PKGCONF_LINK_PKGCONFIG
 	ln -sf pkgconf $(TARGET_DIR)/usr/bin/pkg-config
 endef
 
 define HOST_PKGCONF_INSTALL_WRAPPER
+	$(INSTALL) -m 0775 -D package/pkgconf/target.personality \
+		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
+	$(SED) 's, at STAGING_DIR@,$(STAGING_DIR),g' \
+		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
+	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
+		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
+	ln -sf $(HOST_DIR)/bin/pkgconf \
+		$(HOST_DIR)/bin/$(GNU_TARGET_NAME)-pkg-config
+
 	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
 		$(HOST_DIR)/bin/pkg-config
-	$(SED) 's, at STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \
+	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
 		$(HOST_DIR)/bin/pkg-config
 endef
 
diff --git a/package/pkgconf/target.personality b/package/pkgconf/target.personality
new file mode 100644
index 0000000000..cee4d236c4
--- /dev/null
+++ b/package/pkgconf/target.personality
@@ -0,0 +1,5 @@
+Triplet: @GNU_TARGET_NAME@
+SysrootDir: @STAGING_DIR@
+DefaultSearchPaths: @STAGING_DIR@/usr/lib/pkgconfig:@STAGING_DIR@/usr/share/pkgconfig
+SystemIncludePaths: @STAGING_DIR@/usr/include
+SystemLibraryPaths: @STAGING_DIR@/usr/lib
-- 
2.20.1

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Thomas Petazzoni]

On Tue,  1 Oct 2019 13:41:31 +0100
Thomas Preston <thomas.preston@codethink.co.uk> wrote:

> Buildroot does not reconfigure pkgconf system library and system include
> dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
> library and system include dirs instead of letting the compiler handle
> the logical sysroot. This breaks the -isystem compiler flag, as it
> increases the priority of the system library and system include
> directories. For example:
> 
> 	$ output/host/bin/pkg-config --cflags glib-2.0
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include
> 
> A header in `.../sysroot/usr/include` will be included before a header
> in any directory specified with -isystem flags. Specifically, this
> breaks the Chromium build system, which expects a C++ math.h in a
> bundled LLVM C++ library, and gets a GNU C math.h instead.
> 
> Fix this by telling pkgconf about the sysroot's system library and
> system include directories, so that it doesn't accidentally print them.
> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>

I am wondering if this would not solve
https://bugs.busybox.net/show_bug.cgi?id=11776 which is also related to
-isystem causing problems. I think
https://bugs.busybox.net/show_bug.cgi?id=12131 and
https://bugs.busybox.net/show_bug.cgi?id=12231 are the same issue.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Thomas Preston]

On 01/10/2019 13:41, Thomas Preston wrote:
> The correct way to configure pkgconf when cross-compiling is to use
> pkgconf-personality, rather than using environment variables. The
> personality is selected with a symbolic link mechanism, which we now use
> in the pkg-config wrapper script.
> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> ---
>  package/pkgconf/pkg-config.in      | 11 +----------
>  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
>  package/pkgconf/target.personality |  5 +++++
>  3 files changed, 17 insertions(+), 11 deletions(-)
>  create mode 100644 package/pkgconf/target.personality
> 
> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> index 894069c492..51db4d87e1 100644
> --- a/package/pkgconf/pkg-config.in
> +++ b/package/pkgconf/pkg-config.in
> @@ -1,12 +1,3 @@
>  #!/bin/sh
>  PKGCONFDIR=$(dirname $0)
> -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
> -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
> -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
> -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
> -
> -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
> -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
> -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
> -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
> -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
> +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"

In fact, according to @kaniini (pkgconf maintainer), we can do away with this
wrapper script entirely and let the default symlink `pkg-config -> pkgconf`
load the default.personality. Which can be configured for the buildroot target.

Perhaps there could be a host-pkg-config symlink too (and host.personality),
replacing much of HOST_MAKE_ENV in package/Makefile.in. Where:

	PKG_CONFIG_HOST_BINARY = $(HOST_DIR)/bin/host-pkg-config

I think it's easier to tell how buildroot has set up pkgconf from this kind of
configuration.

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Yann E. MORIN]

Thomas, All,

On 2019-10-02 09:14 +0100, Thomas Preston spake thusly:
> On 01/10/2019 13:41, Thomas Preston wrote:
> > The correct way to configure pkgconf when cross-compiling is to use
> > pkgconf-personality, rather than using environment variables. The
> > personality is selected with a symbolic link mechanism, which we now use
> > in the pkg-config wrapper script.
> > 
> > Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> > ---
> >  package/pkgconf/pkg-config.in      | 11 +----------
> >  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
> >  package/pkgconf/target.personality |  5 +++++
> >  3 files changed, 17 insertions(+), 11 deletions(-)
> >  create mode 100644 package/pkgconf/target.personality
> > 
> > diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> > index 894069c492..51db4d87e1 100644
> > --- a/package/pkgconf/pkg-config.in
> > +++ b/package/pkgconf/pkg-config.in
> > @@ -1,12 +1,3 @@
> >  #!/bin/sh
> >  PKGCONFDIR=$(dirname $0)
> > -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
> > -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
> > -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
> > -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
> > -
> > -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
> > -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
> > -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
> > -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
> > -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
> > +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"
> 
> In fact, according to @kaniini (pkgconf maintainer), we can do away with this
> wrapper script entirely and let the default symlink `pkg-config -> pkgconf`
> load the default.personality. Which can be configured for the buildroot target.

We've already tried to look into the personality feature in the past,
and we concluded back then that we could not easily use it. I don't
remember everything about the reasons, but at least one issue is that
packages do not expect the TUPPLE-pkg-config mechanism, and most do just
call 'pkg-config', so we'd still need to keep our pkg-config wrapper
anyway. As such, using the personality was not so much interesting...

But maybe things have changed now...

Regards,
Yann E. MORIN.

> Perhaps there could be a host-pkg-config symlink too (and host.personality),
> replacing much of HOST_MAKE_ENV in package/Makefile.in. Where:
> 
> 	PKG_CONFIG_HOST_BINARY = $(HOST_DIR)/bin/host-pkg-config
> 
> I think it's easier to tell how buildroot has set up pkgconf from this kind of
> configuration.
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

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

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Thomas Preston]

On 02/10/2019 21:22, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2019-10-02 09:14 +0100, Thomas Preston spake thusly:
>> On 01/10/2019 13:41, Thomas Preston wrote:
>>> The correct way to configure pkgconf when cross-compiling is to use
>>> pkgconf-personality, rather than using environment variables. The
>>> personality is selected with a symbolic link mechanism, which we now use
>>> in the pkg-config wrapper script.
>>>
>>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>>> ---
>>>  package/pkgconf/pkg-config.in      | 11 +----------
>>>  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
>>>  package/pkgconf/target.personality |  5 +++++
>>>  3 files changed, 17 insertions(+), 11 deletions(-)
>>>  create mode 100644 package/pkgconf/target.personality
>>>
>>> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
>>> index 894069c492..51db4d87e1 100644
>>> --- a/package/pkgconf/pkg-config.in
>>> +++ b/package/pkgconf/pkg-config.in
>>> @@ -1,12 +1,3 @@
>>>  #!/bin/sh
>>>  PKGCONFDIR=$(dirname $0)
>>> -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
>>> -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
>>> -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
>>> -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
>>> -
>>> -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
>>> -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
>>> -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
>>> -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
>>> -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
>>> +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"
>>
>> In fact, according to @kaniini (pkgconf maintainer), we can do away with this
>> wrapper script entirely and let the default symlink `pkg-config -> pkgconf`
>> load the default.personality. Which can be configured for the buildroot target.
> 
> We've already tried to look into the personality feature in the past,
> and we concluded back then that we could not easily use it. I don't
> remember everything about the reasons, but at least one issue is that
> packages do not expect the TUPPLE-pkg-config mechanism, and most do just
> call 'pkg-config', so we'd still need to keep our pkg-config wrapper
> anyway. As such, using the personality was not so much interesting...
> 

You can set a default.personality, which is used for the base pkg-config
link, so you could have:

	     pkg-config: default.personality
	host-pkg-config: host.personality

However, you cannot set --static in the pkgconf-personality file (yet...),
which rules out using them for now.

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Thomas Preston]

Hi Thomas,

On 01/10/2019 14:47, Thomas Petazzoni wrote:
> On Tue,  1 Oct 2019 13:41:31 +0100
> Thomas Preston <thomas.preston@codethink.co.uk> wrote:
> 
>> Buildroot does not reconfigure pkgconf system library and system include
>> dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
>> library and system include dirs instead of letting the compiler handle
>> the logical sysroot. This breaks the -isystem compiler flag, as it
>> increases the priority of the system library and system include
>> directories. For example:
>>
>> 	$ output/host/bin/pkg-config --cflags glib-2.0
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include
>>
>> A header in `.../sysroot/usr/include` will be included before a header
>> in any directory specified with -isystem flags. Specifically, this
>> breaks the Chromium build system, which expects a C++ math.h in a
>> bundled LLVM C++ library, and gets a GNU C math.h instead.
>>
>> Fix this by telling pkgconf about the sysroot's system library and
>> system include directories, so that it doesn't accidentally print them.
>>
>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> 
> I am wondering if this would not solve
> https://bugs.busybox.net/show_bug.cgi?id=11776 which is also related to
> -isystem causing problems. I think
> https://bugs.busybox.net/show_bug.cgi?id=12131 and
> https://bugs.busybox.net/show_bug.cgi?id=12231 are the same issue.
> 

I've commented on that first issue, but I don't think this will fix it.
The /usr/include in final -isystem should work because the priority order 
is:

	-I
	-isystem
	logical sysroot /usr/include

Our bug is where a header (GNU C math.h) in the logical sysroot is
included before the expected header (LLVM C++ math.h) in the -isystem
include, because pkgconf incorrectly returned a:

	-I.../sysroot/usr/include.

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Arnout Vandecappelle]

On 03/10/2019 10:28, Thomas Preston wrote:
> On 02/10/2019 21:22, Yann E. MORIN wrote:
>> Thomas, All,
>>
>> On 2019-10-02 09:14 +0100, Thomas Preston spake thusly:
>>> On 01/10/2019 13:41, Thomas Preston wrote:
>>>> The correct way to configure pkgconf when cross-compiling is to use
>>>> pkgconf-personality, rather than using environment variables. The
>>>> personality is selected with a symbolic link mechanism, which we now use
>>>> in the pkg-config wrapper script.
>>>>
>>>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>>>> ---
>>>>  package/pkgconf/pkg-config.in      | 11 +----------
>>>>  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
>>>>  package/pkgconf/target.personality |  5 +++++
>>>>  3 files changed, 17 insertions(+), 11 deletions(-)
>>>>  create mode 100644 package/pkgconf/target.personality
>>>>
>>>> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
>>>> index 894069c492..51db4d87e1 100644
>>>> --- a/package/pkgconf/pkg-config.in
>>>> +++ b/package/pkgconf/pkg-config.in
>>>> @@ -1,12 +1,3 @@
>>>>  #!/bin/sh
>>>>  PKGCONFDIR=$(dirname $0)
>>>> -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
>>>> -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
>>>> -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
>>>> -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
>>>> -
>>>> -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
>>>> -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
>>>> -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
>>>> -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
>>>> -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
>>>> +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"
>>>
>>> In fact, according to @kaniini (pkgconf maintainer), we can do away with this
>>> wrapper script entirely and let the default symlink `pkg-config -> pkgconf`
>>> load the default.personality. Which can be configured for the buildroot target.
>>
>> We've already tried to look into the personality feature in the past,
>> and we concluded back then that we could not easily use it. I don't
>> remember everything about the reasons, but at least one issue is that
>> packages do not expect the TUPPLE-pkg-config mechanism, and most do just
>> call 'pkg-config', so we'd still need to keep our pkg-config wrapper
>> anyway. As such, using the personality was not so much interesting...
>>
> 
> You can set a default.personality, which is used for the base pkg-config
> link, so you could have:
> 
> 	     pkg-config: default.personality
> 	host-pkg-config: host.personality

 I think the personality approach is better than the wrapper script for sure.
However, it's not always easy to override a package's idea of how to call
pkg-config (e.g. quite a few use `pkg-config ...` in the Makefile directly - in
fact, we do the same, in support/kconfig!). Therefore, I think a better approach
is to have a separate "host" and "cross" bin directory, both of which contain a
pkg-config (symlink or wrapper script). For cross-compilation, both directories
are put in PATH, and for host compilation on the host directory.

 The cross dir can then also be populated with the -config script from
STAGING_DIR, which avoids having to pass them explicitly in other packages.

> However, you cannot set --static in the pkgconf-personality file (yet...),
> which rules out using them for now.

 I'm sure that that can be patched...

 Regards,
 Arnout

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Thomas Petazzoni]

On Sat, 5 Oct 2019 15:43:36 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  I think the personality approach is better than the wrapper script for sure.
> However, it's not always easy to override a package's idea of how to call
> pkg-config (e.g. quite a few use `pkg-config ...` in the Makefile directly - in
> fact, we do the same, in support/kconfig!). Therefore, I think a better approach
> is to have a separate "host" and "cross" bin directory, both of which contain a
> pkg-config (symlink or wrapper script). For cross-compilation, both directories
> are put in PATH, and for host compilation on the host directory.

I already expressed my opinion on this before, but I think the
pkg-config vs. TUPLE-pkg-config solution is the right thing to do. It
is really the standard way of doing this, the PKG_CHECK_MODULES()
autoconf macro looks first for TUPLE-pkg-config, before using
pkg-config.

Yes, it's not going to work with a number of packages that directly use
"pkg-config", but they should "simply" be fixed, no?

Another problem with the approach you suggest is that some packages
build both target code and host code. In this case, playing around with
two pkg-config binaries installed in two different locations isn't
going to work well. While with different names, you can have PKG_CONFIG
and PKG_CONFIG_FOR_BUILD with different values.

>  The cross dir can then also be populated with the -config script from
> STAGING_DIR, which avoids having to pass them explicitly in other packages.

That is admittedly an interesting argument.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Arnout Vandecappelle]

On 12/10/2019 22:09, Thomas Petazzoni wrote:
> On Sat, 5 Oct 2019 15:43:36 +0200
> Arnout Vandecappelle <arnout@mind.be> wrote:
> 
>>  I think the personality approach is better than the wrapper script for sure.
>> However, it's not always easy to override a package's idea of how to call
>> pkg-config (e.g. quite a few use `pkg-config ...` in the Makefile directly - in
>> fact, we do the same, in support/kconfig!). Therefore, I think a better approach
>> is to have a separate "host" and "cross" bin directory, both of which contain a
>> pkg-config (symlink or wrapper script). For cross-compilation, both directories
>> are put in PATH, and for host compilation on the host directory.
> 
> I already expressed my opinion on this before, but I think the
> pkg-config vs. TUPLE-pkg-config solution is the right thing to do. It
> is really the standard way of doing this, the PKG_CHECK_MODULES()
> autoconf macro looks first for TUPLE-pkg-config, before using
> pkg-config.
> 
> Yes, it's not going to work with a number of packages that directly use
> "pkg-config", but they should "simply" be fixed, no?

 True. But I think in Buildroot we generally try to be pragmatic, and prefer a
single solution that fixes things for all packages instead of fixing each and
every package individually. At least, if this can be done in a relatively simple
way.


> Another problem with the approach you suggest is that some packages
> build both target code and host code. In this case, playing around with
> two pkg-config binaries installed in two different locations isn't
> going to work well. While with different names, you can have PKG_CONFIG
> and PKG_CONFIG_FOR_BUILD with different values.

 For those cases, it's equally broken at the moment since there's no way to tell
the package that it needs different PKG_CONFIG_LIBDIR for host build. So we need
to do something there...

 Of course, we could use the best of both worlds: use the TUPLE-pkg-config and
make the cross-dir pkg-config a symlink to it (or wrapper script, if
TUPLE-pkg-config can use personality completely).

>>  The cross dir can then also be populated with the -config script from
>> STAGING_DIR, which avoids having to pass them explicitly in other packages.
> 
> That is admittedly an interesting argument.

 That was in fact my original motivation to think of this cross dir, and then I
realized the same could solve the pkg-config issue.

 Regards,
 Arnout

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Arnout Vandecappelle]

 Hi Thomas,

On 01/10/2019 14:41, Thomas Preston wrote:
> Buildroot does not reconfigure pkgconf system library and system include
> dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
> library and system include dirs instead of letting the compiler handle
> the logical sysroot. This breaks the -isystem compiler flag, as it
> increases the priority of the system library and system include
> directories. For example:
> 
> 	$ output/host/bin/pkg-config --cflags glib-2.0
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include
> 
> A header in `.../sysroot/usr/include` will be included before a header
> in any directory specified with -isystem flags. Specifically, this
> breaks the Chromium build system, which expects a C++ math.h in a
> bundled LLVM C++ library, and gets a GNU C math.h instead.
> 
> Fix this by telling pkgconf about the sysroot's system library and
> system include directories, so that it doesn't accidentally print them.
> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> ---
>  package/pkgconf/pkg-config.in | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> index 8795f64b68..894069c492 100644
> --- a/package/pkgconf/pkg-config.in
> +++ b/package/pkgconf/pkg-config.in
> @@ -1,8 +1,12 @@
>  #!/bin/sh
>  PKGCONFDIR=$(dirname $0)
> +DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
> +DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
>  DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
>  DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
>  
> -PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
> +PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
> +	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \

 Since these variables are not overridden by HOST_CONFIGURE_OPTS, they will also
be set for host compilation, which is wrong. However, the worst that can happen
is that it hides some bug during host compilation, because the only thing these
variables do is to remove staging include paths and these are anyway wrong for
host compilation.

 Therefore I've applied to master. With one change though: I changed the order
of the variables to minimize the diff here (i.e. keep LIBDIR first) and to make
it alphabetical (i.e. INCLUDE before LIBRARY, which is anyway more logical as
well IMO).

 Thanks!

 Regards,
 Arnout


> +	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
>  	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
>  	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
> 

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Arnout Vandecappelle]

 In addition to the discussion we had earlier, I have some more thoughts about this.

On 01/10/2019 14:41, Thomas Preston wrote:
> The correct way to configure pkgconf when cross-compiling is to use
> pkgconf-personality, rather than using environment variables. The
> personality is selected with a symbolic link mechanism, which we now use
> in the pkg-config wrapper script.

 I think that as a transient measure, we should still use an environment
variable, but just one this time. E.g. BR2_PKGCONF_PERSONALITY, which is set to
the proper personality (and defaults to the cross personality, like is the case
now).

 Then, when we have a solution for the @STATIC@ thing so even that is no longer
needed, we can add the personality symlinks. When those exist, they can be used
by the smart build systems (i.e. autotools and meson, and cmake by setting
PKG_CONFIG in the environment). And finally we can move the wrapper script to
the cross-dir that I proposed, if we feel like it.

 So in summary, to properly use the personality feature, we should find a
solution for @STATIC at . But until then, I do think it makes sense to use is as
much as possible, like in this patch.

 So I'm just going to give some nitpicking feedback here.

> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> ---
>  package/pkgconf/pkg-config.in      | 11 +----------
>  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
>  package/pkgconf/target.personality |  5 +++++
>  3 files changed, 17 insertions(+), 11 deletions(-)
>  create mode 100644 package/pkgconf/target.personality
> 
> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> index 894069c492..51db4d87e1 100644
> --- a/package/pkgconf/pkg-config.in
> +++ b/package/pkgconf/pkg-config.in
> @@ -1,12 +1,3 @@
>  #!/bin/sh
>  PKGCONFDIR=$(dirname $0)
> -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
> -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
> -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
> -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
> -
> -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
> -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
> -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
> -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
> -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
> +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"

 As long as the @STATIC@ is needed, I wouldn't create
@GNU_TARGET_NAME at -pkg-config, but instead use the --personality command-line option.

 And to properly support host personality, I'd allow an override through the
environment.

> diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
> index 1851ecfca4..66bc61d797 100644
> --- a/package/pkgconf/pkgconf.mk
> +++ b/package/pkgconf/pkgconf.mk
> @@ -11,15 +11,25 @@ PKGCONF_LICENSE = pkgconf license
>  PKGCONF_LICENSE_FILES = COPYING
>  
>  PKG_CONFIG_HOST_BINARY = $(HOST_DIR)/bin/pkg-config
> +PKG_CONFIG_HOST_PERSONALITYD = $(HOST_DIR)/usr/share/pkgconfig/personality.d

 We don't use $(HOST_DIR)/usr any more, it should just be $(HOST_DIR)/share/...

 More importantly, however, there's a problem here. This path is hardcoded in
the executable, which makes it non-relocatable. We try to make the SDK
relocatable. With the wrapper script, it's OK because we use relative paths
there. But it seems that the pkgconf binary encodes a few absolute paths. I
think fixing that will require some pretty invasive modifications to pkgconf...


>  
>  define PKGCONF_LINK_PKGCONFIG
>  	ln -sf pkgconf $(TARGET_DIR)/usr/bin/pkg-config
>  endef
>  
>  define HOST_PKGCONF_INSTALL_WRAPPER
> +	$(INSTALL) -m 0775 -D package/pkgconf/target.personality \

 Why is it executable? And why writable by group?

> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
> +	$(SED) 's, at STAGING_DIR@,$(STAGING_DIR),g' \
> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
> +	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality

 Assuming it doesn't need to be executable, the three commands above can be done
with a single sed command.

	sed -e 's, at STAGING_DIR@,$(STAGING_DIR),g' \
		-e 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
		package/pkgconf/target.personality \
		> $(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality

But then you do need an explicit mkdir.

> +	ln -sf $(HOST_DIR)/bin/pkgconf \
> +		$(HOST_DIR)/bin/$(GNU_TARGET_NAME)-pkg-config
> +
>  	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
>  		$(HOST_DIR)/bin/pkg-config
> -	$(SED) 's, at STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \
> +	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
>  		$(HOST_DIR)/bin/pkg-config
>  endef
>  
> diff --git a/package/pkgconf/target.personality b/package/pkgconf/target.personality
> new file mode 100644
> index 0000000000..cee4d236c4
> --- /dev/null
> +++ b/package/pkgconf/target.personality
> @@ -0,0 +1,5 @@
> +Triplet: @GNU_TARGET_NAME@
> +SysrootDir: @STAGING_DIR@
> +DefaultSearchPaths: @STAGING_DIR@/usr/lib/pkgconfig:@STAGING_DIR@/usr/share/pkgconfig
> +SystemIncludePaths: @STAGING_DIR@/usr/include
> +SystemLibraryPaths: @STAGING_DIR@/usr/lib

 Here as well, the paths are absolute... But in this case, I think they'll get
fixed up by relocate-sdk.sh.

 Regards,
 Arnout

[Buildroot] [PATCH 3/3] pkgconf: Configure using pkgconf-personality

[Arnout Vandecappelle]

 In addition to the discussion we had earlier, I have some more thoughts about this.

On 01/10/2019 14:41, Thomas Preston wrote:
> The correct way to configure pkgconf when cross-compiling is to use
> pkgconf-personality, rather than using environment variables. The
> personality is selected with a symbolic link mechanism, which we now use
> in the pkg-config wrapper script.

 I think that as a transient measure, we should still use an environment
variable, but just one this time. E.g. BR2_PKGCONF_PERSONALITY, which is set to
the proper personality (and defaults to the cross personality, like is the case
now).

 Then, when we have a solution for the @STATIC@ thing so even that is no longer
needed, we can add the personality symlinks. When those exist, they can be used
by the smart build systems (i.e. autotools and meson, and cmake by setting
PKG_CONFIG in the environment). And finally we can move the wrapper script to
the cross-dir that I proposed, if we feel like it.

 So in summary, to properly use the personality feature, we should find a
solution for @STATIC at . But until then, I do think it makes sense to use is as
much as possible, like in this patch.

 So I'm just going to give some nitpicking feedback here.

> 
> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
> ---
>  package/pkgconf/pkg-config.in      | 11 +----------
>  package/pkgconf/pkgconf.mk         | 12 +++++++++++-
>  package/pkgconf/target.personality |  5 +++++
>  3 files changed, 17 insertions(+), 11 deletions(-)
>  create mode 100644 package/pkgconf/target.personality
> 
> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
> index 894069c492..51db4d87e1 100644
> --- a/package/pkgconf/pkg-config.in
> +++ b/package/pkgconf/pkg-config.in
> @@ -1,12 +1,3 @@
>  #!/bin/sh
>  PKGCONFDIR=$(dirname $0)
> -DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
> -DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
> -DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
> -DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
> -
> -PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
> -	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
> -	PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
> -	PKG_CONFIG_SYSROOT_DIR=${PKG_CONFIG_SYSROOT_DIR:-${DEFAULT_PKG_CONFIG_SYSROOT_DIR}} \
> -	exec ${PKGCONFDIR}/pkgconf @STATIC@ "$@"
> +exec ${PKGCONFDIR}/@GNU_TARGET_NAME at -pkg-config @STATIC@ "$@"

 As long as the @STATIC@ is needed, I wouldn't create
@GNU_TARGET_NAME at -pkg-config, but instead use the --personality command-line option.

 And to properly support host personality, I'd allow an override through the
environment.

> diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk
> index 1851ecfca4..66bc61d797 100644
> --- a/package/pkgconf/pkgconf.mk
> +++ b/package/pkgconf/pkgconf.mk
> @@ -11,15 +11,25 @@ PKGCONF_LICENSE = pkgconf license
>  PKGCONF_LICENSE_FILES = COPYING
>  
>  PKG_CONFIG_HOST_BINARY = $(HOST_DIR)/bin/pkg-config
> +PKG_CONFIG_HOST_PERSONALITYD = $(HOST_DIR)/usr/share/pkgconfig/personality.d

 We don't use $(HOST_DIR)/usr any more, it should just be $(HOST_DIR)/share/...

 More importantly, however, there's a problem here. This path is hardcoded in
the executable, which makes it non-relocatable. We try to make the SDK
relocatable. With the wrapper script, it's OK because we use relative paths
there. But it seems that the pkgconf binary encodes a few absolute paths. I
think fixing that will require some pretty invasive modifications to pkgconf...


>  
>  define PKGCONF_LINK_PKGCONFIG
>  	ln -sf pkgconf $(TARGET_DIR)/usr/bin/pkg-config
>  endef
>  
>  define HOST_PKGCONF_INSTALL_WRAPPER
> +	$(INSTALL) -m 0775 -D package/pkgconf/target.personality \

 Why is it executable? And why writable by group?

> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
> +	$(SED) 's, at STAGING_DIR@,$(STAGING_DIR),g' \
> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality
> +	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
> +		$(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality

 Assuming it doesn't need to be executable, the three commands above can be done
with a single sed command.

	sed -e 's, at STAGING_DIR@,$(STAGING_DIR),g' \
		-e 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
		package/pkgconf/target.personality \
		> $(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality

But then you do need an explicit mkdir.

> +	ln -sf $(HOST_DIR)/bin/pkgconf \
> +		$(HOST_DIR)/bin/$(GNU_TARGET_NAME)-pkg-config
> +
>  	$(INSTALL) -m 0755 -D package/pkgconf/pkg-config.in \
>  		$(HOST_DIR)/bin/pkg-config
> -	$(SED) 's, at STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \
> +	$(SED) 's, at GNU_TARGET_NAME@,$(GNU_TARGET_NAME),g' \
>  		$(HOST_DIR)/bin/pkg-config
>  endef
>  
> diff --git a/package/pkgconf/target.personality b/package/pkgconf/target.personality
> new file mode 100644
> index 0000000000..cee4d236c4
> --- /dev/null
> +++ b/package/pkgconf/target.personality
> @@ -0,0 +1,5 @@
> +Triplet: @GNU_TARGET_NAME@
> +SysrootDir: @STAGING_DIR@
> +DefaultSearchPaths: @STAGING_DIR@/usr/lib/pkgconfig:@STAGING_DIR@/usr/share/pkgconfig
> +SystemIncludePaths: @STAGING_DIR@/usr/include
> +SystemLibraryPaths: @STAGING_DIR@/usr/lib

 Here as well, the paths are absolute... But in this case, I think they'll get
fixed up by relocate-sdk.sh.

 Regards,
 Arnout

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Thomas Preston]

On 19/10/2019 22:07, Arnout Vandecappelle wrote:
>  Hi Thomas,
> 
> On 01/10/2019 14:41, Thomas Preston wrote:
>> Buildroot does not reconfigure pkgconf system library and system include
>> dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
>> library and system include dirs instead of letting the compiler handle
>> the logical sysroot. This breaks the -isystem compiler flag, as it
>> increases the priority of the system library and system include
>> directories. For example:
>>
>> 	$ output/host/bin/pkg-config --cflags glib-2.0
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include
>>
>> A header in `.../sysroot/usr/include` will be included before a header
>> in any directory specified with -isystem flags. Specifically, this
>> breaks the Chromium build system, which expects a C++ math.h in a
>> bundled LLVM C++ library, and gets a GNU C math.h instead.
>>
>> Fix this by telling pkgconf about the sysroot's system library and
>> system include directories, so that it doesn't accidentally print them.
>>
>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>> ---
>>  package/pkgconf/pkg-config.in | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
>> index 8795f64b68..894069c492 100644
>> --- a/package/pkgconf/pkg-config.in
>> +++ b/package/pkgconf/pkg-config.in
>> @@ -1,8 +1,12 @@
>>  #!/bin/sh
>>  PKGCONFDIR=$(dirname $0)
>> +DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
>> +DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
>>  DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
>>  DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
>>  
>> -PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
>> +PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
>> +	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
> 
>  Since these variables are not overridden by HOST_CONFIGURE_OPTS, they will also
> be set for host compilation, which is wrong. However, the worst that can happen
> is that it hides some bug during host compilation, because the only thing these
> variables do is to remove staging include paths and these are anyway wrong for
> host compilation.
> 

I agree the staging include paths are wrong for host compilation, but I think 
we should override those variables using HOST_MAKE_ENV because the bug it hides
is the same bug this patch attempts to fix. 

That is, when building for the host some /usr/include headers included with -I
will take over -isystem headers.

Either way, the personality change should fix this properly. For now, I can
send a fixup for this patch, if you agree it is needed.

>  Therefore I've applied to master. With one change though: I changed the order
> of the variables to minimize the diff here (i.e. keep LIBDIR first) and to make
> it alphabetical (i.e. INCLUDE before LIBRARY, which is anyway more logical as
> well IMO).

Ok that makes sense. Thanks for taking the time to review and add this.

[Buildroot] [PATCH 2/3] pkgconf: Add pkgconf system lib and include path

[Arnout Vandecappelle]

On 21/10/2019 11:54, Thomas Preston wrote:
> On 19/10/2019 22:07, Arnout Vandecappelle wrote:
>>  Hi Thomas,
>>
>> On 01/10/2019 14:41, Thomas Preston wrote:
>>> Buildroot does not reconfigure pkgconf system library and system include
>>> dirs to STAGING_DIR. This means that pkgconf prints the sysroot system
>>> library and system include dirs instead of letting the compiler handle
>>> the logical sysroot. This breaks the -isystem compiler flag, as it
>>> increases the priority of the system library and system include
>>> directories. For example:
>>>
>>> 	$ output/host/bin/pkg-config --cflags glib-2.0
>>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include/glib-2.0
>>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/lib/glib-2.0/include
>>> 	-Ioutput/host/bin/../x86_64-buildroot-linux-gnu/sysroot/usr/include
>>>
>>> A header in `.../sysroot/usr/include` will be included before a header
>>> in any directory specified with -isystem flags. Specifically, this
>>> breaks the Chromium build system, which expects a C++ math.h in a
>>> bundled LLVM C++ library, and gets a GNU C math.h instead.
>>>
>>> Fix this by telling pkgconf about the sysroot's system library and
>>> system include directories, so that it doesn't accidentally print them.
>>>
>>> Signed-off-by: Thomas Preston <thomas.preston@codethink.co.uk>
>>> ---
>>>  package/pkgconf/pkg-config.in | 6 +++++-
>>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/package/pkgconf/pkg-config.in b/package/pkgconf/pkg-config.in
>>> index 8795f64b68..894069c492 100644
>>> --- a/package/pkgconf/pkg-config.in
>>> +++ b/package/pkgconf/pkg-config.in
>>> @@ -1,8 +1,12 @@
>>>  #!/bin/sh
>>>  PKGCONFDIR=$(dirname $0)
>>> +DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib
>>> +DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/include
>>>  DEFAULT_PKG_CONFIG_LIBDIR=${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/lib/pkgconfig:${PKGCONFDIR}/../@STAGING_SUBDIR@/usr/share/pkgconfig
>>>  DEFAULT_PKG_CONFIG_SYSROOT_DIR=${PKGCONFDIR}/../@STAGING_SUBDIR@
>>>  
>>> -PKG_CONFIG_LIBDIR=${PKG_CONFIG_LIBDIR:-${DEFAULT_PKG_CONFIG_LIBDIR}} \
>>> +PKG_CONFIG_SYSTEM_LIBRARY_PATH=${PKG_CONFIG_SYSTEM_LIBRARY_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_LIBRARY_PATH}} \
>>> +	PKG_CONFIG_SYSTEM_INCLUDE_PATH=${PKG_CONFIG_SYSTEM_INCLUDE_PATH:-${DEFAULT_PKG_CONFIG_SYSTEM_INCLUDE_PATH}} \
>>
>>  Since these variables are not overridden by HOST_CONFIGURE_OPTS, they will also
>> be set for host compilation, which is wrong. However, the worst that can happen
>> is that it hides some bug during host compilation, because the only thing these
>> variables do is to remove staging include paths and these are anyway wrong for
>> host compilation.
>>
> 
> I agree the staging include paths are wrong for host compilation, but I think 
> we should override those variables using HOST_MAKE_ENV because the bug it hides
> is the same bug this patch attempts to fix. 
> 
> That is, when building for the host some /usr/include headers included with -I
> will take over -isystem headers.

 Not really. $(HOST_DIR)/include is *not* a system include path, it really
should be passed with -I. It doesn't contain the standard library headers that
you expect to come after the -I options.


> Either way, the personality change should fix this properly. For now, I can
> send a fixup for this patch, if you agree it is needed.

 If by fixup you mean: send a new patch that adds
PKG_CONFIG_SYSTEM_LIBRARY/INCLUDE path to HOST_CONFIGURE_OPTS, then yes please
do. Note that I already applied this patch so it has to be a new patch. Make
sure you base it on current master.

 Regards,
 Arnout

>>  Therefore I've applied to master. With one change though: I changed the order
>> of the variables to minimize the diff here (i.e. keep LIBDIR first) and to make
>> it alphabetical (i.e. INCLUDE before LIBRARY, which is anyway more logical as
>> well IMO).
> 
> Ok that makes sense. Thanks for taking the time to review and add this.
>