Bug 107984 - [chromium] Don't use goma to preprocess bindings idl files
Summary: [chromium] Don't use goma to preprocess bindings idl files
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tony Chang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-25 14:31 PST by Tony Chang
Modified: 2013-01-28 09:38 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.62 KB, patch)
2013-01-25 14:34 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (1.82 KB, patch)
2013-01-25 15:40 PST, Tony Chang
no flags Details | Formatted Diff | Diff
Patch (1.82 KB, patch)
2013-01-25 15:50 PST, Tony Chang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Chang 2013-01-25 14:31:36 PST
[chromium] Don't use goma to preprocess bindings idl files
Comment 1 Tony Chang 2013-01-25 14:34:23 PST
Created attachment 184811 [details]
Patch
Comment 2 Antoine Labour 2013-01-25 14:37:21 PST
I'm cool with this, just wondering about weird build environments (a la Chrome OS chroot, though that one looks ok, or Android, that I don't know about), but maybe we already depend on /usr/bin/gcc for something else.
Comment 3 Tony Chang 2013-01-25 14:39:07 PST
(In reply to comment #2)
> I'm cool with this, just wondering about weird build environments (a la Chrome OS chroot, though that one looks ok, or Android, that I don't know about), but maybe we already depend on /usr/bin/gcc for something else.

Ah, I didn't think about Android.  Let's see what the EWS bot thinks.
Comment 4 Tony Chang 2013-01-25 15:20:20 PST
Over email, Nico asks:

> Doesn't it make more sense to let goma fall back to local if it sees a -E parameter? That's what I proposed to the goma folks. (Also, in clang builds, the script should probably use clang?)

Making goma smarter sounds great to me.

I don't think it matters much whether we use clang or gcc for preprocessing idl files.  For example, the Qt port uses moc.
Comment 5 Nico Weber 2013-01-25 15:22:17 PST
Before-collision text:

I think a nicer fix is to let goma fall back to local if it sees -E transparently.

On OS X, /usr/bin/gcc will eventually disappear. Is it possible to set this to the clang binary if clang==1 is set?
Comment 6 Tony Chang 2013-01-25 15:40:50 PST
Created attachment 184821 [details]
Patch
Comment 7 Tony Chang 2013-01-25 15:41:49 PST
(In reply to comment #5)
> I think a nicer fix is to let goma fall back to local if it sees -E transparently.

Yeah, we should rollout after a goma fix is pushed.

> On OS X, /usr/bin/gcc will eventually disappear. Is it possible to set this to the clang binary if clang==1 is set?

Sure.
Comment 8 Nico Weber 2013-01-25 15:46:44 PST
Comment on attachment 184821 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184821&action=review

lgtm

> Source/WebCore/WebCore.gyp/WebCore.gyp:245
> +          ['clang==1 and OS=="mac"', {

nit: This is redundant, clang is always set for OS=="mac". Probably want this for OS="ios" too.

This uses the system clang instead of "our" clang, but for preprocessing I suppose that's cool.
Comment 9 Tony Chang 2013-01-25 15:50:46 PST
Created attachment 184827 [details]
Patch
Comment 10 Nico Weber 2013-01-25 15:51:45 PST
lgtm
Comment 11 Adam Barth 2013-01-26 00:03:21 PST
Comment on attachment 184827 [details]
Patch

I guess it falls to me to mark this r.
Comment 12 WebKit Review Bot 2013-01-26 22:41:09 PST
Comment on attachment 184827 [details]
Patch

Clearing flags on attachment: 184827

Committed r140925: <http://trac.webkit.org/changeset/140925>
Comment 13 WebKit Review Bot 2013-01-26 22:41:13 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Michael[tm] Smith 2013-01-28 09:38:34 PST
Please see bug 108089.

In my environment OSX 10.6.8 environment, the hard-coding of the /usr/bin/clang path from this bug causes the build step that preprocesses the bindings idl files to generate hundreds of "clang: warning: not using the clang compiler for C++ inputs" messages.