Introduce resolvo #3947

jjerphan · 2025-05-19T15:54:39Z

No description provided.

JohanMabille

First review, I'll add more comment later.

JohanMabille · 2025-05-27T08:34:05Z

libmamba/include/mamba/solver/database.hpp

+        class Database;
+    }
+
+    using DatabaseVariant = std::variant<libsolv::Database, resolvo::Database>;


I think a classical inheritance tree (or type erasure) would be more flexible and extensible on the long run than a variant.
I understand this requires further refactoring (up to the APIs used by mamba / micromamba), but I think it is definitely worth doing it. Besides, using abstractions in the higher level should reduce the "common" APIs of the different database classes and simplify the abstraction itself.

Yes, this is temporary for now, and this can later be changed so that the plan from #3387 is implemented.

libmamba/include/mamba/solver/resolvo/database.hpp

JohanMabille · 2025-05-27T09:54:39Z

libmamba/include/mamba/solver/resolvo/database.hpp

+        ID operator[](T value)
+        {
+            return value_to_id[value];
+        }


I think we should be consistent with the map API. operator[] returns a reference to an object either already in the map, or inserted. It might be a bit involved to implement such an operator (it would have to return a reference proxy to update both internal maps), so we might skip it for now and just implement find / insert methods.

libmamba/include/mamba/solver/resolvo/database.hpp

AntoinePrv

This is going in the right direction! I only put a few comments on the code for now, because I think we should split the development in successive steps that would be easy to review and merge. For instance, I would

Focus on an mamba::solver::resolvo implementation disconnected from the rest of mamba that passes tests similar to libsolv/test_solver.cpp. We can leave leave out anything json related to start with, or else plug a dirty implementation that uses libsolv under the hood. If some tests are generic enough, we can introduce a provisional generic Solver API to run thoses tests with all possible solvers and configuration.
Refactor Json parsing and PackageInfo to avoid code-duplication, copies, and exposing private dependencies. I think something with Concept + Composition could help us drastically reduce all the code duplication we already have. Here we can plug some integration tests with data from conda-forge.
Then we can derive a generic API, factory, and plug it into the rest of mamba.

AntoinePrv · 2025-06-23T10:03:58Z

.github/workflows/unix_impl.yml

            -D CMAKE_CXX_COMPILER_LAUNCHER=sccache \
            -D CMAKE_C_COMPILER_LAUNCHER=sccache \
-            -D MAMBA_WARNING_AS_ERROR=ON \
+            -D MAMBA_WARNING_AS_ERROR=OFF \


We should remove this before merging.

AntoinePrv · 2025-06-23T10:10:43Z

libmamba/include/mamba/solver/resolvo/database.hpp

+        bijective_map<::resolvo::NameId, ::resolvo::String> name_pool;
+        bijective_map<::resolvo::StringId, ::resolvo::String> string_pool;
+        bijective_map<::resolvo::VersionSetId, specs::MatchSpec> version_set_pool;
+        bijective_map<::resolvo::SolvableId, specs::PackageInfo> solvable_pool;


Are we sure we want to publicly exposed members?

AntoinePrv · 2025-06-23T10:11:34Z

libmamba/include/mamba/solver/resolvo/database.hpp

+        bool has_package(const specs::MatchSpec& spec)
+        {
+            auto candidates = get_candidates(
+                name_pool.alloc(::resolvo::String(spec.name().to_string()))
+            );
+            return !candidates.candidates.empty();
+        }


Nit: separate declaration from definition.

AntoinePrv · 2025-06-23T10:14:10Z

libmamba/include/mamba/solver/solver_factory.hpp

+#include "mamba/solver/resolvo/database.hpp"
+#include "mamba/solver/resolvo/solver.hpp"
+
+namespace mamba::solver


It would be nice to keep mamba::solver context-free. There could be an enum to choose the solver, and make a method in the context to compute it from experimental_resolvo_solver.

AntoinePrv · 2025-06-23T10:19:27Z

libmamba/include/mamba/specs/package_info.hpp

