[curl] Fixes pkgconfig configuration file #14350

Matioupi · 2020-11-02T08:56:48Z

What does your PR fix?
This PR fixes the libcurl.pc pkgconfig configuration file for windows debug build. It also enables tool on non windows platforms (except uwp)
Which triplets are supported/not supported? Have you updated the CI baseline?

test triplets : x64-windows / x64-windows-static / x86-windows / x86-windows-static / x64-linux / arm64-linux
(tested with/without tool feature enabled) and x64-uwp (core feature only)

Does your PR follow the maintainer guide?

Yes, to the best of my knowledge...

…indows platforms

PhoebeHui · 2020-11-03T09:09:26Z

ports/curl/portfile.cmake

+# Fix the pkgconfig file for debug
+if(NOT DEFINED VCPKG_BUILD_TYPE OR VCPKG_BUILD_TYPE STREQUAL "debug")
+    if(VCPKG_TARGET_IS_WINDOWS)
+        vcpkg_replace_string(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc "-lcurl" "-lcurl-d")
+        vcpkg_replace_string(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc "/lib/zlib.lib" "/debug/lib/zlibd.lib")	
+    endif()
+    file(COPY ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc DESTINATION ${CURRENT_PACKAGES_DIR}/debug/lib/pkgconfig)
+endif()


Could you use vcpkg_fixup_pkgconfig() instead?

Hello, I tried, but without working examples of how to properly us it, I only achieved to get error messages such as :

for additionnal command :

vcpkg_fixup_pkgconfig(DEBUG_FILES libcurl.pc)

-- Fixing pkgconfig file: C:/src/vcpkg/packages/curl_x64-windows/lib/pkgconfig/libcurl.pc -- CHECK_LIB_curl C:/src/vcpkg/packages/curl_x64-windows/lib/zlib.lib_RELEASE:CHECK_LIB_curl C:/src/vcpkg/packages/curl_x64-windows/lib/zlib.lib_RELEASE-NOTFOUND CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:232 (message): Library "curl C:/src/vcpkg/packages/curl_x64-windows/lib/zlib.lib" was not found! If it is a system library use the SYSTEM_LIBRARIES parameter for the vcpkg_fixup_pkgconfig call! Otherwise, correct the *.pc file Call Stack (most recent call first): scripts/cmake/vcpkg_fixup_pkgconfig.cmake:298 (vcpkg_fixup_pkgconfig_check_files) ports/curl/portfile.cmake:125 (vcpkg_fixup_pkgconfig) scripts/ports.cmake:136 (include)

So in the end, I used a search/replace strategy (as in the zlib portfile by the way).

If you have some examples of how to use vcpkg_fixup_pkgconfig() properly (e.g. in this particular case) I'd change it in the PR and can also add the exemple in https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/vcpkg_fixup_pkgconfig.md which do not contain specific exemples.

@Matioupi, it seems vcpkg_fixup_pkgconfig() doesn't handle this case, cc @Neumann-A, he might be interested in this.

I don't have a good suggestion now, replace the string seems the only way that fix the issue now.

vcpkg_fixup_pkgconfig() only fixes the paths to make them work with relative paths and checks if the libraries mentioned within the pc file exists. If they do not exists they need to be corrected. As such a) first correct the library names in the *.pc file and b) call vcpkg_fixup_pkgconfig() after that to handle/correct the paths and make them relative.
vcpkg_fixup_pkgconfig is not intended to automatically fix -l flags!

@Neumann-A, I think the zlib also need to be replaced.

From
Libs.private: -lwldap32 -lwinmm -lws2_32 -ladvapi32 -lcrypt32 E:/vcpkg/clean/vcpkg/installed/x64-windows/lib/zlib.lib

To
Libs.private: -lwldap32 -lwinmm -lws2_32 -ladvapi32 -lcrypt32 -lzlib

Then it will check the zlib.pc file when use vcpkg_fixup_pckconfig.

Neumann-A · 2020-11-10T10:01:06Z

ports/curl/portfile.cmake

+    endif()
+    file(COPY ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc DESTINATION ${CURRENT_PACKAGES_DIR}/debug/lib/pkgconfig)
+endif()
+


add the call after you corrected the *.pc files

Suggested change

vcpkg_fixup_pkgconfig()

Hello, I took into account both your request and the one from @PhoebeHui to change :/vcpkg/clean/vcpkg/installed/x64-windows/lib/zlib.lib style lib to -lzlib. Actually, I had to do it otherwise vcpkg_fixup_pkgconfig() call was failing.
I also added other libs for some feature enabled builds such as :
vcpkg.exe install curl[core,tool,http2,ssh,openssl,winssl,mbedtls,sspi]:x64-windows

…ent) Added vcpkg_fixup_pkgconfig() according to microsoft#14350 (comment)

This reverts commit cb4da8b.

Matioupi · 2020-11-17T11:29:34Z

With commit cb4da8b
build with features such as : .\vcpkg\vcpkg.exe install curl[core,tool,http2,ssh,openssl,winssl,mbedtls,sspi]:x64-windows
fails (where not failing before the commit)

PhoebeHui · 2020-11-18T02:20:45Z

@Matioupi, you need to add the following libs as system libraries likes the commit 'cb4da8b' do.

BTW, I tested them locally, and it has others issues except the following CI failures. So I summit that changes, I haven't test the features. however, it passed with core. I suggest to test this changes locally with defalut and features.

See the CI test failures:

x86-windows/x64-windows/x64-windows-static/arm64-windows

-- Fixing pkgconfig file: D:/packages/curl_x86-windows/lib/pkgconfig/libcurl.pc
-- CHECK_LIB_wldap32_RELEASE:CHECK_LIB_wldap32_RELEASE-NOTFOUND
CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:233 (message):
  Library "wldap32" was not found! If it is a system library use the
  SYSTEM_LIBRARIES parameter for the vcpkg_fixup_pkgconfig call! Otherwise,
  correct the *.pc file
Call Stack (most recent call first):
  scripts/cmake/vcpkg_fixup_pkgconfig.cmake:299 (vcpkg_fixup_pkgconfig_check_files)
  ports/curl/portfile.cmake:144 (vcpkg_fixup_pkgconfig)
  scripts/ports.cmake:136 (include)

x64-uwp:

CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:233 (message):
  Library "crypt32" was not found! If it is a system library use the
  SYSTEM_LIBRARIES parameter for the vcpkg_fixup_pkgconfig call! Otherwise,
  correct the *.pc file
Call Stack (most recent call first):
  scripts/cmake/vcpkg_fixup_pkgconfig.cmake:299 (vcpkg_fixup_pkgconfig_check_files)
  ports/curl/portfile.cmake:144 (vcpkg_fixup_pkgconfig)
  scripts/ports.cmake:136 (include)

x64-linux and x64-osx

-- Fixing pkgconfig file: /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/libcurl.pc
-- CHECK_LIB_c /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libssl.a /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libcrypto.a /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libz.a_RELEASE:CHECK_LIB_c /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libssl.a /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libcrypto.a /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libz.a_RELEASE-NOTFOUND
CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:233 (message):
  Library "c
  /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libssl.a
  /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libcrypto.a
  /mnt/vcpkg-ci/packages/curl_x64-linux/lib/pkgconfig/../../lib/libz.a" was
  not found! If it is a system library use the SYSTEM_LIBRARIES parameter for
  the vcpkg_fixup_pkgconfig call! Otherwise, correct the *.pc file

Matioupi · 2020-11-18T06:51:55Z

@PhoebeHui : Wouldn't it be better to add those libs as Windows system libs "vcpkg wide" ? as proposed in : aee37bd ?

Neumann-A · 2020-11-18T08:22:13Z

scripts/cmake/vcpkg_common_definitions.cmake

@@ -192,5 +192,7 @@ if(VCPKG_TARGET_IS_WINDOWS)
    list(APPEND VCPKG_SYSTEM_LIBRARIES vfw32)
    list(APPEND VCPKG_SYSTEM_LIBRARIES winmm)
    list(APPEND VCPKG_SYSTEM_LIBRARIES wsock32)
-    list(APPEND VCPKG_SYSTEM_LIBRARIES Ws2_32)
+    list(APPEND VCPKG_SYSTEM_LIBRARIES ws2_32)


NO! Add both cases if necessary. vcpkg_fixup_pkgconfig does not do a case insensitive compare!

Ok, so far I've seen no side effet of having ws2_32 instead of Ws2_32 and this change was mostly cosmetic. I believe it's better to have only once the Ws2_32 with the capital W than twice. Will revert to Ws2_32

PhoebeHui · 2020-11-19T06:51:58Z

@Matioupi, the feature 'c-ares' test fail with x64-windows, could you please take a look?

