JSC/CMskeLists.txt includes both JavaScriptCore_INCLUDE_DIRECTORIES and JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES, and JSC/shell/CMakeLists.txt should do it as well. In my build setup ICU include paths happen to be in "system" include path, so JSC library builds fine and shells don't
Created attachment 268140 [details] Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES)
Comment on attachment 268140 [details] Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) LGTM.
Comment on attachment 268140 [details] Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) Sorry for breaking this! I am going to give cq- since this same construct exists in PlatformWin.cmake in this directory. Please fix it there too!
(In reply to comment #3) > Comment on attachment 268140 [details] > Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) > > Sorry for breaking this! > > I am going to give cq- since this same construct exists in PlatformWin.cmake > in this directory. Please fix it there too! Do you mean that PlatformWin.cmake includes ${JavaScriptCore_INCLUDE_DIRECTORIES} ? http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/shell/PlatformWin.cmake#L1 If so, I agree to remove it in PlatformWin.cmake although I don't know this patch is tightly related to remove it.
(In reply to comment #3) > Comment on attachment 268140 [details] > Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) > > Sorry for breaking this! > > I am going to give cq- since this same construct exists in PlatformWin.cmake > in this directory. Please fix it there too! I'm not building for Windows, but for me it seems like it includes ${JavaScriptCore_INCLUDE_DIRECTORIES} twice, first time in CMakeLists.txt and second time in PlatformWin.cmake. Other port don't do it
(In reply to comment #5) > (In reply to comment #3) > > Comment on attachment 268140 [details] > > Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) > > > > Sorry for breaking this! > > > > I am going to give cq- since this same construct exists in PlatformWin.cmake > > in this directory. Please fix it there too! > > I'm not building for Windows, but for me it seems like it includes > ${JavaScriptCore_INCLUDE_DIRECTORIES} twice, first time in CMakeLists.txt > and second time in PlatformWin.cmake. Other port don't do it yes, we meant it needs to be fixed in this patch together.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #3) > > > Comment on attachment 268140 [details] > > > Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) > > > > > > Sorry for breaking this! > > > > > > I am going to give cq- since this same construct exists in PlatformWin.cmake > > > in this directory. Please fix it there too! > > > > I'm not building for Windows, but for me it seems like it includes > > ${JavaScriptCore_INCLUDE_DIRECTORIES} twice, first time in CMakeLists.txt > > and second time in PlatformWin.cmake. Other port don't do it > > yes, we meant it needs to be fixed in this patch together. Shouldn't JavaScriptCore_INCLUDE_DIRECTORIES be removed from PlatformWin.cmake instead?
(In reply to comment #7) > Shouldn't JavaScriptCore_INCLUDE_DIRECTORIES be removed from > PlatformWin.cmake instead? No, it's needed because in PlatformWin.cmake we make jscLib, which isn't needed in non-windows ports. jsc.exe is just a simple program that finds jscLib and starts it after possibly finding some other dlls. I don't think it's critical to include JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES on Windows. It wouldn't hurt, but I don't think it's needed. Commit this adding it to Windows or not, but don't remove JavaScriptCore_INCLUDE_DIRECTORIES from PlatformWin.cmake.
Created attachment 268456 [details] Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES)
Comment on attachment 268456 [details] Added include_directories(JavaScriptCore_SYSTEM_INCLUDE_DIRECTORIES) Clearing flags on attachment: 268456 Committed r194705: <http://trac.webkit.org/changeset/194705>
All reviewed patches have been landed. Closing bug.