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

[feature] conan2 build tool specific recipe #11899

Open
1 task done
bog-dan-ro opened this issue Aug 17, 2022 · 5 comments
Open
1 task done

[feature] conan2 build tool specific recipe #11899

bog-dan-ro opened this issue Aug 17, 2022 · 5 comments
Assignees

Comments

@bog-dan-ro
Copy link

The simplest cmake recipe has over 40 lines of code.
This is a newly created recipe:

from conan import ConanFile
from conan.tools.cmake import CMakeToolchain, CMake, cmake_layout


class helloRecipe(ConanFile):
    name = "hello"
    version = "1.0.0"

    # Optional metadata
    license = "<Put the package license here>"
    author = "<Put your name here> <And your email here>"
    url = "<Package recipe repository url here, for issues about the package>"
    description = "<Description of hello package here>"
    topics = ("<Put some tag here>", "<here>", "<and here>")

    # Binary configuration
    settings = "os", "compiler", "build_type", "arch"
    options = {"shared": [True, False], "fPIC": [True, False]}
    default_options = {"shared": False, "fPIC": True}

    # Sources are located in the same place as this recipe, copy them to the recipe
    exports_sources = "CMakeLists.txt", "src/*", "include/*"

    def config_options(self):
        if self.settings.os == "Windows":
            del self.options.fPIC

    def layout(self):
        cmake_layout(self)

    def generate(self):
        tc = CMakeToolchain(self)
        tc.generate()

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

    def package(self):
        cmake = CMake(self)
        cmake.install()

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

Is it possible to have and inherit CMakeConanFile instead of ConanFile which will have default config_options, layout, generate, build & package methods? package_info can be also auto generated if #11897 is implemented.

This way most of the CMake recipes will be reduced to :

from conan import CMakeConanFile


class helloRecipe(CMakeConanFile):
    name = "hello"
    version = "1.0.0"

    # Optional metadata
    license = "<Put the package license here>"
    author = "<Put your name here> <And your email here>"
    url = "<Package recipe repository url here, for issues about the package>"
    description = "<Description of hello package here>"
    topics = ("<Put some tag here>", "<here>", "<and here>")

    # Binary configuration
    settings = "os", "compiler", "build_type", "arch"
    options = {"shared": [True, False], "fPIC": [True, False]}
    default_options = {"shared": False, "fPIC": True}

    # Sources are located in the same place as this recipe, copy them to the recipe
    exports_sources = "CMakeLists.txt", "src/*", "include/*"

Of course if someone needs to customize any of the steps, will need to override that particular method :

from conan import CMakeConanFile


class helloRecipe(CMakeConanFile):
....
    # Sources are located in the same place as this recipe, copy them to the recipe
    exports_sources = "CMakeLists.txt", "src/*", "include/*"

    # customize generate method
    def generate(self):
        tc = CMakeToolchain(self)
        if self.options.with_fmt:
            tc.variables["WITH_FMT"] = True
        tc.generate()
@memsharded
Copy link
Member

Hi @bog-dan-ro

I guess you are aware of https://docs.conan.io/en/latest/extending/python_requires.html python_requires feature, with it you can implement something very close to it.

You also have the conan new templates (even more powerful in 2.0) that can help with creating new recipes.

But implementing such a generic CMake base class is not in our plans, because it makes future maintainability a nightmare. Once you expose so much condensed behavior in one base class and users start to rely on it, it becomes super complicated to evolve it. But users will inevitably push for improvements, their own customization points, and adding new features to it. It is not worth the pain and necessary efforts to maintain.

@bog-dan-ro
Copy link
Author

I guess you are aware of https://docs.conan.io/en/latest/extending/python_requires.html python_requires feature, with it you can implement something very close to it.

You also have the conan new templates (even more powerful in 2.0) that can help with creating new recipes.

Yup, I used python_requires with conan 1.x for qmake based projects. (Never had time to check conan new templates).

But implementing such a generic CMake base class is not in our plans, because it makes future maintainability a nightmare. Once you expose so much condensed behavior in one base class and users start to rely on it, it becomes super complicated to evolve it. But users will inevitably push for improvements, their own customization points, and adding new features to it. It is not worth the pain and necessary efforts to maintain.

I understand you concerns, but I checked https://github.com/conan-io/examples2 and every single cmake based recipe, has the same config_options, layout, build & package methods. I guess meson, autotools, etc. will have similar methods as well. Even if these methods are not too big, writing the same code for every single recipe is a bit annoying.

As I mentioned above, if someone needs to customize them, he can easily do it, either by overriding the existing methods or by subclassing ConanFile and implement all these methods.

@memsharded
Copy link
Member

I understand you concerns, but I checked https://github.com/conan-io/examples2 and every single cmake based recipe, has the same config_options, layout, build & package methods. I guess meson, autotools, etc. will have similar methods as well. Even if these methods are not too big, writing the same code for every single recipe is a bit annoying.

As I mentioned above, if someone needs to customize them, he can easily do it, either by overriding the existing methods or by subclassing ConanFile and implement all these methods.

The story goes like this:

  • We define the new CMakeConanfile base class, with all the methods, people start to use it
  • A new use case appears for the layout, or some improvement, and we add it as opt-in to the layout() method or cmake_layout() as some new argument.
  • We cannot change the CMakeConanfile base class to use the new improvement without breaking existing users, due to our stability commitment.
  • Now the user base is split between users that could adopt the improvement and those who are stuck with the CMakeConanfile
  • This compounds over time, when new derivative features, bug-fixes and other improvements are built on top of the previous one. And it multiplies by the number of methods and features in the base class.
  • Declaring it "experimental" helps a bit, but not much, users will be annoyed anyway if we change behavior and break their builds.
  • Users might be overriding the methods to provide the new behavior themselves, but at some point this is also very confusing and hard to maintain, making it just a burden for the users to gradually get rid of the base class that became legacy.

Basically, doing that, we would be freezing all those things for a very long time inside the Conan codebase. We have experienced very similar things in 1.X, don't want to repeat the same mistakes, and prefer not to repeat them.

@bog-dan-ro
Copy link
Author

If every cmake recipe will use

    def layout(self):
        cmake_layout(self)

and you'll break cmake_layout(self) doesn't mean it break them all anyway? If you don't break the default cmake_layout(self) behavior, CMakeConanfile::layout will not break either...
Unless there is something else that I fail to see as I'm not a python expert at all !

Anyway, maybe python_requires is a better way.

@memsharded
Copy link
Member

We can easily add an opt-in like cmake_layout(self, new_thing=True) that allows users to move to the new behavior, gradually and under control, without breaking other users. In a few months/releases almost everyone can be using the new de-facto recommended default which is new_thing=True.

This is not true for a common base class implementing a layout() with a call to cmake_layout() which cannot be changed without breaking users using that base class, and making that base class legacy, and the only way to move it forward without breaking is a new base class, but still we cannot retire the legacy one from the codebase without breaking, and our codebase gets larger dirtier, more tests and docs to maintain, etc.

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

No branches or pull requests

2 participants