Skip to content

Conversation

@wolfv
Copy link
Contributor

@wolfv wolfv commented Jul 22, 2025

This PR implements parsing for conditions in MatchSpecs, following the syntax described in conda/ceps#111

Conditions can be used to enable or disable dependencies under certain conditions (such as other packages or virtual packages in the same environment).

They are written in the following way:

  • numpy; if python >=3.12
  • pywin32; if __win
  • foobar; if __unix and python >=3.12
  • complex; if (a or b) and (b and c)

Conditions are only allowed in run dependencies, and not in constraints. Nesting conditions is also not allowed (e.g. foobar; if __win; if __python >=3.12 is invalid).

@wolfv wolfv changed the title add proper condition parsing and proper translation to resolvo feat: parse matchspec conditions and translate to resolvo Jul 23, 2025
@wolfv wolfv requested a review from Copilot July 23, 2025 09:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements parsing and translation of conditional dependencies in MatchSpecs according to the conda CEP specification. It enables dependencies to be conditionally included based on conditions like Python version or virtual packages using syntax such as numpy; if python >=3.12 or foobar; if __unix.

  • Adds parsing support for condition syntax in MatchSpecs with logical operators (and, or) and parentheses
  • Implements translation from parsed conditions to resolvo's conditional requirement system
  • Updates dependency processing to handle conditional requirements throughout the solver pipeline

Reviewed Changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
crates/rattler_conda_types/src/match_spec/condition.rs New module implementing condition parsing with logical operators and MatchSpec evaluation
crates/rattler_conda_types/src/match_spec/parse.rs Updated parser to extract and parse condition clauses from MatchSpec strings
crates/rattler_conda_types/src/match_spec/mod.rs Added condition field to MatchSpec struct and Display implementation
crates/rattler_solve/src/resolvo/mod.rs Modified dependency provider to translate conditions and handle conditional requirements
crates/rattler_solve/tests/backends.rs Added comprehensive test for conditional dependency resolution
crates/rattler_pty/src/unix/pty_session.rs Removed continue statement in error handling loop
Cargo.toml Updated resolvo dependency to development branch with condition support

@wolfv wolfv force-pushed the resolvo-condition branch from 6f0db89 to 8f57c26 Compare July 23, 2025 09:57

/// Represents a condition in a match spec, which can be a match spec itself or a logical combination
#[derive(Debug, Clone, PartialEq, Serialize, Eq, Hash)]
pub enum MatchSpecCondition {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One slight issue I have with this approach is that some information is lost when converting from a string. E.g. converting back to a string loses the order and parenthesis.

Im wondering if we can structure this differently or add a wrapping struct that holds the original statement or something along those lines. Ideally we build a concrete syntax tree.

The reason this is important is that when we report the conditions in an unsat error you want the information to closely resemble the users input.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually never parse the condition back to a string so this is not true.

@wolfv
Copy link
Contributor Author

wolfv commented Aug 13, 2025

@baszalmstra actually we have a few corner cases with our proposed syntax:

            "deep; if foobar >=1.23 *or* and(b >32.12,<=43 *and or(c and d))",

Or in other words, we cannot use and or or in build strings anymore.

Maybe it would be better to go for a "C-like" syntax, using && and || as they are not found and hopefully forbidden in build strings? Looks slightly uglier though!

Copy link
Collaborator

@baszalmstra baszalmstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the liberty to change the code a little bit. I would be happy to merge this now!


/// Represents a condition in a match spec, which can be a match spec itself or a logical combination
#[derive(Debug, Clone, PartialEq, Serialize, Eq, Hash)]
pub enum MatchSpecCondition {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We actually never parse the condition back to a string so this is not true.

#[cfg(feature = "conditional_dependencies")]
pub condition: Option<MatchSpecCondition>,
/// The condition under which this match spec applies (disabled without conditional_dependencies feature).
#[cfg(not(feature = "conditional_dependencies"))]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to keep this field here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I removed it.

@wolfv
Copy link
Contributor Author

wolfv commented Oct 31, 2025

I updated Cargo.lock and merged main :)

@wolfv wolfv enabled auto-merge (squash) October 31, 2025 09:23
@wolfv wolfv disabled auto-merge November 6, 2025 06:35
@wolfv wolfv merged commit 6c9f03d into conda:main Nov 6, 2025
16 of 17 checks passed
@wolfv wolfv deleted the resolvo-condition branch November 6, 2025 06:35
@github-actions github-actions bot mentioned this pull request Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants