Bug 197132 - Progress towards resurrecting Mac CMake build
Summary: Progress towards resurrecting Mac CMake build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-19 19:43 PDT by Alex Christensen
Modified: 2019-06-05 14:12 PDT (History)
6 users (show)

See Also:


Attachments
Patch (12.95 KB, patch)
2019-04-19 19:43 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (17.60 KB, patch)
2019-05-03 17:16 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews212 for win-future (13.49 MB, application/zip)
2019-05-03 19:35 PDT, EWS Watchlist
no flags Details
Patch (24.97 KB, patch)
2019-06-05 13:07 PDT, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2019-04-19 19:43:17 PDT
Progress towards resurrecting Mac CMake build
Comment 1 Alex Christensen 2019-04-19 19:43:39 PDT
Created attachment 367865 [details]
Patch
Comment 2 Alex Christensen 2019-04-19 19:44:04 PDT
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
Comment 3 Alex Christensen 2019-05-03 17:16:32 PDT
Created attachment 369027 [details]
Patch
Comment 4 EWS Watchlist 2019-05-03 19:35:30 PDT
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
Comment 5 EWS Watchlist 2019-05-03 19:35:32 PDT
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
Comment 6 Don Olmstead 2019-05-06 10:12:30 PDT
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.
Comment 7 Alex Christensen 2019-06-05 13:04:55 PDT
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.
Comment 8 Alex Christensen 2019-06-05 13:07:52 PDT
Created attachment 371430 [details]
Patch
Comment 9 Alex Christensen 2019-06-05 13:08:25 PDT
This gets JavaScriptCore and most of WebCore building.  There are still some issues, but this is a good checkpoint to upstream and continue.
Comment 10 Don Olmstead 2019-06-05 13:16:21 PDT
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?
Comment 11 Alex Christensen 2019-06-05 13:41:32 PDT
(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.
Comment 12 WebKit Commit Bot 2019-06-05 14:11:46 PDT
Comment on attachment 371430 [details]
Patch

Clearing flags on attachment: 371430

Committed r246126: <https://trac.webkit.org/changeset/246126>
Comment 13 WebKit Commit Bot 2019-06-05 14:11:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2019-06-05 14:12:22 PDT
<rdar://problem/51455272>