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

libcurl: add version 8.11.1 #25880

Merged
merged 7 commits into from
Jan 2, 2025
Merged

libcurl: add version 8.11.1 #25880

merged 7 commits into from
Jan 2, 2025

Conversation

toge
Copy link
Contributor

@toge toge commented Nov 8, 2024

Summary

Changes to recipe: libcurl/8.11.1

Motivation

There are several improvements since 8.10.1.
websockets flag has been introduced since 8.11.0.
form-api flag has been introduces since 8.3.0.

Details

curl/curl@curl-8_10_1...curl-8_11_1


@ErniGH ErniGH self-assigned this Nov 8, 2024
@jcar87 jcar87 self-assigned this Nov 8, 2024
@AbrilRBS AbrilRBS requested a review from uilianries November 13, 2024 10:12
@toge toge changed the title libcurl: add version 8.11.0 libcurl: add version 8.11.1 Dec 12, 2024
Comment on lines 672 to 674
if Version(self.version) >= "8.10.0":
self.cpp_info.set_property("cmake_additional_variables_prefixes", ["WOLFSS"])

Copy link
Contributor

@czoido czoido Dec 31, 2024

Choose a reason for hiding this comment

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

Is this right? I think that what you want to do instead of this is something like this in the generate method:

        deps = CMakeDeps(self)
        deps.set_property("wolfssl", "cmake_additional_variables_prefixes", ["WolfSSL", "WOLFSSL"])
        deps.set_property("wolfssl", "cmake_file_name", "WolfSSL")
        deps.generate()

which would also make that you can remove these lines:

        # wolfssl
        replace_in_file(self, cmakelists, "find_package(WolfSSL REQUIRED)", "find_package(wolfssl REQUIRED CONFIG)")
        if Version(self.version) < "8.10.0":
            replace_in_file(self, cmakelists, "${WolfSSL_LIBRARIES}", "${wolfssl_LIBRARIES}")
            replace_in_file(self, cmakelists, "${WolfSSL_INCLUDE_DIRS}", "${wolfssl_INCLUDE_DIRS}")
        else:
            replace_in_file(self, cmakelists, "${WOLFSSL_LIBRARIES}", "${wolfssl_LIBRARIES}")
            replace_in_file(self, cmakelists, "${WOLFSSL_INCLUDE_DIRS}", "${wolfssl_INCLUDE_DIRS}")

@czoido
Copy link
Contributor

czoido commented Dec 31, 2024

Hi @toge, thanks for the contribution, let me push some more changes on your PR.

@toge
Copy link
Contributor Author

toge commented Dec 31, 2024

@czoido
Thank you for your review and improvements!
Please go ahead.

@czoido czoido marked this pull request as draft December 31, 2024 06:26
@czoido czoido marked this pull request as ready for review December 31, 2024 09:07
czoido
czoido previously approved these changes Dec 31, 2024

# INTERFACE_LIBRARY (generated by the cmake_find_package generator) targets doesn't have the LOCATION property.
# So skipp the LOCATION check in the CMakeLists.txt
replace_in_file(
Copy link
Contributor

Choose a reason for hiding this comment

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

@perseoGI - please revert the removal of this replace_in_file

This is needed for anyone using CMake older than 3.20 (or 3.22) - otherwise they will be faced with the very error this is trying to solve

CMake Error at CMakeLists.txt:2013 (get_target_property):
  INTERFACE_LIBRARY targets may only have whitelisted properties.  The
  property "LOCATION" is not allowed.


CMake Warning at CMakeLists.txt:2015 (message):
  Bad lib in library list: OpenSSL::SSL


CMake Error at CMakeLists.txt:2013 (get_target_property):
  INTERFACE_LIBRARY targets may only have whitelisted properties.  The
  property "LOCATION" is not allowed.


CMake Warning at CMakeLists.txt:2015 (message):
  Bad lib in library list: OpenSSL::Crypto


CMake Error at CMakeLists.txt:2013 (get_target_property):
  INTERFACE_LIBRARY targets may only have whitelisted properties.  The
  property "LOCATION" is not allowed.


CMake Warning at CMakeLists.txt:2015 (message):
  Bad lib in library list: ZLIB::ZLIB

This is caused by the Conan-generated targets not having the location property set, and older versions of CMake were not allowed to instrospect this property on interface librararies (newer versions are fine).

Please remember to be conservative with these cleanups - this definitely has a chance of causing issues for a (now small) number of users

@jcar87 jcar87 merged commit 3ebeb56 into conan-io:master Jan 2, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants