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

[question] Is new cmake_layout compatible with cpp_info.components ? #9331

Closed
1 task done
Hopobcn opened this issue Jul 27, 2021 · 8 comments · Fixed by #9360
Closed
1 task done

[question] Is new cmake_layout compatible with cpp_info.components ? #9331

Hopobcn opened this issue Jul 27, 2021 · 8 comments · Fixed by #9360
Assignees
Milestone

Comments

@Hopobcn
Copy link
Contributor

Hopobcn commented Jul 27, 2021

Hi, I've been using CMakeToolchain for a while with the CMake build helper that comes with it.
But after updating today I saw that CMake build helper does not accept a build_folder parameter anymore.

Looking at why it happened I arrived at https://github.com/conan-io/conan/pull/8554/files#diff-d72013a45b00a0adf06f4536d6a8c8844461e51b72911937d63e6dda9a3d440aR64 this means that the new way of providing the folder topology is via def layout(self) right?

My first move was to employ the new cmake_layout utility, but I was unable to create a package if this one used self.cpp_info.components

ERROR: 
	ConanException: say/0.1 package_info(): self.cpp_info.components cannot be used with self.cpp_info global values at the same time

Conanfile example:

from conans import ConanFile, CMake
from conan.tools.layout import cmake_layout, LayoutPackager

class Pkg(ConanFile):
    name = "say"
    version = "0.1"
    settings = "os", "compiler", "arch", "build_type"
    generators = "cmake"
    exports_sources = "src/*"

    def layout(self):
        cmake_layout(self)

    def build(self):
        cmake = CMake(self)
        cmake.configure()
        cmake.build()

    def package(self):
        LayoutPackager(self).package()

    def package_info(self):
        self.cpp_info.components["say"].libs = ["say"]

So, I am doing something wrong? Is this intended or just a bug/missing feature?

Thanks!

@memsharded
Copy link
Member

Hi @Hopobcn

I have managed to reproduce this, indeed seems a bug. Thanks for the complete conanfile.py to reproduce, we'll check it asap.

@memsharded memsharded added this to the 1.40 milestone Jul 27, 2021
@memsharded
Copy link
Member

Apparently doing:

            settings = "os", "arch", "compiler", "build_type"

            def layout(self):
                self.cpp.package.components["say"].includedirs = ["include"]

Fixes the issue. However, I think we intended to keep supporting the other package_info() definition as well for self.cpp_info that would be equivalent to self.cpp.package. Probably need the input of @lasote here.

@memsharded
Copy link
Member

BTW, the test to reproduce is:

def test_components_error():
    # https://github.com/conan-io/conan/issues/9331
    client = TestClient()

    conan_hello = textwrap.dedent("""
        import os
        from conans import ConanFile

        from conan.tools.files import save
        class Pkg(ConanFile):
            settings = "os", "arch", "compiler", "build_type"

            def layout(self):
                pass

            def package_info(self):
                self.cpp_info.components["say"].includedirs = ["include"]
            """)

    client.save({"conanfile.py": conan_hello})
    client.run("create . hello/1.0@")

It is fixed by adding self.cpp.package.components["say"].includedirs = ["include"] to layout() (then the package_info() can be removed too, or can stay, it pass either way.

@Hopobcn
Copy link
Contributor Author

Hopobcn commented Jul 28, 2021

Thanks for the fast reply! Yep, can confirm that using the self.cpp.package.components notation works.
Also, note to other readers, .names property seems to not be accepted with self.cpp.package.components. Entries like self.cpp_info.components["say"].names["cmake_find_package"] = "Say" must be translated to self.cpp.package.components["say"].set_property("cmake_find_package", "Say").

@Hopobcn
Copy link
Contributor Author

Hopobcn commented Jul 28, 2021

@memsharded I'm not sure that the generator (cmake_find_package) is able to generate code for self.cpp.package.components.

Using a test_package like this:

cmake_minimum_required(VERSION 3.8)
project(test_package)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()

find_package(say REQUIRED)

add_executable(test_package test_package.cpp)
target_link_libraries(test_package PRIVATE say::Say)
from conans import ConanFile, CMake, tools
import os

class TestConan(ConanFile):
    settings = "os", "compiler", "build_type", "arch"
    generators = "cmake", "cmake_find_package"

    def build(self):
        cmake = CMake(self)
        cmake.configure()
        cmake.build()

    def test(self):
        if not tools.cross_building(self.settings):
            bin_path = os.path.join("bin", "test_pacakge")
            self.run(bin_path, run_environment=True)

The resulting Findsay.cmake doesn't contain any reference to components["say"].

@memsharded
Copy link
Member

Hi @Hopobcn

yes, please check the warning notes in https://docs.conan.io/en/latest/reference/conanfile/methods.html#layout.

It is extremely risky to try to introduce the layout() functionality changes in the old generators. Only the new ones will fully support this functionality (Note that internally we are replacing all the logic of the generators to work, from the self.dependencies, to new cpp_info structure definitions.)

@tapia
Copy link
Contributor

tapia commented Jul 30, 2021

@lasote This error also happens here: conan-io/conan-center-index#6578

Probably the workaround suggested by @memsharded would work here, but that would require a full refactor of the package_info method (not ideal)

@lasote
Copy link
Contributor

lasote commented Aug 2, 2021

on my way!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants