WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
141027
[EFL][GTK] Fix the build after
r179326
https://bugs.webkit.org/show_bug.cgi?id=141027
Summary
[EFL][GTK] Fix the build after r179326
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-
Details
Formatted Diff
Diff
patch for EWS
(15.56 KB, patch)
2015-01-29 08:27 PST
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
Patch
(11.33 KB, patch)
2015-01-29 09:08 PST
,
Csaba Osztrogonác
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug