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

New requirements traits #26

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

memsharded
Copy link
Member

Conan 2.0 will introduce a more thorough and detailed requires model, including different new traits like headers, libs, visible, run, build, force, etc.

These traits will be able to model the special characteristics of the C and C++ compilation model, beyond the existing requires and build_requires model in Conan 1.X, allowing better information propagation and visibility, a more realistic package_id computation, and better representation of more advanced dependency graphs.

This proposal builds on top of the recent improvements in Conan 1.38, including the self.dependencies model to access dependencies information and that is being used already by CMakeDeps, MSBuildDeps, etc. providing an initial validation of flexibility and implementability of the proposal.


  • Upvote 👍 or downvote 👎 to show acceptance or not to the proposal (other reactions will be ignored)
    • Please, use 👀 to acknowledge you've read it, but it doesn't affect your workflow
  • Comment and reviews to suggest changes to all (or part) of the proposal.

@jasal82
Copy link

jasal82 commented Jul 6, 2021

Great proposal! That will solve most of our current problems with the requires. One question regarding the test trait: Do you intend to link that somehow to the test configuration of the current build context (i.e. if testing is enabled or not)? Usually we want to run tests only as part of the package CI but not when it is built as transitive requirement, so building (or fetching) the test requirements would be unnecessary.

@memsharded
Copy link
Member Author

One question regarding the test trait: Do you intend to link that somehow to the test configuration of the current build context (i.e. if testing is enabled or not)? Usually we want to run tests only as part of the package CI but not when it is built as transitive requirement, so building (or fetching) the test requirements would be unnecessary.

The test trait main goal is to differentiate a "product" library dependency, lets say, zlib, from a test only dependency, like gtest. Because they actually are both in the host context, not the build context. Note that the proposed test_requires does not include the trait build=True.

The important trait for test_requires is that it has visible=False, so it is not propagated downstream. However, it is not the same case by default when you depend and fetch a package, than when you have to depend on that package. In the first case, the test_require can be skipped, avoiding fecthing its binary package, but in the second, by default, if the package is going to be built from sources, it will fetch the binary too.

If we want to enable a feature that would allow defining that some test requirements are optional, I think this should go via conf, lets say something in the line:

def requirements(self):
      if self.conf["tools.build:run_tests"]:
             self.test_requires("gtest/1.2") # equivalent to test=True, headers=True, link=True, visible=False

If we get to a consensus, and this is something that I am willing to do, regarding the tools.build:run_tests configuration definition, we could directly embed that logic in the high level test_requires caller (that internally would skip if not enabled). But this sounds like a different feature, not related to the "traits" one, but more on conditionally of requirements, which seems to be an orthogonal mechanism.

@DoDoENT
Copy link

DoDoENT commented Jul 6, 2021

🎉 🎉 🎉
This would finally enable fixing the issue I reported almost 3 years ago.

Well done! 👏 👏 👏

Usually we want to run tests only as part of the package CI but not when it is built as transitive requirement, so building (or fetching) the test requirements would be unnecessary.

I understood the test trait as something that is used only for unit tests (e.g. catch2 or gtest). However, conan would somehow need to know whether the package is being installed in a test context or not (something like conan install --test-context?). Our current workaround is to have option enable_testing for every package and the default cmake-conan invocation of conan install sets it to True.
Unlike you, we do have some cases when a testing-enabled package needs to be used as a transitive requirement - we have our custom modifications over gtest that provide additional features for our needs and we've packaged that into a separate conan package that depends on gtest and the actual package then depends on this package and transitively to gtest.

- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
- ``run`` (default None): This dependency contains some executables, either apps or shared libraries that need to be available to execute (typically in the path, or other system env-vars). By default is None, indicating that Conan can try to deduce if the dependency is needed to execute (if ``options.shared`` is True). This trait can be True for ``build=False``, in that case, the package will contain some executables that can run in the host system when installing it, typically like an end-user application. This trait can be True for ``build=True``, the package will contain executables that will run in the build context, typically while being used to build other packages.
- ``visible`` (default True): This ``require`` will be propagated downstream, even if it doesn’t propagate ``headers``, ``libs`` or ``run`` traits. Requirements that propagate downstream can cause version conflicts. This is by default True, because in most cases, having 2 different versions of the same library in the same dependency graph is at least complicated, if not directly violating ODR or causing linking errors.
- ``transitive_headers`` (default None): The headers of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the headers of the transitive dependency must be available and used in the ``-I<includedirs>`` compilation downstream.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are allowed values here? Is this a boolean value (so, None, True and False) or is it expected to place here the array of folder/file paths that should be transitive?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is expected to be boolean, None, True, False.
Making it that more fine grained will probably be too complex and have a marginal value, maybe even an anti-pattern (it is very difficult to model the dependency internal headers requirements, you think that you would only be propagating include/poco/thread headers to your consumers, but those headers in turn depend on include/poco/system that in turn depends on other headers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree and I thought that the expected type is the boolean, but since it's not explicitly stated, I had to verify that. Maybe add some info that all traits are expected to be boolean? Or will there be a trait that is not a boolean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the package_id_mode will not be boolean, but for all others, yes, they should be (that allows much simpler and systematic implementations later). I will add a note in the doc clarifying this.

- ``transitive_libs`` (default None): The libraries to link with of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the libraries of the transitive dependency must be available and used in the ``-I<libs>`` and ``-L<libdirs>`` compilation downstream.
- ``test`` (default False): this requirement is a test library or framework, like Catch2 or gtest. It is mostly a library that needs to be included and linked, but that will not be propagated downstream.
- ``package_id`` (default None): if the recipe wants to specify how the dependency version affects the current package ``package_id``, can be directly specified here. While it could be also done in the ``package_id()`` method, it seems simpler to be able to specify it in the ``requires`` while avoiding some ambiguities.
- ``force`` (default False): This ``requires`` will force its version in the dependency graph upstream, overriding other existing versions even of transitive dependencies, and also solving potential existing conflicts.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if a package downstream forces a different version of already forced dependency?

For example, for graph A <- B <- C <- D, lets's say that C forces the dependency on A/1.0.0 and D forces the dependency on A/2.0.0. Do we get a conflict or the final resolved version will be A/2.0.0 (most-downstream wins, as today)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more downstream always win, I think that is a good principle that works well in Conan, let the user have the power and control. If your recipe declares a "force" on A/2.0.0, that will be the dependency used, no matter what the other upstream dependencies do.

Actually, the explicit definition of the force trait, allows a much better behavior, allowing not accidentally overriding from downstream, detecting conflicts better, and only forcing overrides when intended (replacing the relatively ugly CONAN_ERROR_ON_OVERRIDE current env-var).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I still don't get it.

If C's recipe declares "force" on A/1.0.0 and D's recipe declares "force" on A/2.0.0, will C's "force" prevent D to force A/2.0.0 or D's desire will be fulfilled and C's "force" will be ignored (in that case, C's force doesn't actually do anything...)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also not sure I understand this trait. I feel it could lead to some very hard to understand, and impossible to resolve conflicts based on what was already said above

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

D force desire will always be fullfilled (same as today). This is the only way to make sure it is always possible to resolve all conflicts.

There is nothing inherently bad to that, in the same way a regular require can be change/overriden from downstream, a force is nothing more than an indication that such requirement should prevail. If we follow the rule that downstream consumers always rule, then the force of D has higher precedence than the force of C. There is no ambiguity, no problem of not resolving conflicts. D forced A/2.0, so A/2.0 will be used upstream, no possibility of conflicts.

This is exactly how the Conan current overrides model works right now, just made explicit (and changing the default conflicting policy).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if D declare A/2.0.0 as forced, that will be forced upstream. If not declared as either "forced" or "override" it will conflict if the version doesn't match (behavior CONAN_ERROR_ON_OVERRIDE, introduced a few releases ago), which would be the case if C declared A/1.0 as forced.

That allows all possible desired behaviors: conflict, pure override (no direct dependency), dependency forced (conflicts solved)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the conceptual model, but I don't love the wording, since it allows saying nonsensical things like self.requires(...,forced=True, override=False) (is that just a normal requires?). Or questionable things like self.requires("foo/[^1.2]",override=True) (it seems odd to override with a non-specific version range).

I also feel like spelling this in self.requires(...,override=True) might be problematic regarding requests like conan-io/conan#2140, conan-io/conan#8017 that have asked to allow overriding via the context (command-line or profile). Presumably one can't, or at least shouldn't, add arbitrary new requires via the command-line or a profile, but if it's all done via calling self.requires with various kwargs the boundary of which flags you could use is a bit blurry.

Maybe separate it into two methods (both to be called inside self.requirements() - self.requires(...) to establish a direct requirement and the propagation kwargs, and self.override("name/version@user/channel") just to specify that a particular reference is overridden if required(in both self and upstream)? To get the effect of forced=True (use it directly and override upstream packages), just call both. Or one could allow just self.override("name") when you've already got a requires that picked a specific version in this recipe. But maybe not - overrides are exotic enough I'm not sure they need that much syntax sugar.

Since that gets it down to a single string-value argument, it would also lend itself to a declarative self.overrides = "name1/version@user/channel","name2" attribute form, and thus to something that could be fit a CLI --overrides argument or profile [overrides] section...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I explicitly call out a dependency, let's say openssl/1.1.1k (because I want to make sure that is the version used), if somewhere in the dependency graph there is a requires openssl/1.1.1f with force=True that would override the newer version I explicitly specified?

Copy link

@ytimenkov ytimenkov Jul 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest separating conflict resolution with defining dependencies into separate functions. Their interface is orthogonal and can't (shouldn't ?) be mixed: If I want to resolve conflict (or put some constraint on package version) other parameters (like visible or headers) are ignored (or at least should - it doesn't make much sense to affect how package is consumed by other packages, might be even impossible).

In other words provide a separate function with clear intent for that, like constraint_requirement or so.

Also to note that I prefer versions should be overridden / resolved by lockfiles.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if I explicitly call out a dependency, let's say openssl/1.1.1k (because I want to make sure that is the version used), if somewhere in the dependency graph there is a requires openssl/1.1.1f with force=True that would override the newer version I explicitly specified?

Still wondering if a specifically called out dependency gets overridden by a force upstream? Seems like it could be hard to trace why were getting a package version that may not be what we intended.

- ``package_id`` (default None): if the recipe wants to specify how the dependency version affects the current package ``package_id``, can be directly specified here. While it could be also done in the ``package_id()`` method, it seems simpler to be able to specify it in the ``requires`` while avoiding some ambiguities.
- ``force`` (default False): This ``requires`` will force its version in the dependency graph upstream, overriding other existing versions even of transitive dependencies, and also solving potential existing conflicts.
- ``override`` (default False): The same as the ``force`` trait, but not adding a ``direct`` dependency. If there is no transitive dependency to override, this ``require`` will be discarded. This trait only exists at the time of defining a ``requires``, but it will not exist as an actual ``requires`` once the graph is fully evaluated
- ``direct`` (default True): If the dependency is a direct one, that is, it has explicitly been declared by the current recipe, or if it is a transitive one.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does mean to require a package non-directly?

I understand the need for the field in self.dependencies to be used in generate method as already described in conan 1.38 documentation, but what would it mean to self.requires('pkg/1.0.0', direct=False)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Probably that trait cannot be directly defined in recipes, it should be hardwired for self.requires("dep/1.0",....), and then use it as read trait when visiting your self.dependencies. Lets clarify this in the proposal.


```python
def requirements(self):
self.requires(ref, headers=True, libs=True, run=None, visible=True, …)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the extension to ref as tuple also planned?

Would it be possible to write something like

class MyRecipe(ConanFile):
    ...
    requires = (
        'pkg1/1.0.0',
        ('gtest/1.10.0', 'test'),
        ('pkg2/2.0.0', 'visible=False,package_id=True')
    )

or something similar?

Currently it's possible to provide only a single string override or private.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the patterns I see both from ConanCenter and customers, there is clear trend to use more and more def requirements() method. So I'd suggest:

  • Leave the plain requires = "zlib/1.2.11" for the standard library requirement case, with all defaults
  • For anything that deviates from the defaults, need to use the method requirements() form, as it provides better attribute checking, is more idiomatic in general.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this proposal will create a discrepancy in behavior/functionality... would it be possible to remove (or deprecate) the attribute mechanism? I'd love to avoid the confusion of two similarly name workflows and which when to use when and having to write hooks or best practices.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would assume the point of keeping the attribute form defined is to still have a simple, purely declarative format to that can be defined equivalently in conanfile.py and conanfile.txt.

Though this proposal certainly does undermine that by adding new consumer-oriented kwargs which could only accessed by a conanfile.py.

@ericriff
Copy link

ericriff commented Jul 6, 2021

I think this will be really useful and this might be nitpicking but I'm not sold on the parameters names. I think their functionality is not obvious based on their names.
For example, without reading the docs it is not clear at all what run, build or visible mean.


- ``headers`` (default True): This trait indicates that there are headers that are going to be #included from this package at compile time. The dependency will be in the host context.
- ``libs`` (default True): The dependency contains some library or artifact that will be used at link time of the consumer. The dependency will be in the host context. This trait will be true for direct shared and static libraries, but could be false for indirect static libraries that are consumed via a shared library.
- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So - is it an error to have build True while libs and/or headers true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, maybe not, for some special cases, like a compiler, lets say MinGW, that is a build_require, for the "build" context, with gcc executables (build=True), but it will also contain some headers and libraries associated to the compiler. In a first step, I wouldn't constraint or check incompatibility on traits, but work towards the positive cases: Is there a package out there that would be in the build context but still require to link with it? Should it be the current dual "protobuf in the build context for protoc, and protobuf in the host context for protobuf library"?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok then I would reword:

 This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.

which makes it sound mutually exclusive.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, lets clarify that in the doc.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a first step, I wouldn't constraint or check incompatibility on traits, but work towards the positive cases: Is there a package out there that would be in the build context but still require to link with it?

We are using conan to package simulink sfunctions. They have a "mex" part, i.e. a library required in the build context, as well as a library to be linked in the host context. With conan 1.x we didn't find a good way to model this, yet. Hopefully conan 2.x with this proposal will help us here.

Copy link

@lasote lasote Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using conan to package simulink sfunctions. They have a "mex" part, i.e. a library required in the build context, as well as a library to be linked in the host context. With conan 1.x we didn't find a good way to model this, yet. Hopefully conan 2.x with this proposal will help us here.

@Da-LiFe In that case, you should be able to require the library twice, one with build=True and one with build=False. They won't conflict.

Conan's main declaration of dependencies is the ``requires`` relationship. Mostly 2 variants of it are also available in Conan 1.X: ``build_requires`` that aim to express a dependency to a build tool, like cmake, and ``private`` which is discouraged for most cases.

But this model does not provide enough information to describe some specifics of the C/C++ compilation model, for example in the general case, when one library is linking another library, the consumers of the former shouldn’t have visibility over the headers of the later one. A ``requires`` trait called ``transitive_headers`` could control this visibility.
It seems the potential combinations of dependencies usage is too high to consider approaches like the current one, defining high level names as ``build_requires``, ``test_requires``, ``requires_shared``, etc. does not scale.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, but potentially some separation reduces the cases where the options are in conflict. e.g. does it make sense to have transitive_headers for a build requires?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, with the current model it doesn't make sense to have a build_require containing headers, because that build_require lives in the build context, which can be different platform than the host context, and headers will need to compile and run in the host context. The only possible exceptions are compilers/ndks, but as they don't use the typical build-system library inclusion mechanism (there is no need for a find_package(gcc) or find_package(android_ndk), not sure yet about the necessity of modeling these headers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I have the desire to filter out invalid settings via the function interface...but I get it's challenging.

@memsharded
Copy link
Member Author

I think their functionality is not obvious based on their names.
For example, without reading the docs it is not clear at all what run, build or visible mean.

@ericriff We have iterated the naming a couple of times, trying to be both concise and meaningful, but this is certainly challenging. If you have some suggestion to improve those names, please share them, they will be welcome.

Copy link

@Daniel-Roberts-Bose Daniel-Roberts-Bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this proposal I very positive however, I think some of the proposed traits such as force and override may cause more problems than they are worth. They could create some very confusing and impossible to resolve errors if multiple transitive packages are trying to force the same requirement in a complex graph.

In this case would you end up with two versions of the same package being used in the same graph? That seems dangerous to me.

@a4z
Copy link

a4z commented Jul 7, 2021

looks good, even if I think some options have the potential to make things rather complex.
To me it seems it raises the detail level and being a conan build master becomes more and more an own profession, instead of something that developer will be able to do as part of their development.
I understand that this is due to the nature of C/C++ builds, but on the other hand, when things become too complex you might lose new users, because they will think: before reading through the docs, I have what I need implemented with a handful of scripts (if that might be true or not is an other topic)

@memsharded
Copy link
Member Author

looks good, even if I think some options have the potential to make things rather complex.
To me it seems it raises the detail level and being a conan build master becomes more and more an own profession, instead of something that developer will be able to do as part of their development.
I understand that this is due to the nature of C/C++ builds, but on the other hand, when things become too complex you might lose new users, because they will think: before reading through the docs, I have what I need implemented with a handful of scripts (if that might be true or not is an other topic)

This is a good point, and one of the reasons we have been investing a lot of time implementing some of these ideas already in proof of concepts, playing with them, etc. It seems so far that the balance is very good, and it is possible to implement very reasonable behaviors with the defaults, much better than Conan 1.X, with just the basic requires = "mydep/1.0", or self.requires("mydep/1.0"), while giving extra flexibility for power users that need it for more complex enterprise scenarios. One of the goals of Conan 2.0 is actually that recipes, like those in ConanCenter will be shorter and simpler than today.

- ``libs`` (default True): The dependency contains some library or artifact that will be used at link time of the consumer. The dependency will be in the host context. This trait will be true for direct shared and static libraries, but could be false for indirect static libraries that are consumed via a shared library.
- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
- ``run`` (default None): This dependency contains some executables, either apps or shared libraries that need to be available to execute (typically in the path, or other system env-vars). By default is None, indicating that Conan can try to deduce if the dependency is needed to execute (if ``options.shared`` is True). This trait can be True for ``build=False``, in that case, the package will contain some executables that can run in the host system when installing it, typically like an end-user application. This trait can be True for ``build=True``, the package will contain executables that will run in the build context, typically while being used to build other packages.
- ``visible`` (default True): This ``require`` will be propagated downstream, even if it doesn’t propagate ``headers``, ``libs`` or ``run`` traits. Requirements that propagate downstream can cause version conflicts. This is by default True, because in most cases, having 2 different versions of the same library in the same dependency graph is at least complicated, if not directly violating ODR or causing linking errors.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of questions

  1. Do you envision self.requires(... ,headers=True, libs=True, visible=False) as a valid combination?
  2. Is a requires(visible=False) resolved in the host profile but not propagated (like requires(private=True was), or is the recipe not loaded at all (like build_requires)?
  3. How do force and visible interact? I.e. does visible=False make this requirement ignore a force (since the downstream can't see that it even exists) or is it still subject to being overridden (even though they don't see the result)?

Context

My use case (which we've discussed before, e.g. conan-io/conan#4753 (comment)) is for libraries which use other code in their implementation, but export a very narrow and controlled ABI. I.e. in the case of COM servers, the only exported symbol is the factory method DllGetClassObject (and perhaps DllRegisterServer) and everything else is accessed through vtbls defined by pure virtual interfaces using only a limited set of [oleautomation] types (https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-oaut/7b5fa59b-d8f6-4a47-9695-630d3c10363e). Nothing about the implementation is exposed - what's inside might be implemented in C/C++, but it could equally be replaced by a .net language, python, perl, VBScript, whatever, without the consumer/caller being changed or even recompiled. This keeps the one-definition rule far away :-)

You don't actually even pass such libraries to the linker; instead you just bring in the pure-virtual interface definitions and then dynamically load some implementation as a runtime plugin using CoLoadLibrary (perhaps indirectly via CoGetClassObject, or CoCreateInstance). Even if there are shared-library dependencies, CoLoadLibrary uses LOAD_WITH_ALTERED_SEARCH_PATH to find dependencies adjacent to the server's module file (rather than the consumer), and a well-behaved component will also use its own activation context and ISOLATION_AWARE_ENABLED to prevents the consumer from seeing any shared libraries which it loaded. So the caller should completely not care when there are static/shared libraries inside a C++ implementation.

The reason I ask 2 (whether the recipe is resolved but not propagated, vs not loaded at all), is that these COM servers are generally compatible packages down to "os","arch", but the transitive requirements I'd like to label with visible=False likely use more settings (e.g. "compiler" at least). Yet the consumer might not even be using the same compiler (or might not even be in C++). So there's no need (and little reason to even expect) that the consumer's profile will fulfill the settings constraints of all transitive requirements.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you envision self.requires(... ,headers=True, libs=True, visible=False) as a valid combination?

Yes, this would be a completely private requirement, isolated, non-conflicting library. Might happen in applications or shared libraries linking static or header libraries. By default visible=True, because it seems a sensible default to assume that the same version of the transitive dependency is desired, even if you have multiple shared libraries and applications in the same dependency graph. Putting the visible=False will allow to have shared1->zlib1 and shared2->zlib2 in the same dependency graph (app -> (shared1 and shared2).

Is a requires(visible=False) resolved in the host profile but not propagated (like requires(private=True was), or is the recipe not loaded at all (like build_requires)?

As by the previous accepted proposal #20, the graph will always be complete in terms of recipes, including build_requires, and including visible=False requires. The trait will make:

  • To not create conflicts down the graph.
  • To not propagate the headers and linkage requirements further down the graph
  • If the final binary is not necessary (existing downstream binary, no need to rebuild from source), then fetching the package binary might be avoided, for performance.

How do force and visible interact? I.e. does visible=False make this requirement ignore a force (since the downstream can't see that it even exists) or is it still subject to being overridden (even though they don't see the result)?

The force trait defines a direct dependency to the caller. But it will not force it to "invisible" upstream requirements, as they are not propagated, and they do not conflict. The force trait does solve conflicts forcing the version, but do not forces things that do not exist for it.

The override is similar, it can override dependencies, but only the visible ones.

This is the proposal, because the visible=False is something to be used mainly when you want to allow different versions of the dependencies to co-exist in the same graph at the same time. It is probably only needed in special circumstances, and many dependencies graph will want to use the same versions upstream. But if the graph defined visible=False to allow precisely this thing, because most likely some of the packages have limitations to upgrade versions, then the force or override will probably not solve the issue. Because there will be different requires upstream like requiring zlib1, zlib2 and zlib3 by different packages, and if force/override did that, it will override all of them, and some will not work. Trying to attempt to do something like "override only the zlib dependency of the boost dependency" seems a rabbit hole.

Regarding your use case, I think this proposal will help in propagating better the information, headers, linkage, but the other part that you are describing is more related to the binary model, which this proposal doesn't address.

If you create such ABI stable package, even if it isolates completely the implementation details, even the language, the package_id of such package will probably be a function of its dependencies settings. Compiling your dependencies in a more modern version of Clang, will most likely create better code, with less bugs, and your final binary gathering that native code, should have a different package_id than the previous one that was compiled using a gcc 4.9. So passing the compiler=clang, compiler.version=9 to the installation of your package is still very relevant to get the right package binary.

The only way to achieve this level of isolation and independence, via a very aggressive configuration/settings erasure, would be the concept of "re-packaging", and introducing a completely new type of relationships that could model this (I don't think this could be done just by a "trait", it seems like a completely different thing, like the python_requires). Given that such model will also introduce a physical barrier for CI flows, and Conan computation of lockfiles, build-order, etc to work properly, this needs to be carefully considered.

Copy link

@puetzk puetzk Jul 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As by the previous accepted proposal #20, the graph will always be complete in terms of recipes, including build_requires, and including visible=False requires.

Yes, we discussed the problems there too, up through #20 (comment), but it didn't really come to a conclusion (which was OK at that time, but this is the next piece to get detailed). So... when you say "complete in terms of recipes", does really mean just the recipe, or do you meant you want to resolve the package_id/package_info too (even when it won't get downloaded/propagated downstream). The former seems OK, the latter quite problematic.

Or to make it specific, if I have:

  1. App with requires=FooLib,
  2. FooLib recipe with self.requires("BarLib",visible=False),
    FooLib has a well-controlled ABI, and a correspondingly generous compatible_packages allowing the selected package_id to be quite different than App's profile
  3. BarLib recipe with either settings restrictions or a configure() method might throw ConanInvalidConfiguration
  4. The host context of App is not suitable to build or directly call BarLib (violates those settings restriction or configure() would fail).

Will visible=False stop after resolving/loading the BarLib recipe but before propagating settings to it (since conan doesn't need to select a binary for an invisible package)? Or will App's profile settings be propagated to Bar despite the fact binaries are not needed, triggering the ConanInvalidConfiguration and making the conan install fail? It feels very strange to require that App use a custom profile picking per-package settings for BarLib (and all its transitive requirements), when App isn't even going to be told that BarLib exists in the end.

Obviously there must have been some other profile used as the host context while building FooLib (corresponding to the binary chosen by compatible_package) which was compatible with BarLib. But App has no direct means to know what that profile was. The conaninfo.txt for the selected FooLib binary could know (there must have been something that worked when it was built), but compatible_packages are defined not to influence the dependency graph, and are in any case processed too late to help.

Also, since it's not only requirements but even build_requirements that will now have this behavior, similar questions arise regarding the build context too. If FooLib has an existing binary with suitable ABI, but a build_requirement that is not compatible with App's build context (perhaps App is being cross-compiled but FooLib doesn't support that). Such cases where different packages need different build environments cannot support conan install App --build FooLib, but they're possibly today and IMO should continue to be possible when the necessary binaries have already been created and are available in the cache or on artifactory.

For the other two questions, you gave the answer I was hoping for :-)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way to achieve this level of isolation and independence, via a very aggressive configuration/settings erasure, would be the concept of "re-packaging",

Yes,that's basically what we do now - there are commonly two recipes, one for obtaining dependencies and building the binaries, and an entirely different one (to be used used with export-pkg) to copy things into the cache for upload consumption or upload. The latter simply fails to mention the majority of the settings/requirements (omitting all those which would be visible=False and are not important to a consumer). This course has lots of drawbacks: there's duplication that one can mess up accidentally not synchronize when changing the two recipes, one can never use conan create or --build missing, and the conaninfo.txt entirely fails to record what requirements/settings the uploaded binary really is (even in the [full_requires]/[full_settings]` that would normally know about things that were excluded from package_id).

Hence I'd love to see things evolve to where visible=False and compatible_packages become adequate for this kind of library :-)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... when you say "complete in terms of recipes", does really mean just the recipe, or do you meant you want to resolve the package_id/package_info too (even when it won't get downloaded/propagated downstream). The former seems OK, the latter quite problematic.

