Bug 141027 – [EFL][GTK] Fix the build after r179326

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Csaba Osztrogonác]

SSIA

I started working on it.

[Csaba Osztrogonác]

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]

Regarding the availability of DTrace on Linux check: https://bugs.webkit.org/show_bug.cgi?id=140673#c18

[Carlos Garcia Campos]

(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]

Created attachment 245616 [details]
Patch

Add ENABLE(DTRACE)

[Csaba Osztrogonác]

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.

[Csaba Osztrogonác]

(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]

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]

(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]

(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]

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]

Created attachment 245626 [details]
Patch

bug141033 already fixed, patch for final review

[Csaba Osztrogonác]

ping for review?

EFL and GTK build is still broken - since 15 hours!

[Alexey Proskuryakov]

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 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 on attachment 245626 [details]
Patch

Clearing flags on attachment: 245626

Committed r179346: <http://trac.webkit.org/changeset/179346>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.