Bug 198493 – [CMake] Add WebKit::PAL target

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Don Olmstead]

Add WebKit::PAL target

[Don Olmstead]

Created attachment 371207 [details]
Patch

[Don Olmstead]

Created attachment 371220 [details]
Patch

[Don Olmstead]

Created attachment 371319 [details]
Patch

[EWS Watchlist]

Comment on attachment 371319 [details]
Patch

Attachment 371319 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12376776

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/unregister-then-register-new-script.https.html

[EWS Watchlist]

Created attachment 371338 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.5

[EWS Watchlist]

Comment on attachment 371319 [details]
Patch

Attachment 371319 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12379872

New failing tests:
http/wpt/service-workers/service-worker-networkprocess-crash.html

[EWS Watchlist]

Created attachment 371364 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Don Olmstead]

Created attachment 391819 [details]
WIP Patch

[Don Olmstead]

Created attachment 391821 [details]
WIP Patch

[Don Olmstead]

Created attachment 391824 [details]
WIP Patch

[Don Olmstead]

Created attachment 391827 [details]
Patch

[Don Olmstead]

Created attachment 391897 [details]
Patch

[Michael Catanzaro]

Comment on attachment 391897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391897&action=review

targets :D

> Source/cmake/target/PAL.cmake:14
> +    # Just assuming Windows for the moment
> +    add_library(WebKit::PAL STATIC IMPORTED)
> +    set_target_properties(WebKit::PAL PROPERTIES
> +        IMPORTED_LOCATION ${WEBKIT_LIBRARIES_RUNTIME_DIR}/PAL${DEBUG_SUFFIX}.dll
> +        IMPORTED_IMPLIB ${WEBKIT_LIBRARIES_LINK_DIR}/PAL${DEBUG_SUFFIX}.lib
> +        # Should add Apple libraries here when https://bugs.webkit.org/show_bug.cgi?id=205085 lands
> +        INTERFACE_LINK_LIBRARIES "WebKit::WTF"
> +    )

Unguarded Windows stuff in a cross-platform file doesn't seem good.

[Michael Catanzaro]

Comment on attachment 391897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391897&action=review

> Source/cmake/target/PAL.cmake:4
> +if (NOT TARGET WebKit::PAL)
> +    if (NOT INTERNAL_BUILD)
> +        message(FATAL_ERROR "WebKit::PAL target not found")
> +    endif ()

I see, I missed that we'll never reach the Windows-specific stuff.

[Fujii Hironori]

Comment on attachment 391897 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391897&action=review

LGTM2

> Source/WebCore/PAL/pal/CMakeLists.txt:43
> +# linked otherwise it can be linked directly

Theoretically, this also should be wrapped into the interface library WebKit::WTF, shound't it?

[Don Olmstead]

(In reply to Fujii Hironori from comment #15)
> Comment on attachment 391897 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=391897&action=review
> 
> LGTM2
> 
> > Source/WebCore/PAL/pal/CMakeLists.txt:43
> > +# linked otherwise it can be linked directly
> 
> Theoretically, this also should be wrapped into the interface library
> WebKit::WTF, shound't it?

WebKit::WTF is used for building JSC and some test stuff. I don't think you can technically modify it to include JSC without that part of the build freaking out.

After writing that if statement to make the decision of what to link to PAL I started thinking of how a CMake macro or function is really needed for anything trying to link a WebKit framework. The complexity is because we have so many different variants of the framework libraries.

Mac/PlayStation - STATIC bmalloc/WTF exposed through SHARED JSC
Windows - SHARED WTF and JavaScriptCore
WPE - STATIC everything except WebKit

I think the best way forward is to have a function/macro for when you link a WebKit framework. So the above would be WEBKIT_FRAMEWORK_LINK(PAL WTF) and under the covers it would figure that out and provide book keeping on what it is you're supposed to link in the end.

[WebKit Commit Bot]

Comment on attachment 391897 [details]
Patch

Clearing flags on attachment: 391897

Committed r257587: <https://trac.webkit.org/changeset/257587>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.