Bug 192658 – Resurrect Mac CMake build

• Component: CMake
• Status: RESOLVED FIXED

[Alex Christensen]

Resurrect Mac CMake build

[Alex Christensen]

Created attachment 357210 [details]
Patch

[EWS Watchlist]

Attachment 357210 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/cf/CFURLExtras.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 21 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Don Olmstead]

Comment on attachment 357210 [details]
Patch

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

Overall looks fine. Just fix those and we're good to go.

> Source/WebCore/PlatformMac.cmake:73
> +    "${PAL_DIR}"

Pal is copied so you shouldn't have to put this here.

> Source/WebCore/PlatformMac.cmake:81
> +    "${WEBCORE_DIR}/layout/layouttree"

All the layout stuff should go into the root CMakeLists. I actually have this patched locally since there were a number of includes.

This should only have mac specific stuff.

> Source/WebKitLegacy/PlatformMac.cmake:25
> +    "${WEBKITLEGACY_DIR}/mac"
> +    "${WEBKITLEGACY_DIR}/mac/Carbon"
> +    "${WEBKITLEGACY_DIR}/mac/DefaultDelegates"
> +    "${WEBKITLEGACY_DIR}/mac/DOM"
> +    "${WEBKITLEGACY_DIR}/mac/History"
> +    "${WEBKITLEGACY_DIR}/mac/icu"
> +    "${WEBKITLEGACY_DIR}/mac/Misc"
> +    "${WEBKITLEGACY_DIR}/mac/Panels"
> +    "${WEBKITLEGACY_DIR}/mac/Plugins"
> +    "${WEBKITLEGACY_DIR}/mac/Plugins/Hosted"
> +    "${WEBKITLEGACY_DIR}/mac/Storage"
> +    "${WEBKITLEGACY_DIR}/mac/WebCoreSupport"
> +    "${WEBKITLEGACY_DIR}/mac/WebInspector"
> +    "${WEBKITLEGACY_DIR}/mac/WebView"

This is in System includes?

> Source/cmake/OptionsMac.cmake:45
> +WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WIRELESS_PLAYBACK_TARGET PRIVATE ON)

A bunch more options were added that I thought were turned on in Mac. Like Paint API.

[Don Olmstead]

Comment on attachment 357210 [details]
Patch

Just testing reviewer powers :)

[Don Olmstead]

Comment on attachment 357210 [details]
Patch

There are also idl files missing under the Modules/applepay directory. Not sure if we should include this in the root CMake or just make it Mac specific.

[Don Olmstead]

*** Bug 191502 has been marked as a duplicate of this bug. ***

[Don Olmstead]

*** Bug 191503 has been marked as a duplicate of this bug. ***

[Alex Christensen]

They should eventually be added, but this is step 1.  That is step 4:

Step 1: Get it to mostly compile
Step 2: Get it to entirely compile and link
Step 3: Get it to run (the XPC services have never worked with CMake)
Step 4: Make it equivalent to the current Xcode build.  This is the hardest step.

[Don Olmstead]

(In reply to Alex Christensen from comment #8)
> They should eventually be added, but this is step 1.  That is step 4:
> 
> Step 1: Get it to mostly compile
> Step 2: Get it to entirely compile and link
> Step 3: Get it to run (the XPC services have never worked with CMake)
> Step 4: Make it equivalent to the current Xcode build.  This is the hardest
> step.

For #4 are you talking about replacing the Xcode build and going to a CMake based everything?

If so I know annulen has mentioned using ExternalProject https://cmake.org/cmake/help/latest/module/ExternalProject.html and it feels like this is how the internal Apple Win build is doing things. I've thought about giving it a try but I don't have access to any internal builds so I'm not sure how to completely match it.

If you want to in the new year work together on it I'd be interested. I think it'd simplify the header copying we should be doing.

[Alex Christensen]

For #4 I'm not making any promises, but if such a thing were to exist it would help some people consider it.

[Alex Christensen]

Created attachment 357847 [details]
Patch

[EWS Watchlist]

Attachment 357847 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/cf/CFURLExtras.cpp:27:  Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 357847 [details]
Patch

OK, r=me as Alex described this is a first step to resurrect macOS CMake build.
CMake build can offer various fancy tools. For example, compile_commands.json can be generated, and it can be an input of many fancy tools (e.g. clang-tidy). CMake ports are useful and thus I think it is worth resurrecting.

[Alex Christensen]

http://trac.webkit.org/r239556