[curl] Add feature idn2

[JackBoosY]

• add curl feature idn2 to avoid to find the system idn2 library and headers.

Fixes #14798.

Note: already test all features on x86-windows and x64-windows-static successfully.

[klalumiere]

Hi JackBoosY!

I see that this pull request is still a work in progress. Do you have an ETA for when it's going to be ready? Right now the port curl and the 39 ports that depend on it are broken on Linux (I don't understand why the CI didn't catch this 🙁 ). If you tell me that your ETA is more a few days, I could do a 2-liner pull request that allows the port to work again on Linux while waiting for the cleaner solution you are developing, i.e. doing

vcpkg_fixup_pkgconfig(SYSTEM_LIBRARIES pthread dl c) -> # vcpkg_fixup_pkgconfig(SYSTEM_LIBRARIES pthread dl c).

Port-Version: 3 -> Port-Version: 4

is an ugly (or more precisely, an incorrect) but effective way to make curl work again. 🙂

[JackBoosY]

Hi @klalumiere, it should be fixed on my latest commit, can you test it?
The reason for this issue is that some curl dependencies are not added as other dependencies (add option to CMakeLists.txt and use find_package/find_library to find them), and I've disabled them.

[JackBoosY]

Also, @martti-cloudkey can you please test this PR?

Thanks.

[klalumiere]

The problem with lber has been replaced by a problem with ldap 🙁

-- Installing: /home/klalumiere/Downloads/vcpkg/packages/curl_x64-linux/share/curl/curl-config
-- Fixing pkgconfig file: /home/klalumiere/Downloads/vcpkg/packages/curl_x64-linux/lib/pkgconfig/libcurl.pc
-- CHECK_LIB_ldap_RELEASE:CHECK_LIB_ldap_RELEASE-NOTFOUND
CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:233 (message):
  Library "ldap" 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:187 (vcpkg_fixup_pkgconfig)
  scripts/ports.cmake:136 (include)


Error: Building package curl:x64-linux failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `./vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: curl:x64-linux
  Vcpkg version: 2020.11.12-unknownhash

Additionally, attach any relevant sections from the log files above.
[21:04:48] klalumiere@WKS-000791:~/Downloads/vcpkg$ git status
On branch dev/jack/14798
Your branch is up to date with 'origin/dev/jack/14798'.

nothing to commit, working tree clean

[JackBoosY]

@KaibaLopez Can you just modify VCPKG_ROOT/port/curl/portfile.cmake line 71:

...
    OPTIONS ${FEATURE_OPTIONS}
        -DCURL_DISABLE_LDAP=OFF
        ${UWP_OPTIONS}
...

And try again (do not use my changes)?

Thanks.

[JackBoosY]

@klalumiere Any feedback with my comment?

[klalumiere]

@KaibaLopez Can you just modify VCPKG_ROOT/port/curl/portfile.cmake line 71:

...
    OPTIONS ${FEATURE_OPTIONS}
        -DCURL_DISABLE_LDAP=OFF
        ${UWP_OPTIONS}
...

And try again (do not use my changes)?

Thanks.

I had exactly the same problem with -DCURL_DISABLE_LDAP=OFF (that is, ./vcpkg install curl fails). However, adding -DCURL_DISABLE_LDAP=ON did solve the problem (I think there was a confusion with the double negative 🙂).

[JackBoosY]

@klalumiere Yep, my mistake. It should be ON. I will continue fix this issue today.

[JackBoosY]

@klalumiere Can you remove DCURL_DISABLE_LDAP=ON, install libldap2-dev and libsasl2-dev using apt or yum then try again?

Thanks.

We cannot simply disable ldap because it is indeed enabled and used as a default feature on some machines.
And feature LDAPS has some problem that needs to wait for the official reply, see #14030.

[klalumiere]

So as you (maybe) saw on #14865, I'm not able to reproduce the problem I had anymore on master. However, I still have problem with the branch JackBoosY:dev/jack/14798. I built in a Docker container running the image rikorose/gcc-cmake. I installed vim with

apt-get update && apt-get install vim --no-install-recommends

and that's all. Without any changes to the branch, I ran ./vcpkg install curl:x64-linux-dynamic with the triplet x64-linux-dynamic from the doc, i.e.

set(VCPKG_TARGET_ARCHITECTURE x64)
set(VCPKG_CRT_LINKAGE dynamic)
set(VCPKG_LIBRARY_LINKAGE dynamic) # This changed from static to dynamic

set(VCPKG_CMAKE_SYSTEM_NAME Linux)

It fails, with the output

CMake Error at scripts/cmake/vcpkg_fixup_pkgconfig.cmake:235 (message):
  Unhandled string
  "/root/vcpkg/packages/curl_x64-linux-dynamic/lib/pkgconfig/../../lib/libssl.so
  /root/vcpkg/packages/curl_x64-linux-dynamic/lib/pkgconfig/../../lib/libcrypto.so
  /root/vcpkg/packages/curl_x64-linux-dynamic/lib/pkgconfig/../../lib/libz.so"
  was found! If it is a system library use the SYSTEM_LIBRARIES parameter for
  the vcpkg_fixup_pkgconfig call! Otherwise, correct the *.pc file or add the
  case to vcpkg_fixup_pkgconfig
Call Stack (most recent call first):
  scripts/cmake/vcpkg_fixup_pkgconfig.cmake:299 (vcpkg_fixup_pkgconfig_check_files)
  ports/curl/portfile.cmake:188 (vcpkg_fixup_pkgconfig)
  scripts/ports.cmake:136 (include)


Error: Building package curl:x64-linux-dynamic failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `./vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: curl:x64-linux-dynamic
  Vcpkg version: 2020.11.12-unknownhash

