[vlc-devel] [PATCH v2 3/5] contrib: detect the pkg-config variant to use when cross-compiling

[Alexandre Janniaux]

> > I assume it was supposed to be in `ifdef HAVE_CROSS_COMPILE`
> > as the value would already be defined anyway so this line
> > would never assign anything then.

Ok, reading is hard...

I'd still prefer that we use the given PKG_CONFIG but fallback
on the correct variant if not specified and it's working.

>
On Fri, Mar 20, 2020 at 08:22:44AM +0100, Steve Lhomme wrote:
> On 2020-03-19 17:25, Alexandre Janniaux wrote:
> > Hi,
> >
> > On Thu, Mar 19, 2020 at 05:02:29PM +0100, Steve Lhomme wrote:
> > > On 2020-03-19 16:34, Alexandre Janniaux wrote:
> > > > Hi,
> > > >
> > > > On Wed, Mar 18, 2020 at 09:46:14AM +0100, Steve Lhomme wrote:
> > > > > And use it for all contribs detection and building.
> > > > >
> > > > > on Debian, the x86_64-w64-mingw32-pkg-config may exist and run but give an
> > > > > error when actually trying to find a package because the package architecture
> > > > > doesn't exist for that target. So we have to test it thoroughly.
> > > > >
> > > > > If we revert to pkg-config when cross-compiling we also force the
> > > > > PKG_CONFIG_LIBDIR, otherwise we leave it untouched as the system/environment may
> > > > > provide more that we know.
> > > > >
> > > > > Both /usr/$(HOST)/lib/pkgconfig and /usr/lib/$(HOST)/pkgconfig variants exist,
> > > > > at least on Debian.
> > > > > ---
> > > > >    contrib/src/main.mak | 20 +++++++++++++++++---
> > > > >    1 file changed, 17 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/contrib/src/main.mak b/contrib/src/main.mak
> > > > > index 555d96a4b42..bff9cd82413 100644
> > > > > --- a/contrib/src/main.mak
> > > > > +++ b/contrib/src/main.mak
> > > > > @@ -82,6 +82,7 @@ RANLIB ?= ranlib
> > > > >    STRIP ?= strip
> > > > >    WIDL ?= widl
> > > > >    WINDRES ?= windres
> > > > > +PKG_CONFIG ?= pkg-config
> > > > >    else
> > > > >    ifneq ($(findstring $(origin CC),undefined default),)
> > > > >    CC := $(HOST)-gcc
> > > > > @@ -99,6 +100,22 @@ RANLIB ?= $(HOST)-ranlib
> > > > >    STRIP ?= $(HOST)-strip
> > > > >    WIDL ?= $(HOST)-widl
> > > > >    WINDRES ?= $(HOST)-windres
> > > > > +
> > > > > +# On Debian x86_64-w64-mingw32-pkg-config exists, runs but returns an error when checking packages
> > > > > +ifeq ($(shell unset PKG_CONFIG_LIBDIR; $(HOST)-pkg-config --version 1>/dev/null 2>/dev/null || echo FAIL),)
> > > > > +PKG_CONFIG ?= $(HOST)-pkg-config
> > > > > +else
> > > > > +# Use the regular pkg-config and set some PKG_CONFIG_LIBDIR ourselves
> > > > > +PKG_CONFIG = pkg-config
> > > >
> > > > The PKG_CONFIG assignment above is missing an additional ?= right?
> > > > Otherwise there is no point in putting a ?= above.
> > >
> > > No in the former case the HOST-pkg-config is tested but the user is allowed
> > > to change the value.
> > > In the latter case we force the PKG_CONFIG_LIBDIR and so we don't want to
> > > use whatever the user provided that might mess with what we're doing.
> >
> > This seems wrong to me.
> >
> > On the one hand, you're basically allowing pkg-config to be
> > overridden when not in cross compilation, in the situation
> > where there is almost no point in changing pkg-config.
> >
> > https://github.com/videolan/vlc/blob/master/contrib/src/main.mak#L77
> >
> > On the other hand, here, you check for $(HOST)-pkg-config,
> > mentioning the following text:
> >
> > # On Debian x86_64-w64-mingw32-pkg-config exists, runs but returns an error when checking packages
> > ifeq ($(shell unset PKG_CONFIG_LIBDIR; $(HOST)-pkg-config --version 1>/dev/null 2>/dev/null || echo FAIL),)
> > PKG_CONFIG ?= $(HOST)-pkg-config
> >
> > But it still in the `ifndef HAVE_CROSS_COMPILE` so you're
> > basically always setting PKG_CONFIG to pkg-config on
> > non-cross-compilation builds.
> >
> > I assume it was supposed to be in `ifdef HAVE_CROSS_COMPILE`
> > as the value would already be defined anyway so this line
> > would never assign anything then.
>
> Nope. The comment gives a hint. The HOST-pkg-config on Debian (dunno about
> others) is broken in intricate ways. Going back to pkg-config is the way to
> go. To do that you just need to tweak the PKG_CONFIG_LIBDIR to use the same
> cross compilation environment HOST-pkg-config would use.
>
> In the end HOST-pkg-config is always pkg-config with some environment set
> for a particular platform. The regular pkg-config can be made to perform
> exactly the same. And that's what this patch does. (it will never be
> universal and match all system specific ways but it's close enough).
>
> > And then when talking albout PKG_CONFIG_LIBDIR:
> >
> > > In the latter case we force the PKG_CONFIG_LIBDIR and so we don't want to
> > > use whatever the user provided that might mess with what we're doing.
> >
> > It feels like saying "the user might provide an invalid pkg-config
> > executable so prevent him to do that". But then why would you prevent
> > the user to set CC/CXX for example?
>
> Because the user is probably not aware that we are going to mess with the
> PKG_CONFIG_LIBDIR when (s)he provides a pkg-config. It's usually only
> PKG_CONFIG_PATH that is changed for a particular project.
>
> > In addition, it means that if there is a $(HOST)-pkg-config in the
> > path, we could provide an invalid PKG_CONFIG value, but when there
> > is not it would be forbidden, which is more confusing than expecting
> > the buildsystem to fail if the user pkg-config is invalid.
>
> Do you mean we should also test that plain pkg-config is usable too ?
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel