feat: Add support for optional dependencies #1019

prsabahrami · 2025-01-08T17:07:17Z

Description

I have updated the matchspec parser to parse the list of features. (e.g. sympy[version=1.2, optional_features=[feat1, feat2]] into [feat1, feat2]).
In addition, I added SolverPackageRecord::RecordWithFeature which represents a package with a single feature. Hence, the goal is to turn sympy[feat1, feat2] into sympy[feat1], sympy[feat2]. We also need to assure that both of these adhere to the correct version of sympy.

…ional features to the solver

baszalmstra

Very nice work thus far! I think you almost got it!

...s/src/repo_data/snapshots/rattler_conda_types__repo_data__patches__test__patch_purl.snap.new

crates/rattler_conda_types/src/repo_data/mod.rs

crates/rattler_solve/src/resolvo/mod.rs

tdejager · 2025-01-09T11:03:11Z

@baszalmstra @prsabahrami Do you think we need to version the MatchSpec implementation in rust with this feature addition? Or maybe have an explicit cargo feature that marks this as an extension? That way consumers of the libary cannot seralize into matchspecs that cannot be parse by e.g conda?

baszalmstra · 2025-01-09T11:07:42Z

@tdejager That seems smart!

wolfv · 2025-01-09T12:22:13Z

I think we might even want to gate the whole feature behind a compile time option for now? Until we have a CEP that passed?

…inor fixes

…ameWithoutFeature

…PackageNameWithoutFeature" This reverts commit 44c381b.

prsabahrami · 2025-01-09T21:12:25Z

@baszalmstra I have updated the pool to take in NameType which an enum of either single String or a pair of name and feature. Please let me know what you think!

...src/snapshots/rattler_conda_types__prefix_record__test__pysocks-1_7_1-pyh0701188_6_json.snap

crates/rattler_lock/src/parse/models/v6/conda_package_data.rs

crates/rattler_solve/src/resolvo/mod.rs

…lockfile

crates/rattler_solve/src/resolvo/mod.rs

baszalmstra · 2025-01-13T16:10:59Z

crates/rattler_solve/src/resolvo/mod.rs

+        let feature = value
+            .optional_features
+            .as_ref()
+            .and_then(|features| features.first().cloned());


I guess this should be None?

We need to have an assertion here to make sure that only 1 feature is in the optional_features of value and then select the only one as the SolverMatchSpec::feature. I'm not sure why it would be None.

It should be None initially. In parse_match_spec we then explicitly add a feature.

crates/rattler_solve/src/resolvo/mod.rs

baszalmstra · 2025-01-13T16:39:34Z

crates/rattler_solve/src/resolvo/mod.rs

+        let feature = value
+            .optional_features
+            .as_ref()
+            .and_then(|features| features.first().cloned());


It should be None initially. In parse_match_spec we then explicitly add a feature.

crates/rattler_solve/src/resolvo/mod.rs

test-data/channels/dummy-optional-dependencies/noarch/repodata.json

baszalmstra

Im still thinking about the name optional_features. I wonder if its descriptive enough.

Im a little on the fence about the name features. It implies that a certain "feature" is enabled but in reality the only thing that changes is that an extra set of dependencies is required. This might ofcourse cause new functionality to be unlocked but not necessarily.

Im starting to think that the name extras might be better.. Wdyt?

Also optional is a bit strange in this context. Optional to me would imply that a solution without the optional features is also valid, where it is in fact not. Perhaps in the matchspec we could just drop this part? E.g. foobar[features=[baz]]

.github/workflows/rust-compile.yml

crates/rattler_conda_types/src/match_spec/mod.rs

crates/rattler_conda_types/Cargo.toml

crates/rattler_conda_types/src/match_spec/mod.rs

crates/rattler_conda_types/src/match_spec/parse.rs

crates/rattler_conda_types/src/repo_data/mod.rs

crates/rattler_solve/Cargo.toml

baszalmstra · 2025-01-23T14:27:18Z

crates/rattler_solve/src/libsolv_c/mod.rs

