Bug 184127

Summary:

[CMake] WebKit should link to WebCore as a PRIVATE library if WebCore is a static library

Product:

WebKit

Reporter:

Fujii Hironori <fujii.hironori>

Component:

WebKit2

Assignee:

Fujii Hironori <fujii.hironori>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, annulen, bfulgham, don.olmstead, mcatanzaro, pvollan, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

174003

Attachments:

Description	Flags
WIP patch	none
WIP patch	none
WIP patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	none
Patch	annulen: review+

Fujii Hironori

Reported 2018-03-28 23:48:24 PDT

[CMake] WebKit should link to WebCore as a PRIVATE library While trying to build TestWebKit for Windows port (Bug 174003), I see the following linkage error. > ------ Build started: Project: TestWebKitLib, Configuration: Debug x64 ------ > WebCore.lib(UnifiedSource311.obj) : error LNK2005: "struct WTFLogChannel * __cdecl WebCore::notImplementedLoggingChannel(void)" (?notImplementedLoggingChannel@WebCore@@YAPEAUWTFLogChannel@@XZ) already defined in WebKit2.lib(WebKit2.dll) > Creating library C:/webkit/gb/WebKitBuild/Debug/lib64/TestWebKitLib.lib and object C:/webkit/gb/WebKitBuild/Debug/lib64/TestWebKitLib.exp > C:\webkit\gb\WebKitBuild\Debug\bin64\TestWebKitLib.dll : fatal error LNK1169: one or more multiply defined symbols found > Done building project "TestWebKitLib.vcxproj" -- FAILED. > ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ========== This happens because TestWebKitLib links to both WebCore and WebKit. But, TestWebKitLib explicitly links only with WebKit, WebCore is propagated because WebKit links WebCore as PUBLIC.

Attachments
WIP patch (2.40 KB, patch) 2018-03-29 00:41 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
WIP patch (4.99 KB, patch) 2018-03-29 03:44 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
WIP patch (3.17 KB, patch) 2018-03-29 20:34 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (6.17 KB, patch) 2018-03-29 22:19 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (6.52 KB, patch) 2018-04-01 20:18 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (6.43 KB, patch) 2018-04-02 18:37 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (7.58 KB, patch) 2018-04-03 18:47 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (10.55 KB, patch) 2018-04-05 19:07 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Patch (10.68 KB, patch) 2018-04-06 03:58 PDT, Fujii Hironori	annulen: review+	Details Formatted Diff Diff
Show Obsolete (8) View All Add attachment proposed patch, testcase, etc.

Fujii Hironori

Comment 1 2018-03-29 00:41:16 PDT

Created attachment 336754 [details] WIP patch This patch doen't work for ports using generate-forwarding-headers.pl. Because generate-forwarding-headers.pl creates forwarding headers only for headers included by WebKit2. Then, include paths should contain all WebCore directories.

Fujii Hironori

Comment 2 2018-03-29 03:44:44 PDT

Created attachment 336762 [details] WIP patch

Fujii Hironori

Comment 3 2018-03-29 20:34:24 PDT

Created attachment 336836 [details] WIP patch

Fujii Hironori

Comment 4 2018-03-29 22:19:07 PDT

Created attachment 336840 [details] Patch

Michael Catanzaro

Comment 5 2018-03-30 07:49:17 PDT

Let's see what Konstantin thinks.

Konstantin Tokarev

Comment 6 2018-03-30 08:53:42 PDT

This change is wrong for cases when WebCore is built as SHARED library, because all users of WebKit need to link WebCore as well

Fujii Hironori

Comment 7 2018-04-01 20:18:18 PDT

Created attachment 336969 [details] Patch

Konstantin Tokarev

Comment 8 2018-04-02 05:08:41 PDT

