RESOLVED FIXED 197174
[CMake] Refactor WEBKIT_MAKE_FORWARDING_HEADERS into WEBKIT_COPY_FILES
https://bugs.webkit.org/show_bug.cgi?id=197174
Summary [CMake] Refactor WEBKIT_MAKE_FORWARDING_HEADERS into WEBKIT_COPY_FILES
Don Olmstead
Reported 2019-04-22 13:01:23 PDT
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.
Attachments
Patch (14.42 KB, patch)
2019-05-01 18:15 PDT, Don Olmstead
no flags
Patch (15.92 KB, patch)
2019-05-01 18:27 PDT, Don Olmstead
no flags
Don Olmstead
Comment 1 2019-05-01 18:15:12 PDT Comment hidden (obsolete)
Don Olmstead
Comment 2 2019-05-01 18:27:20 PDT
Created attachment 368746 [details] Patch Hopefully fix jsc only builds.
WebKit Commit Bot
Comment 3 2019-05-02 13:37:47 PDT
Comment on attachment 368746 [details] Patch Clearing flags on attachment: 368746 Committed r244881: <https://trac.webkit.org/changeset/244881>
WebKit Commit Bot
Comment 4 2019-05-02 13:37:48 PDT
All reviewed patches have been landed. Closing bug.
Zan Dobersek
Comment 5 2019-05-03 01:17:15 PDT
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
Comment 6 2019-05-03 07:27:50 PDT
Re-opened since this is blocked by bug 197559
Guillaume Emont
Comment 7 2019-05-03 07:32:41 PDT
(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
Comment 8 2019-05-03 09:12:44 PDT
(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
Comment 9 2022-12-14 15:47:40 PST
Macro was removed
Note You need to log in before you can comment on or make changes to this bug.