Fix the CLD2 GN build on the Mac.

Fix the CLD2 GN build on the Mac.

R=andrewhayden@chromium.org, brettw@chromium.org
BUG=

Committed: https://crrev.com/eef4664f84cd06acc4d99e629a3f4eefb3df4367
Cr-Commit-Position: refs/heads/master@{#303842}

[Dirk Pranke, 2014-11-12 02:46:04]

https://codereview.chromium.org/718943002/diff/1/third_party/cld_2/BUILD.gn
File third_party/cld_2/BUILD.gn (right):

https://codereview.chromium.org/718943002/diff/1/third_party/cld_2/BUILD.gn#n...
third_party/cld_2/BUILD.gn:95: source_set("cld_2") {
Both this target and the one below have no actual object files. On the mac, this
isn't allowed (libtool explodes).

In GYP, we work around this in the generator by checking to see if there are any
input object files, and if there aren't, we set the target to type 'none'
instead of type 'static library'.

This, however, seems like a misfeature.

Changing this to a source set works, but seems like it might also be the wrong
way to do things. We could change this to a 'group', but groups probably can't
have 'sources' (which in this case only include headers).

Brett, what do you think the right thing to do here is?

[brettw, 2014-11-12 05:30:58]

this looks good to me. LGTM

[Andrew Hayden (chromium.org), 2014-11-12 10:08:52]

On 2014/11/12 05:30:58, brettw wrote:
> this looks good to me. LGTM

Seems fine to me as well. Sorry about breaking the mac GN bot. I got all-green
from the CQ, so I don't understand what I missed here.

This file has drifted out of sync slightly with the GYP file; I'll re-sync them
shortly. The cld2_platform_impl target needs to be parameterized; see
"cld2_platform_support" variable definition comments in cld2.gyp. If I manage to
get the review ready before you're ready to land this, we can slurp both my
change and this fix at once. I'll see to it now...

[Dirk Pranke, 2014-11-12 15:50:46]

On 2014/11/12 10:08:52, Andrew Hayden wrote:
> On 2014/11/12 05:30:58, brettw wrote:
> > this looks good to me. LGTM
> 
> Seems fine to me as well. Sorry about breaking the mac GN bot. I got all-green
> from the CQ, so I don't understand what I missed here.

You didn't miss anything; the mac bots are not yet on the CQ or the waterfall.
That
will hopefully be fixed soon, but in the meantime these sorts of breakages are
expected.

[Dirk Pranke, 2014-11-12 15:51:01]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-11-12 15:52:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/718943002/1

[Andrew Hayden (chromium.org), 2014-11-12 16:02:53]

On 2014/11/12 15:52:33, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/718943002/1

LGTM as well :P thanks! I've got a patch almost ready (turned out to be a bit
harder than expected due to a bug in the GYP file, and I'm trying to use
gypi_to_gn now). But I'll just merge this with mine. Thank you for the fix!

[Dirk Pranke, 2014-11-12 16:08:34]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2014-11-12 16:09:26]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2014-11-12 16:09:29]

On 2014/11/12 16:02:53, Andrew Hayden wrote:
> On 2014/11/12 15:52:33, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >  https://chromium-cq-status.appspot.com/patch-status/718943002/1
> 
> LGTM as well :P thanks! I've got a patch almost ready (turned out to be a bit
> harder than expected due to a bug in the GYP file, and I'm trying to use
> gypi_to_gn now). But I'll just merge this with mine. Thank you for the fix!

Oh, sorry for the collision, I didn't read your reply properly :(. That's what I
get for
trying to do work before I'm fully awake ...

[commit-bot: I haz the power, 2014-11-12 16:10:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/718943002/1

[commit-bot: I haz the power, 2014-11-12 16:45:07]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2014-11-12 16:46:23]

Patchset 1 (id:??) landed as
https://crrev.com/eef4664f84cd06acc4d99e629a3f4eefb3df4367
Cr-Commit-Position: refs/heads/master@{#303842}