Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1530723002:
GN: Don't default to using lib64 in PKG_CONFIG_PATH (Closed)
DescriptionGN: Don't default to using lib64 in PKG_CONFIG_PATH
This change makes gn's pkg-config.py match more
closely the behavior of pkg-config-wrapper.
pkg-config-wrapper is always run with the third
argument set to '<(system_libdir)', which defaults to
'lib', which means that gyp never using lib64 in
practice. Even though system_libdir is supported in
gyp, there is currently no equivalent that I can find
in gn, so I left a TODO.
This change is required before we can land the sysroot
change that populates /usr/lib/pkgconfig:
https://codereview.chromium.org/1531513002
BUG=569947
Committed: https://crrev.com/b05b60bf32ce6f01253888ad0a0d1e0cbdac84b0
Cr-Commit-Position: refs/heads/master@{#365415}
Patch Set 1 #Patch Set 2 : #
Messages
Total messages: 28 (11 generated)
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH BUG= ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to 'system_libdir', which defaults to 'lib', which means that the libpath was never set to lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required https://codereview.chromium.org/1531513002 BUG=569947 ==========
sbc@chromium.org changed reviewers: + dpranke@chromium.org
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to 'system_libdir', which defaults to 'lib', which means that the libpath was never set to lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required https://codereview.chromium.org/1531513002 BUG=569947 ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required https://codereview.chromium.org/1531513002 BUG=569947 ==========
The CQ bit was checked by sbc@chromium.org to run a CQ dry run
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required https://codereview.chromium.org/1531513002 BUG=569947 ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002, BUG=569947 ==========
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530723002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530723002/20001
dpranke@chromium.org changed reviewers: + thestig@chromium.org
lgtm, though I'm not sure I understand the implications of this properly, so I'd like thestig@ to look at it as well.
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002, BUG=569947 ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002 BUG=569947 ==========
Does anyone ever set the gyp system_libdir variable?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
It looks as though build/common.gypi is the only pkg-config-wrapper caller that passes in a $system_libdir. The other users, e.g. webrtc, do not. So they default to the code being removed.
On 2015/12/15 22:56:06, Lei Zhang wrote: > Does anyone ever set the gyp system_libdir variable? Seem very unlikely
On 2015/12/15 23:00:51, Lei Zhang wrote: > It looks as though build/common.gypi is the only pkg-config-wrapper caller that > passes in a $system_libdir. The other users, e.g. webrtc, do not. So they > default to the code being removed. Well.. the default "lib64" doesn't exist in the sysroot. Do you think that webrtc needs the lib64 default?
On 2015/12/15 23:12:28, Sam Clegg wrote: > On 2015/12/15 23:00:51, Lei Zhang wrote: > > It looks as though build/common.gypi is the only pkg-config-wrapper caller > that > > passes in a $system_libdir. The other users, e.g. webrtc, do not. So they > > default to the code being removed. > > Well.. the default "lib64" doesn't exist in the sysroot. > > Do you think that webrtc needs the lib64 default? I guess as an alternative we could introduce a new gn variable that defaults to 'lib' so that default would be overidden in the same way that it is today in gyp? That would be the more conservative change I guess, but only if people actually would want to use that variable.
On 2015/12/15 23:31:15, Sam Clegg wrote: > On 2015/12/15 23:12:28, Sam Clegg wrote: > > On 2015/12/15 23:00:51, Lei Zhang wrote: > > > It looks as though build/common.gypi is the only pkg-config-wrapper caller > > that > > > passes in a $system_libdir. The other users, e.g. webrtc, do not. So they > > > default to the code being removed. > > > > Well.. the default "lib64" doesn't exist in the sysroot. > > > > Do you think that webrtc needs the lib64 default? > > I guess as an alternative we could introduce a new gn variable that defaults to > 'lib' so that default would be overidden in the same way that it is today in > gyp? That would be the more conservative change I guess, but only if people > actually would want to use that variable. The general approach we've taken with the GN migration is to only add complexity as needed (or as we're sure we will need it); it sounds like in this case we're not sure if it's needed yet, so I'd prefer to not add it. Make sense?
On 2015/12/15 23:12:28, Sam Clegg wrote:
> On 2015/12/15 23:00:51, Lei Zhang wrote:
> > It looks as though build/common.gypi is the only pkg-config-wrapper caller
> that
> > passes in a $system_libdir. The other users, e.g. webrtc, do not. So they
> > default to the code being removed.
>
> Well.. the default "lib64" doesn't exist in the sysroot.
>
> Do you think that webrtc needs the lib64 default?
Well, at least locally, in the sysroots there's no usr/lib/pkgconfig or
usr/lib64/pkgconfig. What I do have is
usr/lib/{x86_64,i386}-linux-gnu/pkgconfig, and usr/share/pkgconfig, which is a
symlink to the former.
On 2015/12/16 01:03:30, Lei Zhang wrote:
> On 2015/12/15 23:12:28, Sam Clegg wrote:
> > On 2015/12/15 23:00:51, Lei Zhang wrote:
> > > It looks as though build/common.gypi is the only pkg-config-wrapper caller
> > that
> > > passes in a $system_libdir. The other users, e.g. webrtc, do not. So they
> > > default to the code being removed.
> >
> > Well.. the default "lib64" doesn't exist in the sysroot.
> >
> > Do you think that webrtc needs the lib64 default?
>
> Well, at least locally, in the sysroots there's no usr/lib/pkgconfig or
> usr/lib64/pkgconfig. What I do have is
> usr/lib/{x86_64,i386}-linux-gnu/pkgconfig, and usr/share/pkgconfig, which is a
> symlink to the former.
Thats what the other change does:
https://codereview.chromium.org/1531513002
It moves usr/lib/{x86_64,i386}-linux-gnu/pkgconfig/* to usr/lib/pkgconfig/*
This makes the sysroots less "multi-arch", but easier to work with.
I'm not 100% happy with this solution, but it fixes the problem at hand. There
are alternatives, but AFAICT they would involve more complexity in the gn build
(adding the new system_libdir setting), or more complexity in the pkg-config
wrapper (attempting to auto-detect the correct pkg-config dir).
Thoughts?
On 2015/12/16 01:26:02, Sam Clegg wrote:
> On 2015/12/16 01:03:30, Lei Zhang wrote:
> > On 2015/12/15 23:12:28, Sam Clegg wrote:
> > > On 2015/12/15 23:00:51, Lei Zhang wrote:
> > > > It looks as though build/common.gypi is the only pkg-config-wrapper
caller
> > > that
> > > > passes in a $system_libdir. The other users, e.g. webrtc, do not. So
they
> > > > default to the code being removed.
> > >
> > > Well.. the default "lib64" doesn't exist in the sysroot.
> > >
> > > Do you think that webrtc needs the lib64 default?
> >
> > Well, at least locally, in the sysroots there's no usr/lib/pkgconfig or
> > usr/lib64/pkgconfig. What I do have is
> > usr/lib/{x86_64,i386}-linux-gnu/pkgconfig, and usr/share/pkgconfig, which is
a
> > symlink to the former.
>
> Thats what the other change does:
> https://codereview.chromium.org/1531513002
>
> It moves usr/lib/{x86_64,i386}-linux-gnu/pkgconfig/* to usr/lib/pkgconfig/*
> This makes the sysroots less "multi-arch", but easier to work with.
>
> I'm not 100% happy with this solution, but it fixes the problem at hand.
There
> are alternatives, but AFAICT they would involve more complexity in the gn
build
> (adding the new system_libdir setting), or more complexity in the pkg-config
> wrapper (attempting to auto-detect the correct pkg-config dir).
>
> Thoughts?
Ah, sure. That works for me.
lgtm
The CQ bit was checked by sbc@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1530723002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1530723002/20001
Message was sent while issue was closed.
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002 BUG=569947 ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002 BUG=569947 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002 BUG=569947 ========== to ========== GN: Don't default to using lib64 in PKG_CONFIG_PATH This change makes gn's pkg-config.py match more closely the behavior of pkg-config-wrapper. pkg-config-wrapper is always run with the third argument set to '<(system_libdir)', which defaults to 'lib', which means that gyp never using lib64 in practice. Even though system_libdir is supported in gyp, there is currently no equivalent that I can find in gn, so I left a TODO. This change is required before we can land the sysroot change that populates /usr/lib/pkgconfig: https://codereview.chromium.org/1531513002 BUG=569947 Committed: https://crrev.com/b05b60bf32ce6f01253888ad0a0d1e0cbdac84b0 Cr-Commit-Position: refs/heads/master@{#365415} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b05b60bf32ce6f01253888ad0a0d1e0cbdac84b0 Cr-Commit-Position: refs/heads/master@{#365415} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