Additionally, attach any relevant sections from the log files above.

I'll try removing -DCURL_DISABLE_LDAP=ON next, although it doesn't seems to be related to the error. I'll try installing the libs you suggested after. It might take a while. Building is super slow in a container 🙁

[klalumiere]

Actually... Shouldn't the string /root/vcpkg/packages/curl_x64-linux-dynamic/lib/pkgconfig/../../lib/libssl.so be replaced by -lssl in the pkgconfig? This seems to be what is done elsewhere in the portfile.cmake. Oh, I see, e.g. portfile.cmake:139

vcpkg_replace_string(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc "${CURRENT_INSTALLED_DIR}/debug/lib/libssl.a" "-lssl")

There should also be

vcpkg_replace_string(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc "${CURRENT_INSTALLED_DIR}/debug/lib/libssl.so" "-lssl")

Couldn't we use a regular expression instead? Something in the spirit of

vcpkg_replace_string(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc "${CURRENT_INSTALLED_DIR}/debug/lib/lib(.*)\\.so" "-l\1")

[klalumiere]

We could add a function

function(vcpkg_replace_string_regex filename match_string replace_string)
    file(READ ${filename} _contents)
    string(REGEX REPLACE "${match_string}" "${replace_string}" _contents "${_contents}")
    file(WRITE ${filename} "${_contents}")
endfunction()

and then use it as I was suggesting, but without my mistake 😆

vcpkg_replace_string_regex(${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/libcurl.pc
    "${CURRENT_INSTALLED_DIR}/debug/lib/lib(.*)\.so" "-l\\1")

[klalumiere]

I had practically the same error than here without -DCURL_DISABLE_LDAP=ON, the only difference being of course the line number, i.e. `ports/curl/portfile.cmake:187 (vcpkg_fixup_pkgconfig)

[JackBoosY]

cc @ras0219 for review this PR. I'm not sure my changes is correct.

[klalumiere]

Merging master did fix the build on Linux with the triplet x64-linux-dynamic. 🥳

The systematic warning when we build on a system different than Windows is a bit unfortunate. 🙁 . Could this warning instead be added to cURL's CMakeLists.txt? I believe that once inside the CMakeList.txt, we might have the information necessary to know if libldap2-dev and libsasl2-dev will be required for the build. We could print the warning only if we detect that they are necessary, and that they can't be found. 🤔

[JackBoosY]

@klalumiere Adding the message to CMakeLists.txt is not a good choice, because I'm not sure if these two packages are needed, and curl builds successfully in my ubuntu without that.

[klalumiere]

@klalumiere Adding the message to CMakeLists.txt is not a good choice, because I'm not sure if these two packages are needed, and curl builds successfully in my ubuntu without that.

I'm not sure I understand. Right now the warning is inside the portfile.cmake. How is it better to put the warning inside the portfile.cmake than to put it in the CMakeLists.txt? As I mentioned above, the advantages of putting it inside the CMakeLists.txt is that you can search (find_library or find_package) for e.g. ldap2 and, if it is not found, print the warning. What are the advantages of putting the warning inside te portfile.cmake?

It also works on my machine (Ubuntu 20.04). dpkg --list libldap2-dev tells me that this package is installed, but dpkg --list libsasl2-dev informs me that there is no such package on my machine. It probably would be worth it to verify what happen inside a Docker container where none of the packages are installed. If none are required, we can juste remove the warning altogether.

[JackBoosY]

@klalumiere Normally, the warning message like that always put into portfile.cmake, such as pixel, glfw3, spdk etc.
We need to detect and prepare these dependencies before configuration, rather than dynamically checking during configuration. Otherwise vcpkg will report error logs instead of error message.

[klalumiere]

I retried to install curl inside the Docker image rikorose/gcc-cmake with your latest changes. I tried with the triplets x64-linux and x64-linux-dynamic. Both work, and I made sure that neither libldap2-dev nor libsasl2-dev are installed. Hence, I don't think that the warning is required: it works without libldap2-dev and libsasl2-dev.

Good point about the error logs instead of error message. I think the way spdk does it is smart, i.e. it uses FIND_PATH to print (an error in this case) only if there's actually a problem.

[BillyONeal]

@JackBoosY Why is this "requires:discussion"? It looks like a normal bugfix to me?

[JackBoosY]

@BillyONeal Discussion for feature lber2 in curl's source. I'm not sure which method we should choose to solve that.