Would be nice to have actual targets for Apple frameworks.
Created attachment 385321 [details] WIP Patch
Created attachment 385332 [details] WIP Patch
Created attachment 385334 [details] WIP Patch
Comment on attachment 385334 [details] WIP Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385334&action=review Hey Apple folks, Currently this is a WIP but it should build with both AppleWin, WinCairo and FTW. I also believe this should be usable with Mac but maybe Alex should check that out. If this works for you all then the next steps are 1. Handle ${DEBUG_SUFFIX} and have debug and release targets 2. Add in all the libraries applicable for Apple. Let me know what you all think here. > Source/cmake/FindApple.cmake:82 > +function(_FIND_APPLE_FRAMEWORK framework) > + set(OPTIONS "") > + set(oneValueArgs HEADER) > + set(multiValueArgs LIBRARY_NAMES) > + cmake_parse_arguments(opt "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) > + > + # Find include directory and library > + find_path(${framework}_INCLUDE_DIR NAMES ${opt_HEADER}) > + find_library(${framework}_LIBRARY NAMES ${opt_LIBRARY_NAMES}) > + > + if (${framework}_INCLUDE_DIR AND ${framework}_LIBRARY) > + add_library(Apple::${framework} UNKNOWN IMPORTED) > + set_target_properties(Apple::${framework} PROPERTIES > + INTERFACE_INCLUDE_DIRECTORIES "${${framework}_INCLUDE_DIR}" > + IMPORTED_LOCATION "${${framework}_LIBRARY}" > + ) > + set(${framework}_FOUND ON PARENT_SCOPE) > + if (Apple_FIND_REQUIRED_${framework}) > + set(Apple_LIBS_FOUND ${Apple_LIBS_FOUND} "${framework} (required): ${${framework}_LIBRARY}" PARENT_SCOPE) > + else () > + set(Apple_LIBS_FOUND ${Apple_LIBS_FOUND} "${framework} (optional): ${${framework}_LIBRARY}}" PARENT_SCOPE) > + endif () > + else () > + if (Apple_FIND_REQUIRED_${framework}) > + set(_Apple_REQUIRED_LIBS_FOUND NO PARENT_SCOPE) > + set(Apple_LIBS_NOT_FOUND ${Apple_LIBS_NOT_FOUND} "${framework} (required)" PARENT_SCOPE) > + else () > + set(Apple_LIBS_NOT_FOUND ${Apple_LIBS_NOT_FOUND} "${framework} (optional)" PARENT_SCOPE) > + endif () > + endif () > + > + mark_as_advanced(${framework}_INCLUDE_DIR ${framework}_LIBRARY) > +endfunction() This is the function that searches for the libraries and headers and then creates an Apple:<C> target. If the framework is found then it adds it to the found list, and includes the path to the library. If the framework is not found then it adds it to the not found list. If the components are REQUIRED then if there's anything not found the module will assert and stop the build. Currently it doesn't handle the ${DEBUG_SUFFIX} case but it wouldn't be too painful to have it do a check within this function. With an imported library we should be able to set the path to the debug binaries as well. > Source/cmake/FindApple.cmake:96 > +if ("CoreFoundation" IN_LIST Apple_FIND_COMPONENTS) > + _FIND_APPLE_FRAMEWORK(CoreFoundation > + HEADER CoreFoundation/CoreFoundation.h > + LIBRARY_NAMES CoreFoundation CFlite > + ) > +endif () > + > +if ("CoreGraphics" IN_LIST Apple_FIND_COMPONENTS) > + _FIND_APPLE_FRAMEWORK(CoreGraphics > + HEADER CoreGraphics/CoreGraphics.h > + LIBRARY_NAMES CoreGraphics > + ) > +endif () Here we just add a check for whether a particular Apple framework has been requested. If so it just calls the function. This should be pretty easy to expand further. > Source/cmake/FindApple.cmake:111 > +if (NOT Apple_FIND_QUIETLY) > + if (Apple_LIBS_FOUND) > + message(STATUS "Found the following Apple libraries:") > + foreach (found ${Apple_LIBS_FOUND}) > + message(STATUS " ${found}") > + endforeach () > + endif () > + if (Apple_LIBS_NOT_FOUND) > + message(STATUS "The following Apple libraries were not found:") > + foreach (found ${Apple_LIBS_NOT_FOUND}) > + message(STATUS " ${found}") > + endforeach () > + endif () > +endif () This just prints out the results. The output was modeled after the output of FindICU.cmake. For example WinCairo will print the following -- Found the following Apple libraries: -- CoreFoundation (required): C:/webkit/WebKitLibraries/win/lib64/CFLite.lib -- Found Apple: YES While AppleWin will print -- Found the following Apple libraries: -- CoreFoundation (required): C:/cygwin/home/buildbot/WebKitLibraries/win/lib64/CoreFoundation.lib -- Found Apple: Y
Created attachment 385408 [details] Patch This completely migrates the Windows ports. The Mac port is left as an exercise for the reader, most likely Alex.
Created attachment 385412 [details] Patch
I doubt it has any use for Mac as CMake natively supports finding and linking Apple frameworks
(In reply to Konstantin Tokarev from comment #7) > I doubt it has any use for Mac as CMake natively supports finding and > linking Apple frameworks Ok good to know. Wasn't sure how one would make targets out of them. Currently there's find_library calls in the Mac CMake.
(In reply to Don Olmstead from comment #8) > Currently there's find_library calls in the Mac CMake. find_library works fine with frameworks, and linking framework in Apple way automatically adds include paths
Comment on attachment 385412 [details] Patch I'm not too opposed to this change, but I don't know what it improves and there's a high risk that it will break something in our hard-to-test internal build even once debug suffixes are added.
Comment on attachment 385412 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=385412&action=review (In reply to Alex Christensen from comment #10) > Comment on attachment 385412 [details] > Patch > > I'm not too opposed to this change, but I don't know what it improves and > there's a high risk that it will break something in our hard-to-test > internal build even once debug suffixes are added. Modern CMake wants everything to be targets. Its a lot cleaner in this case and it'll propagate things like include directories and linking information automatically. With the Apple internal build it includes the OptionsAppleWin.cmake file for each directory so it will have the targets available. I'd just like someone to run through with a real Apple internal build before landing. > Source/cmake/FindApple.cmake:115 > +if ("CoreFoundation" IN_LIST Apple_FIND_COMPONENTS) > + _FIND_APPLE_FRAMEWORK(CoreFoundation > + HEADER CoreFoundation/CoreFoundation.h > + LIBRARY_NAMES CoreFoundation CFlite Any AppleWin internal build stuff that requires ${DEBUG_SUFFIX} would just be inputted into the LIBRARY_NAMES area. Maybe perarne can verify that works as expected?
Created attachment 391085 [details] Patch
Created attachment 391102 [details] Patch
Comment on attachment 391102 [details] Patch I think this looks good. I'd like someone else to double check that things work before landing.
(In reply to Alex Christensen from comment #14) > Comment on attachment 391102 [details] > Patch > > I think this looks good. I'd like someone else to double check that things > work before landing. I can do a local test compile.
Created attachment 391117 [details] Patch Fix the style goof
Comment on attachment 391117 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391117&action=review Per Arne thank you for verifying the internal build. I think everything should be good but feel free to ping me if you hit any issues. > Source/cmake/FindApple.cmake:87 > + LIBRARY_NAMES ASL${DEBUG_SUFFIX} I just added ${DEBUG_SUFFIX} to all the library names. This should be fine for the internal build. You'd just want to make sure that it links _debug.lib in all its builds. Should be able to see this from the log. > Source/cmake/OptionsAppleWin.cmake:62 > set(CMAKE_REQUIRED_LIBRARIES > - "${WEBKIT_LIBRARIES_LINK_DIR}/CoreFoundation${DEBUG_SUFFIX}.lib" > - "${WEBKIT_LIBRARIES_LINK_DIR}/AVFoundationCF${DEBUG_SUFFIX}.lib" > - "${WEBKIT_LIBRARIES_LINK_DIR}/QuartzCore${DEBUG_SUFFIX}.lib" > - "${WEBKIT_LIBRARIES_LINK_DIR}/libdispatch${DEBUG_SUFFIX}.lib" > + Apple::AVFoundationCF > + Apple::CoreFoundation > + Apple::QuartzCore > + Apple::libdispatch This is the only change that I'm worried about. Only the Apple internal build will be successful with all the different options being set. The documentation for CMAKE_REQUIRED_LIBRARIES says that targets are ok but remember Per Arne that all the options should end up on with the internal build.
Sorry, ignore the previous comment, it does seem to find the libraries.
(In reply to Per Arne Vollan from comment #19) > Sorry, ignore the previous comment, it does seem to find the libraries. Yes, libraries are found, build is in progress. I will let you know if I see any issues.
I see the following error: CMake Error at AppleInternal/tools/cmake/WebKitMacros.cmake:142 (add_library): Target "JavaScriptCore" links to target "ICU::uc,Apple::CoreFoundation" but the target was not found. Perhaps a find_package() call is missing for an IMPORTED target, or an ALIAS target is missing? Call Stack (most recent call first): CMakeLists.txt:1316 (WEBKIT_FRAMEWORK_DECLARE)
Created attachment 391191 [details] Patch Fix the issue Per Arne had. Used a , instead of ; in the targets.
There seems to be some compile errors: Creating library C:/Projects//WebKit/Debug/build32/lib32/JavaScriptCore_debug.lib and object C:/Projects//WebKit/Debug/build32/lib32/JavaScriptCore_debug.exp UnifiedSource-f2e18ffc-35.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) bool __cdecl WTF::isMainRunLoop(void)" (__imp_?isMainRunLoop@WTF@@YA_NXZ) referenced in function "void __cdecl WTF::removeIterator<class JSC::CallFrame *,struct WTF::KeyValuePair<class JSC::CallFrame *,class s td::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> > >,struct WTF::KeyValuePairKeyExtractor<struct WTF::KeyValuePair<class JSC::CallFrame *,class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct J SC::CheckpointOSRExitSideState> > > >,struct WTF::PtrHash<class JSC::CallFrame *>,struct WTF::HashMap<class JSC::CallFrame *,class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> >,struct WTF::PtrHash<class JSC::CallFrame *>,struct WTF::HashTraits<class JSC::CallFrame *>,struct WTF::HashTraits<class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> > > >::KeyValuePairTraits,struct WTF::HashTraits<class JSC::CallFrame *> >(class WTF::HashTableConstIterator<class JSC::CallFrame *,struct WTF::KeyValuePair<class JSC::CallFrame *,class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> > >,struct WTF::KeyValuePairKeyExtractor<struct WTF::KeyValuePair<class JSC::CallFrame *,class std::unique_ptr<st ruct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> > > >,struct WTF::PtrHash<class JSC::CallFrame *>,struct WTF::HashMap<class JSC::CallFrame *,class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::Checkpoin tOSRExitSideState> >,struct WTF::PtrHash<class JSC::CallFrame *>,struct WTF::HashTraits<class JSC::CallFrame *>,struct WTF::HashTraits<class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct JSC::CheckpointOSRExitSideState> > > >::KeyValuePairTraits,struct WTF::HashT raits<class JSC::CallFrame *> > *)" (??$removeIterator@PAVCallFrame@JSC@@U?$KeyValuePair@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@WTF@@U?$KeyValuePairKeyExtractor@U?$KeyValuePair@PAVCallFrame@JSC@@V?$unique_ptr@UCheckp ointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@WTF@@@4@U?$PtrHash@PAVCallFrame@JSC@@@4@UKeyValuePairTraits@?$HashMap@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@U?$PtrHash@PAVCall Frame@JSC@@@WTF@@U?$HashTraits@PAVCallFrame@JSC@@@6@U?$HashTraits@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@6@@4@U?$HashTraits@PAVCallFrame@JSC@@@4@@WTF@@YAXPAV?$HashTableConstIterator@PAVCallFrame@JSC@@U?$KeyValuePair@PAVCallFrame@JSC@@ V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@WTF@@U?$KeyValuePairKeyExtractor@U?$KeyValuePair@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@WTF@@@4@U?$PtrHas h@PAVCallFrame@JSC@@@4@UKeyValuePairTraits@?$HashMap@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@U?$PtrHash@PAVCallFrame@JSC@@@WTF@@U?$HashTraits@PAVCallFrame@JSC@@@6@U?$HashTraits@V?$unique_ptr@UCheckpointOSRExitSideState @JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@6@@4@U?$HashTraits@PAVCallFrame@JSC@@@4@@0@@Z) [C:\Projects\\WebKit\Debug\build32\JavaScriptCore.vcxproj] [C:\Projects\\WebKit\Source\JavaScriptCore\JavaScriptCore.vcxproj\JavaScriptCore.proj] UnifiedSource-0284c6ac-2.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) bool __cdecl WTF::isMainRunLoop(void)" (__imp_?isMainRunLoop@WTF@@YA_NXZ) [C:\Projects\\WebKit\Debug\build32\JavaScriptCore.vcxproj] [C:\Projects\\WebKit\Source\JavaScriptCore\JavaSc riptCore.vcxproj\JavaScriptCore.proj]
(In reply to Per Arne Vollan from comment #23) > There seems to be some compile errors: > > Creating library > C:/Projects//WebKit/Debug/build32/lib32/JavaScriptCore_debug.lib and object > C:/Projects//WebKit/Debug/build32/lib32/JavaScriptCore_debug.exp > UnifiedSource-f2e18ffc-35.obj : error LNK2019: unresolved external symbol > "__declspec(dllimport) bool __cdecl WTF::isMainRunLoop(void)" > (__imp_?isMainRunLoop@WTF@@YA_NXZ) referenced in function "void __cdecl > WTF::removeIterator<class JSC::CallFrame *,struct WTF::KeyValuePair<class > JSC::CallFrame *,class s > td::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::CheckpointOSRExitSideState> > >,struct > WTF::KeyValuePairKeyExtractor<struct WTF::KeyValuePair<class JSC::CallFrame > *,class std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct J > SC::CheckpointOSRExitSideState> > > >,struct WTF::PtrHash<class > JSC::CallFrame *>,struct WTF::HashMap<class JSC::CallFrame *,class > std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::CheckpointOSRExitSideState> >,struct > WTF::PtrHash<class JSC::CallFrame *>,struct > WTF::HashTraits<class JSC::CallFrame *>,struct WTF::HashTraits<class > std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::CheckpointOSRExitSideState> > > > >::KeyValuePairTraits,struct WTF::HashTraits<class JSC::CallFrame *> >(class > WTF::HashTableConstIterator<class > JSC::CallFrame *,struct WTF::KeyValuePair<class JSC::CallFrame *,class > std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::CheckpointOSRExitSideState> > >,struct > WTF::KeyValuePairKeyExtractor<struct WTF::KeyValuePair<class JSC::CallFrame > *,class std::unique_ptr<st > ruct JSC::CheckpointOSRExitSideState,struct std::default_delete<struct > JSC::CheckpointOSRExitSideState> > > >,struct WTF::PtrHash<class > JSC::CallFrame *>,struct WTF::HashMap<class JSC::CallFrame *,class > std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::Checkpoin > tOSRExitSideState> >,struct WTF::PtrHash<class JSC::CallFrame *>,struct > WTF::HashTraits<class JSC::CallFrame *>,struct WTF::HashTraits<class > std::unique_ptr<struct JSC::CheckpointOSRExitSideState,struct > std::default_delete<struct JSC::CheckpointOSRExitSideState> > > > >::KeyValuePairTraits,struct WTF::HashT > raits<class JSC::CallFrame *> > *)" > (??$removeIterator@PAVCallFrame@JSC@@U?$KeyValuePair@PAVCallFrame@JSC@@V?$uni > que_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExit > SideState@JSC@@@std@@@std@@@WTF@@U?$KeyValuePairKeyExtractor@U?$KeyValuePair@ > PAVCallFrame@JSC@@V?$unique_ptr@UCheckp > ointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@ > @std@@@std@@@WTF@@@4@U?$PtrHash@PAVCallFrame@JSC@@@4@UKeyValuePairTraits@?$Ha > shMap@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$def > ault_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@U?$PtrHash@PAVCall > Frame@JSC@@@WTF@@U?$HashTraits@PAVCallFrame@JSC@@@6@U?$HashTraits@V?$unique_p > tr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExitSideS > tate@JSC@@@std@@@std@@@6@@4@U?$HashTraits@PAVCallFrame@JSC@@@4@@WTF@@YAXPAV?$ > HashTableConstIterator@PAVCallFrame@JSC@@U?$KeyValuePair@PAVCallFrame@JSC@@ > V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointO > SRExitSideState@JSC@@@std@@@std@@@WTF@@U?$KeyValuePairKeyExtractor@U?$KeyValu > ePair@PAVCallFrame@JSC@@V?$unique_ptr@UCheckpointOSRExitSideState@JSC@@U?$def > ault_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@WTF@@@4@U?$PtrHas > h@PAVCallFrame@JSC@@@4@UKeyValuePairTraits@?$HashMap@PAVCallFrame@JSC@@V?$uni > que_ptr@UCheckpointOSRExitSideState@JSC@@U?$default_delete@UCheckpointOSRExit > SideState@JSC@@@std@@@std@@U?$PtrHash@PAVCallFrame@JSC@@@WTF@@U?$HashTraits@P > AVCallFrame@JSC@@@6@U?$HashTraits@V?$unique_ptr@UCheckpointOSRExitSideState > @JSC@@U?$default_delete@UCheckpointOSRExitSideState@JSC@@@std@@@std@@@6@@4@U? > $HashTraits@PAVCallFrame@JSC@@@4@@0@@Z) > [C:\Projects\\WebKit\Debug\build32\JavaScriptCore.vcxproj] > [C:\Projects\\WebKit\Source\JavaScriptCore\JavaScriptCore. > vcxproj\JavaScriptCore.proj] > UnifiedSource-0284c6ac-2.obj : error LNK2001: unresolved external symbol > "__declspec(dllimport) bool __cdecl WTF::isMainRunLoop(void)" > (__imp_?isMainRunLoop@WTF@@YA_NXZ) > [C:\Projects\\WebKit\Debug\build32\JavaScriptCore.vcxproj] > [C:\Projects\\WebKit\Source\JavaScriptCore\JavaSc > riptCore.vcxproj\JavaScriptCore.proj] Are there other linker errors? It looks like WTF::isMainRunLoop is properly exported.
Pull request: https://github.com/WebKit/WebKit/pull/7179
Committed 257428@main (d9aab5144958): <https://commits.webkit.org/257428@main> Reviewed commits have been landed. Closing PR #7179 and removing active labels.