Bug 149624 - Build WebCore JS Builtins according the ENABLE flags
Summary: Build WebCore JS Builtins according the ENABLE flags
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-29 04:47 PDT by youenn fablet
Modified: 2015-09-29 07:05 PDT (History)
2 users (show)

See Also:

Attachments
Patch (20.59 KB, patch)
2015-09-29 05:08 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (20.43 KB, patch)
2015-09-29 06:13 PDT, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2015-09-29 04:47:05 PDT 
WebCore builtins are currently compiled and linked even if the related WebIDL is not (conditional flag off).
This should be improved.
Comment 1 youenn fablet 2015-09-29 05:08:00 PDT 
Created attachment 262067 [details]
Patch
Comment 2 WebKit Commit Bot 2015-09-29 05:10:55 PDT 
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.
Comment 3 Csaba Osztrogonác 2015-09-29 05:34:09 PDT 
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
Comment 4 Csaba Osztrogonác 2015-09-29 05:36:23 PDT 
Otherwise the cmake part looks good to me. (and it builds too)
Comment 5 youenn fablet 2015-09-29 05:47:32 PDT 
(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...
Comment 6 Csaba Osztrogonác 2015-09-29 05:53:00 PDT 
(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". ;)
Comment 7 youenn fablet 2015-09-29 06:03:47 PDT 
(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.
Comment 8 Csaba Osztrogonác 2015-09-29 06:07:19 PDT 
Comment on attachment 262067 [details]
Patch

r+ ;) 

Please fix the vcxproj issue before landing. 
blindly, because Win EWS and buildbot now use cmake)
Comment 9 youenn fablet 2015-09-29 06:13:07 PDT 
Created attachment 262068 [details]
Patch for landing
Comment 10 youenn fablet 2015-09-29 06:14:32 PDT 
(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.
Comment 11 WebKit Commit Bot 2015-09-29 07:05:39 PDT 
Comment on attachment 262068 [details]
Patch for landing

Clearing flags on attachment: 262068

Committed r190309: <http://trac.webkit.org/changeset/190309>
Comment 12 WebKit Commit Bot 2015-09-29 07:05:43 PDT 
All reviewed patches have been landed.  Closing bug.