@@ -94,7 +94,7 @@ impl super::SolverImpl for Solver {
    >(
        &mut self,
        task: SolverTask<TAvailablePackagesIterator>,
-    ) -> Result<Vec<RepoDataRecord>, SolveError> {
+    ) -> Result<SolverResult, SolveError> {


Can you create an issue for this so we dont forget?

baszalmstra · 2025-01-23T14:28:33Z

crates/rattler_solve/src/libsolv_c/mod.rs

@@ -94,7 +94,7 @@ impl super::SolverImpl for Solver {
    >(
        &mut self,
        task: SolverTask<TAvailablePackagesIterator>,
-    ) -> Result<Vec<RepoDataRecord>, SolveError> {
+    ) -> Result<SolverResult, SolveError> {


Furthermore, maybe it would be good to error out if a matchspec with extras is used. That way we provide a clear error message when people try it.

baszalmstra · 2025-01-23T14:33:22Z

...e/tests/snapshots/backends__resolvo__solve_dummy_repo_optional_depends_feature_conflict.snap

+│  └─ foo 4.0.2
+└─ foo[with-bar] * cannot be installed because there are no viable options:
+   └─ foo[with-bar] 3.0.2 would require
+      └─ foo ==3.0.2 py36h1af98f8_1[md5=fb731d9290f0bcbf3a054665f33ec94f, sha256=67a63bec3fd3205170eaad532d487595b8aaceb9814d13c6858d7bac3ef24cd4], which cannot be installed because there are no viable options:


Mm this error is pretty uninformative. Especially if there will be multiple solvables that provide that extra. It will probably list every single solvable and extra here.

Perhaps it would be possible to merge all the SolvableWithFeature solvables of a single version into a single solvable? I assume that will result in a much clearer error message and reduce the total number of solvables..

.../tests/snapshots/backends__resolvo__solve_dummy_repo_optional_depends_recursive_feature.snap

Co-authored-by: Bas Zalmstra <[email protected]>

prsabahrami · 2025-01-23T15:05:55Z

Created an issue for supporting this in libsolv. #1032

…tler into feature_optional_depends

…ional_depends`

prsabahrami added 3 commits January 8, 2025 11:27

Updating the parser to parse Matchspec with features and adding opt…

21d3d3e

…ional features to the solver

Adding the package with different features to all-requirements.

73de02e

Run fmt

187cc92

prsabahrami changed the title ~~Adding support for optional dependencies~~ feat: Add support for optional dependencies Jan 8, 2025

prsabahrami added 2 commits January 9, 2025 02:03

Add separate features as separate packages

2141a10

Add optional_dependencies to to repodata query

cd6347c

baszalmstra reviewed Jan 9, 2025

View reviewed changes

prsabahrami added 3 commits January 9, 2025 09:39

Remove the new snapshots and update the previous ones

964f896

Create new MatchSpec instead of parsing the main package along with m…

4653c79

…inor fixes

Add support for packages with optional_features as as dependencis

0d56281

prsabahrami requested review from baszalmstra and wolfv January 9, 2025 17:18

prsabahrami added 5 commits January 9, 2025 12:19

Remove extra repodata used for testing

08431ca

Refactor parse_match_spec from Vec<VersionSetId> to VersionSetId.

59a99be

Refactor PackageName into enum of PackageNameWithFeature and PackageN…

44c381b

…ameWithoutFeature

Revert "Refactor PackageName into enum of PackageNameWithFeature and …

6b11582

…PackageNameWithoutFeature" This reverts commit 44c381b.

Switch from String to NameType to handle names with features

3031f38

baszalmstra reviewed Jan 10, 2025

View reviewed changes

prsabahrami added 3 commits January 10, 2025 10:59

update resolvo/mod.rs

7e1a48a

Fix conda_package_data to ensure optional_depends is included in the …

f699c64

…lockfile

Updated snapshots

afa26a5

prsabahrami requested a review from baszalmstra January 10, 2025 18:15

baszalmstra reviewed Jan 13, 2025

View reviewed changes

Replace optional_features with SolverMatchSpec::feature

19913c5

baszalmstra reviewed Jan 13, 2025

View reviewed changes

Update the From function to set features as None initially

73e9c46

prsabahrami added 3 commits January 16, 2025 16:54

update the ci to test experimental features as well

68f9b9a

run fmt

ac0a9d7

update the tests

f4d5dd0

wolfv reviewed Jan 17, 2025

View reviewed changes

test-data/channels/dummy-optional-dependencies/noarch/repodata.json Outdated Show resolved Hide resolved

prsabahrami added 3 commits January 17, 2025 12:15

Update the tests

7356302

Update py-rattler/solver.rs

3bde1ab

fix solver.rs and replace Hashmap with Vec

0fa69ea

baszalmstra reviewed Jan 19, 2025

View reviewed changes

.github/workflows/rust-compile.yml Outdated Show resolved Hide resolved

prsabahrami added 5 commits January 22, 2025 00:26

replace optional_features with extras

ef77099

Merge branch 'main' into feature_optional_depends

09b16ae

run fmt

8d051d5

Update rust-compile.yml

0f6a82b

update the extras parser and add tests

e79beb3

baszalmstra reviewed Jan 23, 2025

View reviewed changes

Update mod.rs

7d7b356

Co-authored-by: Bas Zalmstra <[email protected]>

prsabahrami added 12 commits January 23, 2025 10:09

Error out if extras is used with libsolv

28e52ff

Merge branch 'feature_optional_depends' of github.com:prsabahrami/rat…

9e976c9

…tler into feature_optional_depends

rename feature name to experimental_extras

73868cd

run fmt

1436ce0

Update feature flag from extras to experimental_extras in parse.rs

ef7da73

Refactor MatchSpec display to format extras correctly

e7d603c

Refactor optional dependencies to use extra_depends instead of `opt…

342c1f9

…ional_depends`

update the parser and add failing tests

2f129b9

add more versions of foo that provide the extra with-bar

af17aee

update the snapshot

1a2f2d0

remove extra snapshots

7cbcfaa

Merge branch 'main' into feature_optional_depends

b373841

prsabahrami requested a review from baszalmstra January 27, 2025 15:59

baszalmstra merged commit fb92618 into conda:main Jan 27, 2025
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add support for optional dependencies #1019

feat: Add support for optional dependencies #1019

prsabahrami commented Jan 8, 2025

baszalmstra left a comment

tdejager commented Jan 9, 2025

baszalmstra commented Jan 9, 2025

wolfv commented Jan 9, 2025

prsabahrami commented Jan 9, 2025

baszalmstra Jan 13, 2025

prsabahrami Jan 13, 2025

baszalmstra Jan 13, 2025

baszalmstra Jan 13, 2025

baszalmstra left a comment

baszalmstra Jan 23, 2025

baszalmstra Jan 23, 2025

baszalmstra Jan 23, 2025

prsabahrami commented Jan 23, 2025

feat: Add support for optional dependencies #1019

feat: Add support for optional dependencies #1019

Conversation

prsabahrami commented Jan 8, 2025

Description

baszalmstra left a comment

Choose a reason for hiding this comment

tdejager commented Jan 9, 2025

baszalmstra commented Jan 9, 2025

wolfv commented Jan 9, 2025

prsabahrami commented Jan 9, 2025

baszalmstra Jan 13, 2025

Choose a reason for hiding this comment

prsabahrami Jan 13, 2025

Choose a reason for hiding this comment

baszalmstra Jan 13, 2025

Choose a reason for hiding this comment

baszalmstra Jan 13, 2025

Choose a reason for hiding this comment

baszalmstra left a comment

Choose a reason for hiding this comment

baszalmstra Jan 23, 2025

Choose a reason for hiding this comment

baszalmstra Jan 23, 2025

Choose a reason for hiding this comment

baszalmstra Jan 23, 2025

Choose a reason for hiding this comment

prsabahrami commented Jan 23, 2025