Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lcms] Backfill compat for lcms::lcms #20034

Merged
merged 6 commits into from
Sep 23, 2021

Conversation

ras0219-msft
Copy link
Contributor

Backfill a few issues unaddressed in #19551:

  1. Follow upstream naming since we're breaking the binary names -- remove d suffix in debug
  2. Add back compat support for lcms::lcms since there's no need to break existing users
  3. Remove deprecated helpers (vcpkg_configure_cmake)
  4. Switch to version scheme

ports/lcms/usage Outdated Show resolved Hide resolved
ports/lcms/portfile.cmake Outdated Show resolved Hide resolved
ports/lcms/usage Outdated Show resolved Hide resolved
ports/lcms/usage Outdated Show resolved Hide resolved
@NancyLi1013 NancyLi1013 added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Sep 8, 2021
@c72578
Copy link
Contributor

c72578 commented Sep 8, 2021

Concerning the removal of the d suffix in debug, please consider ports/libraw/lcms2_debug_fix.patch:

+find_library(LCMS2_LIBRARY_DEBUG NAMES lcms2d liblcms2d lcms-2d liblcms-2d

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 7e044226c8a6f43dac7d9e0efd8edbf8ff5ecd04 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/lcms.json b/versions/l-/lcms.json
index 7e02096..a41a3b6 100644
--- a/versions/l-/lcms.json
+++ b/versions/l-/lcms.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "2325b67210a3ae0feb4348fb889e1e55436363b8",
+      "git-tree": "d10fd604e23cd1145f606998d19f607641bce13b",
       "version": "2.12",
       "port-version": 1
     },

Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy as long as @c72578 's comment is addressed.

ports/lcms/portfile.cmake Outdated Show resolved Hide resolved
@ras0219-msft
Copy link
Contributor Author

Because vcpkg builds each flavor separately, the results will still be correct even given the patch: CMake will only ever find a "release" version (which is actually the debug version when built in debug) and will use that regardless of the consumer's configuration.

@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013 NancyLi1013 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Sep 23, 2021
@NancyLi1013 NancyLi1013 added the info:internal This PR or Issue was filed by the vcpkg team. label Sep 23, 2021
@BillyONeal BillyONeal merged commit 674e616 into microsoft:master Sep 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants