Bug 170594

Summary:

[CMake][Win] Conditionally select DLL CRT or static CRT

Product:

WebKit

Reporter:

Fujii Hironori <fujii.hironori>

Component:

WebKit Misc.

Assignee:

Don Olmstead <don.olmstead>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, bfulgham, commit-queue, don.olmstead, isaac+webkit, pvollan, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

179908

Attachments:

Description	Flags
WIP patch	none
WIP patch	none
Similar to the Fujii's patch, but some extra files covered	none
Patch	none
Patch	achristensen: review+
Patch	none
Patch	none

Fujii Hironori

Reported 2017-04-07 03:18:11 PDT

Brent suggested to use DLL CRT instead of static CRT (bug 157067 comment 6). MSDN: CRT Library Features https://msdn.microsoft.com/en-us/library/abx4dbyh.aspx One benefit of DLL CRT is avoiding heap mismatch crash (Bug 156831). Any other benefits?

Attachments
WIP patch (2.56 KB, patch) 2017-04-07 03:18 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
WIP patch (2.60 KB, patch) 2017-04-10 00:15 PDT, Fujii Hironori	no flags	Details Formatted Diff Diff
Similar to the Fujii's patch, but some extra files covered (4.63 KB, patch) 2017-05-19 01:15 PDT, isaac+webkit	no flags	Details Formatted Diff Diff
Patch (7.16 KB, patch) 2017-11-20 19:48 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (7.24 KB, patch) 2017-11-21 11:57 PST, Don Olmstead	achristensen: review+	Details Formatted Diff Diff
Patch (7.27 KB, patch) 2017-11-27 11:52 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Patch (7.28 KB, patch) 2017-11-27 11:53 PST, Don Olmstead	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Fujii Hironori

Comment 1 2017-04-07 03:18:46 PDT

Created attachment 306483 [details] WIP patch

Fujii Hironori

Comment 2 2017-04-10 00:15:09 PDT

Created attachment 306665 [details] WIP patch AppleWin port reports mismatch CRT type linkage error: >WebKitSystemInterface.lib(WebKitSystemInterface.obj) : error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MD_DynamicRelease' in StorageAreaImpl.obj WebKitLibraries/win/lib32/WebKitSystemInterface.lib needs to be compiled with /MD.

isaac+webkit

Comment 3 2017-05-19 01:15:19 PDT

Created attachment 310631 [details] Similar to the Fujii's patch, but some extra files covered As mentioned on webkit-dev ( https://lists.webkit.org/pipermail/webkit-dev/2017-May/029141.html ), I have been working on this as well. I've also changed the vsprops files as well, but from looking at Fujii's patch (which I prefer over mine), it looks like these aren't modified. Does this mean they are no longer used and can be removed?

Alex Christensen

Comment 4 2017-05-22 10:46:16 PDT

Unfortunately we can't remove the vsprops files yet.

Don Olmstead

Comment 5 2017-11-20 19:48:30 PST

Created attachment 327383 [details] Patch This patch takes a different approach since WebKitSystemInterface.lib might not be compiled with /MD anytime soon. This introduces MSVC_STATIC_RUNTIME which AppleWin has as ON. Then the compile and linker flags are modified according to whether /MD or /MT should be specified.

Don Olmstead

Comment 6 2017-11-20 20:38:19 PST

Error from AppleWin is around FourCC.h again....

Don Olmstead

Comment 7 2017-11-21 11:57:02 PST

Created attachment 327424 [details] Patch Put setting of MSVC_STATIC_RUNTIME before the include of OptionsWin and adding some logging for the value. This should get AppleWin working.

Alex Christensen

Comment 8 2017-11-27 09:21:25 PST

Comment on attachment 327424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327424&action=review > Source/cmake/OptionsAppleWin.cmake:1 > +# FIXME: Remove after WebKitSystemInterface.lib is compiled with /MD I'm not sure we ever intend to do this. We would have to do changes to how we distribute Windows binaries and merge modules, etc. > Source/cmake/OptionsMSVC.cmake:66 > + set(MSVC_RUNTIME_LINKER_FLAGS "") I don't think this is necessary.

Don Olmstead

Comment 9 2017-11-27 11:52:53 PST

Created attachment 327655 [details] Patch Fixing review comments

Don Olmstead

Comment 10 2017-11-27 11:53:18 PST

Created attachment 327656 [details] Patch

WebKit Commit Bot

Comment 11 2017-11-27 13:16:53 PST

Comment on attachment 327656 [details] Patch Clearing flags on attachment: 327656 Committed r225191: <https://trac.webkit.org/changeset/225191>

WebKit Commit Bot

Comment 12 2017-11-27 13:16:55 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 13 2017-11-27 13:18:36 PST

<rdar://problem/35705731>

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