Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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