Bug 223442

Summary: [CMake] Use an imported target for the LCMS2 library
Product: WebKit Reporter: Adrian Perez <aperez>
Component: Tools / TestsAssignee: Adrian Perez <aperez>
Status: RESOLVED FIXED    
Severity: Normal CC: annulen, cgarcia, clopez, don.olmstead, ews-watchlist, gyuyoung.kim, ryuan.choi, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=223340
Bug Depends on: 177185    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch for landing none

Adrian Perez
Reported 2021-03-18 06:43:54 PDT
SSIA =]
Attachments
Patch (9.96 KB, patch)
2021-03-18 06:51 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch v2 (9.48 KB, patch)
2021-03-19 14:09 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Patch for landing (9.58 KB, patch)
2021-03-23 14:01 PDT, Adrian Perez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Adrian Perez
Comment 1 2021-03-18 06:51:15 PDT
Created attachment 423594 [details] Patch
Adrian Perez
Comment 2 2021-03-18 06:52:31 PDT
This patch also includes a fix for bug #223340 — if we land this first, then we can skip that; if the other one lands first, I'll need to update and reupload this one. Either way is fine 👌️
Adrian Perez
Comment 3 2021-03-19 14:04:01 PDT
I am going to update this now that the patch in bug #223340 has landed.
Adrian Perez
Comment 4 2021-03-19 14:09:26 PDT
Created attachment 423775 [details] Patch v2 Rebased =)
EWS
Comment 5 2021-03-23 10:30:34 PDT
/Volumes/Data/worker/Commit-Queue/build/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).
Don Olmstead
Comment 6 2021-03-23 12:25:24 PDT
Comment on attachment 423775 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=423775&action=review > Source/cmake/FindLCMS2.cmake:100 > +mark_as_advanced(LCMS2_INCLUDE_DIR LCMS2_LIBRARIES) This should be LCMS2_LIBRARY. You don't create LCMS2_LIBRARIES until further down. If you copy-pastaed it from somewhere else we should probably fix that. I thought I fixed the cases but could've missed one.
Adrian Perez
Comment 7 2021-03-23 13:58:58 PDT
(In reply to Don Olmstead from comment #6) > Comment on attachment 423775 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=423775&action=review > > > Source/cmake/FindLCMS2.cmake:100 > > +mark_as_advanced(LCMS2_INCLUDE_DIR LCMS2_LIBRARIES) > > This should be LCMS2_LIBRARY. You don't create LCMS2_LIBRARIES until further > down. > > If you copy-pastaed it from somewhere else we should probably fix that. I > thought I fixed the cases but could've missed one. Good catch, thanks. This was only my fault; the rest of the CMake modules (at least the ones updated to follow the imported target style) look fine. I'll fix this before landing.
Adrian Perez
Comment 8 2021-03-23 14:01:32 PDT
Created attachment 424062 [details] Patch for landing
EWS
Comment 9 2021-03-23 14:38:29 PDT
Committed r274903: <https://commits.webkit.org/r274903> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424062 [details].
Radar WebKit Bug Importer
Comment 10 2021-03-23 14:39:24 PDT
<rdar://problem/75755925>
Note You need to log in before you can comment on or make changes to this bug.