conan-io / cmake-conan

CMake wrapper for conan C and C++ package manager
MIT License
828 stars 251 forks source link

[bug] conanfile.txt and conanfile.py behave inconsistently #662

Open pschichtel opened 3 months ago

pschichtel commented 3 months ago

Describe the bug

I had this conanfile.txt:

[requires]
openssl/3.2.2

[options]
openssl/*:shared=True

and I converted it to this conanfile.py (in order to make the shared option dynamic eventually):

from conan import ConanFile

class LibDataChannel(ConanFile):
    settings = "os"
    requires = "openssl/3.2.2"

    def configure(self):
        # self.options["openssl"].shared = self.settings.os != "Windows"
        self.options["openssl"].shared = True

My expection would be, that these behave identically, but they don't.

When installing the dependencies through cmake using conan-cmake, cmake properly finds the openssl dir, but only with the .txt. When I switch to the .py it stops finding the OpenSSL_DIR and fails.

How to reproduce it

in https://github.com/pschichtel/libdatachannel-java you can run ./gradlew :compileNativeForWindowsX8664 in the root directly. When you replace the jni/conanfile.txt by the mentioned .py file cmake fails due to being unable to find the OpenSSL_DIR.

pschichtel commented 3 months ago

I'm not sure if this is an issue here or in https://github.com/conan-io/cmake-conan, happy to create another one over there.

czoido commented 3 months ago

Hi @pschichtel,

Thanks for the question. I think that the problem could be that you need to add the CMakeDeps generator to your conanfile.py and also expand the settings to include at least the build_type. If you check your logs there should be a warning emitted by cmake-conan saying something like: Cmake-conan: CMakeDeps generator was not defined in the conanfile

Please try with something like this:

from conan import ConanFile

class LibDataChannel(ConanFile):
    settings = "os", "arch", "compiler", "build_type"
    requires = "openssl/3.2.2"
    generators = "CMakeDeps"
    def configure(self):
        # self.options["openssl"].shared = self.settings.os != "Windows"
        self.options["openssl"].shared = True

Hope this helps.

pschichtel commented 3 months ago

thanks @czoido, this version indeed works:

from conan import ConanFile

class LibDataChannel(ConanFile):
    settings = "os", "arch", "compiler", "build_type"
    requires = "openssl/3.2.2"
    generators = "CMakeDeps"

    def configure(self):
        self.options["openssl"].shared = self.settings.os != "Windows"

I still wonder: why do I need the generator explicitly in the python version, when I don't need it in the text version?

czoido commented 3 months ago

I still wonder: why do I need the generator explicitly in the python version, when I don't need it in the text version?

That's related to these lines of cmake-conan, it behaves a bit different when you use a conanfile.txt or conanfile.py, but the warning is always there. Check here: https://github.com/conan-io/cmake-conan/blob/8bf396b92f37f8942d0be24453e8aa268d61ecba/conan_provider.cmake#L572-L585

I think that's to prevent fails and try not to break users from cmake-conan 1.x in the simple conanfile.txt version and as said in the comments in the code it will be mandatory in the near future to have the generator in the conanfile. I'll check with the team if we need to introduce a more visible warning there or something.

Anyway, I'm glad that it finally worked. I'll keep the issue open until we decide if we have to make something in the cmake-conan side. Thanks a lot!

pschichtel commented 3 months ago

Ah this makes sense. I was actually not aware I have warnings, I guess it gets drowned by all the log output from the actual compilation.

I could imagine having some form of "strict mode" that just outright fails things like this.

czoido commented 3 months ago

Hi @pschichtel,

Thanks for the feedback, I'm transferring this to the cmake-conan repo to check if we want to handle the issue there in some way.

memsharded commented 3 months ago

Thanks for the feedback. This tool will remove the automatic injection of CMakeDeps and require conanfiles to explicitly define it, the warning is preventing users to start defining it explicitly to avoid future failure.