Bug 149624 – Build WebCore JS Builtins according the ENABLE flags

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

WebCore builtins are currently compiled and linked even if the related WebIDL is not (conditional flag off).
This should be improved.

[youenn fablet]

Created attachment 262067 [details]
Patch

[WebKit Commit Bot]

Attachment 262067 [details] did not pass style-queue:


ERROR: Source/WebCore/bindings/js/WebCoreJSClientData.h:48:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Csaba Osztrogonác]

Comment on attachment 262067 [details]
Patch

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

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:6661
> +    <ClCompile Include="$(ConfigurationBuildDir)\obj$(PlatformArchitecture)\$(ProjectName)\DerivedSources\WebCoreJSBuiltins.cpp">

WebCoreJSBuiltins.cpp is in Source/WebCore/bindings/js, not in DerivedSources

[Csaba Osztrogonác]

Otherwise the cmake part looks good to me. (and it builds too)

[youenn fablet]

(In reply to comment #3)
> Comment on attachment 262067 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=262067&action=review
> 
> > Source/WebCore/WebCore.vcxproj/WebCore.vcxproj:6661
> > +    <ClCompile Include="$(ConfigurationBuildDir)\obj$(PlatformArchitecture)\$(ProjectName)\DerivedSources\WebCoreJSBuiltins.cpp">
> 
> WebCoreJSBuiltins.cpp is in Source/WebCore/bindings/js, not in DerivedSources

OK.
Will fix that at landing time or future patch.

(In reply to comment #4)
> Otherwise the cmake part looks good to me. (and it builds too)

Thanks, let's see what the win bot will say.

This patch compiles with streams API disabled on the GTK port.
If you have time to double check on your specific environment...

[Csaba Osztrogonác]

(In reply to comment #5)
> This patch compiles with streams API disabled on the GTK port.
> If you have time to double check on your specific environment...

I meant that I checked and it works fine, when I wrote "and it builds too". ;)

[youenn fablet]

(In reply to comment #6)
> (In reply to comment #5)
> > This patch compiles with streams API disabled on the GTK port.
> > If you have time to double check on your specific environment...
> 
> I meant that I checked and it works fine, when I wrote "and it builds too".
> ;)

Great then, and win bot is happy as well.
Just after landing it, I will probably trigger a clean build on windows bots, just in case.
Waiting for r+ now.

[Csaba Osztrogonác]

Comment on attachment 262067 [details]
Patch

r+ ;) 

Please fix the vcxproj issue before landing. 
blindly, because Win EWS and buildbot now use cmake)

[youenn fablet]

Created attachment 262068 [details]
Patch for landing

[youenn fablet]

(In reply to comment #8)
> Comment on attachment 262067 [details]
> Patch
> 
> r+ ;) 

Thanks :)

> Please fix the vcxproj issue before landing. 
> blindly, because Win EWS and buildbot now use cmake)

Done.

[WebKit Commit Bot]

Comment on attachment 262068 [details]
Patch for landing

Clearing flags on attachment: 262068

Committed r190309: <http://trac.webkit.org/changeset/190309>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.