oss-review-toolkit / ort

A suite of tools to automate software compliance checks.
https://oss-review-toolkit.org
Apache License 2.0
1.61k stars 309 forks source link

Conan 2 support #8383

Open rettichschnidi opened 8 months ago

rettichschnidi commented 8 months ago

It seems like ORT currently supports only Conan 1.x. For my needs, I need support for version 2.

Questions:

sschuberth commented 8 months ago

For me, creating a new conan2 package-manager plugin (based on the existing one) would be the easiest way to achieve my goal.

The way the ORT analyzer works is that it needs to be able to decide "from the outside" of the project under analysis (like by files being present or not), which package manager implementation it should use. So if there is a reliable way to tell whether a project is a Conan 1 or Conan 2, then implementing Conan 2 as a separate plugin should work straightly.

As I'm not a Conan expert, for me there are a bunch of more questions to clarify, like: Is Conan 2 backwards compatible to Conan 1? So could we use only a Conan 2 binary to power both Conan 1 and Conan 2 analyzers? Or only have a single analyzer plugin for both on the ORT side after all?

Maybe @jsteinhofff can chime in here?

sschuberth commented 8 months ago

I guess this is good read:

rettichschnidi commented 8 months ago

The way the ORT analyzer works is that it needs to be able to decide "from the outside" of the project under analysis (like by files being present or not), which package manager implementation it should use. So if there is a reliable way to tell whether a project is a Conan 1 or Conan 2, then implementing Conan 2 as a separate plugin should work straightly.

With some support of the user, this should be possible by adding required_conan_version to a conanfile.py. This does not seem to be possible for conanfile.txt files.

Some additional thoughts/questions

As I'm not a Conan expert, ...

Same here

Is Conan 2 backwards compatible to Conan 1?

(Partially):

Recipes using latest 1.X integrations will be compatible with Conan 2.

I have no idea how many Conan 1.x recipes are not using "latest 1.X integrations", and therefore might fail to build with Conan 2. The simple ones I am using work seamlessly with Conan 2. The ones contained as test case in ORT do not (build_requires is no longer understood).

So could we use only a Conan 2 binary to power both Conan 1 and Conan 2 analyzers?

Nope. Not if we want to support every project out there/not breaking backward compatibility.

Or only have a single analyzer plugin for both on the ORT side after all?

By now, contradictory to my initial gut feeling, I think we should try to.

sschuberth commented 8 months ago

ORT runs just one project at a time, correct?

Well, ORT only analyzes one project of the same type (i.e. manager by the same package managers) at the same time. Other analyzers may run in parallel by default.

If so, can we just deduct the used Conan version from the binary available on the system.

No, because we need to be able to support both Conan 1 and 2 at the same time, so also both binaries need to be available at the same time.

Can we expect ORT users to to set the required_conan_version variable in their conanfile.py?

In short, IMO, no. Of course we could document this as a requirements to run ORT, but I'm already seeing the bug reports that ORT does not work for users who don't specify that.

Can we be forward looking, and default to version 2?

TBH, I don't know, as I have no feeling about how widespread Conan 2 is compared to version 1.

rettichschnidi commented 8 months ago

If so, can we just deduct the used Conan version from the binary available on the system.

No, because we need to be able to support both Conan 1 and 2 at the same time, so also both binaries need to be available at the same time.

How would you do this?

When installed via pip, the binaries for version 1.x and 2.x are both called conan. To install two versions, those would need to be installed in different (virtual) Python environments. Does ORT typically source a Python environment before running Conan?

Can we expect ORT users to to set the required_conan_version variable in their conanfile.py?

In short, IMO, no. Of course we could document this as a requirements to run ORT, but I'm already seeing the bug reports that ORT does not work for users who don't specify that.

As I have not (yet) found a way to reliably detect whether a recipe works with Conan 1.x, 2.x or both versions, I think one way or another some support by the user will be needed. Question is whether it should be the users of Conan version 1.x or 2.x.

Can we be forward looking, and default to version 2?

TBH, I don't know, as I have no feeling about how widespread Conan 2 is compared to version 1.

Does this really matter? Conan 2 is the future, and it will dominate sooner or later. The intention behind my question was to figure out if ORT values "status quo can run unchanged" over "we make it easy to run modern stuff".

sschuberth commented 8 months ago

How would you do this?

I don't know yet 😸 But generally, ORT allows you run any of its supported package managers in any combination. Maybe we should simply consider Conan 2 to be a special case of Conan 1, and allow to configure it similar to the Python version.

Does ORT typically source a Python environment before running Conan?

No.

I think one way or another some support by the user will be needed.

Ok, again similar to Python support then, as we cannot always reliably detect the Python version that should be used, but the user simply has to know and configure it for ORT.

Does this really matter? Conan 2 is the future, and it will dominate sooner or later.

IMO it matters, because we should make ORT easy to use for the majority of users. If Conan 2 is there but not yet widely adopted, I don't see a point in making the default now, but maybe only later.

jsteinhofff commented 8 months ago

Hi there, not sure I can contribute much since I don't have a good understanding (any more) how the conan package manager integration works . Having a quick look at the code I get the impression that it boils down to

About the acceptable of Conan 2 I can only share from our company that I think we are quite far away from general adoption. We use Conan to share packages across teams, departments and even business units, this makes such a breaking change extremely difficult to tackle. No single entity can upgrade in isolation, supporting both versions is also cumbersome, and orchestrating a synchronized change everywhere is basically impossible in such far spread scenario.

But due to this effect one could argue that rarely both versions would be coexisting in a single project/product/whatever the granularity is on which ORT runs. So I would suspect that a configuration option of the version to use is acceptable, it would probably then make sense to verify that the Conan command line tool indeed matches to this configuration.

For what the default shall be of such configuration... I would assume the default should not break existing ORT instances, so it should be conan1.

rettichschnidi commented 5 months ago

Here is my (still very hacky) WIP branch: https://github.com/husqvarnagroup/ort/tree/gardena/rs/conan-2

TODOs:

nnobelis commented 4 months ago

A comment relative to Conan 2: Some artifactories can serve artifacts in "revision" mode (https://docs.conan.io/1/versioning/revisions.html#package-revisions).

If this is the case, Conan 1 will fail:

conan user -r artifactory -p <password> <user>
ERROR: The remote at 'https://<artifactory>' only works with revisions enabled.
Set CONAN_REVISIONS_ENABLED=1 or set 'general.revisions_enabled = 1' at the 'conan.conf'.

You have then two choices: either define the specified variable, or use Conan 2.

sschuberth commented 5 days ago

@rettichschnidi turns out that out of own need I was taking a brief look at this again. Do you have a pointer to a good Conan 2 stand-alone project to test with?