Skip to content

Commit

Permalink
Add no-default-feature option
Browse files Browse the repository at this point in the history
  • Loading branch information
olivier-lacroix committed Mar 31, 2024
1 parent 2ac0d54 commit 1e80d92
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 65 deletions.
9 changes: 3 additions & 6 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -539,18 +539,15 @@ platforms = ["linux-64", "osx-arm64"]
### The `environments` table
The `environments` table allows you to define environments that are created using the features defined in the `feature` tables.

!!! important
`default` is always implied when creating environments.
If you don't want to use the `default` feature you can keep all the non feature tables empty.

The environments table is defined using the following fields:

- `features: Vec<Feature>`: The features that are included in the environment set, which is also the default field in the environments.
- `solve-group: String`: The solve group is used to group environments together at the solve stage.
- `features`: The features that are included in the environment, which is also the default field in the environments. Unless `no-default-feature` is set to `false`; the default feature is always included.
- `solve-group`: The solve group is used to group environments together at the solve stage.
This is useful for environments that need to have the same dependencies but might extend them with additional dependencies.
For instance when testing a production environment with additional test dependencies.
These dependencies will then be the same version in all environments that have the same solve group.
But the different environments contain different subsets of the solve-groups dependencies set.
- `no-default-feature`: Whether to include the default feature in that environment. The default is to include the default feature.

