conan icon indicating copy to clipboard operation
conan copied to clipboard

[bug] CMakeDeps with mixed static and shared libraries.

Open jsallay opened this issue 2 years ago • 13 comments

Environment Details (include every applicable attribute)

  • Operating System+version: ubuntu 20.04 (docker)
  • Compiler+version: gcc9
  • Conan version: 2.0-beta2
  • Python version: 3.8.10

Steps to reproduce (Include if Applicable)

I am having an issue when a shared library has a static dependency. The CMakeDeps generator fails. I'll provide a minimal reproducer below.

# This is a static dependency
from conan import ConanFile

class t1Conan(ConanFile):
    name = "t1"
    version = "0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": False}
# shared library that depends on the static t1
from conan import ConanFile

class t2Conan(ConanFile):
    name = "t2"
    version = "0.1.0"
    requires = "t1/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}
   
    # Could be a component instead of the whole package.
    def package_info(self):
        self.cpp_info.requires.append("t1::t1")
# shared library that depends on shared t2 
from conan import ConanFile

class t3Conan(ConanFile):
    name = "t3"
    version = "0.1.0"
    requires = "t2/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}
    generators = "CMakeDeps"
    settings = "os", "arch", "compiler", "build_type"

Logs (Executed commands with output) (Include/Attach if Applicable)

Running conan create t1; conan create t2; conan create t3, we get the following error creating package t3:

t3/0.1.0: ERROR: Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/templates/__init__.py", line 43, in render
    context = self.context
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/templates/target_data.py", line 44, in context
    dependency_filenames = self._get_dependency_filenames()
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/templates/target_data.py", line 200, in _get_dependency_filenames
    req = direct_host[dep_name]
  File "/usr/local/lib/python3.8/dist-packages/conans/model/dependencies.py", line 61, in __getitem__
    return self.get(name)
  File "/usr/local/lib/python3.8/dist-packages/conans/model/dependencies.py", line 56, in get
    raise KeyError("'{}' not found in the dependency set".format(ref))
KeyError: "'t1' not found in the dependency set"

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/conans/client/generators/__init__.py", line 95, in write_generators
    generator.generate()
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/cmakedeps.py", line 46, in generate
    generator_files = self.content
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/cmakedeps.py", line 88, in content
    self._generate_files(require, dep, ret, find_module_mode=False)
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/cmakedeps.py", line 101, in _generate_files
    ret[data_target.filename] = data_target.render()
  File "/usr/local/lib/python3.8/dist-packages/conan/tools/cmake/cmakedeps/templates/__init__.py", line 45, in render
    raise ConanException("error generating context for '{}': {}".format(self.conanfile, e))
conans.errors.ConanException: error generating context for 't2/0.1.0': "'t1' not found in the dependency set"

ERROR: Error in generator 'CMakeDeps': error generating context for 't2/0.1.0': "'t1' not found in the dependency set"

There are several small changes that seem to avoid the error (however I don't believe that they are acceptable workarounds):

  • build t3 with shared=False
  • Remove the requires.append("t1::t1") from t2 package_info
  • Build/use t1 as a shared library.

jsallay avatar Sep 05 '22 12:09 jsallay

Hi @jsallay

I think the issue is defining self.cpp_info.requires.append("t1::t1") in the first place. This feature is only intended and designed to work when t1 package has components defined. Does this still happen if you define components in t1?

The other constrainst, like using shared options, yes, that is not acceptable to change if you want that option. But you really don't need to define that self.cpp_info.requires.append("t1::t1") if the package doesn0 have components, why wouldn't this be a good solution?

memsharded avatar Sep 05 '22 15:09 memsharded

@memsharded, The same problem occurs with components. I've updated t1 and t2 as follows.

class t1Conan(ConanFile):
    name = "t1"
    version = "0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": False}


    def package_info(self):
        self.cpp_info.components["comp1"].set_property("cmake_target_name", "t1::comp1")
        self.cpp_info.components["comp2"].set_property("cmake_target_name", "t1::comp2")
class t2Conan(ConanFile):
    name = "t2"
    version = "0.1.0"
    requires = "t1/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}

    def package_info(self):
        self.cpp_info.requires.append("t1::comp1")

I still get the same error in conan create t3. I was unaware of the requires only being needed for components. I based this issue off of the zeromq recipe which contains in the package_info:

if self.options.encryption == "libsodium":
            self.cpp_info.components["libzmq"].requires.append("libsodium::libsodium")

(libsodium does not define any components).

Is this a change for v2 or is that how it is supposed to work in v1 as well?

jsallay avatar Sep 05 '22 16:09 jsallay

Thanks for the help, I have managed to reproduce.

The issue comes from the definition of:

def package_info(self):
                self.cpp_info.requires.append("t1::comp1")

in t2.

