Use gyp targets to link in X11 libraries instead of setting -lXfoo

Use gyp targets to link in X11 libraries instead of setting -lXfoo

This sets up gyp targets for the various X11 libraries we use and uses
pkg-config to determine the correct cflags/ldflags incantations to use to
link against them instead of listing out -lXfoo lines in libraries sections
in gyp targets throughout the tree.  This is more likely to generate the right
lines and makes the dependencies a bit more explicit.

BUG=339518
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248939

[jamesr, 2014-02-04 00:07:47]

This tries to sort out the X11 dependencies / link lines a bit by declaring
dependencies on a gyp target to link against X11 libraries instead of more
directly manipulating the link line.  Many targets are depending on
ui/ui.gyp:ui's all_dependent_settings line that adds link lines - those will
have to be cleaned up in a follow-up patch to avoid this getting too big.

One thing I don't understand is that pkg-config doesn't seem to know anything
about Xss, although it definitely needs to be on the link line for some targets.

[Elliot Glaysher, 2014-02-04 00:20:26]

lgtm

[piman, 2014-02-04 00:25:13]

LGTM+nits

https://codereview.chromium.org/153223002/diff/30001/build/linux/system.gyp
File build/linux/system.gyp (right):

https://codereview.chromium.org/153223002/diff/30001/build/linux/system.gyp#n...
build/linux/system.gyp:801: ['_toolset=="target"', {
nit: you don't need that conditional if you don't declare multiple toolsets.
(same with other ones).

https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp
File ui/ui.gyp (right):

https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp#newcode537
ui/ui.gyp:537: '-lXrender',
nit: won't all these be inherited now, since they're in dependencies?

[jamesr, 2014-02-04 00:27:14]

https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp
File ui/ui.gyp (right):

https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp#newcode537
ui/ui.gyp:537: '-lXrender',
On 2014/02/04 00:25:14, piman wrote:
> nit: won't all these be inherited now, since they're in dependencies?

Without this all_dependent_settings entries many targets (mostly unit tests)
that link against libui.so but do not declare a dependency directly on
ui/ui.gyp:ui do not have the -lXrender/etc lines on their final link lines and
thus fail to link.  I think the right fix is for these targets to either declare
dependencies on ui.gyp:ui or (if they use X11 directly) on the relevant X11
targets, but I haven't chased them all down yet.

[jamesr, 2014-02-04 00:27:43]

Note that I'm somewhat guessing about the link_settings / all_dependent_settings
interactions based on observed behavior - what gyp actually does is confusing.

[piman, 2014-02-04 00:38:44]

On Mon, Feb 3, 2014 at 4:27 PM, <jamesr@chromium.org> wrote:

>
> https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp
> File ui/ui.gyp (right):
>
> https://codereview.chromium.org/153223002/diff/30001/ui/ui.gyp#newcode537
> ui/ui.gyp:537: '-lXrender',
> On 2014/02/04 00:25:14, piman wrote:
>
>> nit: won't all these be inherited now, since they're in dependencies?
>>
>
> Without this all_dependent_settings entries many targets (mostly unit
> tests) that link against libui.so but do not declare a dependency
> directly on ui/ui.gyp:ui


How do they link against libui.so without depending on ui/ui.gyp:ui ?


> do not have the -lXrender/etc lines on their
> final link lines and thus fail to link.  I think the right fix is for
> these targets to either declare dependencies on ui.gyp:ui or (if they
> use X11 directly) on the relevant X11 targets, but I haven't chased them
> all down yet.
>

Ok, I mean, LGTM to make progress on that either way, just wondering how it
fails.


>
> https://codereview.chromium.org/153223002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jamesr, 2014-02-04 00:59:57]

On 2014/02/04 00:38:44, piman wrote:
> > Without this all_dependent_settings entries many targets (mostly unit
> > tests) that link against libui.so but do not declare a dependency
> > directly on ui/ui.gyp:ui
> 
> 
> How do they link against libui.so without depending on ui/ui.gyp:ui ?

They do depend transitively on libui.so, but not directly.  I don't fully
understand how link_settings are supposed to propagate through gyp so I'm not
sure exactly how the thing propagates.  Look forward to this fix in a patch
coming soon!

[jamesr, 2014-02-04 23:00:40]

I think this is as green as it's gonna be on the trybots - it's
compiling+linking everywhere, which is the important bit.

ben@ - could you review ui/ and give OWNERS coverage for the gyp files
elsewhere? I don't think it's worth tracking down individual owners for each.

[Sergey Ulanov, 2014-02-05 00:33:19]

remoting LGTM

[Ben Goodger (Google), 2014-02-05 05:17:36]

lgtm

[jamesr, 2014-02-05 05:51:28]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-02-05 05:54:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/153223002/160001

[commit-bot: I haz the power, 2014-02-05 09:09:35]

Change committed as 248939