The recipes will be downloaded no matter what and the settings/options will be passed to the recipe, because for example, you will need them in case you have conditional transitive requirements or any other thing that might depend on the configuration (e.g configure() method).

But, if once the graph is built, if the binary is not needed (e.g public=False and binary available for the dependant), the package_id calculation might be skipped because it is not visible for the final consumer.

Will visible=False stop after resolving/loading the BarLib recipe but before propagating settings to it (since conan doesn't need to select a binary for an invisible package)?

As said before, nope, that is not possible because the graph depends on the settings also, not only the binaries.


Introduce the concept of "requirements traits", to better model the relations between packages. Requirement traits are specifiers of the Conan ``requires`` that can be defined in recipes. These traits can allow defining the current Conan 1.X ``build_requires`` behavior with some ``requires`` traits.

This proposal does not affect ``python_requires``, because of their nature and evaluation at recipe parsing time, they are very different to regular ``requires`` and ``build_requires``, and they would not benefit from this proposal.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know that I fully agree with "would not benefit". One example would be the use case of a custom build helper - in such a case the python_requires really should really be resolved in the build contex (and especially any transitive requires it might have, e.g. the tool itself, should be in the build context), and be visible=False (it's unimportant to the consumer, and need not be resolved unless this recipe is being built), etc.

My one existing use of this sort (a qmake build helper) is actually quite old and still uses the original PYTHONPATH approach, which is more satisfactory in that you can make the build helper a build_requires and do the import myhelper only inside the def build(self): ... body.

This is mostly satisfactory in that it loads the recipe in the right context and at the right time (only when building), but it does have the drawback that it's a normal python import statement - which means it's not fully isolated. If you have multiple requirements being built in the same conan install run, and they build_require different versions of this helper, the caching of loaded modules in sys.modules mean whoever runs first will load the version from their environment's PYTHONPATH, and later recipes will get that already-loaded version (even if their build_requires specified a different version.

python_requires would address that isolation, but (today) doesn't support it being in build context.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is much different to have a build_requires containing some python code, that can be used to build, than a python_requires. There are packages out there, like https://conan.io/center/meson which is a python code, that can be used as build_requires.

python_requires is much much different, because it needs to be evaluated at recipe parsing time, which happens way before any package binary is fetched. So all the traits that relate to binaries: headers, libs, run, transitive_headers, transitive_libs, directly not apply.

So python_requires are not in the build context, and they are not in the host context. The build and host contexts are settings, and python_requires do not have settings, because they do not have binaries. They are python code that becomes part of the recipe that calls them, without any context.

The reason here could be that the tool you are requiring is more than just some python code to be reused, but a complete build tool that is wrapped in python, and this is the reason why it cannot be put inside a python_requires?
I think the best line of action for this case might be to try to use a thin layer over such a tool, and launch it as a subprocess, instead of importing it directly. Specially if you can have different versions of that tool in your dependency graph.

The python packaging and importing ecosystem is also complicated, and Conan was not designed to solve that problem, so this would be very difficult to solve: basically you are trying to import the same modules, different versions from different locations in the same python interpreter, something Python was not designed for, so this would be challenging for any tool out there, not only Conan.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build and host contexts are settings, and python_requires do not have settings, because they do not have binaries.

Ah. I admit I have not actually written much using python_requires, and I had managed to overlook that they did not have settings at all . Which would seem to imply they also simply don't carry transitive requires either? Given all that the statement that "they would not benefit from this proposal" makes more sense.

I was thinking of examples like https://docs.conan.io/en/latest/extending/custom_build_helper.html (which recommends the use of python_requires) to obtain WafBuildEnvironment, and thinking (as a what-if) of such a build helper that was either contained or required a binary package that actually contained the waf build tool itself.

But it sounds like that is a categorical error - if one wanted to do that, you would need both a python_requires and a build_requires (which might, or might not, be the same reference, but are loaded at very different ways). And it seems to be legal to have both python_requires and build_requires for the same reference. That's only documented in the context of an unrelated workaround, but here it would be simply be the right thing to do - the python_requires to get the build helper (in no context) and a build_requires to get the tool (in build context).

Thank you for the explanation, that hopefully makes my mental model of what python_requires are for and how they work a bit more accurate.

basically you are trying to import the same modules, different versions from different locations in the same python interpreter, something Python was not designed for

I concur, but it seemed like this scoping/versioning was was at least part of what python_requires was trying to solve, and https://docs.conan.io/en/latest/howtos/python_code_reuse.html says

To reuse Python code, from Conan 1.7 there is a new python_requires() feature. See: Python requires: reusing Python code in recipes This “how to” might be deprecated and removed in the future. It is left here for reference only.

So it seemed like it might be intended that python_requires evolve to replace such usage. But in any I now see why that still wouldn't create a reason to label python_requires as build/test/etc, and thus wouldn't be directly related to this proposal.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on the same page as @memsharded: what I've successfully used, 3 parts:

  1. A Toolchain or Deps class (former generator) - takes conanfile in constructor.
  2. A Mixin to instantiate Toolchain class and call it in generate() method. Also provides build_requirements() method to inject a dependency on the build tool.
  3. The tool itself.

Former 2 are part of python_require and is a pure Python code (strictly a mixin can be skipped). The latter is a ordinary requirement intended to be used in the build context. May have settings.

The trick is all mixins need to play nicely together and call super().

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, what @ytimenkov is saying sounds a reasonable approach, that might scale and be reasonable to maintain. The truth is that so far we haven't found any blocker for using the same reference as python_require and regular requires or build_requires, and while seems a bit weird to use the same exact reference for both things, it also seems to work, thanks to the fact that the resolution, context, etc are completely decoupled.

We will probably try to come up with more insights about if it is a good practice or not to use the same reference.

@puetzk
Copy link

puetzk commented Jul 8, 2021

@Daniel-Roberts-Bose

In this case would you end up with two versions of the same package being used in the same graph? That seems dangerous to me.

Not by using force or override (those prevent this from happening, by changing upstream requirement to match the version chosen by a downstream recipe). But visible=False (which produces isolated sub-trees that don't see each other) would allow that to happen - that's in fact the purpose of it.

@sztomi
Copy link

sztomi commented Jul 11, 2021

I think this is an excellent proposal and will make our lives a lot easier.

visible (default True): This require will be propagated downstream, even if it doesn’t propagate headers, libs or run traits. Requirements that propagate downstream can cause version conflicts. This is by default True, because in most cases, having 2 different versions of the same library in the same dependency graph is at least complicated, if not directly violating ODR or causing linking errors.

Is visible=False intended to replace private requires? Our use-case is the following: we have auxiliary executables that are distributed as conan packages. These are bundled in the final build of our product (so we pretty much just copy things from the package dir to the build dir). For a long time, we used to use build_requires to hide the dependencies of these auxiliary packages (so as to not require their dependencies transitively in the final consuming conanfile). For example, if we required a statically linked python executable package, we did not want to add bzip2 to the dependency graph. But this is a misuse of build_requires and it has its own problems, particularly with package revisions.

@memsharded
Copy link
Member Author

memsharded commented Jul 11, 2021

Is visible=False intended to replace private requires? Our use-case is the following: we have auxiliary executables that are distributed as conan packages. These are bundled in the final build of our product (so we pretty much just copy things from the package dir to the build dir). For a long time, we used to use build_requires to hide the dependencies of these auxiliary packages (so as to not require their dependencies transitively in the final consuming conanfile). For example, if we required a statically linked python executable package, we did not want to add bzip2 to the dependency graph. But this is a misuse of build_requires and it has its own problems, particularly with package revisions.

Yes, it is. This is a trait that is by default applied to build_requires (in Conan 1), but the idea is that this trait applied to requires in the host context (regular libraries) can achieve this decoupling. Just a note about this decoupling:

  • The visible=False will not avoid bringing the dependencies recipes, as approved in a previous proposals. Even if the dependency is not visible it can definitely define the binary and package_id of the consumer, so the dedependencies recipes might be necessary to evaluate that, and they will be downloaded if necessary.
  • The visible=False will avoid propagating linkage and inclusion requirements. Downstream consumers will not see the upstream transitive dependencies hidden by this trait.
  • The above implies that in many cases it will be possible to avoid fetching the binaries (which is the expensive stage).

Does this sound good?

@sztomi
Copy link

sztomi commented Jul 12, 2021

visible=False will not avoid bringing the dependencies recipes

Yeah I think that's fine, as long as we only fetch the recipes.

@ytimenkov
Copy link

Did anyone do any research on modules and how they're going to play with the proposal? Probably this decision can be postponed, but IMHO it's good to have some proof of concept that modules will fit the proposed model.

Copy link

@ytimenkov ytimenkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have strong opinion on naming?
This proposal reminded me CMake's model: visibility / propagation: PUBLIC / INTERFACE / PRIVATE and kind: COMPILE / LINK (maybe even RUNTIME).

I find it clearer that headers / libs traits as the area where it's used: during compilation or linking. Just to consider...

@ytimenkov
Copy link

Another thought about visible vs transitive...

I have a gut feeling that instead a depth of transitivity should be defined somehow: affects only direct parent or everything up (actually up until a node which breaks transitivity encountered).

Also this should be specified independently for headers (or compilation step) and libraries (link step) and maybe even runtime. For example to address the case of "private" static library where headers are not exposed in the public headers, but library needs to be linked all the way up.

This (as I understand) resembles CMake's model: when you consume target (that's what new-style target_link_library does, right?) its dependencies automatically added to the consumer's public interface if it's public and kept only for linking otherwise. Well, pretty much like class inheritance works in C++... But again, this needs to be per-stage: e.g. consuming privately for compilation but publicly for linking.


The traits are:

- ``headers`` (default True): This trait indicates that there are headers that are going to be #included from this package at compile time. The dependency will be in the host context.
Copy link

@ytimenkov ytimenkov Jul 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the separation of headers / libs / run (or compile / link / run) are there plans to mark parts of packages that way and using this information especially in deploy generator?

For example if I'm deploying a package to run I'm not interested in pulling headers or static libraries (i.e. all those things which are not with run=True).

Also splitting package may provide some savings on bandwidth (especially with fat static libraries), because a package with static library may still provide some runtime resources or configuration files.

In this context I could think also about test and debug contexts. "test" is probably a subclass of "run", but is some extra packages. Also it should affect only direct consumers (i.e. "test" requirements for any dependency shouldn't be installed).

"debug" (or rather debug info) on the other hand is good to deploy for the whole graph and may be deployed totally independently (via deploy generator) or as a part of development workflow (e.g. depending on a flag or a config option).

EDIT: I think this is the same as requested in this comment: #27 (comment)

- ``transitive_headers`` (default None): The headers of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the headers of the transitive dependency must be available and used in the ``-I<includedirs>`` compilation downstream.
- ``transitive_libs`` (default None): The libraries to link with of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the libraries of the transitive dependency must be available and used in the ``-I<libs>`` and ``-L<libdirs>`` compilation downstream.
- ``test`` (default False): this requirement is a test library or framework, like Catch2 or gtest. It is mostly a library that needs to be included and linked, but that will not be propagated downstream.
- ``package_id`` (default None): if the recipe wants to specify how the dependency version affects the current package ``package_id``, can be directly specified here. While it could be also done in the ``package_id()`` method, it seems simpler to be able to specify it in the ``requires`` while avoiding some ambiguities.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love this one ❤

``build_requires`` will remain to be a high-level definition of requirements, but internally it is implemented as:

```python
req = Requirement(ref, headers=False, libs=False, build=True, run=True, visible=False,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminded me flatbuffers (aka tool + lib):

In this case either duplicate requirements should be allowed or there should be a way to specify that a requirement is for both host and build contexts and will be duplicated internally.

If same requirement can be added multiple times it will be a tough job (IMHO) to specify all possible cases or combinations. Then why not to add with more than one combination of traits?

The second case is better, but then it limits possibility to use different versions in different contexts (maybe it's not bad). In this case a parameter context: enum or string fits better than build: bool.

@sztomi
Copy link

sztomi commented Jul 13, 2021

Do you have strong opinion on naming?
This proposal reminded me CMake's model: visibility / propagation: PUBLIC / INTERFACE / PRIVATE and kind: COMPILE / LINK (maybe even RUNTIME).

I would also prefer that. However, so far Conan seemed to stick with the GNU terminology. Even though I dislike that, consistency is better than choosing the kinds of names I would like more. Currently, if I need to think about the meaning of a word, I can think in terms of how GNU uses it.

@DoDoENT
Copy link

DoDoENT commented Jul 13, 2021

I would also prefer that. However, so far Conan seemed to stick with the GNU terminology. Even though I dislike that, consistency is better than choosing the kinds of names I would like more. Currently, if I need to think about the meaning of a word, I can think in terms of how GNU uses it.

"GNU terminology" == "Generally Not Used terminology"

😛 :trollface::trollface:😛

I also prefer calling the host system "host" and target system "target" 😛

@ytimenkov
Copy link

I also prefer calling the host system "host" and target system "target" 😛

I wonder how many votes would such proposal get, considering all breaking changes on the CLI and API... (at least one from me 😉).

@DoDoENT
Copy link

DoDoENT commented Jul 13, 2021

I wonder how many votes would such proposal get, considering all breaking changes on the CLI and API... (at least one from me 😉).

Well, conan 2.0 will be breaking change anyway.

Let's test that theory. Add 👍 to this comment if you would support such a proposal and 👎 if you wouldn't 😛

@puetzk
Copy link

puetzk commented Jul 13, 2021

visible=False will not avoid bringing the dependencies recipes

Yeah I think that's fine, as long as we only fetch the recipes.

Yes, but I don't think that's the proposal - it sure sounds like the intent is to fetch the recipes, and resolve settings/options/package_id, in case someone wants to use full_package_mode() on a build_requires.

Which seems problematic in complex cases (may force the consumer profile to come up with valid settings for a bunch of build tools it doesn't actually use), but if/when it does, that problem is with #20, not this proposal.

@memsharded
Copy link
Member Author

Do you have strong opinion on naming?
This proposal reminded me CMake's model: visibility / propagation: PUBLIC / INTERFACE / PRIVATE and kind: COMPILE / LINK (maybe even RUNTIME).

We have thought about this, actually in one of my preliminary proof of concepts the traits were called more in the line of include, link, and instead of visible it was called public. After some thinking and discussion, we finally named as the current proposal shows. This is some of the rationale:

  • The visible trait was called differently (not public or the like) because it is not related to headers inclusion or to library linkage at all, it is only about version and configuration conflict. Packages can contain data files only, or just executables. So we avoided the terminology public/private here, to precisely avoid confusion.
  • The INTERFACE CMake propagation that for example is applied to header-only targets, probably doesn't make the same sense when talking about packages. We cannot make a package to be just "interface" of the upstream dependencies, because the package itself, for building, for example, if it has unittests, it will still need to link the upstream dependencies. If you have a header-only recipe in a repository, and you clone it and execute conan install, you want to get the xxx-config.cmake files of the dependencies pointing to the libraries. If we apply the CMake interface concept, you shouldn't be getting those, because you are just an interface package.
  • If we aimed to use just 1 trait, like headers with values public|private, it would still fall short. The headers trait could still be False, as the current package will not be using the headers of such dependency. Then, we would still need 3 different values, and we cannot make it a boolean.
  • Boolean/binary traits are one order of magnitude easier to manage and understand. It only allows True/False values and semantics, otherwise, at least just from the UI point of view, we would need to handle strings, or enum, that makes the interface uglier. The way conflicts can be quickly detected is doing boolean operations (when upstream package names are the same), and composing operations are typically ORs (if you have a diamond and one branch tells you don't need the headers headers=False and the other branch tells you headers=True, the resulting trait is just ORing both (or as many branches closing the diamond).
  • Separating the traits headers and transitive_headers for example, really helps in simplifying things. A headers=True default can be easily defined for regular requires and transitive_headers could for example automatically deduced by default, if not specified by the user depending on the package_type (a header-only library will have transitive_headers=True of its dependencies, for example. All this will not modify or affect the headers trait, and this makes it way simpler to reason about.

So we are deciding between:

  • headers: {False, interface, public, private}
  • libs: {False, interface, public, private}

And

  • headers: {False, True}
  • libs: {False, True}
  • transitive_headers: {False, True}
  • transitive_libs: {False, True}

So the first proposal not being the same as CMake proposes with the False value, and a probably confusing interface concept, together with the fact that binary traits are way more intuitive to handle and UI is simpler, we do propose the second one so far. If it weren't for the CMake precedent, the second form feels clearly superior in terms of easier to understand and manage.

@memsharded
Copy link
Member Author

Yes, but I don't think that's the proposal - it sure sounds like the intent is to fetch the recipes, and resolve settings/options/package_id, in case someone wants to use full_package_mode() on a build_requires.

@puetzk The aim is that even settings/options are not fully specified in some cases, and the package_id couldn't be fully computed, if the downstream consumers do not depend on the full_package_mode() it would be able to evaluate and work, by skipping the unnecessary upstream binaries. Of course, if the full package_id of the dependency needs to be evaluated, it will be necessary to provide the right settings/options for computing it, and getting the downstream resultant package_id, but that is something that users can control, right?

@puetzk
Copy link

puetzk commented Jul 15, 2021

The aim is that even settings/options are not fully specified in some cases, and the package_id couldn't be fully computed, if the downstream consumers do not depend on the full_package_mode() it would be able to evaluate and work

If that's going to be the case (in some to-be-determined fashion), then great, I'll shut up on this for now and watch for that proposal :-). I like this proposal and it doesn't make things worse; this thread is still really about my 👎 on #20. I only brought it up again here since this proposal seemed like the follow-up to #20 (comment) and I still don't see how it solves the problems we discussed there.

but that is something that users can control, right?

I didn't see how, so I assumed not; given how the RequirementInfo object (currently) works, conan doesn't know the ABI mode in time to avoid computing upstream package_id values. At entry to package_id(), each self.info.requires[] is already populated with self.full_package_id = pref.id; and the methods like semver_mode() and full_package_mode(), etc just control what gets copied into the hash. But the recipe had to run to work out pref.id before constructing RequirementInfo objects, which has to happen before package_id() even begins. So if there are missing settings, constraint violations, or ConanInvalidConfiguration exceptions during configure(), etc, those errors will have been been fatal before conan could know whether package_id(self) calls self.info.requires["my_other_lib"].full_package_mode() or not.

That said, 2.0 proposals have changed lots of the parts around this, and future proposals will undoubtedly change even more. So the fact it doesn't work today doesn't preclude there being some something coming (or just something I'm overlooking).

But without some way to avoid configuring the upstream recipe, it becomes impossible to consume existing binary packages in a build context that couldn't have compiled them.

E.g. if libfoo has build_requires such that it has to cross-compiled (perhaps it build_requires a tool whose recipe only supports running on some other os), in conan 1.x it was perfectly possible for a downstream conan install to consume existing binaries (since build_requires are entirely ignored when not building). Since #20, all build_requires now potentially impact package_id, so you don't know whether or not you have existing binaries until you've worked out in detail what tools you'd have used to do a build in this context. If the answer is "I couldn't build it here", then one can't find out what pkg_id/compatible_package to look for.

@ytimenkov
Copy link

We have thought about this, actually in one of my preliminary proof of concepts the traits were called more in the line of include, link,

@memsharded Let me summarize what I came to based on this proposal.

First, define contexts in which package (or actually parts of the package):

  1. Compilation (use this word instead of "headers" or "includes", because flags and defines also used here).
  2. Linking (ditto, instead of "libraries" because linker flags)
  3. Run
  4. Testing. This is special case of running, mostly in context of running on CI because more artifacts need to be produced - reports or logs. It's different from run case in a way that it's always not transitive (somewhat like build context in Conan 1.0, but for host profile and potentially extra dependencies).
  5. Debugging (primarily sources and debug symbols - binary files).

For each we have files and metadata. For example, for compilation it's headers + defines, for linking it's libraries and paths to them, for running binaries or assets and environment variables.

Files go into binary package while metadata is kept in the recipe (from my comment about splitting binary packages).

Consuming also could be either "private" - means that neither meta information nor files are propagated further, or "public" - when it is propagated. (I think "visible" in the proposal).

Then the major trick: specialize "visibility" and propagation not per package, but per context.

For example, if package A requires package B and it's compilation context as "public" then all of B's compilation information is added to A's compilation information. When package Z consumes package A it get's A's compilation information and B's compilation information.

Different stages' ("compilation" and "linking") visibility shouldn't affect each other. If requirement's compilation information is not propagated (private / invisible) it doesn't mean same for link: in this case a header-only library used as implementation (== not exposed in headers) may still require extra libraries to link.

"private" / invisible link means (as pointed out in this proposal) that binaries are embedded into the artifacts: a static library into shared one or application.

That would be convenient (as I mentioned in other comments) if files could be marked as belonging to different contexts. In this case if a static library also has some resources to be used in runtime, a shared library may not expose further link information from its dependency, but runtime files could still be deployed.

Another note: I don't think that focus on "headers-only" libraries is necessary. A header-only library with dependencies is as good as a library. And even it may require some link information (e.g. -pthread flag or linking to sockets library) which needs to be propagated.

That's how it is seen by Conan and generators on the bottom level. For recipe it may be simplified (as with other proposal with package type). Maybe simple "require as interface" and "require as implementation" will be enough to deduce correct propagation for each context given package type is known for both packages.

In a way it's not that far from what build systems do under the hood when you set up dependencies between targets (I'm most familiar with CMake and Waf)...

@sztomi
Copy link

sztomi commented Jul 15, 2021

@memsharded this paints a little different picture and I feel like we are missing something without a true private requires (in fact, the current meaning of "private" is a little confusing to me). I understand that conan wants to take the private requirements into account when computing the package ID, but I would really like a "I KNOW WHAT I'M DOING" mode where conan allows me to worry about the binary compatibility (which is trivial in the case of auxiliary binary packages).

For example, I could implement true private requires in conan 1.x doing on of the following:

  • For any private requirement, call the conan binary from the recipe to fetch them (so that conan doesn't know about them while building my recipe)

In this case, I'm working against conan. It's essentially about building a wall so that conan doesn't see the things I don't want it to take into account while computing the package ID. "True private" for me means that outside building the package, conan wouldn't need to fetch the dependencies that are marked as such because I decided that I want them private, i.e. left out from the model.

@foundry-markf
Copy link

I welcome this proposal, as it begins to address some very inelegant workarounds we've had to model dependencies, e.g. don't make a static library from package A transient when it's completely contained in a shared library in downstream package B.

As much as I like being explicit in scripts, I have some concerns that this might complicate recipes if default behaviours (such as marking up the package types in the other PR) aren't sufficient, but very willing to see where this goes. I have noted that one of goals is to make the Conan Center recipes shorter and simpler, so I'm hoping this will work out in our own recipes too.

@puetzk
Copy link

puetzk commented Jul 21, 2021

if there are missing settings, constraint violations, or ConanInvalidConfiguration exceptions during configure(), etc, those errors will have been been fatal before conan could know whether package_id(self) calls self.info.requires["my_other_lib"].full_package_mode() or not.

I thought of one possibility here. Assuming that conan 2.0 intends to make the new validate() function introduced by 1.32 non-experimental, one could argue that all "this binary can't exist" checks should be moved there rather than in configure(). And one could also then redefine where/when declarative restrictions in a settings attribute (see the part starting with "You can restrict existing settings and accepted values") so that they are also diagnosed at the validate() step (rather than up-front as they are now).

Then it might work out that PACKAGE_ID_INVALID only propagates if that requirement used full_package_mode(), because otherwise the package_id didn't end up being part of the RequirementInfo.sha calculation.

@cguentherTUChemnitz
Copy link

Could these require traits also be a possible solution to handle those problems like here? conan-io/conan-center-index#5607

Some projects produce additional build binary tooling for their own library build. For example this is the case for grpc. This makes it currently complicated to get cross-compilation for this in conan-center-index. The current siuation is: it needs binaries from its own reciepe as build_dependencies. The linked merge request is able to handle this, but jenkins ci gets red because of detected recursion.

Can these traits here used to break the recursion detection? Or would it already be possible to enhance the recursion detection to split between build and host requirements?


- ``headers`` (default True): This trait indicates that there are headers that are going to be #included from this package at compile time. The dependency will be in the host context.
- ``libs`` (default True): The dependency contains some library or artifact that will be used at link time of the consumer. The dependency will be in the host context. This trait will be true for direct shared and static libraries, but could be false for indirect static libraries that are consumed via a shared library.
- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@memsharded I think we should update the definition of the build trait as it is not accurate that a build=True implies: an application or executable, that would be build=True and run=True. A build=True and run=False is also possible as has been suggested here.


- ``headers`` (default True): This trait indicates that there are headers that are going to be #included from this package at compile time. The dependency will be in the host context.
- ``libs`` (default True): The dependency contains some library or artifact that will be used at link time of the consumer. The dependency will be in the host context. This trait will be true for direct shared and static libraries, but could be false for indirect static libraries that are consumed via a shared library.
- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
Copy link

@lasote lasote Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using conan to package simulink sfunctions. They have a "mex" part, i.e. a library required in the build context, as well as a library to be linked in the host context. With conan 1.x we didn't find a good way to model this, yet. Hopefully conan 2.x with this proposal will help us here.

@Da-LiFe In that case, you should be able to require the library twice, one with build=True and one with build=False. They won't conflict.

- ``libs`` (default True): The dependency contains some library or artifact that will be used at link time of the consumer. The dependency will be in the host context. This trait will be true for direct shared and static libraries, but could be false for indirect static libraries that are consumed via a shared library.
- ``build`` (default False): This dependency is a build tool, an application or executable, like cmake, that is used exclusively at build time, it is not linked/embedded into binaries, and will be in the build context.
- ``run`` (default None): This dependency contains some executables, either apps or shared libraries that need to be available to execute (typically in the path, or other system env-vars). By default is None, indicating that Conan can try to deduce if the dependency is needed to execute (if ``options.shared`` is True). This trait can be True for ``build=False``, in that case, the package will contain some executables that can run in the host system when installing it, typically like an end-user application. This trait can be True for ``build=True``, the package will contain executables that will run in the build context, typically while being used to build other packages.
- ``visible`` (default True): This ``require`` will be propagated downstream, even if it doesn’t propagate ``headers``, ``libs`` or ``run`` traits. Requirements that propagate downstream can cause version conflicts. This is by default True, because in most cases, having 2 different versions of the same library in the same dependency graph is at least complicated, if not directly violating ODR or causing linking errors.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... when you say "complete in terms of recipes", does really mean just the recipe, or do you meant you want to resolve the package_id/package_info too (even when it won't get downloaded/propagated downstream). The former seems OK, the latter quite problematic.

The recipes will be downloaded no matter what and the settings/options will be passed to the recipe, because for example, you will need them in case you have conditional transitive requirements or any other thing that might depend on the configuration (e.g configure() method).

But, if once the graph is built, if the binary is not needed (e.g public=False and binary available for the dependant), the package_id calculation might be skipped because it is not visible for the final consumer.

Will visible=False stop after resolving/loading the BarLib recipe but before propagating settings to it (since conan doesn't need to select a binary for an invisible package)?

As said before, nope, that is not possible because the graph depends on the settings also, not only the binaries.

require.run # boolean
```

``build_requires`` will remain to be a high-level definition of requirements, but internally it is implemented as:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to update this. As build_requires is becoming a generic concept with run=Noneand the specific "build tool" name will be different, something like build_tool_require and will have the run=True

- ``visible`` (default True): This ``require`` will be propagated downstream, even if it doesn’t propagate ``headers``, ``libs`` or ``run`` traits. Requirements that propagate downstream can cause version conflicts. This is by default True, because in most cases, having 2 different versions of the same library in the same dependency graph is at least complicated, if not directly violating ODR or causing linking errors.
- ``transitive_headers`` (default None): The headers of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the headers of the transitive dependency must be available and used in the ``-I<includedirs>`` compilation downstream.
- ``transitive_libs`` (default None): The libraries to link with of the dependency will be visible downstream or not. The default None allows Conan to auto detect this trait, for example, if the current package is a ``header-only`` one, and it depends on another library (header only, or static/shared), the libraries of the transitive dependency must be available and used in the ``-I<libs>`` and ``-L<libdirs>`` compilation downstream.
- ``test`` (default False): this requirement is a test library or framework, like Catch2 or gtest. It is mostly a library that needs to be included and linked, but that will not be propagated downstream.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add for clarifying that the dependency will be in the host context.


In order to smooth the upgrade to Conan 2.0, a ``**kwargs`` unused argument might be added to ``self.requires()`` in Conan 1.x, even if whatever is passed there is completely ignored in Conan 1.X, but that would allow to start specifying required traits in Conan 1.X recipes.

The explicit, high-level ``test_requires`` might also be introduced in Conan 1.X as an alias to ``self.build_requires(...., force_host_context=True)``.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the build_tool_requires("foo/1.0") to the migration plan.

@memsharded
Copy link
Member Author

This proposal has been fully implemented and released in 2.0-alpha1, and already iterated (by different maintainer) in 2.0-alpha2. So far this proposal is looking very good, and it will be able to bring Conan graph model and dependency resolution in 2.0 to a better place.

Thanks all for the fruitful discussions and feedback!

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