Skip to content

[proj4] Install pc file#20458

Merged
vicroms merged 5 commits intomicrosoft:masterfrom
dg0yt:proj-pc-file
Oct 22, 2021
Merged

[proj4] Install pc file#20458
vicroms merged 5 commits intomicrosoft:masterfrom
dg0yt:proj-pc-file

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Oct 1, 2021

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 1, 2021

@strega-nil-ms This change uses VCPKG_SYSTEM_LIBRARIES from vcpkg_common_definitions.cmake to determine the C++ standard library.
However, this variable doesn't have the literal name (stdc++ etc.) but escapes + (stdc\+\+). Is this intentional?
(I also don't see how a triplet could override the C++ library e.g. for clang toolchains instead of g++ toolchains.)

@dg0yt dg0yt marked this pull request as draft October 1, 2021 14:43
@dg0yt dg0yt marked this pull request as ready for review October 1, 2021 19:46
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

I need time to review, so assigning to myself.

@strega-nil-ms
Copy link
Contributor

strega-nil-ms commented Oct 8, 2021

After discussion, we've agreed that VCPKG_SYSTEM_LIBRARIES is not used anywhere and therefore we should just remove it entirely; the correct way to check for these things is to ask CMake which libraries it needs to link to privately. @ras0219 is currently trying to figure out how to query CMake for these things.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 8, 2021

After discussion, we've agreed that VCPKG_SYSTEM_LIBRARIES is not used anywhere and therefore we should just remove it entirely; the correct way to check for these things is to ask CMake which libraries it needs to link to privately. @ras0219 is currently trying to figure out how to query CMake for these things.

Thanks for feedback. Well, removal was the most likely outcome that I expected when I started using it.
I wonder if you should deprecate the variable in one "release" before removing it before removing it in another, if you care for user-side overlay ports and registries. It is not used anywhere in vcpkg, but it is not an internal feature.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 13, 2021

@ras0219 is currently trying to figure out how to query CMake for these things.

I don't want to delay this PR. I use the same condition now as port gdal does for linking proj.

@dg0yt dg0yt marked this pull request as draft October 13, 2021 19:18
@dg0yt dg0yt marked this pull request as ready for review October 15, 2021 05:25
@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 15, 2021
@JackBoosY
Copy link
Contributor

Ping @strega-nil-ms for response.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 15, 2021

FTR I did the experimentation in #20480. This is the result from the lesson-learnt.

@JackBoosY JackBoosY added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Oct 15, 2021
@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 20, 2021
@vicroms vicroms merged commit a901b1e into microsoft:master Oct 22, 2021
@dg0yt dg0yt deleted the proj-pc-file branch October 22, 2021 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants