Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[question] components: requirements handling #7444

Closed
1 task done
ericLemanissier opened this issue Jul 28, 2020 · 9 comments
Closed
1 task done

[question] components: requirements handling #7444

ericLemanissier opened this issue Jul 28, 2020 · 9 comments

Comments

@ericLemanissier
Copy link
Contributor

ericLemanissier commented Jul 28, 2020

When using components, all the requirements of the recipe have to be explicitly associated with one of the components, or conan refuses the recipe. How should we handle the following cases?

  1. The requirement is used only in a private plugin, which is loaded at run-time but to which consumers should not link directly, eg qt platform plugins
  2. the requirement is used only in an executable, eg libelf in glib: https://github.com/GNOME/glib/blob/b79747eee1f5a840eb8b369b82e09f888ddf7a0d/gio/meson.build#L936
@jgsogo
Copy link
Contributor

jgsogo commented Nov 3, 2020

Maybe the check should run only for public requirements. If it is used internally, it should be a private requirement. wdyt, @danimtb? Does it make sense? It would be something really easy to add to the check.

@ericLemanissier
Copy link
Contributor Author

ericLemanissier commented Nov 3, 2020

I don't think that private requirements is the way to solve this. The fact that a dynamic library is a plugin not intended to be linked to by the consumer does not imply that the requirements of this plugins have to be private. It could lead to ODR violation in some cases, if some other package in the dependency graph depend on this requirement too.
Maybe the best way to go is to create components with empty libs attribute ? (this is what I ended up doing in glib for an executable, but It could be done for dynamic plugins too)

@jgsogo
Copy link
Contributor

jgsogo commented Nov 3, 2020

If you are using some shared library as a plugin downstream, you might face some DLLHell issue (libraries with the same name), but you shouldn't get an ODR violation (by definition you are not linking with a plugin). Maybe I'm missing something or some edge case.

...or maybe we are not talking about the same scenario. I think that if the requirement is private, it shouldn't appear in the package_info() function (this function declares the information available to consumers), if it is private no information about that requirement should be available to consumers. Conan graph algorithm will take care of conflicts and ODR violations (now, it won't allow two different versions of the same library).

Even if you bypass the check as you did in glib, Conan will generate the gresource component that is propagating information about libelf, which is exactly what we were trying to avoid, right?

@ericLemanissier
Copy link
Contributor Author

If you are using some shared library as a plugin downstream, you might face some DLLHell issue (libraries with the same name), but you shouldn't get an ODR violation (by definition you are not linking with a plugin). Maybe I'm missing something or some edge case.

You are not linking to it, but the fact that it is loaded at runtime is an ODR violation if several parts of the same program use different versions of a library. (let's imagine said library has global static data)

Even if you bypass the check as you did in glib, Conan will generate the gresource component that is propagating information about libelf, which is exactly what we were trying to avoid, right?

With dynamic plugins, my main goal was to avoid consumers to link to the plugin directly, but you are right that the workaround does not prevent consumers from linking to the plugin's requirements, which might not be desirable too. Maybe what we truly need is to add a boolean attribute named private at the component level ? This way it is still guaranteed to have a single version of all the packages in the dependency graph, but the information of private components is not transmitted to consumers.

@jgsogo
Copy link
Contributor

jgsogo commented Nov 3, 2020

Components (or requires member in components) don't guarantee to have one single version of all the packages in the dependency graph. That check is performed by Conan way before entering the package_id method. Conan first builds the graph and resolves conflicts, and then worries about the information to be propagated to consumers, before it knows nothing about declared components.

The current implementation of private requirements might have some flaws and it needs to be improved, but I really think it is the way to go. If the requirement is private it doesn't propagate (it can't appear in any components[].requires declaration), but we need to provide a way to ensure ODR and DLLHell are taken into account and warn (or raise) the consumer about it.

@ericLemanissier
Copy link
Contributor Author

Components (or requires member in components) don't guarantee to have one single version of all the packages in the dependency graph.

Yes, the ODR violation has nothing to do with components, but only with using private requirements: They break this guarantee.

The current implementation of private requirements might have some flaws and it needs to be improved, but I really think it is the way to go. If the requirement is private it doesn't propagate (it can't appear in any components[].requires declaration), but we need to provide a way to ensure ODR and DLLHell are taken into account and warn (or raise) the consumer about it.

Well, that we need to change both the implementation and the documentation of the private requirements, because currently it says It might be necessary in some extreme cases, like having to use two different versions of the same library (provided that they are totally hidden in a shared library, for example), but it is mostly discouraged otherwise.. This is why it feels like a different feature to me, but if it's ok to break existing users of private requirements, I have no problem with changing this

@jgsogo
Copy link
Contributor

jgsogo commented Nov 4, 2020

Yes, probably I'm thinking (and talking) with the new graph model in mind and you are being realistic about the current features we have. Sorry, I didn't notice. For Conan 2.0 we have realized we need a more powerful graph model and I think this is one of the issues to take into account and should fit into the new modeling.

In the transition to Conan 2.0 we would really like not to break any recipe, so we try to avoid adding syntax/member/attributes to Conan 1.x if they are not going to be used in 2.0. With this premise, I try to think about how I would implement the feature in Conan 2.0 and, if a new method/attribute is not going to be needed, then it could be better to wait than to add future-legacy to the recipes. Sometimes the feature is urgent and cannot wait until the next major version, then we will add whatever is needed, but if the feature can wait it is better to postpone it (Conan 2.0 will start development very soon).

@danimtb
Copy link
Member

danimtb commented Nov 4, 2020

Related to the issue #7907. Now in Conan 1.31 you can use private requirements without components failing

@ohanar
Copy link
Contributor

ohanar commented Nov 19, 2020

From my perspective, the components with empty libs is the correct direction for plugins. For openscenegraph, the plugins are all listed out as individual components with their requires appropriately set -- when built as a shared library (which is what you should do 99%+ of the time), the plugins' components have empty libs.

For executables, I think the issue should be punted to #7240. My current workaround has to just been to make a dump component and throw all the executables requires in there. I also use this technique for most instances of private requires, because I still want the guarantee of only having one copy of a library in my graph to avoid DLLHell.

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

No branches or pull requests

4 participants