Bug 197174 – [CMake] Refactor WEBKIT_MAKE_FORWARDING_HEADERS into WEBKIT_COPY_FILES

• Component: CMake
• Status: RESOLVED FIXED

[Don Olmstead]

WEBKIT_MAKE_FORWARDING_HEADERS should just copy files and not do anything else to the target framework.

Dependencies between the headers should be explicitly set. It is currently adding a dependency on the target framework. This shouldn't be the case.

Also this macro can be used to copy over other file types, such as scripts, that are currently just copied with file(COPY). If any of those files change then this isn't going to propagate which is a problem.

[Don Olmstead]

Created attachment 368743 [details]
Patch

[Don Olmstead]

Created attachment 368746 [details]
Patch

Hopefully fix jsc only builds.

[WebKit Commit Bot]

Comment on attachment 368746 [details]
Patch

Clearing flags on attachment: 368746

Committed r244881: <https://trac.webkit.org/changeset/244881>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Zan Dobersek]

Comment on attachment 368746 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368746&action=review

> Source/JavaScriptCore/CMakeLists.txt:1308
> +add_dependencies(JavaScriptCorePrivateFrameworkHeaders JavaScriptCore)

Any reason why the dependency is reversed for JavaScriptCorePrivateFrameworkHeaders?
It's causing build failures for me on WPE, where CustomGlobalObjectClassTest.c from the testapi target is trying to include the JSObjectRefPrivate.h forwarding header which doesn't (yet?) exist.

Switching the targets fixes it for me.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 197559

[Guillaume Emont]

(In reply to WebKit Commit Bot from comment #6)
> Re-opened since this is blocked by bug 197559

Rolling out because this breaks 3 EWS bots (jsc-armv7-ews, jsc-mips-ews and jsc-i386-ews) and would likely start to break buildbots once we do a clean build. The change suggested by Žan seems to fix it, but I don't know whether this would have ill side effects on other platforms, hence the rollout to give time to Dom to figure this out.

[Don Olmstead]

(In reply to Zan Dobersek from comment #5)
> Comment on attachment 368746 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368746&action=review
> 
> > Source/JavaScriptCore/CMakeLists.txt:1308
> > +add_dependencies(JavaScriptCorePrivateFrameworkHeaders JavaScriptCore)
> 
> Any reason why the dependency is reversed for
> JavaScriptCorePrivateFrameworkHeaders?
> It's causing build failures for me on WPE, where
> CustomGlobalObjectClassTest.c from the testapi target is trying to include
> the JSObjectRefPrivate.h forwarding header which doesn't (yet?) exist.
> 
> Switching the targets fixes it for me.

I see the problem here but I'm not sure what the fix is so adding some JSC folks into the mix.

If you look in Source/JavaScriptCore/API/tests/CustomGlobalObjectClassTest.c theres

#include <JavaScriptCore/JSObjectRefPrivate.h>

If you look in ExecutionTimeLimitTest.cpp there's

#include "JSObjectRefPrivate.h"

I see a bunch of these inconsistencies in there. So yes that's why you saw this build failure.

JSC folks I would assume that testapi should just be using <JavaScriptCore/*.h> for includes. Is that a fair assessment?

Also should any of the other text executables and JSC itself be using <JavaScriptCore/*.h> style? I'm happy to fix this before relanding this patch.

[Don Olmstead]

Macro was removed