Feat/generic pool

[tdejager]

Description

PR that makes the Rust libsolv solver generic on a couple of traits and the current conda ecosystem uses these traits. The traits you currently need are:

• VersionTrait defines what a version specification is. We might be able to get rid of some requirements on this later. For Conda this is a PackageRecord.
• VersionSet this is the set of version which you can use to match something that implements VersionTrait.
• DependencyProvider this has a function that determines how candidates selected by the solver should be ordered. This is custom per ecosystem.

The reason for doing this is to be able to integrate other ecosystems like PyPi into the solver and see if this can work. We now know the generic requirements currently needed by the solver.

All the conda specific code has been moved to rattler_solve. I think if all tests succeed this is in state that it could potentially be merged. All the tests are succeeding locally so I suppose the whole thing should be a drop in replacement.

It does have a performance regression, namely that we are copying a lot of PackageRecords. Regression range from 13% - 77% depending on the test, so that's quite a lot. However, I think it would be good to review and merge before it gets bigger :) Then look at PackageRecord improvement.

In the solver tests I've made a bespoke version and matching spec that could potentially be a source of inspiration for an implementation.

Todo's in code

Some TODO's have been added to the code where we could potentially improve some aspects.
Some notable todo's:

• The sorting method in conda requires a cache of candidates to highest versions, could we potentially make this more generic? Maybe even a generic caching crate like lru would just work.
• There is a Name type in the VersionTrait that might be able to move to Pool
• In the libsolv tests, I've made generic methods for adding packages and dependecies in the Pool, this puts some more requirements on the Traits of pulling these requirements into the traits. This goes against the last point, however. And we should see what we prefer. It's also totally possible to keep these methods ecosystem specific, with some potential duplication.
• Can we avoid NewTypes somehow when implementing the traits in rattler_solve or is this something we actually want? :)
• Can we make the arguments to sort_candidates type-safe as of now they can can by any SolvableId it would be nice to be able to actually use the subset returned by the pool when matching on the name. Could introduce a new type for this that can only be constructed in such a situation.

What's next?

• Making use of the &PackageRecord type instead of the PackageRecord value, avoiding copies.
• Try writing the same for PyPi and see what we still need to change, while keeping the solver code generic.

[wolfv]

Wow, impressive that it's passing our test suite! Awesome!
I was wondering if the error message changes are really necessary or if we can keep the current way of displaying the "matchspec" form?
I haven't done a deeper review of the changes yet.

I am also not sure how we should proceed given the performance regression. That might be a little annoying to end-users since we already enabled the new solver in pixi.

Should we make a solver-development branch instead?

[baszalmstra]

I think I can solve the performance regressions tomorrow!

[tdejager]

@baszalmstra @wolfv Okay so we'll keep this PR open, until the performance regressions are fixed?

[baszalmstra]

Yes! should be fixed pretty soon!

[tdejager]

Wow, impressive that it's passing our test suite! Awesome! I was wondering if the error message changes are really necessary or if we can keep the current way of displaying the "matchspec" form? I haven't done a deeper review of the changes yet.

I am also not sure how we should proceed given the performance regression. That might be a little annoying to end-users since we already enabled the new solver in pixi.

Should we make a solver-development branch instead?

It's for the internal tests anyway I think the error message and format used there is a little more generic because it removes the ambiguity of the upper bound, which I hope helps a random person looking at the solver tests for the first time. Also decoupling it more from conda in the same time.

[wolfv]

Ah, OK, so to confirm, the error messages we would see in pixi are the same?

[tdejager]

Ah, OK, so to confirm, the error messages we would see in pixi are the same?

For sure!

[wolfv]

I was running the benchmark locally and it was only ~10% slower max so I'm fine with merging

[tdejager]

When I added LTO it is the same again on my M1.