Bug 62399 – [chromium] make features.gypi the same as features_override.gypi

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

[Tony Chang]

[chromium] make features.gypi the same as features_override.gypi

[Tony Chang]

Created attachment 96630 [details]
Patch

[Tony Chang]

This removes the nesting in features.gypi.  Instead, the two feature_defines lists are now merged together.  As far as I can tell, duplicate values are removed so we don't get -D listed twice on the command line.

I tried to find the code in gyp that's removing the duplicates, but I couldn't seem to find it.  Maybe Mark knows where that is so I can be more confident?

[James Robinson]

Comment on attachment 96630 [details]
Patch

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

> Source/WebKit/chromium/features.gypi:51
> +      'ENABLE_FULLSCREEN_API=1',

you need to remove this too - see https://bugs.webkit.org/show_bug.cgi?id=62397

[Mark Mentovai]

Tony, look for the word “singleton” in input.py.

[Tony Chang]

Created attachment 96639 [details]
Patch

[Tony Chang]

Removed ENABLE_FULLSCREEN_API and found singleton in the gyp source :)  We remove duplicate values that don't start with a -.

[WebKit Review Bot]

Comment on attachment 96639 [details]
Patch

Attachment 96639 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8810060

[James Robinson]

This should compile after http://trac.webkit.org/changeset/88490

[James Robinson]

Comment on attachment 96639 [details]
Patch

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

> Source/WebKit/chromium/features.gypi:158
> +# Local Variables:

did you mean to check this section in?

[Tony Chang]

Comment on attachment 96639 [details]
Patch

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

>> Source/WebKit/chromium/features.gypi:158
>> +# Local Variables:
> 
> did you mean to check this section in?

It's in features_override.gypi and I just copied the whole file over.  I don't care about it one way or the other.  We should probably decide whether or not we need this and update all our .gyp(i) files.

[James Robinson]

I don't know what it is or what it does

[Tony Chang]

(In reply to comment #11)
> I don't know what it is or what it does

Oh, it's for vim.  It lets vim users get the proper indenting and tab behavior when they open the file (I think this is normally done based on file extension?).  I'm not actually a vim user, so I don't care :)

[WebKit Review Bot]

Comment on attachment 96639 [details]
Patch

Attachment 96639 [details] did not pass cr-mac-ews (chromium):
Output: http://queues.webkit.org/results/8826214

[Tony Chang]

Created attachment 96753 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 96753 [details]
Patch

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

> Source/WebKit/chromium/features.gypi:162
> +# Local Variables:
> +# tab-width:2
> +# indent-tabs-mode:nil
> +# End:
> +# vim: set expandtab tabstop=2 shiftwidth=2:

What's that?!

[Tony Chang]

(In reply to comment #15)
> (From update of attachment 96753 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=96753&action=review
> 
> > Source/WebKit/chromium/features.gypi:162
> > +# Local Variables:
> > +# tab-width:2
> > +# indent-tabs-mode:nil
> > +# End:
> > +# vim: set expandtab tabstop=2 shiftwidth=2:
> 
> What's that?!

Ok, I'll remove it before landing.  I'm also working on removing this from all our gyp(i) files.

[Tony Chang]

Committed r88548: <http://trac.webkit.org/changeset/88548>