-- CHECK_LIB_curl E:/vcpkg/14350/vcpkg/packages/curl_x64-windows/lib/cares.lib_RELEASE:CHECK_LIB_curl E:/vcpkg/14350/vcpkg/packages/curl_x64-windows/lib/cares.lib_RELEASE-NOTFOUND
CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:233 (message):
  Library "curl E:/vcpkg/14350/vcpkg/packages/curl_x64-windows/lib/cares.lib"
  was not found! If it is a system library use the SYSTEM_LIBRARIES parameter
  for the vcpkg_fixup_pkgconfig call! Otherwise, correct the *.pc file
Call Stack (most recent call first):
  scripts/cmake/vcpkg_fixup_pkgconfig.cmake:299 (vcpkg_fixup_pkgconfig_check_files)
  ports/curl/portfile.cmake:166 (vcpkg_fixup_pkgconfig)
  scripts/ports.cmake:136 (include)

Other features test passed with x64-windows.

Matioupi · 2020-11-20T05:00:44Z

Should be fixed now. I guess the regressions in C.I. are unrelated to those commits

PhoebeHui · 2020-11-20T06:15:26Z

@Matioupi, please ignore it, currently it has been fixed, I have rerun the CI testing.

All features test passed with x64-windows.

strega-nil · 2020-11-20T20:18:25Z

Alright, thanks @Matioupi :D

* Fix pkgconfig for windows debug build. Re-allow tool feature on non windows platforms * Fix tool feature * Modified debug/release prvate libs according to #14350 (comment) Added vcpkg_fixup_pkgconfig() according to #14350 (comment) * Delete portfile.cmake.orig * Fix pkgconfig * Revert "Fix pkgconfig" This reverts commit cb4da8b. * Bump port version to 3 * add missing system libs for windows. made case type coherent for ws2_32 * Fix build on x64-linux triplet * Reverted ws2_32 to Ws2_32 as discudssed in #14350 (comment) * More fixes for Linux build with features * Fix for feature c-ares * Formatting * First fix attemps for #14681 * More fix for #14681 (Windows only for now) * Add brotli dependcy when brotli feature is enabled * Fixed indentation Co-authored-by: Phoebe Ma <[email protected]>

* Fix pkgconfig for windows debug build. Re-allow tool feature on non windows platforms * Fix tool feature * Modified debug/release prvate libs according to microsoft/vcpkg#14350 (comment) Added vcpkg_fixup_pkgconfig() according to microsoft/vcpkg#14350 (comment) * Delete portfile.cmake.orig * Fix pkgconfig * Revert "Fix pkgconfig" This reverts commit cb4da8b. * Bump port version to 3 * add missing system libs for windows. made case type coherent for ws2_32 * Fix build on x64-linux triplet * Reverted ws2_32 to Ws2_32 as discudssed in microsoft/vcpkg#14350 (comment) * More fixes for Linux build with features * Fix for feature c-ares * Formatting Co-authored-by: Phoebe Ma <[email protected]>

* Fix pkgconfig for windows debug build. Re-allow tool feature on non windows platforms * Fix tool feature * Modified debug/release prvate libs according to microsoft/vcpkg#14350 (comment) Added vcpkg_fixup_pkgconfig() according to microsoft/vcpkg#14350 (comment) * Delete portfile.cmake.orig * Fix pkgconfig * Revert "Fix pkgconfig" This reverts commit cb4da8b. * Bump port version to 3 * add missing system libs for windows. made case type coherent for ws2_32 * Fix build on x64-linux triplet * Reverted ws2_32 to Ws2_32 as discudssed in microsoft/vcpkg#14350 (comment) * More fixes for Linux build with features * Fix for feature c-ares * Formatting * First fix attemps for microsoft/vcpkg#14681 * More fix for microsoft/vcpkg#14681 (Windows only for now) * Add brotli dependcy when brotli feature is enabled * Fixed indentation Co-authored-by: Phoebe Ma <[email protected]>

Matioupi added 2 commits November 1, 2020 14:53

Fix pkgconfig for windows debug build. Re-allow tool feature on non w…

d4eb60b

…indows platforms

Fix tool feature

dda7c72

LilyWangL assigned PhoebeHui Nov 3, 2020

PhoebeHui requested changes Nov 3, 2020

View reviewed changes

