conda-forge / curl-feedstock

A conda-smithy repository for curl.
BSD 3-Clause "New" or "Revised" License
3 stars 42 forks source link

Add linking tests #130

Closed AntoinePrv closed 11 months ago

AntoinePrv commented 11 months ago

Checklist

conda-forge-webservices[bot] commented 11 months ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

AntoinePrv commented 11 months ago

Hi @isuruf @conda-forge/curl thanks for commenting. This PR is for starting the conversation and debugging the move to libsolv-static>=8.4.0 in micromamba.

What are your thoughts?

isuruf commented 11 months ago

I think this test shows that it is not easy to link with libcurl (libcurl-static in particular) and that such tests should be added to make sure downstream package do not break.

It was easier before and the added complexity is exactly because of micromamba introduced in https://github.com/conda-forge/curl-feedstock/pull/112

The libcurl -> cmake -> libcurl cycle exists only in the test. It is not ideal, since it risks linking with the wrong libcurl, but it remains an implementation detail that does not leak outside of this repo.

No, it's not. When building a new version of curl, let's say curl 9.0, then cmake will be incompatible with the new curl version and result in a solver error when trying to build curl 9.0

Why go with CMake? It is easier to start with, but mainly, curl has a CMakeLists.txt and I would very much like to make follow-up PR that uses it to build the feedstock. Using CMake, will allow to keep all this complexity (that I am not enven sure i got right) inside a single CMake target target_link_library(mylib CURL::libcurl). This is valuable for micromamba but also every other user of this package. Again, the cycle would not leak outside the feedstock. CMake target files for libcurl and libcurl-static would most likely conflict (due to how Curl's CMakeLists.txt is structured), and so I suggest making two fully independent and incompatible package, that have an exclusion rule between them, as it has been done in some other packages that provide CMake targets.

Those reasons seem like a good enough reason to not go with CMake. CMake already has a FindCURL.cmake file built-in. So, why not use that?

AntoinePrv commented 11 months ago

CMake already has a FindCURL.cmake file built-in. So, why not use that?

The FindXXX are meant for libraries not built with CMake. FindCURL probaly originates from a time where Curl had not adopted CMake. I have not used it, but it would be a second class citizen to CURLConfig.cmake generated when building with CMake. The former is a CMake best effort to find CURL, while the latter is the correct up to date thing containing the exact options and targets that were used at build time.

No, it's not. When building a new version of curl, let's say curl 9.0, then cmake will be incompatible with the new curl version and result in a solver error when trying to build curl 9.0

If I understand correctly, putting cmake in as a build dependency would not make this cycle / incompatibility. The build environment would contain cmake > old-libcurl, while the libcurl being built does not exists yet (and is installed in a separate environment). I agree that for testing we have the case that you mention. I'm happy to move that to a non-CMake solution for it, but still think building with CMake is desirable.

isuruf commented 11 months ago

I agree that for testing we have the case that you mention. I'm happy to move that to a non-CMake solution for it, but still think building with CMake is desirable.

No, it's not. It's bound to bring up incompatibilities. See https://conda-forge.org/docs/maintainer/knowledge_base.html#moving-from-an-autotools-build-to-a-cmake-build

AntoinePrv commented 11 months ago

Closing then as I don't know other ways to do it.