Bug 223442 - [CMake] Use an imported target for the LCMS2 library
Summary: [CMake] Use an imported target for the LCMS2 library
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords: InRadar
Depends on: 177185
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-18 06:43 PDT by Adrian Perez
Modified: 2021-03-23 14:39 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.96 KB, patch)
2021-03-18 06:51 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch v2 (9.48 KB, patch)
2021-03-19 14:09 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff
Patch for landing (9.58 KB, patch)
2021-03-23 14:01 PDT, Adrian Perez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2021-03-18 06:43:54 PDT
SSIA =]
Comment 1 Adrian Perez 2021-03-18 06:51:15 PDT
Created attachment 423594 [details]
Patch
Comment 2 Adrian Perez 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 👌️
Comment 3 Adrian Perez 2021-03-19 14:04:01 PDT
I am going to update this now that the patch in bug #223340 has landed.
Comment 4 Adrian Perez 2021-03-19 14:09:26 PDT
Created attachment 423775 [details]
Patch v2

Rebased =)
Comment 5 EWS 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).
Comment 6 Don Olmstead 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.
Comment 7 Adrian Perez 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.
Comment 8 Adrian Perez 2021-03-23 14:01:32 PDT
Created attachment 424062 [details]
Patch for landing
Comment 9 EWS 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].
Comment 10 Radar WebKit Bug Importer 2021-03-23 14:39:24 PDT
<rdar://problem/75755925>