Bug 107984 – [chromium] Don't use goma to preprocess bindings idl files

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Chang]

[chromium] Don't use goma to preprocess bindings idl files

[Tony Chang]

Created attachment 184811 [details]
Patch

[Antoine Labour]

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.

[Tony Chang]

(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.

[Tony Chang]

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.

[Nico Weber]

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?

[Tony Chang]

Created attachment 184821 [details]
Patch

[Tony Chang]

(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.

[Nico Weber]

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.

[Tony Chang]

Created attachment 184827 [details]
Patch

[Nico Weber]

lgtm

[Adam Barth]

Comment on attachment 184827 [details]
Patch

I guess it falls to me to mark this r.

[WebKit Review Bot]

Comment on attachment 184827 [details]
Patch

Clearing flags on attachment: 184827

Committed r140925: <http://trac.webkit.org/changeset/140925>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[sideshowbarker]

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.