Skip to content

[openssl] Fix exported config#20831

Merged
BillyONeal merged 4 commits intomicrosoft:masterfrom
dg0yt:ssl-config
Oct 27, 2021
Merged

[openssl] Fix exported config#20831
BillyONeal merged 4 commits intomicrosoft:masterfrom
dg0yt:ssl-config

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Oct 18, 2021

Describe the pull request

@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 19, 2021
Copy link
Contributor Author

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Concerning risk: some ports may suddenly start to discover openssl via png-config.

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

Did you test the usage?

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 20, 2021

Did you test the usage?

CMake: No. I think it is safe to assume that it works as well as CMake's recent changes, and that at least one port tested in vcpkg CI uses a canonical find_package(OpenSSL).
Of course we might move on, identify such a port, and remove all the OpenSSL workarounds and patches added by vcpkg.

Pkg-config: No. Every port is using OpenSSL workarounds and patches. Or simply doesn't use openssl. Note that the missing file was discovered with #20556 (libnice) or more precisely for nghttp2 (#20556 (comment)).
Of course we might move on and try to enable openssl in nghttp2. But there is no request for it. Maybe @Neumann-A wants to test with libnice?

But note that this is a by-product of starting to use pkg-config files for nmake/windows in port gdal where invalid exported cmake config made its way into netcdf-c's pkg-config files, breaking the build of gdal. So this going to be part of a solid CI tested network of dependencies around gdal. I'm sure I will touch a lot of ports in that network which use hard-wired transitive dependencies at the moment. That's why I'm reluctant to add additional ports to my list. (I'm just fixing gdal's dependencies for mingw builds again.)

@JackBoosY JackBoosY added 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. labels Oct 20, 2021
@JackBoosY
Copy link
Contributor

I'd like other guys to double confirm this.

Name: OpenSSL-libcrypto
Description: OpenSSL cryptography library
Libs: -L"${libdir}" -llibcrypto
Libs.private: -lcrypt32 -lws2_32
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't MinGW need these too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mingw already gets pc files, as it is using the unix branch of the port. libcrypto.pc for mingw-dynamic has:

Libs.private: -lws2_32 -lgdi32 -lcrypt32

But my change for windows is based on consuming ports.

@BillyONeal BillyONeal merged commit 1baf31a into microsoft:master Oct 27, 2021
@BillyONeal
Copy link
Member

Thanks for the help!

@dg0yt dg0yt deleted the ssl-config branch November 13, 2021 04:46
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