conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.14k stars 970 forks source link

[bug] options in conanfile.txt are not applied to build requirements #14221

Open sophieeihpos opened 1 year ago

sophieeihpos commented 1 year ago

Environment details

Steps to reproduce

level_0/conanfile.py:

from conan import ConanFile

class BasicConanfile(ConanFile): name = "level_0" description = "A basic recipe" license = "" homepage = "" options = { "copy_header": [True, False], } default_options = { "copy_header": True, } def export_sources(self): pass

def requirements(self):
    pass

def build_requirements(self):
    pass

def generate(self):
    pass

def build(self):
    pass

def package(self):
    pass

level_1/conanfile.py:

from conan import ConanFile

class BasicConanfile(ConanFile): name = "level_1" description = "A basic recipe" license = "" homepage = ""

def requirements(self):
    pass

def build_requirements(self):
    self.tool_requires("level_0/1.0.0")

def generate(self):
    pass

def build(self):
    pass

def package(self):
    pass

conanfile.txt:

[requires] level_1/1.0.0 [options] *:copy_header=False

Reproduce:

conan export level_0 --version 1.0.0 conan export level_1 --version 1.0.0 conan install --profile:build default--profile:host default --build=missing conanfile.txt

Package for level_0 has option copy_header set to True although requested False in conanfile.txt

Logs

No response

memsharded commented 1 year ago

Hi @sophieeihpos

Thanks for your report.

It seems that this is expected behavior, and by design. tool_requires are private dependencies (visible=True trait) to the package requiring it. It means the consumer recipes don't have visibility over them, and consequently, they cannot change their options. The consumer conanfile.txt cannot see the level_0 requirement because it is private to level_1 recipe. If you convert your conanfile.txt to a conanfile.py and iterate the self.dependencies, you will not see level_0 there, as it is not visible to it.

To set "build" context options you can do it:

sophieeihpos commented 1 year ago

It is confusing when conanfile.txt already has the options but we need to repeat the options in the command "conan install --profile:build default--profile:host default --build=missing -o:b *:copy_header=True conanfile.txt" We will need to copy all the options to the command line. Thanks for clarifying.

memsharded commented 1 year ago

Definitions of options values is in general not recommended in conanfiles. It is there mostly for historic reasons, but the idea is that options are configuration, and as such it belongs to profiles or command line. The conanfiles shouldn't be hardcoding any configuration, specifically they shouldn't try to define options values for their dependencies in the general case.

We will need to copy all the options to the command line.

Not all of them, only the "build" ones. Build or tool-requires are in general orders of magnitude less than regular requires, and they also contain way lees options to configure them.

Please make sure that you are not abusing tool_requires. They are intended for tools like cmake, ninja or similar, but not for libraries with headers and compiled libraries. A copy_header option for a tool_require sounds a bit unusual.

sophieeihpos commented 1 year ago

Consuming with one profile is much easier than creating more profiles considering many users. From onboarding perspective, keeping all pkg_name/version and options in one place is easier than keeping more profiles. From consuming perspective, a user might consume package A with profile A and package B with profile B, if more than one profile is supported. This would increase complexity in user interface and our toolchain ( we use bazel).

We have about 40+ name/version under [requires] in conanfile.txt. Would "copy all options" work without telling which ones are tool-requires ?

The example given is only for experiment purpose. The majority of the recipes we use are from conan-center-index.

memsharded commented 1 year ago

Consuming with one profile is much easier than creating more profiles considering many users.

You can still have 1 single "default_build" profile that defines the options that you want. You can even configure that profile as the default build profile so it will be transparent for users. At the end of the day you are talking only about 1 configuration item, putting it in the conanfile or in 1 extra profiles.

We have about 40+ name/version under [requires] in conanfile.txt. Would "copy all options" work without telling which ones are tool-requires ?

Not sure what you mean with copy all options. [requires] is not an issue here, it is only the options of the tool_requires that matter, so not sure how this is connected.

The majority of the recipes we use are from conan-center-index.

The recommended way to use them in production would be building packages from a fork. In that sense, the simplest approach would be to change the default option value in the tool recipe directly, before building your binaries.

sophieeihpos commented 1 year ago

Adding a package currently needs changing in the conan-center-index fork repo, adding the package and options in the conanfile.txt in the onboard repo. The profiles are in a separate repo, would require additional changes. In the onboard repo, a mapping would be needed for keeping which projects using which profile.

"copy all options" means copy all the [options] in the conanfile.txt to the command line -o . As we list all the pkg_name/version there, if we can skip finding out which ones are tool_requires, would be easier.

A fork can not keep two option values. Users might need different default option values.

memsharded commented 1 year ago

Users might need different default option values.

Then, those are not default values. They are custom user values, and the way to do it for tool_requires is to specify it in the "build" profile. Because trying to add them in the conanfile.txt would violate all the visibility constraints in the graph.

A fork can not keep two option values. Users might need different default option values.

A conanfile.txt cannot keep two values either, different conanfiles would be needed too. But conanfiles define the "what", not the how. If there are different "projects", such projects will contain a "conanfile" + some profiles that work for that project. Even a conf that works for that project, and all of that can be set up with conan config install managing one configuration per-project.

All of this, still doesn't explain why a private build-requires need to be changed from the downstream. If you can specify which real world package is that one, and why other package rather than the one that is defining a tool_require to it needs to change some options, that could clarify the use case. The whole idea of tool_requires is that a package can do self.tool_requires("cmake/...) and the rest of the world doesn't even know and they don't need to know that this exist, which options are used if any, or how it works. It is a private implementation detail of the recipe doing the tool-requires. A much different story is regular requires, and for this reason, options are propagated to requires.

sophieeihpos commented 1 year ago

Yes, we keep more than one conanfile.txt files for conflicting versions/options. My main point is keeping these in more than one profiles requires more effort from us as well as our users. We will copy all the options to the command line as the solution.

For your question regarding build-requires, we upload all packages including build-requires packages. The build-requires are included as part of package id calculation, so we should keep rather that discard. We would like to control the options set for all packages. Cmake ( or a package in general) can be under tool-requires and requires. If it is listed under requires in conanfile.txt as well, two sets of packages will be created.

memsharded commented 1 year ago

We will copy all the options to the command line as the solution.

But doing things in command line is generally the approach that has more cognitive overhead and is more challenging from users. That approach is typically only acceptable for CI, but not for developers.

Cmake ( or a package in general) can be under tool-requires and requires.

What would be the case of doing a regular requires() to CMake package? Other packages like protobuf would make more sense, but CMake? Can you please clarify?