Bug 205085 – [CMake] Add targets for Apple Frameworks

• Component: CMake
• Status: RESOLVED FIXED

[Don Olmstead]

Would be nice to have actual targets for Apple frameworks.

[Don Olmstead]

Created attachment 385321 [details]
WIP Patch

[Don Olmstead]

Created attachment 385332 [details]
WIP Patch

[Don Olmstead]

Created attachment 385334 [details]
WIP Patch

[Don Olmstead]

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

[Don Olmstead]

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.

[Don Olmstead]

Created attachment 385412 [details]
Patch

[Konstantin Tokarev]

I doubt it has any use for Mac as CMake natively supports finding and linking Apple frameworks

[Don Olmstead]

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

[Konstantin Tokarev]

(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

[Alex Christensen]

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.

[Don Olmstead]

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?

[Don Olmstead]

Created attachment 391085 [details]
Patch

[Don Olmstead]

Created attachment 391102 [details]
Patch

[Alex Christensen]

Comment on attachment 391102 [details]
Patch

I think this looks good.  I'd like someone else to double check that things work before landing.

[Per Arne Vollan]

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

[Don Olmstead]

Created attachment 391117 [details]
Patch

Fix the style goof

[Don Olmstead]

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.

[Per Arne Vollan]

Sorry, ignore the previous comment, it does seem to find the libraries.

[Per Arne Vollan]

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

[Per Arne Vollan]

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)

[Don Olmstead]

Created attachment 391191 [details]
Patch

Fix the issue Per Arne had. Used a , instead of ; in the targets.

[Per Arne Vollan]

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]

[Don Olmstead]

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

[Don Olmstead]

Pull request: https://github.com/WebKit/WebKit/pull/7179

[EWS]

Committed 257428@main (d9aab5144958): <https://commits.webkit.org/257428@main>

Reviewed commits have been landed. Closing PR #7179 and removing active labels.