Bug 198493

Summary: [CMake] Add WebKit::PAL target
Product: WebKit Reporter: Don Olmstead <don.olmstead>
Component: CMakeAssignee: Don Olmstead <don.olmstead>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, commit-queue, ews-watchlist, fujii.hironori, gyuyoung.kim, mcatanzaro, rniwa, ryuan.choi, sergio
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
annulen: review+
Patch
none
Patch
ews-watchlist: commit-queue-
Archive of layout-test-results from ews125 for ios-simulator-wk2
none
Archive of layout-test-results from ews105 for mac-highsierra-wk2
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch
none
Patch none

Don Olmstead
Reported 2019-06-03 13:17:59 PDT
Add WebKit::PAL target
Attachments
Patch (5.61 KB, patch)
2019-06-03 13:24 PDT, Don Olmstead 		annulen: review+
DetailsFormatted DiffDiff
Patch (5.71 KB, patch)
2019-06-03 15:40 PDT, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (6.51 KB, patch)
2019-06-04 12:43 PDT, Don Olmstead 		ews-watchlist: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from ews125 for ios-simulator-wk2 (2.55 MB, application/zip)
2019-06-04 14:40 PDT, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews105 for mac-highsierra-wk2 (3.01 MB, application/zip)
2019-06-04 19:48 PDT, EWS Watchlist 		no flags
Details
WIP Patch (5.96 KB, patch)
2020-02-26 17:33 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
WIP Patch (5.96 KB, patch)
2020-02-26 17:49 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
WIP Patch (5.96 KB, patch)
2020-02-26 18:15 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (8.91 KB, patch)
2020-02-26 18:24 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Patch (8.09 KB, patch)
2020-02-27 12:27 PST, Don Olmstead 		no flags
DetailsFormatted DiffDiff
Show Obsolete (9) View All Add attachment proposed patch, testcase, etc.
Don Olmstead
Comment 1 2019-06-03 13:24:30 PDT Comment hidden (obsolete)
Don Olmstead
Comment 2 2019-06-03 15:40:11 PDT Comment hidden (obsolete)
Don Olmstead
Comment 3 2019-06-04 12:43:56 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 4 2019-06-04 14:40:32 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 5 2019-06-04 14:40:34 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 6 2019-06-04 19:48:00 PDT Comment hidden (obsolete)
EWS Watchlist
Comment 7 2019-06-04 19:48:02 PDT Comment hidden (obsolete)
Don Olmstead
Comment 8 2020-02-26 17:33:06 PST Comment hidden (obsolete)
Don Olmstead
Comment 9 2020-02-26 17:49:45 PST Comment hidden (obsolete)
Don Olmstead
Comment 10 2020-02-26 18:15:09 PST Comment hidden (obsolete)
Don Olmstead
Comment 11 2020-02-26 18:24:03 PST Comment hidden (obsolete)
Don Olmstead
Comment 12 2020-02-27 12:27:48 PST
Created attachment 391897 [details] Patch
Michael Catanzaro
Comment 13 2020-02-27 13:11:14 PST
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 14 2020-02-27 13:12:48 PST
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 15 2020-02-27 13:13:27 PST
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
Comment 16 2020-02-27 13:53:19 PST
(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 17 2020-02-27 14:37:07 PST
Comment on attachment 391897 [details] Patch Clearing flags on attachment: 391897 Committed r257587: <https://trac.webkit.org/changeset/257587>
WebKit Commit Bot
Comment 18 2020-02-27 14:37:09 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.