Bug 193045

Summary: Add ENABLE_UNIFIED_BUILDS option to cmake ports
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, cadubentzen, darin, ddkilzer, don.olmstead, fujii.hironori, keith_miller, mcatanzaro, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch don.olmstead: review+

Yusuke Suzuki
Reported 2018-12-28 00:57:19 PST
Add ENABLE_UNIFIED_BUILDS option to cmake ports
Attachments
Patch (17.61 KB, patch)
2018-12-28 00:59 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (18.15 KB, patch)
2018-12-28 01:01 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (23.22 KB, patch)
2018-12-28 01:11 PST, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (8.90 KB, patch)
2018-12-28 01:14 PST, Yusuke Suzuki 		don.olmstead: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2018-12-28 00:59:55 PST
Created attachment 358115 [details] Patch
Yusuke Suzuki
Comment 2 2018-12-28 01:01:58 PST
Created attachment 358116 [details] Patch
Yusuke Suzuki
Comment 3 2018-12-28 01:11:47 PST
Created attachment 358117 [details] Patch
Yusuke Suzuki
Comment 4 2018-12-28 01:14:53 PST
Created attachment 358118 [details] Patch
Yusuke Suzuki
Comment 5 2018-12-28 01:16:48 PST
Note that this patch itself does not fix the build failures with non-unified builds. This change allows us to generate non-unified compile_commands.json with CMake.
Konstantin Tokarev
Comment 6 2018-12-28 09:18:26 PST
Shouldn't we name it ENABLE_ALLINONE_BUILD to match name of option which was available in the past?
Michael Catanzaro
Comment 7 2018-12-28 09:21:29 PST
Well it's not really ALLINONE
Yusuke Suzuki
Comment 8 2018-12-28 09:29:45 PST
(In reply to Konstantin Tokarev from comment #6) > Shouldn't we name it ENABLE_ALLINONE_BUILD to match name of option which was > available in the past? Do we use ENABLE_UNIFIED_BUILDS option in practice in the past? I think this name is the most appropriate one to this functionality. So if it does not pose actual problems, I would like to use this name. If this poses the problem, the other name like USE_UNIFIED_BUILDS also sounds nice to me. But I think “All In One” is misleading since our unified builds bundle (typically) 8 source files to one source bundle, not bundling all the sources into one bundle.
Yusuke Suzuki
Comment 9 2018-12-28 09:54:08 PST
(In reply to Michael Catanzaro from comment #7) > Well it's not really ALLINONE Ah, Michael was fast. My second comment was duplicate :P
Don Olmstead
Comment 10 2018-12-30 13:13:57 PST
Comment on attachment 358118 [details] Patch r=me Thank you so much for doing this! Can't wait to build without Unified Sources.
Yusuke Suzuki
Comment 11 2018-12-30 13:17:33 PST
(In reply to Don Olmstead from comment #10) > Comment on attachment 358118 [details] > Patch > > r=me > > Thank you so much for doing this! Can't wait to build without Unified > Sources. :D This allows me to use YCM with fully enabled completions!
Yusuke Suzuki
Comment 12 2018-12-30 13:19:15 PST
Committed r239561: <https://trac.webkit.org/changeset/239561>
Radar WebKit Bug Importer
Comment 13 2018-12-30 13:20:26 PST
<rdar://problem/46983277>
Don Olmstead
Comment 14 2018-12-30 13:26:06 PST
Will also be nice for integrating some clang tooling like tidy and IWYU. Next step might be to enable those and have it default to OFF when either are enabled.
Note You need to log in before you can comment on or make changes to this bug.