Note this is a definition for the consumers of t2, letting them know: You also depend on t1::comp1. This is not the case for the default linkage requirements in 2.0. If t2 (shared) is linking t1 (static), it is assumed that t1 is embedded and hidden in t2 (unless some transitive_libs=True or transitive_headers=True requirement trait is defined.

Conceptually this could be addressed doing something like this:

def package_info(self):
      if not (self.options.shared and not self.dependencies["t1"].options.shared):
           self.cpp_info.requires.append("t1::comp1")

However that sounds like a complicated UX. I will try to relax the requirements for components propagations with traits.

memsharded avatar Sep 05 '22 16:09 memsharded

trying it in https://github.com/conan-io/conan/pull/12033, in case you want to test the source branch

memsharded avatar Sep 05 '22 17:09 memsharded

Great. Thank you for looking at it. I don't know if I will have time today, but I will definitely try it out.

On Mon, Sep 5, 2022, 1:07 PM James @.***> wrote:

trying it in #12033 https://github.com/conan-io/conan/pull/12033, in case you want to test the source branch

— Reply to this email directly, view it on GitHub https://github.com/conan-io/conan/issues/12027#issuecomment-1237316234, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPWDXH4HNRHGKP4ZIY5IW3V4YSETANCNFSM6AAAAAAQE7AW3I . You are receiving this because you were mentioned.Message ID: @.***>

jsallay avatar Sep 05 '22 17:09 jsallay

That commit fixes the issue in the test case, but I am still seeing the same error with my actual recipe. I did some digging and there is something odd going on with the validate function that appears to be independent of components. Here is a reproducer.

class t1Conan(ConanFile):
    name = "t1"
    version = "0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": False}
class t2Conan(ConanFile):
    name = "t2"
    version = "0.1.0"
    requires = "t1/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}
class t3Conan(ConanFile):
    name = "t3"
    version = "0.1.0"
    requires = "t2/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}
    generators = "CMakeDeps"
    settings = "os", "arch", "compiler", "build_type"

    def validate(self):
        x = self.dependencies["t2"]

In t3, It looks like any access of self.dependencies in validate triggers the same problem that I had before. I checked the 2.0 docs and it does that it should be safe to use self.dependencies in the validate method.

If you feel like this is too different from the original issue, let me know and I can open a new one.

jsallay avatar Sep 05 '22 19:09 jsallay

Thanks for the feedback, this is exactly the kind of feedback we want for 2.0.

The issue happens because 2.0 is filtering out information from dependencies when they are not needed. We need to think a bit more what interface we give for dependencies:

  • It seems it makes sense that validate() can access information from dependencies, even if the binary is skipped (binary = SKIP). It will not have access to the actual binary or package_folder, but the meta-information seems doable
  • On the other hand, skipped, unused binaries (SKIP) need to be filtered out for all generators, so they are not taken into account. Maybe we need to translate the responsibility of skipped binaries to generators?

memsharded avatar Sep 05 '22 21:09 memsharded

Good hint in existing code:

@staticmethod
    def from_node(node):
        # TODO: Probably the BINARY_SKIP should be filtered later at the user level, not forced here
        d = OrderedDict((require, ConanFileInterface(transitive.node.conanfile))
                        for require, transitive in node.transitive_deps.items()
                        if transitive.node.binary != BINARY_SKIP)
        return ConanFileDependencies(d)

memsharded avatar Sep 05 '22 21:09 memsharded

Hi @jsallay

I am trying to reproduce this, but it seems that

def validate(self):
        x = self.dependencies["t2"]

is working. Are you sure you don't mean the generate() method instead?

memsharded avatar Sep 07 '22 10:09 memsharded

I'm seeing it in the validate method. I started up a fresh docker container and installed the develop2 branch to be safe and I'm still seeing it. Here are the exact instructions I ran:

docker run -it --rm ubuntu:20.04 bash
apt-get update && apt-get install python3-pip vim git
pip install git+https://github.com/conan-io/conan.git@develop2
mkdir ~/t1 ~/t2 ~/t3
# Copy file contents into conanfiles

~/t1/conanfile.py

from conan import ConanFile

class t1Conan(ConanFile):
    name = "t1"
    version = "0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": False}

~/t2/conanfile.py

from conan import ConanFile

class t2Conan(ConanFile):
    name = "t2"
    version = "0.1.0"
    requires = "t1/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}

~/t3/conanfile.py

from conan import ConanFile

class t3Conan(ConanFile):
    name = "t3"
    version = "0.1.0"
    requires = "t2/0.1.0"
    options = {"shared": [True, False]}
    default_options = {"shared": True}
    generators = "CMakeDeps"
    settings = "os", "arch", "compiler", "build_type"

    def validate(self):
        x = self.dependencies["t2"]
conan create ~/t1
conan create ~/t2
conan create ~/t3
# Fails with: ERROR: Error in generator 'CMakeDeps': error generating context for 't1/0.1.0': include is not absolute

jsallay avatar Sep 07 '22 12:09 jsallay

Note that duplicating static libraries into potentially several shared libraries is a recipe for disaster when they have global variables.

johan-boule avatar Sep 12 '22 22:09 johan-boule

Note that duplicating static libraries into potentially several shared libraries is a recipe for disaster when they have global variables.

Yes, we know. But there are users (more than we would expect) that for some reasons have this setup, and they need conan to support it, and Conan 2.0 traits better allow it than in 1.X. It is critical to ensure that no API or ABI things are leaked through the shared libraries, but it is definitely possible (risky but possible) to do with some care. In that sense we try not to be opinionated and provide tools that can help.

@jsallay this was an issue slightly deeper than expected, but we are working on it in https://github.com/conan-io/conan/pull/12055, hopefully for next beta.

memsharded avatar Sep 13 '22 12:09 memsharded

@memsharded No worries. I am very appreciative of the effort you guys are putting in. Conan 2 is going to be a great product.

jsallay avatar Sep 13 '22 13:09 jsallay

Closed by https://github.com/conan-io/conan/pull/12033

czoido avatar Oct 11 '22 10:10 czoido