Summary: | JSC build fails on Big Sur 11.0.1 beta | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergey Rubanov <chi187> | ||||||||||
Component: | CMake | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Trivial | CC: | annulen, ap, ddkilzer, ews-watchlist, fpizlo, gyuyoung.kim, krollin, ryuan.choi, sergio, smoley, ysuzuki | ||||||||||
Priority: | P4 | Keywords: | DoNotImportToRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Other | ||||||||||||
Attachments: |
|
Description
Sergey Rubanov
2020-11-04 07:41:49 PST
Created attachment 413161 [details]
Patch
Comment on attachment 413161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413161&action=review r- for the cmake minimum version change. > CMakeLists.txt:9 > -cmake_minimum_required(VERSION 3.10) > +cmake_minimum_required(VERSION 3.18) Is this needed to use the find_library() function? I only have 3.17.x installed on my macOS 10.15.6 system, so I think this would break (Apple) folks using CMake on macOS. Oh, actually cmake minimum required version update is not required, I misunderstood this issue at first https://gitlab.kitware.com/cmake/cmake/-/issues/21007 and forgot to remove this change in CMakeLists Created attachment 413183 [details]
Patch
Comment on attachment 413183 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413183&action=review r=me > Source/cmake/WebKitFindPackage.cmake:94 > + # Apple just has a single tbd for ICU Nit: Add period at the end of the comment. (Not necessary to land, but you might want to make this change while fixing the ChangeLog.) > ChangeLog:8 > + * CMakeLists.txt: Nit: Remove this line as this file is no longer changed. Created attachment 413190 [details]
Patch
(In reply to Sergey Rubanov from comment #6) > Created attachment 413190 [details] > Patch Can you set cq+, or do I need to do it? (I didn't want to set it if you had planned to later.) (In reply to David Kilzer (:ddkilzer) from comment #7) > (In reply to Sergey Rubanov from comment #6) > > Created attachment 413190 [details] > > Patch > > Can you set cq+, or do I need to do it? (I didn't want to set it if you had > planned to later.) Sorry, I'm new here and this is my first patch =) What is cq+ or where can I read about that? Created attachment 413227 [details]
Patch for landing
Committed r269410: <https://trac.webkit.org/changeset/269410> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413227 [details]. |