```toml title="Simplest example"
[environments]
Expand Down
7 changes: 2 additions & 5 deletions src/cli/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,14 @@ pub async fn execute(args: Args) -> miette::Result<()> {
.iter()
.map(|env| {
let tasks = env
.tasks(None, true)
.tasks(None)
.ok()
.map(|t| t.into_keys().cloned().collect())
.unwrap_or_default();

EnvironmentInfo {
name: env.name().clone(),
features: env
.features(true)
.map(|feature| feature.name.clone())
.collect(),
features: env.features().map(|feature| feature.name.clone()).collect(),
solve_group: env
.solve_group()
.map(|solve_group| solve_group.name().to_string()),
Expand Down
4 changes: 2 additions & 2 deletions src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ fn command_not_found<'p>(project: &'p Project, explicit_environment: Option<Envi
let available_tasks: HashSet<TaskName> =
if let Some(explicit_environment) = explicit_environment {
explicit_environment
.tasks(Some(Platform::current()), true)
.tasks(Some(Platform::current()))
.into_iter()
.flat_map(|tasks| tasks.into_keys())
.map(ToOwned::to_owned)
Expand All @@ -208,7 +208,7 @@ fn command_not_found<'p>(project: &'p Project, explicit_environment: Option<Envi
.into_iter()
.filter(|env| verify_current_platform_has_required_virtual_packages(env).is_ok())
.flat_map(|env| {
env.tasks(Some(Platform::current()), true)
env.tasks(Some(Platform::current()))
.into_iter()
.flat_map(|tasks| tasks.into_keys())
.map(ToOwned::to_owned)
Expand Down
2 changes: 1 addition & 1 deletion src/cli/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ pub fn execute(args: Args) -> miette::Result<()> {
let tasks = project
.environment(&env)
.ok_or(miette!("Environment `{}` not found in project", env))?
.tasks(Some(Platform::current()), true)?
.tasks(Some(Platform::current()))?
.into_keys()
.collect_vec();
if tasks.is_empty() {
Expand Down
38 changes: 15 additions & 23 deletions src/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,8 @@ impl<'p> Environment<'p> {
.join(self.environment.name.as_str())
}

/// Returns references to the features that make up this environment. The default feature is
/// always added at the end.
pub fn features(
&self,
include_default: bool,
) -> impl DoubleEndedIterator<Item = &'p Feature> + 'p {
/// Returns references to the features that make up this environment.
pub fn features(&self) -> impl DoubleEndedIterator<Item = &'p Feature> + 'p {
let environment_features = self.environment.features.iter().map(|feature_name| {
self.project
.manifest
Expand All @@ -121,10 +117,10 @@ impl<'p> Environment<'p> {
.expect("feature usage should have been validated upfront")
});

if include_default {
Either::Left(environment_features.chain([self.project.manifest.default_feature()]))
} else {
if self.environment.no_default_feature {
Either::Right(environment_features)
} else {
Either::Left(environment_features.chain([self.project.manifest.default_feature()]))
}
}

Expand All @@ -138,7 +134,7 @@ impl<'p> Environment<'p> {
/// used instead. However, these are not considered during deduplication. This means the default
/// channels are always added to the end of the list.
pub fn channels(&self) -> IndexSet<&'p Channel> {
self.features(true)
self.features()
.filter_map(|feature| match feature.name {
// Use the user-specified channels of each feature if the feature defines them. Only
// for the default feature do we use the default channels from the project metadata
Expand Down Expand Up @@ -171,7 +167,7 @@ impl<'p> Environment<'p> {
/// Features can specify which platforms they support through the `platforms` key. If a feature
/// does not specify any platforms the features defined by the project are used.
pub fn platforms(&self) -> HashSet<Platform> {
self.features(true)
self.features()
.map(|feature| {
match &feature.platforms {
Some(platforms) => &platforms.value,
Expand All @@ -196,11 +192,10 @@ impl<'p> Environment<'p> {
pub fn tasks(
&self,
platform: Option<Platform>,
include_default: bool,
) -> Result<HashMap<&'p TaskName, &'p Task>, UnsupportedPlatformError> {
self.validate_platform_support(platform)?;
let result = self
.features(include_default)
.features()
.flat_map(|feature| feature.targets.resolve(platform))
.rev() // Reverse to get the most specific targets last.
.flat_map(|target| target.tasks.iter())
Expand All @@ -215,10 +210,7 @@ impl<'p> Environment<'p> {
name: &TaskName,
platform: Option<Platform>,
) -> Result<&'p Task, UnknownTask> {
match self
.tasks(platform, true)
.map(|tasks| tasks.get(name).copied())
{
match self.tasks(platform).map(|tasks| tasks.get(name).copied()) {
Err(_) | Ok(None) => Err(UnknownTask {
project: self.project,
environment: self.name().clone(),
Expand Down Expand Up @@ -259,7 +251,7 @@ impl<'p> Environment<'p> {
/// the features that make up the environment. If multiple features specify a requirement for
/// the same system package, the highest is chosen.
pub fn local_system_requirements(&self) -> SystemRequirements {
self.features(true)
self.features()
.map(|feature| &feature.system_requirements)
.fold(SystemRequirements::default(), |acc, req| {
acc.union(req)
Expand All @@ -273,7 +265,7 @@ impl<'p> Environment<'p> {
/// requirement for the same package that both requirements are returned. The different
/// requirements per package are sorted in the same order as the features they came from.
pub fn dependencies(&self, kind: Option<SpecType>, platform: Option<Platform>) -> Dependencies {
self.features(true)
self.features()
.filter_map(|f| f.dependencies(kind, platform))
.map(|deps| Dependencies::from(deps.into_owned()))
.reduce(|acc, deps| acc.union(&deps))
Expand All @@ -289,7 +281,7 @@ impl<'p> Environment<'p> {
&self,
platform: Option<Platform>,
) -> IndexMap<PyPiPackageName, Vec<PyPiRequirement>> {
self.features(true)
self.features()
.filter_map(|f| f.pypi_dependencies(platform))
.fold(IndexMap::default(), |mut acc, deps| {
// Either clone the values from the Cow or move the values from the owned map.
Expand All @@ -316,7 +308,7 @@ impl<'p> Environment<'p> {
/// The activation scripts of all features are combined in the order they are defined for the
/// environment.
pub fn activation_scripts(&self, platform: Option<Platform>) -> Vec<String> {
self.features(true)
self.features()
.filter_map(|f| f.activation_scripts(platform))
.flatten()
.cloned()
Expand All @@ -343,7 +335,7 @@ impl<'p> Environment<'p> {

/// Returns true if the environments contains any reference to a pypi dependency.
pub fn has_pypi_dependencies(&self) -> bool {
self.features(true).any(|f| f.has_pypi_dependencies())
self.features().any(|f| f.has_pypi_dependencies())
}
}

Expand Down Expand Up @@ -449,7 +441,7 @@ mod tests {

assert!(manifest
.default_environment()
.tasks(Some(Platform::Osx64), true)
.tasks(Some(Platform::Osx64))
.is_err())
}

Expand Down
4 changes: 2 additions & 2 deletions src/project/grouped_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ impl<'p> GroupedEnvironment<'p> {
/// Returns the features of the group
pub fn features(&self) -> impl DoubleEndedIterator<Item = &'p Feature> + 'p {
match self {
GroupedEnvironment::Group(group) => Either::Left(group.features(true)),
GroupedEnvironment::Environment(env) => Either::Right(env.features(true)),
GroupedEnvironment::Group(group) => Either::Left(group.features()),
GroupedEnvironment::Environment(env) => Either::Right(env.features()),
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/project/manifest/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ pub struct Environment {
/// An optional solver-group. Multiple environments can share the same solve-group. All the
/// dependencies of the environment that share the same solve-group will be solved together.
pub solve_group: Option<usize>,

/// Wether to include the default feature automatically or not
pub no_default_feature: bool,
}

/// Helper struct to deserialize the environment from TOML.
Expand All @@ -134,6 +137,8 @@ pub(super) struct TomlEnvironment {
#[serde(default)]
pub features: PixiSpanned<Vec<String>>,
pub solve_group: Option<String>,
#[serde(default)]
pub no_default_feature: bool,
}

pub(super) enum TomlEnvironmentMapOrSeq {
Expand Down
15 changes: 10 additions & 5 deletions src/project/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,7 @@ impl<'de> Deserialize<'de> for ProjectManifest {
features: Vec::new(),
features_source_loc: None,
solve_group: None,
no_default_feature: false,
});
environments.by_name.insert(EnvironmentName::Default, 0);
}
Expand All @@ -1033,11 +1034,14 @@ impl<'de> Deserialize<'de> for ProjectManifest {
environments.by_name.insert(name.clone(), environment_idx);

// Decompose the TOML
let (features, features_source_loc, solve_group) = match env {
TomlEnvironmentMapOrSeq::Map(env) => {
(env.features.value, env.features.span, env.solve_group)
}
TomlEnvironmentMapOrSeq::Seq(features) => (features, None, None),
let (features, features_source_loc, solve_group, no_default_feature) = match env {
TomlEnvironmentMapOrSeq::Map(env) => (
env.features.value,
env.features.span,
env.solve_group,
env.no_default_feature,
),
TomlEnvironmentMapOrSeq::Seq(features) => (features, None, None, false),
};

// Add to the solve group if defined
Expand Down Expand Up @@ -1068,6 +1072,7 @@ impl<'de> Deserialize<'de> for ProjectManifest {
features,
features_source_loc,
solve_group,
no_default_feature,
});
}

Expand Down
2 changes: 1 addition & 1 deletion src/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ impl Project {
/// TODO: Remove this function and use the tasks from the default environment instead.
pub fn tasks(&self, platform: Option<Platform>) -> HashMap<&TaskName, &Task> {
self.default_environment()
.tasks(platform, true)
.tasks(platform)
.unwrap_or_default()
}

Expand Down
19 changes: 7 additions & 12 deletions src/project/solve_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,10 @@ impl<'p> SolveGroup<'p> {

/// Returns all features that are part of the solve group.
///
/// If `include_default` is `true` the default feature is also included.
///
/// All features of all environments are combined and deduplicated.
pub fn features(
&self,
include_default: bool,
) -> impl DoubleEndedIterator<Item = &'p manifest::Feature> + 'p {
pub fn features(&self) -> impl DoubleEndedIterator<Item = &'p manifest::Feature> + 'p {
self.environments()
.flat_map(move |env| env.features(include_default))
.flat_map(move |env| env.features())
.unique_by(|feat| &feat.name)
}

Expand All @@ -85,7 +80,7 @@ impl<'p> SolveGroup<'p> {
/// the environments that share the same solve group. If multiple environments specify a
/// requirement for the same system package, the highest is chosen.
pub fn system_requirements(&self) -> SystemRequirements {
self.features(true)
self.features()
.map(|feature| &feature.system_requirements)
.fold(SystemRequirements::default(), |acc, req| {
acc.union(req)
Expand All @@ -100,7 +95,7 @@ impl<'p> SolveGroup<'p> {
/// different requirements per package are sorted in the same order as the features they came
/// from.
pub fn dependencies(&self, kind: Option<SpecType>, platform: Option<Platform>) -> Dependencies {
self.features(true)
self.features()
.filter_map(|feat| feat.dependencies(kind, platform))
.map(|deps| Dependencies::from(deps.into_owned()))
.reduce(|acc, deps| acc.union(&deps))
Expand All @@ -117,7 +112,7 @@ impl<'p> SolveGroup<'p> {
&self,
platform: Option<Platform>,
) -> IndexMap<PyPiPackageName, Vec<PyPiRequirement>> {
self.features(true)
self.features()
.filter_map(|f| f.pypi_dependencies(platform))
.fold(IndexMap::default(), |mut acc, deps| {
// Either clone the values from the Cow or move the values from the owned map.
Expand Down Expand Up @@ -149,7 +144,7 @@ impl<'p> SolveGroup<'p> {
/// used instead. However, these are not considered during deduplication. This means the default
/// channels are always added to the end of the list.
pub fn channels(&self) -> IndexSet<&'p Channel> {
self.features(true)
self.features()
.filter_map(|feature| match feature.name {
// Use the user-specified channels of each feature if the feature defines them. Only
// for the default feature do we use the default channels from the project metadata
Expand All @@ -175,7 +170,7 @@ impl<'p> SolveGroup<'p> {

/// Returns true if any of the environments contain a feature with any reference to a pypi dependency.
pub fn has_pypi_dependencies(&self) -> bool {
self.features(true).any(|f| f.has_pypi_dependencies())
self.features().any(|f| f.has_pypi_dependencies())
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/task/task_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,10 @@ impl<'p, D: TaskDisambiguation<'p>> SearchEnvironments<'p, D> {
};

// Find all the task and environment combinations
let include_default_feature = true;
let mut tasks = Vec::new();
for env in environments.iter() {
if let Some(task) = env
.tasks(self.platform, include_default_feature)
.tasks(self.platform)
.ok()
.and_then(|tasks| tasks.get(&name).copied())
{
Expand Down
12 changes: 6 additions & 6 deletions tests/task_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub async fn add_remove_task() {
.unwrap();

let project = pixi.project().unwrap();
let tasks = project.default_environment().tasks(None, true).unwrap();
let tasks = project.default_environment().tasks(None).unwrap();
let task = tasks.get(&<TaskName>::from("test")).unwrap();
assert!(matches!(task, Task::Plain(s) if s == "echo hello"));

Expand All @@ -35,7 +35,7 @@ pub async fn add_remove_task() {
pixi.project()
.unwrap()
.default_environment()
.tasks(None, true)
.tasks(None)
.unwrap()
.len(),
0
Expand All @@ -61,7 +61,7 @@ pub async fn add_command_types() {
.unwrap();

let project = pixi.project().unwrap();
let tasks = project.default_environment().tasks(None, true).unwrap();
let tasks = project.default_environment().tasks(None).unwrap();
let task2 = tasks.get(&<TaskName>::from("test2")).unwrap();
let task = tasks.get(&<TaskName>::from("test")).unwrap();
assert!(matches!(task2, Task::Execute(cmd) if matches!(cmd.cmd, CmdArgs::Single(_))));
Expand All @@ -80,7 +80,7 @@ pub async fn add_command_types() {
.execute()
.unwrap();
let project = pixi.project().unwrap();
let tasks = project.default_environment().tasks(None, true).unwrap();
let tasks = project.default_environment().tasks(None).unwrap();
let task = tasks.get(&<TaskName>::from("testing")).unwrap();
assert!(matches!(task, Task::Alias(a) if a.depends_on.first().unwrap().as_str() == "test"));
}
Expand Down Expand Up @@ -138,7 +138,7 @@ pub async fn add_remove_target_specific_task() {
let project = pixi.project().unwrap();
let task = *project
.default_environment()
.tasks(Some(Platform::Win64), true)
.tasks(Some(Platform::Win64))
.unwrap()
.get(&<TaskName>::from("test"))
.unwrap();
Expand All @@ -159,7 +159,7 @@ pub async fn add_remove_target_specific_task() {
assert_eq!(
project
.default_environment()
.tasks(Some(Platform::Win64), true)
.tasks(Some(Platform::Win64))
.unwrap()
.len(),
// The default task is still there
Expand Down

0 comments on commit 1e80d92

Please sign in to comment.