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

feat: add --priority arg to project channel add #2086

Merged
merged 10 commits into from
Sep 23, 2024
10 changes: 10 additions & 0 deletions crates/pixi_manifest/src/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@
}
}

impl From<(NamedChannelOrUrl, Option<i32>)> for PrioritizedChannel {
fn from((value, priority): (NamedChannelOrUrl, Option<i32>)) -> Self {
Self {
channel: value,
priority: priority,

Check failure on line 30 in crates/pixi_manifest/src/channel.rs

View workflow job for this annotation

GitHub Actions / Cargo Lint

redundant field names in struct initialization
}
}

Check warning on line 32 in crates/pixi_manifest/src/channel.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/channel.rs
}


pub enum TomlPrioritizedChannelStrOrMap {
Map(PrioritizedChannel),
Str(NamedChannelOrUrl),
Expand Down
66 changes: 58 additions & 8 deletions crates/pixi_manifest/src/manifests/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@
};

use indexmap::{Equivalent, IndexSet};
use itertools::Itertools;

Check warning on line 12 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
use miette::{miette, IntoDiagnostic, NamedSource, WrapErr};
use pixi_spec::PixiSpec;
use rattler_conda_types::{ChannelConfig, MatchSpec, PackageName, Platform, Version};
use toml_edit::DocumentMut;
use rattler_conda_types::{ChannelConfig, MatchSpec, PackageName, Platform, Version
};
use serde::Deserialize;
use toml_edit::{DocumentMut, Table, Value};

use crate::{

Check warning on line 20 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
consts,
channel::TomlPrioritizedChannelStrOrMap,
error::{DependencyError, TomlError, UnknownFeature},
manifests::{ManifestSource, TomlManifest},
pypi::PyPiPackageName,
Expand Down Expand Up @@ -528,17 +531,55 @@
let current = match feature_name {
FeatureName::Default => &mut self.parsed.project.channels,
FeatureName::Named(_) => self.get_or_insert_feature_mut(feature_name).channels_mut(),
};

Check warning on line 534 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
let to_add: IndexSet<_> = channels.into_iter().collect();
let new: IndexSet<_> = to_add.difference(current).cloned().collect();
let new_channels: IndexSet<_> = new.clone().into_iter().map(|channel| channel.channel).collect();

// clear channels with modified priority
current.retain(|c| ! new_channels.contains(&c.channel));

// Add the channels to the manifest
// Add the updated channels to the manifest
current.extend(new.clone());

// Then to the TOML document
let channels = self.document.get_array_mut("channels", feature_name)?;

for channel in new.iter() {

Check warning on line 548 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
channels.push(channel.channel.to_string())
// lookup index for changed priority and update in-place
// is there an easier lookup for deserialize to use here?
let index = channels.iter().map(
|value| {
let deserializer = value.to_string().trim().parse::<toml_edit::de::ValueDeserializer>().unwrap();
TomlPrioritizedChannelStrOrMap::deserialize(deserializer)
}
).position(
|c| match c {
Ok(c) => c.into_prioritized_channel().channel == channel.channel,
Err(_) => false,
}
);

// push if not found, remove & insert if found
let mut push = |item: Value| {
if index.is_some() {
channels.remove(index.unwrap());

Check failure on line 566 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Lint

called `unwrap` on `index` after checking its variant with `is_some`
channels.insert(index.unwrap(), item);

Check failure on line 567 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Lint

called `unwrap` on `index` after checking its variant with `is_some`
} else {
channels.push(item);
}
};
match channel.priority {
Some(priority) => {
let mut table = Table::new().into_inline_table();
table.insert("channel", channel.channel.to_string().into());

Check warning on line 575 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
table.insert("priority", i64::from(priority).into());
push(table.into());
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of you could implement the toml_edit::Value for this PrioritizedChannel. Which then might be reused in different places.

},
None => {
push(channel.channel.to_string().into());
},
}
}

Ok(())
Expand All @@ -564,19 +605,28 @@
.iter()
.position(|x| x.channel == c.channel)
.ok_or_else(|| miette::miette!("channel {} does not exist", c.channel.as_str()))
.map(|_| c)
.map(|_| c.channel)
})

Check warning on line 609 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
.collect::<Result<_, _>>()?;

let retained: IndexSet<_> = current.difference(&to_remove).cloned().collect();
let retained: IndexSet<_> = current.iter().filter(
|channel| ! to_remove.contains(&channel.channel)).cloned().collect();

// Remove channels from the manifest
current.retain(|c| retained.contains(c));

Check warning on line 617 in crates/pixi_manifest/src/manifests/manifest.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/crates/pixi_manifest/src/manifests/manifest.rs
// And from the TOML document
let retained = retained.iter().map(|c| c.channel.as_str()).collect_vec();
let channels = self.document.get_array_mut("channels", feature_name)?;
channels.retain(|x| retained.contains(&x.as_str().unwrap()));
channels.retain(
|value| {
let deserializer = value.to_string().trim().parse::<toml_edit::de::ValueDeserializer>().unwrap();
let parsed = TomlPrioritizedChannelStrOrMap::deserialize(deserializer);
match parsed {
Ok(c) => retained.contains(&c.into_prioritized_channel()),
Err(_) => false,
}
}
);

Ok(())
}
Expand Down
6 changes: 5 additions & 1 deletion src/cli/project/channel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
pub struct AddRemoveArgs {
/// The channel name or URL
#[clap(required = true, num_args=1..)]
pub channel: Vec<NamedChannelOrUrl>,

Check warning on line 27 in src/cli/project/channel/mod.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/src/cli/project/channel/mod.rs

/// Specify the channel priority
#[clap(long, num_args=1)]
pub priority: Option<i32>,

/// Don't update the environment, only modify the manifest and the
/// lock-file.
#[clap(long)]
Expand All @@ -35,10 +39,10 @@
#[clap(long, short)]
pub feature: Option<String>,
}

Check warning on line 42 in src/cli/project/channel/mod.rs

View workflow job for this annotation

GitHub Actions / Cargo Format

Diff in /home/runner/work/pixi/pixi/src/cli/project/channel/mod.rs
impl AddRemoveArgs {
fn prioritized_channels(&self) -> impl IntoIterator<Item = PrioritizedChannel> + '_ {
self.channel.iter().cloned().map(PrioritizedChannel::from)
self.channel.iter().cloned().map(|channel| PrioritizedChannel::from((channel, self.priority)))
}

fn feature_name(&self) -> FeatureName {
Expand Down
Loading