[webkit-reviews] review requested: [Bug 205085] [CMake] Add targets for Apple Frameworks : [Attachment 391117] Patch

[bugzilla-daemon at webkit.org]

Don Olmstead <don.olmstead at sony.com> has asked	for review:
Bug 205085: [CMake] Add targets for Apple Frameworks
https://bugs.webkit.org/show_bug.cgi?id=205085

Attachment 391117: Patch

https://bugs.webkit.org/attachment.cgi?id=391117&action=review




--- Comment #17 from Don Olmstead <don.olmstead at sony.com> ---
Comment on attachment 391117
  --> https://bugs.webkit.org/attachment.cgi?id=391117
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.