+        [[nodiscard]] static auto from_json(
+            const std::string_view& filename,
+            simdjson::ondemand::object& pkg,
+            const CondaURL& repo_url,
+            const std::string& channel_id
+        ) -> expected_parse_t<PackageInfo>;
+


This ought to be a private helpers somewhere. simdjson is a private dependency and I think it should stay that way.

Perhaps a concept or view-like wrapper could help. On idea I had was to make a PackageInfo template that take a data storage class object. Then there could be simdjson, libsolv, resolvo, and inline std::string... implementations.
Let me know if you want to brainstorm it together, I'd be happy to!

jjerphan · 2025-06-25T08:13:39Z

Note that this is completely work in progress, I am trying to have something which works first (while being pulled in many directions).

Refactor Json parsing and PackageInfo to avoid code-duplication, copies, and exposing private dependencies. I think something with Concept + Composition could help us drastically reduce all the code duplication we already have. Here we can plug some integration tests with data from conda-forge.

Yes, this would be useful to also resolve #3954. I think this should be tackled first before all.

Signed-off-by: Julien Jerphanion <[email protected]>

Many changes are required to be able to use hashmaps. Signed-off-by: Julien Jerphanion <[email protected]>

Required to use `std::unordered_map` with custom types. Signed-off-by: Julien Jerphanion <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]>

Resolve the problem with the locking. Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Wolf Vollprecht <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

Signed-off-by: Julien Jerphanion <[email protected]>

wolfv · 2025-06-25T08:23:49Z

I keep wondering if it wouldn't be easier to just add C++ bindings to rattler at this point ...

jjerphan added the release::enhancements For enhancements PRs or implementing features label May 19, 2025

jjerphan force-pushed the integration-resolvo branch 4 times, most recently from 54d499b to a3c5531 Compare May 22, 2025 10:22

JohanMabille reviewed May 27, 2025

View reviewed changes

jjerphan force-pushed the integration-resolvo branch from a3c5531 to 28f9086 Compare May 28, 2025 13:08

jjerphan force-pushed the integration-resolvo branch from e9c2cf7 to 096c6e5 Compare June 11, 2025 15:05

AntoinePrv reviewed Jun 23, 2025

View reviewed changes

jjerphan and others added 20 commits June 25, 2025 10:16

Depend on resolvo

ebe9e91

Signed-off-by: Julien Jerphanion <[email protected]>

Add resolvo-cpp to the mamba environment

237ad2a

Signed-off-by: Julien Jerphanion <[email protected]>

Add a test for resolvo

b8795f7

Many changes are required to be able to use hashmaps. Signed-off-by: Julien Jerphanion <[email protected]>

Add specializations of std::hash

7bab460

Required to use `std::unordered_map` with custom types. Signed-off-by: Julien Jerphanion <[email protected]>

Implement most methods

d2a3160

Signed-off-by: Julien Jerphanion <[email protected]>

wip

4a0b12c

Signed-off-by: Julien Jerphanion <[email protected]>

wip

baca592

Signed-off-by: Julien Jerphanion <[email protected]>

wip: Use custom pools

5d326dd

Signed-off-by: Julien Jerphanion <[email protected]>

Minimal Reproducer of the problem

183e365

Signed-off-by: Julien Jerphanion <[email protected]>

Also sort on build number

c28affb

Signed-off-by: Julien Jerphanion <[email protected]>

test: Addition of PackageInfo to PackageDatabase

de9d09b

Signed-off-by: Julien Jerphanion <[email protected]>

test: Filter solvables

0fc9676

Signed-off-by: Julien Jerphanion <[email protected]>

Add more strings and names to pools

7f05c6f

Signed-off-by: Julien Jerphanion <[email protected]>

Initialize Candidates.{favored,locked} to nullptr

e97546d

Resolve the problem with the locking. Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Wolf Vollprecht <[email protected]>

wip

6dbe87c

Signed-off-by: Julien Jerphanion <[email protected]>

Parse repodata.json

b5a7dcd

