search

conan-io / conan

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

[question] Check if any requires is in editable mode #17310

Open DanielKopp94 opened 3 days ago

DanielKopp94 commented 3 days ago

What is your question?

In order to mark a package with a certain version if it was built with at least one require in editable mode it would be needed to get this information when running a conan build of the top level package.

Lets assume a simple example:

We have four modules: A, B, C, D

where

A requires B and C B requires D C requires D

Now when doing a conan build it would be needed to know if any of B, C or D is in editable mode. Is there any possibility?

Have you read the CONTRIBUTING guide?

memsharded commented 3 days ago

Hi @DanielKopp94

Thanks for your question.

In order to mark a package with a certain version if it was built with at least one require in editable mode it would be needed to get this information when running a conan build of the top level package.

I am curious, why would you want to do this in the first place. As described in https://docs.conan.io/2/knowledge/guidelines.html, developers shouldn't be able to upload packages, and the editables is a developer feature, so marking packages built against editables seems a bit redundant.

Now when doing a conan build it would be needed to know if any of B, C or D is in editable mode. Is there any possibility?

At the moment there is nothing explicit in the Conan interfaces to do this, exactly because editables are intended to be fully transparent. The idea is exactly that a consumer of a package is completely unaware if a package is in the Conan cache or it is in the user folder as editable. It is possible to do an easy check of self.dependencies[dep].package_folder and check if it is in the Conan cache or not, but that would probably be not a very elegant solution.

DanielKopp94 commented 3 days ago

The reason we need to have this is to mark those packages properly. We are working with binaries that are loaded to devices. And yes the rule is not to put packages built using editable mode onto the artifactory.

But. Developers sometimes are not following rules or are not aware that they left a module in editable mode. Therefore we decided to have a explicit version marking for such packages. This way on one hand everybody getting such a packages from others (also shared within other channels like mail) will be aware of the type of packages. And in case such a binary is on a drive used in R&D somebody reading the version sees that a experimental package is currently on the device and can updated to a non-experimental package.

So in this case best practices apply but there are exceptions that not all are always following all the rules and we have cases where we share such packages due to pair-programming or fast test and development cycles.

I will try the idea you suggested and see if it works for our use case. Even if it is not the most elegant solution it helps

memsharded commented 3 days ago

Thanks for the feedback.

As this sounds like a relatively "orthogonal" quality check, I have another quick question, how do you plan to implement this? Have you considered adding a hook for example, that does the task, and lets say, generate a file inside the package_folder that contains this information? Or maybe into the package_metadata? Because in that way it is not necessary to modify all recipes to add anything, but the checks happen by injecting some functionality via hooks.

Note this sounds related to the new feature we are working in this PR: https://github.com/conan-io/conan/pull/17203, to be able to define a plugin that generates extra manifests files inside packages metadata, for sboms and the like. Your case, which is knowing something about a package dependency, is a "manifest" case too.

DanielKopp94 commented 3 days ago

As it is needed already in the final binary we will implement it in a way that it becomes part of the version of the package => similar to the maturity of a package (alpha, beta, ...) we set the maturity to something like local build.

At the moment this will only be done on the top-level package. This at the moment does not ensure that I first create a module in a lower level and use then the artifact in the local cache to create my top-level package. Doing it with hooks actually is a really good idea and we have to think about it. Thank you for the idea.

We saw that there is some work in progress for sbom and once it is finished it is for sure a interesting topic for us. Also for other information as well.

memsharded commented 3 days ago

As it is needed already in the final binary we will implement it in a way that it becomes part of the version of the package => similar to the maturity of a package (alpha, beta, ...) we set the maturity to something like local build.

Please recall that the user/channel shouldnt be used for this but a regular version.

This would introduce some challenges. The version of a package is a function purely of the input sources, not the dependencies. Very often it will match a git tag. A conan package version and set_version() are evaluated at export time (that happens for conan export/export-pkg/create commands), when the dependency graph is not computed at all. It is impossible to know if the dependencies of a package are editable because there are no self.dependencies at all at that point.

