| Summary: | [EFL][GTK] Fix the build after r179326 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Csaba Osztrogonác <ossy> | ||||||||
| Component: | New Bugs | Assignee: | 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
Csaba Osztrogonác
2015-01-29 00:11:37 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. Regarding the availability of DTrace on Linux check: https://bugs.webkit.org/show_bug.cgi?id=140673#c18 (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. Created attachment 245616 [details]
Patch
Add ENABLE(DTRACE)
note: I reverted my previous change, removed uuid from the dependency list: http://trac.webkit.org/changeset/179341 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. (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? 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. (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 (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 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 Created attachment 245626 [details] Patch bug141033 already fixed, patch for final review ping for review? EFL and GTK build is still broken - since 15 hours! 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). 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. Comment on attachment 245626 [details] Patch Clearing flags on attachment: 245626 Committed r179346: <http://trac.webkit.org/changeset/179346> All reviewed patches have been landed. Closing bug. |