Signed-off-by: Julien Jerphanion <[email protected]>

wip: Test consistency with libsolv

4ff86b7

Signed-off-by: Julien Jerphanion <[email protected]>

wip

5f577a1

Signed-off-by: Julien Jerphanion <[email protected]>

test: mlflow=2.12.2 explicit

d05f0d2

Signed-off-by: Julien Jerphanion <[email protected]>

Debug

c7d32dc

Signed-off-by: Julien Jerphanion <[email protected]>

jjerphan and others added 28 commits June 25, 2025 10:17

Adapt Database and related signatures

623b3ff

Signed-off-by: Julien Jerphanion <[email protected]>

Use to_string instead of str

44b563e

Signed-off-by: Julien Jerphanion <[email protected]>

Define a type for the variant

8afe9a4

Signed-off-by: Julien Jerphanion <[email protected]>

Use DatabaseVariant for MTransaction and friends

92df455

Signed-off-by: Julien Jerphanion <[email protected]>

Deduplicate configurable for using resolvo

2590c52

Signed-off-by: Julien Jerphanion <[email protected]>

Use DatabaseVariant for repoquery

8350a72

Signed-off-by: Julien Jerphanion <[email protected]>

Log solver used

05eb464

Signed-off-by: Julien Jerphanion <[email protected]>

WIP solver integration

0926f05

Signed-off-by: Julien Jerphanion <[email protected]>

Implement for resolvo

d727e79

Signed-off-by: Julien Jerphanion <[email protected]>

Use resolvo 0.1.0 for now

f0487f6

Signed-off-by: Julien Jerphanion <[email protected]>

Pass ChannelParams to solver::resolvo::Database

0976153

Signed-off-by: Julien Jerphanion <[email protected]>

Revert changes made to expected_caster.hpp

bb06b21

Signed-off-by: Julien Jerphanion <[email protected]>

Adapt create_repo_from_pkgs_dir

b50b59a

Signed-off-by: Julien Jerphanion <[email protected]>

Merge database_has_package implementations

28fda54

Signed-off-by: Julien Jerphanion <[email protected]>

Deduplicate database_has_package

7a7fdc2

Signed-off-by: Julien Jerphanion <[email protected]>

Resolvo on the install path

413dd82

Signed-off-by: Julien Jerphanion <[email protected]>

More install path

4bce239

Signed-off-by: Julien Jerphanion <[email protected]>

Define lsplit_track_features inline

cac9ad6

Signed-off-by: Julien Jerphanion <[email protected]>

Parse respodata.json

3e00e99

Signed-off-by: Julien Jerphanion <[email protected]>

Complete implementation of solver::resolvo::Database

2748de1

Signed-off-by: Julien Jerphanion <[email protected]>

Unify naming of DatabaseVariant instances

71e83ac

Signed-off-by: Julien Jerphanion <[email protected]>

Ignore packages signatures when using resolvo for now

0a2a79d

Signed-off-by: Julien Jerphanion <[email protected]>

Only create solver::libsolv::Database if needed

01f324f

Signed-off-by: Julien Jerphanion <[email protected]>

Revert change made to test_env_lockfile.cpp

a790564

Signed-off-by: Julien Jerphanion <[email protected]>

Post rebase unification

61a4d1a

Signed-off-by: Julien Jerphanion <[email protected]>

Rename Mapping to bijective_map

0c3136c

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

Align APIs

d9c2e01

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Johan Mabille <[email protected]>

Post rebase adaptations

107188f

Signed-off-by: Julien Jerphanion <[email protected]>

jjerphan force-pushed the integration-resolvo branch from bbbd192 to 107188f Compare June 25, 2025 08:18

Uh oh!

Introduce resolvo #3947

Are you sure you want to change the base?

Introduce resolvo #3947

Uh oh!

Conversation

jjerphan commented May 19, 2025

Uh oh!

JohanMabille left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AntoinePrv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjerphan commented Jun 25, 2025

Uh oh!

wolfv commented Jun 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AntoinePrv left a comment •

edited

Loading