Comment on attachment 336969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336969&action=review > Source/WebCore/CMakeLists.txt:1994 > + target_link_libraries(WebCore PUBLIC ANGLESupport) ANGLESupport must be PRIVATE > Source/WebKit/PlatformGTK.cmake:461 > +list(APPEND WebKit_PRIVATE_LIBRARIES There is no real need for new variable - just add 'PUBLIC' and 'PRIVATE' keywords to WebKit_LIBRARIES.

Fujii Hironori

Comment 9 2018-04-02 06:21:26 PDT

Comment on attachment 336969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336969&action=review >> Source/WebCore/CMakeLists.txt:1994 >> + target_link_libraries(WebCore PUBLIC ANGLESupport) > > ANGLESupport must be PRIVATE No. I tried PRIVATE, but I got compilation errors. I'll attach the error message for the records, tomorrow. >> Source/WebKit/PlatformGTK.cmake:461 >> +list(APPEND WebKit_PRIVATE_LIBRARIES > > There is no real need for new variable - just add 'PUBLIC' and 'PRIVATE' keywords to WebKit_LIBRARIES. I think I need to switch back to PUBLIC again. > list(APPEND WebKit_PRIVATE_LIBRARIES PRIVATE WebCore PUBLIC) I'll remake the patch.

Konstantin Tokarev

Comment 10 2018-04-02 06:37:56 PDT

Comment on attachment 336969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336969&action=review >>> Source/WebCore/CMakeLists.txt:1994 >>> + target_link_libraries(WebCore PUBLIC ANGLESupport) >> >> ANGLESupport must be PRIVATE > > No. > I tried PRIVATE, but I got compilation errors. I'll attach the error message for the records, tomorrow. Ah, that's because of include directories. Same problem as what you are doing with WebCoreHeaderInterface

Fujii Hironori

Comment 11 2018-04-02 18:23:35 PDT

Comment on attachment 336969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336969&action=review >>> Source/WebKit/PlatformGTK.cmake:461 >>> +list(APPEND WebKit_PRIVATE_LIBRARIES >> >> There is no real need for new variable - just add 'PUBLIC' and 'PRIVATE' keywords to WebKit_LIBRARIES. > > I think I need to switch back to PUBLIC again. GTK port is using ADD_WHOLE_ARCHIVE_TO_LIBRARIES. https://trac.webkit.org/browser/webkit/trunk/Source/WebKit/PlatformGTK.cmake?rev=228590#L668 Do you think I should modify ADD_WHOLE_ARCHIVE_TO_LIBRARIES to remove PUBLIC and PRIVATE keywords?

Fujii Hironori

Comment 12 2018-04-02 18:31:32 PDT

Comment on attachment 336969 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=336969&action=review >>>> Source/WebCore/CMakeLists.txt:1994 >>>> + target_link_libraries(WebCore PUBLIC ANGLESupport) >>> >>> ANGLESupport must be PRIVATE >> >> No. >> I tried PRIVATE, but I got compilation errors. I'll attach the error message for the records, tomorrow. > > Ah, that's because of include directories. Same problem as what you are doing with WebCoreHeaderInterface I tried again and no problem happens today. I'll make ANGLESupport PRIVATE.

Fujii Hironori

Comment 13 2018-04-02 18:37:08 PDT

Created attachment 337050 [details] Patch

Fujii Hironori

Comment 14 2018-04-03 18:47:06 PDT

Created attachment 337138 [details] Patch

Fujii Hironori

Comment 15 2018-04-04 16:53:49 PDT

Hi Konstantin, I addressed the review feedbacks. Could you review again?

Konstantin Tokarev

Comment 16 2018-04-04 18:07:40 PDT

Comment on attachment 337138 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337138&action=review > Source/WebKit/CMakeLists.txt:726 > + set(WebKit_LIBRARIES WebCoreHeaderInterface PRIVATE WebCore PUBLIC) While this line is syntactically correct, I think we should write such code like set(WebKit_LIBRARIES PUBLIC WebCoreHeaderInterface PRIVATE WebCore ) i.e. all libraries/targets have their "access specifiers" in front of them, instead of providing access specifier for unknown follow-up list

Fujii Hironori

Comment 17 2018-04-05 19:07:12 PDT

Created attachment 337327 [details] Patch

Fujii Hironori

Comment 18 2018-04-05 19:33:04 PDT

Comment on attachment 337327 [details] Patch Thank you for the review, Konstantin. I did it.

Konstantin Tokarev

Comment 19 2018-04-06 03:33:40 PDT

Comment on attachment 337327 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337327&action=review > Source/WebKit/PlatformGTK.cmake:463 > + PRIVATE ${GTK_UNIX_PRINT_LIBRARIES} Don't repeat PRIVATE list(APPEND WebKit_LIBRARIES PRIVATE A B C PUBLIC ... )

Fujii Hironori

Comment 20 2018-04-06 03:58:28 PDT

Created attachment 337354 [details] Patch

Fujii Hironori

Comment 21 2018-04-08 19:58:49 PDT

Committed r230385: <https://trac.webkit.org/changeset/230385>

Radar WebKit Bug Importer

Comment 22 2018-04-08 19:59:19 PDT

<rdar://problem/39270000>

Note You need to log in before you can comment on or make changes to this bug.