| Message ID | 20191001124132.107700-3-thomas.preston@codethink.co.uk |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] pkgconf: Split pkgconf command into multi-line | expand |
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@-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.
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@-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@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
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@-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.
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@-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
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
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
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@. 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@-pkg-config @STATIC@ "$@" As long as the @STATIC@ is needed, I wouldn't create @GNU_TARGET_NAME@-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,@STAGING_DIR@,$(STAGING_DIR),g' \ > + $(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality > + $(SED) 's,@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,@STAGING_DIR@,$(STAGING_DIR),g' \ -e 's,@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,@STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \ > + $(SED) 's,@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
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@. 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@-pkg-config @STATIC@ "$@" As long as the @STATIC@ is needed, I wouldn't create @GNU_TARGET_NAME@-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,@STAGING_DIR@,$(STAGING_DIR),g' \ > + $(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality > + $(SED) 's,@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,@STAGING_DIR@,$(STAGING_DIR),g' \ -e 's,@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,@STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \ > + $(SED) 's,@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
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@-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,@STAGING_DIR@,$(STAGING_DIR),g' \ + $(PKG_CONFIG_HOST_PERSONALITYD)/$(GNU_TARGET_NAME).personality + $(SED) 's,@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,@STAGING_SUBDIR@,$(STAGING_SUBDIR),g' \ + $(SED) 's,@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
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