Bug 197132 – Progress towards resurrecting Mac CMake build

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Alex Christensen]

Progress towards resurrecting Mac CMake build

[Alex Christensen]

Created attachment 367865 [details]
Patch

[Alex Christensen]

Comment on attachment 367865 [details]
Patch

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

> Source/WebCore/CMakeLists.txt:2041
> +    COMMAND ${PERL_EXECUTABLE} ${JavaScriptCore_SCRIPTS_DIR}/xxd.pl WHLSLStandardLibrary ${WEBCORE_DIR}/Modules/webgpu/WHLSL/WHLSLStandardLibrary.txt ${WebCore_DERIVED_SOURCES_DIR}/WHLSLStandardLibrary.h ${WebCore_DERIVED_SOURCES_DIR}/WHLSLStandardLibrary.cpp

Something isn't quite right here.  It doesn't make WHLSLStandardLibrary.cpp

[Alex Christensen]

Created attachment 369027 [details]
Patch

[EWS Watchlist]

Comment on attachment 369027 [details]
Patch

Attachment 369027 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12094875

New failing tests:
http/tests/css/filters-on-iframes.html

[EWS Watchlist]

Created attachment 369036 [details]
Archive of layout-test-results from ews212 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Don Olmstead]

Comment on attachment 369027 [details]
Patch

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

> Source/WebKitLegacy/PlatformMac.cmake:-434
> -WEBKIT_CREATE_FORWARDING_HEADERS(WebKitLegacy DIRECTORIES ${WebKitLegacy_FORWARDING_HEADERS_DIRECTORIES} FILES ${WebKitLegacy_FORWARDING_HEADERS_FILES})

Can we just get rid of this macro while doing this work? I know its still used in Source/WebKit but we should be able to copy there as well.

> Source/WebKitLegacy/PlatformMac.cmake:430
> +WEBKIT_MAKE_FORWARDING_HEADERS(WebKitLegacy
> +    TARGET_NAME WebKitLegacyFrameworkHeaders
> +    DESTINATION ${WebKitLegacy_FRAMEWORK_HEADERS_DIR}/WebKitLegacy
> +    FILES ${WebKitLegacy_FORWARDING_HEADERS_FILES}
> +    FLATTENED
>  )

You can move this into the main CMakeLists.txt since both ports are using this.

[Alex Christensen]

Comment on attachment 369027 [details]
Patch

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

>> Source/WebKitLegacy/PlatformMac.cmake:-434
>> -WEBKIT_CREATE_FORWARDING_HEADERS(WebKitLegacy DIRECTORIES ${WebKitLegacy_FORWARDING_HEADERS_DIRECTORIES} FILES ${WebKitLegacy_FORWARDING_HEADERS_FILES})
> 
> Can we just get rid of this macro while doing this work? I know its still used in Source/WebKit but we should be able to copy there as well.

Eventually.  Not with this patch.

>> Source/WebKitLegacy/PlatformMac.cmake:430
>>  )
> 
> You can move this into the main CMakeLists.txt since both ports are using this.

I'd like to avoid touching the windows build in this patch.  We can combine those in a followup patch.

[Alex Christensen]

Created attachment 371430 [details]
Patch

[Alex Christensen]

This gets JavaScriptCore and most of WebCore building.  There are still some issues, but this is a good checkpoint to upstream and continue.

[Don Olmstead]

Comment on attachment 371430 [details]
Patch

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

r=me looks like a good start! Only concern is the library path.

One thing I could use assistance with is creating a Find module for Apple specific frameworks for Apple builds (Win and Mac). Pop on IRC to discuss sometime.

> Source/cmake/target/icu.cmake:-3
> -if (NOT TARGET ICU::data)

Any reason for getting rid of the `if (NOT TARGET ICU::data)`? was something failing for you? Its fine as long as this is only included once.

> Source/cmake/target/icu.cmake:9
> +    set(ICU_I18N_LIBRARY /usr/lib/libicucore.dylib)
> +    set(ICU_UC_LIBRARY /usr/lib/libicucore.dylib)
> +    set(ICU_DATA_LIBRARY /usr/lib/libicucore.dylib)

Is this always the path? Should we use a find_library for icucore.dylib in a private variable and then just set everything using that?

[Alex Christensen]

(In reply to Don Olmstead from comment #10)
> Comment on attachment 371430 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371430&action=review
> 
> r=me looks like a good start! Only concern is the library path.
> 
> One thing I could use assistance with is creating a Find module for Apple
> specific frameworks for Apple builds (Win and Mac). Pop on IRC to discuss
> sometime.
> 
> > Source/cmake/target/icu.cmake:-3
> > -if (NOT TARGET ICU::data)
> 
> Any reason for getting rid of the `if (NOT TARGET ICU::data)`? was something
> failing for you? Its fine as long as this is only included once.
It failed with the condition and didn't without the condition.  I'm not sure why it was ever there.

> 
> > Source/cmake/target/icu.cmake:9
> > +    set(ICU_I18N_LIBRARY /usr/lib/libicucore.dylib)
> > +    set(ICU_UC_LIBRARY /usr/lib/libicucore.dylib)
> > +    set(ICU_DATA_LIBRARY /usr/lib/libicucore.dylib)
> 
> Is this always the path? Should we use a find_library for icucore.dylib in a
> private variable and then just set everything using that?
This is always the path.

[WebKit Commit Bot]

Comment on attachment 371430 [details]
Patch

Clearing flags on attachment: 371430

Committed r246126: <https://trac.webkit.org/changeset/246126>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/51455272>