The only possibility would be to do 2 separate steps, gather the information doing first a conan install, and then using that information in the later export step, but that sounds a bit too artificial, not being able to do a conan create in a single step too.

In general I would advise against adding information to version that is not about the sources but about the package binary.

DanielKopp94 commented 3 days ago

Please recall that the user/channel shouldnt be used for this but a regular version.

We are not using user/channel at all for it.

In general I would advise against adding information to version that is not about the sources but about the package binary.

Here we distinguish two different cases. One is that our binaries are built with a build server -> There we are not going to do any operations as such for checking editable mode yes or no. And at any time we know if we are on a build server or building on a local machine of a developer.

If a build is done on a developer machine we need to implement such a functionality as this is a critical situation for us and such packages need to be recognizable even if we are excluding artifactory. So the package itself needs a stamp to tell that to everybody who gets such a package. Adding the information about editable mode used yes/no is an additional information that is requested by our team.

memsharded commented 3 days ago

If a build is done on a developer machine we need to implement such a functionality as this is a critical situation for us and such packages need to be recognizable even if we are excluding artifactory. So the package itself needs a stamp to tell that to everybody who gets such a package. Adding the information about editable mode used yes/no is an additional information that is requested by our team.

There might be something about the use case or the flow that I don't understand yet:

In the second case, even if we wanted to try to bypass the process error, the problem for assigning a version to it that depends on the dependencies remains. It is not possible with a single conan create to define a version for the package that depends on the dependencies, because the version is computed well before the dependencies are computed.

DanielKopp94 commented 3 days ago

If the package is in "editable" mode, it is not really a package. It is not published, it doesn't really get a version, it cannot be shared with others

That unfortunately not true 100%. Even if build in editable mode it gets a version. And in our case you will get a binary that you put on your device. To clarify, we are talking here about embedded devices. So the device can be tested locally by the develop. As devices are shared it may be the case that somebody else gets it and then it needs to be visible what kind of package is running on it. In this case we need to mark such a package properly.

If the package is not in editable mode, but it is being built in the cache, but it depends on editable packages, that I would consider it mostly like a process error. I'd try to block it in the first place, not allowing this to be usable or shared at all. The hook that detects this situation should block the creation of the package raising an exception.

I think that's a good point. Blocking with a hook the create step for a package in editable mode. I have not yet thought about that.

memsharded commented 3 days ago

Ok, got it.

I am thinking that this could be connected to the problem of "workspace" definition, see current PR: https://github.com/conan-io/conan/pull/17272

The workspace is a way to automatically manage editables together, with some extension capabilities, like the ability to define packages AND versions of packages that are in editable mode dynamically, check for example the def test_dynamic_editables(self, api): test in that PR, with a dynamic editables() function that can define packages and versions of those packages dynamically, based on some local workspace logic. This still cannot change one package version based on the dependencies (if editable or not), but it can dynamically change the versions when the packages are in editable with conanws.py logic.

DanielKopp94 commented 2 days ago

@memsharded, thank you for all the input and nice discussion. Now i got some really good ideas on how to proceed. I will close the issue as for me it is fine.

In case as conan at the moment does not allow to do a create for a module in editable mode maybe it would be useful to extend this to not allow to do a conan create in case any requirement is in editable mode.

memsharded commented 1 day ago

@memsharded, thank you for all the input and nice discussion. Now i got some really good ideas on how to proceed. I will close the issue as for me it is fine.

Thanks very much for the feedback. Don't hesitate to create new tickets as necessary for any further question!

In case as conan at the moment does not allow to do a create for a module in editable mode maybe it would be useful to extend this to not allow to do a conan create in case any requirement is in editable mode.

Actually I think this would deserve to re-open the ticket and consider this idea to be built in. Certainly building packages in the cache that depends on editables deserve at least a warning (which can be converted to an error with warn_as_error). Let's try to add something, at least the warning at the moment, and we can re-think if it would be worth to go further in some scenarios.