From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH-FOR-NEXT v1 3/6] pkgconf: add host-pkg-config wrapper
Date: Thu, 22 Feb 2018 10:41:46 +0100 [thread overview]
Message-ID: <20180222104146.5d4efe6e@windsurf.lan> (raw)
In-Reply-To: <0e296fe2-3359-581f-6d3d-d3bc3cac06eb@mind.be>
Hello,
Thanks for the feedback!
On Thu, 22 Feb 2018 10:27:13 +0100, Arnout Vandecappelle wrote:
> > I've been thinking about this for a while, and I remember having a
> > discussion about this with some other Buildroot developer a while ago.
> > I think the most correct thing to do would be:
> >
> > $(HOST_DIR)/bin/pkg-config returns results valid for native compilation
> >
> > $(HOST_DIR)/bin/<tuple>-pkg-config returns results valid for cross-compilation
>
> This sounds like a good idea, but as you note below, could lead to breakage as
> well.
Yes.
>
> An alternative would be (IIRC this idea was launched by ThomasDS (added in Cc)):
>
> $(HOST_DIR)/bin/ contains binaries valid for native compilation (i.e. no
> cross-compiler, cross-pkg-config, ...)
>
> $(HOST_DIR)/<tuple>/bin contains binaries valid for cross-compilation
Changing this will also lead to a lot of breakage. All our users who
have scripts doing $(HOST_DIR)/bin/<tuple>-gcc will be broken. I would
even qualify it as an even more radical change, breaking even more
stuff.
And more importantly, the result is not great. I think the most correct
result is to have a single HOST_DIR/bin, with both native and cross
compilation tools, with cross-compilation tools prefixed by <tuple>.
> > I.e, the current pkg-config wrapper should be renamed
> > <tuple>-pkg-config, and pkg-config should behave like a normal native
> > pkg-config, except that it provides results for libraries located in
> > $(HOST_DIR).
> >
> > The autoconf PKG_CHECK_MODULES() macro uses PKG_PROG_PKG_CONFIG(),
> > which internally uses AC_PATH_TOOL(). And AC_PATH_TOOL() will first
> > search for the program with the host machine tuple, and warn if the
> > program cannot be found with this tuple. From
> > https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/Generic-Programs.html:
> >
> > """
> > When cross-compiling, this macro will issue a warning if no program
> > prefixed with the host type could be found. For more information, see
> > Specifying Target Triplets.
>
> Weird, I checked a couple of log files and I could see no such warning. Ah,
> that's because we pass PKG_CONFIG=... in the environment.
Yes, I believe it is because we pass PKG_CONFIG=.
> Is it the idea to remove that from the environment then?
Not necessarily, because PKG_CONFIG may be used by non-autoconf based
packages. Some non-autotools packages do:
FOO=`$PKG_CONFIG ...`
> Anyway, the PATH-based alternative will not remove this warning, but that
> shouldn't be different from the current situation.
>
> > """
> >
> > I know this change will:
> >
> > - Potentially break a number of packages we have in Buildroot, which
> > directly use pkg-config without first trying to use
> > <tuple>-pkg-config
>
> I.e. anything not using autotools? Well, most will probably heed the
> PKG_CONFIG=... in the environment.
Most yes, but not all. Some of them do:
FOO=`pkg-config ...`
which currently works because PATH contains $(HOST_DIR)/bin, and
pkg-config returns correct results thanks to us injecting the proper
PKG_CONFIG_LIBDIR and PKG_CONFIG_SYSROOT_DIR depending on whether we're
building a host or target package.
> The PATH-based alternative doesn't have this potential breakage.
>
>
> > - Potentially break a number of downstream users who are using
> > pkg-config.
>
> The PATH-based alternative reduces this problem, downstream users just have to
> add $(HOST_DIR)/tuple/bin to their path (and Buildroot will do this for post-xxx
> scripts).
Well, they still have to change to $(HOST_DIR)/tuple/bin, which means
it breaks stuff for them anyway.
> > However:
> >
> > - It would solve that once you add $(HOST_DIR)/bin to your PATH, you
> > cannot anymore do native builds because "pkg-config" returns results
> > that are not relevant for native builds. I already saw a number of
> > people affected by this.
>
> The PATH-based alternative solves this as well.
>
> Note BTW that neither alternative solves the problem when building a host-tool
> during a target build (and yes, we have seen this problem already in some
> packages). AFAICS, autotools will also for host builds use the PKG_CONFIG passed
> in the environment or discovered through the tuple. Same for CMake.
Potentially autotools could be taught about PKG_CONFIG_FOR_BUILD, just
like it does for CC_FOR_BUILD and al. But that's obviously not
supported by packages today.
> > - It would comply with the standard autoconf expectations.
>
> With the PATH-based alternative, it might make sense to have the cross-stuff
> both in $(HOST_DIR)/bin with the tuple prefix, and in $(HOST_DIR)/tuple/bin
> without the prefix. That way, we get the advantages of both: comply with
> autoconf expectations, and avoid breaking packages or downstream users.
I'm still not convinced about changing the HOST_DIR/ organization. It's
a massive change, affecting everything, and not just pkg-config, and
the outcome is less nice than what we have today, for my perspective.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
next prev parent reply other threads:[~2018-02-22 9:41 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-21 14:27 [Buildroot] [PATCH-FOR-NEXT v1 0/6] Qt5 bump latest version to 5.10.1 Gaël PORTAY
2018-02-21 14:27 ` [Buildroot] [PATCH-FOR-NEXT v1 1/6] package: add libnpsr host package Gaël PORTAY
2018-02-21 21:27 ` Thomas Petazzoni
2018-02-22 14:29 ` Gaël PORTAY
2018-02-22 16:38 ` Baruch Siach
2018-02-21 14:27 ` [Buildroot] [PATCH-FOR-NEXT v1 2/6] package: add libnss " Gaël PORTAY
2018-02-21 21:32 ` Thomas Petazzoni
2018-02-21 14:27 ` [Buildroot] [PATCH-FOR-NEXT v1 3/6] pkgconf: add host-pkg-config wrapper Gaël PORTAY
2018-02-21 21:50 ` Thomas Petazzoni
2018-02-22 9:27 ` Arnout Vandecappelle
2018-02-22 9:41 ` Thomas Petazzoni [this message]
2018-02-22 10:56 ` Arnout Vandecappelle
2018-02-25 20:38 ` Thomas De Schampheleire
2018-02-25 20:53 ` Yann E. MORIN
2018-02-25 21:56 ` Thomas De Schampheleire
2018-02-21 14:27 ` [Buildroot] [PATCH-FOR-NEXT v1 4/6] qt5: bump latest version to 5.10.1 Gaël PORTAY
2018-02-21 21:56 ` Thomas Petazzoni
2018-02-23 18:32 ` Peter Seiderer
2018-02-21 14:28 ` [Buildroot] [PATCH-FOR-NEXT v1 5/6] qt5webengine: satisfy new requirements for 5.10 Gaël PORTAY
2018-02-21 21:58 ` Thomas Petazzoni
2018-02-22 14:42 ` Gaël PORTAY
2018-02-21 14:28 ` [Buildroot] [PATCH-FOR-NEXT v1 6/6] qt5webengine: set ninja host pkg-config tool Gaël PORTAY
2018-02-21 21:59 ` Thomas Petazzoni
2018-02-22 14:44 ` Gaël PORTAY
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180222104146.5d4efe6e@windsurf.lan \
--to=thomas.petazzoni@bootlin.com \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox