Bug 141027

Summary: [EFL][GTK] Fix the build after r179326
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Blocker CC: aestes, andersca, ap, bdakin, beidson, benjamin, cgarcia, clopez, cmarcelo, commit-queue, darin, enrica, gyuyoung.kim, hyatt, kling, mitz, mjs, mrowe, ossy, sam, simon.fraser, thorton
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 141033    
Bug Blocks: 140673    
Attachments:
Description Flags
Patch
mitz: review-
patch for EWS
none
Patch none

Csaba Osztrogonác
Reported 2015-01-29 00:11:37 PST
SSIA I started working on it.
Attachments
Patch (11.02 KB, patch)
2015-01-29 03:59 PST, Carlos Garcia Campos
mitz: review-
patch for EWS (15.56 KB, patch)
2015-01-29 08:27 PST, Csaba Osztrogonác
no flags
Patch (11.33 KB, patch)
2015-01-29 09:08 PST, Csaba Osztrogonác
no flags
Csaba Osztrogonác
Comment 1 2015-01-29 00:32:56 PST
I won't be available in the following 2.5-3 hours. If anybody is interested in fixing the build, feel free to pick it up.
Carlos Alberto Lopez Perez
Comment 2 2015-01-29 02:59:30 PST
Regarding the availability of DTrace on Linux check: https://bugs.webkit.org/show_bug.cgi?id=140673#c18
Carlos Garcia Campos
Comment 3 2015-01-29 03:07:12 PST
(In reply to comment #2) > Regarding the availability of DTrace on Linux check: > https://bugs.webkit.org/show_bug.cgi?id=140673#c18 I think this should be guarded by #ifdefs anyway, and make dtrace and uuid as optional deps if we eventually support it.
Carlos Garcia Campos
Comment 4 2015-01-29 03:59:04 PST
Created attachment 245616 [details] Patch Add ENABLE(DTRACE)
Csaba Osztrogonác
Comment 5 2015-01-29 04:10:57 PST
note: I reverted my previous change, removed uuid from the dependency list: http://trac.webkit.org/changeset/179341
mitz
Comment 6 2015-01-29 07:11:30 PST
Comment on attachment 245616 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245616&action=review > Source/WebKit2/ChangeLog:8 > + Add ENABLE_DTRACE only enabled in Cocoa platforms to guard the JavaScriptCore uses HAVE(DTRACE). I think that’s also more appropriate here, since it’s not about enabling a WebKit feature, but about whether a platform feature is available.
Csaba Osztrogonác
Comment 7 2015-01-29 07:16:20 PST
(In reply to comment #6) > Comment on attachment 245616 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245616&action=review > > > Source/WebKit2/ChangeLog:8 > > + Add ENABLE_DTRACE only enabled in Cocoa platforms to guard the > > JavaScriptCore uses HAVE(DTRACE). I think that’s also more appropriate here, > since it’s not about enabling a WebKit feature, but about whether a platform > feature is available. It is a Mac only hack in JavasScriptCore: http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/Configurations/Base.xcconfig Any hint how to make it cross platform and work inside WebKit2 too?
Csaba Osztrogonác
Comment 8 2015-01-29 07:23:35 PST
https://trac.webkit.org/changeset/41350 was a dirty Mac only hack, which moved it from Platform.h to xcconfig. But now HAVE_DTRACE is true unconditonally since: https://trac.webkit.org/changeset/132860/trunk/Source/JavaScriptCore/Configurations/Base.xcconfig I think, it's high time to move it back to the end of Platform.h.
Csaba Osztrogonác
Comment 9 2015-01-29 07:39:26 PST
(In reply to comment #8) > https://trac.webkit.org/changeset/41350 was a dirty Mac only hack, which > moved it from Platform.h to xcconfig. > > But now HAVE_DTRACE is true unconditonally since: > https://trac.webkit.org/changeset/132860/trunk/Source/JavaScriptCore/ > Configurations/Base.xcconfig > > I think, it's high time to move it back to the end of Platform.h. new bug report for moving this back to Platform.h: bug141033
Carlos Garcia Campos
Comment 10 2015-01-29 07:44:26 PST
(In reply to comment #6) > Comment on attachment 245616 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245616&action=review > > > Source/WebKit2/ChangeLog:8 > > + Add ENABLE_DTRACE only enabled in Cocoa platforms to guard the > > JavaScriptCore uses HAVE(DTRACE). I think that’s also more appropriate here, > since it’s not about enabling a WebKit feature, but about whether a platform > feature is available. I used ENABLE with the idea of being able to disable it for production builds
Csaba Osztrogonác
Comment 11 2015-01-29 08:27:30 PST
Created attachment 245623 [details] patch for EWS patch for EWS, not for landing, s/ENABLE(DTRACE)/HAVE(DTRACE)/g in the patch of Carlos + fix from bug141033
Csaba Osztrogonác
Comment 12 2015-01-29 09:08:37 PST
Created attachment 245626 [details] Patch bug141033 already fixed, patch for final review
Csaba Osztrogonác
Comment 13 2015-01-29 09:11:13 PST
ping for review? EFL and GTK build is still broken - since 15 hours!
Alexey Proskuryakov
Comment 14 2015-01-29 09:37:40 PST
Comment on attachment 245626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245626&action=review > Source/WebKit2/ChangeLog:9 > + Guard MessageRecorder implementation with USE(DTRACE) > + for platforms not supporting DTrace. This patch breaks dtrace support on Mac, because only JavaScriptCore project turns on ENABLE(DTRACE).
Alexey Proskuryakov
Comment 15 2015-01-29 09:38:45 PST
Comment on attachment 245626 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=245626&action=review >> Source/WebKit2/ChangeLog:9 >> + for platforms not supporting DTrace. > > This patch breaks dtrace support on Mac, because only JavaScriptCore project turns on ENABLE(DTRACE). I see, this is only part of the fix. Seems OK to me.
WebKit Commit Bot
Comment 16 2015-01-29 10:23:44 PST
Comment on attachment 245626 [details] Patch Clearing flags on attachment: 245626 Committed r179346: <http://trac.webkit.org/changeset/179346>
WebKit Commit Bot
Comment 17 2015-01-29 10:23:51 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.