Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: channel priority logic #1332

Merged
merged 3 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 98 additions & 2 deletions src/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,6 @@ mod tests {
);
}

// TODO: Add a test to verify that feature specific channels work as expected.

#[test]
fn test_default_platforms() {
let manifest = Project::from_str(
Expand Down Expand Up @@ -490,6 +488,104 @@ mod tests {
);
}

#[test]
fn test_channel_feature_priority() {
let manifest = Project::from_str(
Path::new("pixi.toml"),
r#"
[project]
name = "foobar"
channels = ["a", "b"]
platforms = ["linux-64", "osx-64"]

[feature.foo]
channels = ["c", "d"]

[feature.bar]
channels = ["e", "f"]

[feature.barfoo]
channels = ["a", "f"]

[environments]
foo = ["foo"]
foobar = ["foo", "bar"]
barfoo = {features = ["barfoo"], no-default-feature=true}
"#,
)
.unwrap();

// All channels are added in order of the features and default is last
let foobar_channels = manifest.environment("foobar").unwrap().channels();
assert_eq!(
foobar_channels
.into_iter()
.map(|c| c.name.clone().unwrap())
.collect_vec(),
vec!["c", "d", "e", "f", "a", "b"]
);

let foo_channels = manifest.environment("foo").unwrap().channels();
assert_eq!(
foo_channels
.into_iter()
.map(|c| c.name.clone().unwrap())
.collect_vec(),
vec!["c", "d", "a", "b"]
);

// The default feature is not included in the channels, so only the feature channels are included.
let barfoo_channels = manifest.environment("barfoo").unwrap().channels();
assert_eq!(
barfoo_channels
.into_iter()
.map(|c| c.name.clone().unwrap())
.collect_vec(),
vec!["a", "f"]
)
}

#[test]
fn test_channel_feature_priority_with_redefinition() {
let manifest = Project::from_str(
Path::new("pixi.toml"),
r#"
[project]
name = "test"
channels = ["d", "a", "b"]
platforms = ["linux-64"]

[environments]
foo = ["foo"]

[feature.foo]
channels = ["a", "c", "b"]

"#,
)
.unwrap();

let foobar_channels = manifest.environment("default").unwrap().channels();
assert_eq!(
foobar_channels
.into_iter()
.map(|c| c.name.clone().unwrap())
.collect_vec(),
vec!["d", "a", "b"]
);

// Check if the feature channels are sorted correctly,
// and that the remaining channels from the default feature are appended.
let foo_channels = manifest.environment("foo").unwrap().channels();
assert_eq!(
foo_channels
.into_iter()
.map(|c| c.name.clone().unwrap())
.collect_vec(),
vec!["a", "c", "b", "d"]
);
}

#[test]
fn test_channel_priorities() {
let manifest = Project::from_str(
Expand Down
8 changes: 3 additions & 5 deletions src/project/has_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,15 @@ pub trait HasFeatures<'p> {
/// If a feature does not specify any channel the default channels from the project metadata are
/// used instead.
fn channels(&self) -> IndexSet<&'p Channel> {
// We reverse once before collecting into an IndexSet, and once after,
// to ensure the default channels of the project are added to the end of the list.
let mut channels: IndexSet<_> = self
// Collect all the channels from the features in one set,
// deduplicate them and sort them on feature index, default feature comes last.
let channels: IndexSet<_> = self
.features()
.flat_map(|feature| match &feature.channels {
Some(channels) => channels,
None => &self.project().manifest.parsed.project.channels,
})
.rev()
.collect();
channels.reverse();

// The prioritized channels contain a priority, sort on this priority.
// Higher priority comes first. [-10, 1, 0 ,2] -> [2, 1, 0, -10]
Expand Down
Loading