PhoebeHui changed the title ~~Curl fixes~~ [curl] Fixes pkgconfig configuration file Nov 3, 2020

PhoebeHui added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Nov 3, 2020

PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Nov 10, 2020

PhoebeHui approved these changes Nov 10, 2020

View reviewed changes

PhoebeHui removed the info:reviewed Pull Request changes follow basic guidelines label Nov 10, 2020

Neumann-A reviewed Nov 10, 2020

View reviewed changes

PhoebeHui added the requires:author-response label Nov 11, 2020

Matioupi and others added 4 commits November 14, 2020 09:55

Modified debug/release prvate libs according to microsoft#14350 (comm…

8e9fe42

…ent) Added vcpkg_fixup_pkgconfig() according to microsoft#14350 (comment)

Merge remote-tracking branch 'upstream/master' into curl_fixes

415f5f0

Delete portfile.cmake.orig

7a0ca0e

Fix pkgconfig

cb4da8b

PhoebeHui force-pushed the curl_fixes branch from cb4da8b to 2938eec Compare November 17, 2020 10:08

Revert "Fix pkgconfig"

a9c9474

This reverts commit cb4da8b.

Matioupi force-pushed the curl_fixes branch from 2938eec to a9c9474 Compare November 17, 2020 11:30

Bump port version to 3

c9ce6b5

add missing system libs for windows. made case type coherent for ws2_32

aee37bd

Fix build on x64-linux triplet

7f65283

Neumann-A reviewed Nov 18, 2020

View reviewed changes

Matioupi added 2 commits November 18, 2020 09:33

Reverted ws2_32 to Ws2_32 as discudssed in microsoft#14350 (comment)

0f48be7

More fixes for Linux build with features

da88a18

PhoebeHui added the requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function label Nov 19, 2020

Matioupi added 2 commits November 19, 2020 09:47

Fix for feature c-ares

d5d42b2

Formatting

bed80ce

PhoebeHui approved these changes Nov 20, 2020

View reviewed changes

PhoebeHui added info:reviewed Pull Request changes follow basic guidelines and removed requires:all-feature-testing vcpkg install port[all features supported by that port] needs to be demonstrated to function requires:author-response labels Nov 20, 2020

strega-nil merged commit 878d7ea into microsoft:master Nov 20, 2020

StarGate-One mentioned this pull request Nov 20, 2020

[curl] x64-windows build failure with features brotli,c-ares,core,http2,mbedtls,non-http,openssl,schannel,ssh,ssl,sspi,tool,winssl after curl Fixes pkgconfig configuration file (#14350) #14681

Closed

Matioupi mentioned this pull request Nov 23, 2020

[curl] Fixes pkgconfig file #14715

Merged

klalumiere mentioned this pull request Dec 1, 2020

[curl] Mitigate issue #14834: port broken on Linux #14865

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[curl] Fixes pkgconfig configuration file #14350

[curl] Fixes pkgconfig configuration file #14350

Matioupi commented Nov 2, 2020

PhoebeHui Nov 3, 2020

Matioupi Nov 3, 2020

PhoebeHui Nov 10, 2020 •

edited

Loading

Neumann-A Nov 10, 2020

PhoebeHui Nov 10, 2020

Neumann-A Nov 10, 2020

Matioupi Nov 14, 2020

Matioupi commented Nov 17, 2020

PhoebeHui commented Nov 18, 2020 •

edited

Loading

Matioupi commented Nov 18, 2020

Neumann-A Nov 18, 2020

Matioupi Nov 18, 2020

PhoebeHui commented Nov 19, 2020 •

edited

Loading

Matioupi commented Nov 20, 2020

PhoebeHui commented Nov 20, 2020

strega-nil commented Nov 20, 2020

[curl] Fixes pkgconfig configuration file #14350

[curl] Fixes pkgconfig configuration file #14350

Conversation

Matioupi commented Nov 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhoebeHui Nov 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Matioupi commented Nov 17, 2020

PhoebeHui commented Nov 18, 2020 • edited Loading

Matioupi commented Nov 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhoebeHui commented Nov 19, 2020 • edited Loading

Matioupi commented Nov 20, 2020

PhoebeHui commented Nov 20, 2020

strega-nil commented Nov 20, 2020

PhoebeHui Nov 10, 2020 •

edited

Loading

PhoebeHui commented Nov 18, 2020 •

edited

Loading

PhoebeHui commented Nov 19, 2020 •

edited

Loading