Fix CURL ssl option for CPR recipe

[uilianries]

Specify library name and version: cpr/1.3.0

fixes #3206

/cc @FohlenAFK

• I've read the guidelines for contributing.
• I've followed the PEP8 style guides for Python code in the recipes.
• I've used the latest Conan client version.
• I've tried at least one configuration locally with the
  conan-center hook activated.

[madebr]

I think this should be moved to build and changed in a check:

if self.options["libcurl"].with_openssl != "openssl":
    raise ConanInvalidConfiguration("cpr requires libcurl built with the option with_ssl=\"openssl\"")

[uilianries]

Makes more sense!

[SpaceIm]

I disagree, because build() is not always called, and you probably want that recipe works out of the box.

[madebr]

Then the check should (also) be called in package_info.

The options of libcurl are available and it will always be run on consumption.

[SpaceIm]

Yes maybe, but anyway I believe that upstream option should be forced if it's a strong requirement of this recipe. What happen if default option of libcurl is not with_ssl = openssl? No binary is produced in CCI? Downstream recipes shouldn't rely on upstream default options.

[madebr]

Forcing of options is not correct as it will override a user passing e.g. -o libcurl:with_ssl="gnutls". This is surprising behavior for a user.
It's better to tell the user that he is wrong about an option then to override it.
Let's not treat the user as an idiot.

This recipe is not relying on libcurl default options as it actively checks the option.

[SpaceIm]

It's even more surprising for a user to have a recipe not building with its default options. This recipe relies on libcurl default option (which is not openssl on Macos by the way) to build out of the box.

If you just launch conan install somerecipe/x.x.x and it raises because of some options in transitive dependencies, it's quite surprising.

Depending on recipe design, an option could be a feature, or a component which could has been its own recipe.

In conan 2.0, precedence of values should change conan-io/conan#7786

[madebr]

@SpaceIm
Agreed it's not fun for a user when it doesn't work out-of-the box (which it does at the moment), but forcing will ultimately cause problems when 2 recipes require the opposite. As long as the error message is actionable by the user, (s)he isn't puzzled by weird compiler errors.

Thanks for the link to the conan issue.
The proposed validate function is nice to write the checks for options of dependencies in only one place instead, but will not solve the matter of (forcing of) default options, I think.

[SpaceIm]

On the same topic:

• in old and new libcurl recipe (without and with with_ssl option): the default value for ssl lib on Macos is darwinssl
• in old libcurl recipe, forcing with_openssl=True didn't have any effect when darwin_ssl was already True (which was the case for default options, it was with_openssl=True and darwin_ssl=True), and on Macos darwin_ssl always had precedence in old design. So CPR was somehow broken on Macos, if openssl is strictly required.
• in new libcurl recipe, forcing with_ssl =openssl will indeed force using openssl in libcurl. But it's not the default value on Macos, so there is no pre build binaries in CCI for such package ids.
• cpr recipe claims that openssl support is unstable, so why not disabling it by default?

[uilianries]

Okay, let's keep forcing option.

[conan-center-bot]

Failure in build 2 (f20cf6ac63c05c275a95f9609c31a37c1902d6b0):

• Error processing recipe (ref 'cpr/1.4.0'): Windows x86_64, Debug, Visual Studio 15, MTd. Options: cpr:shared-True
  Unexpected error happened:

cpr/1.4.0: WARN: OpenSSL support is not stable yet. whoshuu/cpr#31
libcurl/7.67.0: WARN: with_openssl, with_winssl, darwin_ssl and with_wolfssl options are deprecated. Use with_ssl option instead.
openssl/1.1.1g: ERROR: Error downloading binary package: 'openssl/1.1.1g:963bb116781855de98dbb23aaac41621e5d312d8'
ERROR: 503: Service Temporarily Unavailable. [Remote: central]

[conan-center-bot]

All green in build 3 (f20cf6ac63c05c275a95f9609c31a37c1902d6b0)! 😊

• cpr/1.3.0: Generated 124 packages (+ 12 invalid config from build()). All logs here
• cpr/1.4.0: Generated 124 packages (+ 12 invalid config from build()). All logs here
• cpr/1.5.0: Generated 124 packages (+ 12 invalid config from build()). All logs here

[SpaceIm]

After looking a little bit cpr source code and build files, I'm wondering if this option with_openssl makes sense. It doesn't change cpr binaries nor interface definitions. it's just a redundant option trying to control libcurl option.

[uilianries]

After looking a little bit cpr source code and build files, I'm wondering if this option with_openssl makes sense. It doesn't change cpr binaries nor interface definitions. it's just a redundant option trying to control libcurl option.

I vote for removing

[madebr]

I vote for removing

Same,
@SpaceIm Thanks for checking this

[conan-center-bot]

Some configurations of 'cpr/1.3.0' failed in build 5 (9aef944dee975d2ed709585396f6c86860f71a0e):

• Macos x86_64, Release, apple-clang 11.0, libc++ . Options: cpr:shared-False
  • Log download
  • Log error Download
  • Profile
• Access to all the logs
• Macos x86_64, Debug, apple-clang 10.0, libc++ . Options: cpr:shared-False
  • Log download
  • Log error Download
  • Profile
• Access to all the logs
• Macos x86_64, Debug, apple-clang 11.0, libc++ . Options: cpr:shared-False
  • Log download
  • Log error Download
  • Profile
• Access to all the logs
• Macos x86_64, Release, apple-clang 10.0, libc++ . Options: cpr:shared-False
  • Log download
  • Log error Download
  • Profile
• Access to all the logs

[ghost]

I detected other pull requests that are modifying cpr/all recipe:

• add cpr/1.5.2 + fix options #3813

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.