Skip to content

Commit

Permalink
feat: add --prepend option for pixi project channel add (#2447)
Browse files Browse the repository at this point in the history
trying to fix #2349 

### Example:

If the current `pixi.toml` has:

```
[project]
channels = ["conda-forge"]
```

After running `pixi project channel add pytorch --prepend` , the
channels list will become:

```
[project]
channels = ["pytorch", "conda-forge"]
```

unit test regarding the above functionality is passing:
![Screenshot from 2024-11-10
02-46-18](https://github.com/user-attachments/assets/30973f40-6a2e-471b-abea-6e13195f99d0)


this is my first time trying to contribute to pixi, and I am not that
good in Rust too, so any kind of help will be very much appreciated. I
will be happy to make requested code changes for this PR.
  • Loading branch information
mrswastik-robot authored Nov 11, 2024
1 parent 5ab7fac commit 689cf9d
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 20 deletions.
99 changes: 82 additions & 17 deletions crates/pixi_manifest/src/manifests/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,13 +523,17 @@ impl Manifest {
&mut self,
channels: impl IntoIterator<Item = PrioritizedChannel>,
feature_name: &FeatureName,
prepend: bool,
) -> miette::Result<()> {
// Get current and new platforms for the feature
// First collect all the new channels
let to_add: IndexSet<_> = channels.into_iter().collect();

// Get the current channels and update them
let current = match feature_name {
FeatureName::Default => &mut self.parsed.project.channels,
FeatureName::Named(_) => self.get_or_insert_feature_mut(feature_name).channels_mut(),
};
let to_add: IndexSet<_> = channels.into_iter().collect();

let new: IndexSet<_> = to_add.difference(current).cloned().collect();
let new_channels: IndexSet<_> = new
.clone()
Expand All @@ -540,16 +544,25 @@ impl Manifest {
// clear channels with modified priority
current.retain(|c| !new_channels.contains(&c.channel));

// Add the updated channels to the manifest
current.extend(new.clone());
let current_clone = current.clone();
// Create the final channel list in the desired order
let final_channels = if prepend {
let mut new_set = new.clone();
new_set.extend(current.iter().cloned());
new_set
} else {
let mut new_set = current.clone();
new_set.extend(new.clone());
new_set
};

// Then to the TOML document
// Update both the parsed channels and the TOML document
*current = final_channels.clone();

// Update the TOML document
let channels = self.document.get_array_mut("channels", feature_name)?;
// clear and recreate from current list
channels.clear();
for channel in current_clone.iter() {
channels.push(Value::from(channel.clone()));
for channel in final_channels {
channels.push(Value::from(channel));
}

Ok(())
Expand Down Expand Up @@ -1424,7 +1437,7 @@ platforms = ["linux-64", "win-64"]
[dependencies]
[feature.test.dependencies]
"#;
"#;

let mut manifest = Manifest::from_str(Path::new("pixi.toml"), file_contents).unwrap();

Expand All @@ -1433,13 +1446,13 @@ platforms = ["linux-64", "win-64"]
let conda_forge =
PrioritizedChannel::from(NamedChannelOrUrl::Name(String::from("conda-forge")));
manifest
.add_channels([conda_forge.clone()], &FeatureName::Default)
.add_channels([conda_forge.clone()], &FeatureName::Default, false)
.unwrap();

let cuda_feature = FeatureName::Named("cuda".to_string());
let nvidia = PrioritizedChannel::from(NamedChannelOrUrl::Name(String::from("nvidia")));
manifest
.add_channels([nvidia.clone()], &cuda_feature)
.add_channels([nvidia.clone()], &cuda_feature, false)
.unwrap();

let test_feature = FeatureName::Named("test".to_string());
Expand All @@ -1450,6 +1463,7 @@ platforms = ["linux-64", "win-64"]
PrioritizedChannel::from(NamedChannelOrUrl::Name(String::from("test2"))),
],
&test_feature,
false,
)
.unwrap();

Expand All @@ -1465,7 +1479,7 @@ platforms = ["linux-64", "win-64"]

// Try to add again, should not add more channels
manifest
.add_channels([conda_forge.clone()], &FeatureName::Default)
.add_channels([conda_forge.clone()], &FeatureName::Default, false)
.unwrap();

assert_eq!(
Expand Down Expand Up @@ -1494,10 +1508,12 @@ platforms = ["linux-64", "win-64"]
.into_iter()
.collect::<IndexSet<_>>()
);

// Try to add again, should not add more channels
manifest
.add_channels([nvidia.clone()], &cuda_feature)
.add_channels([nvidia.clone()], &cuda_feature, false)
.unwrap();

assert_eq!(
manifest
.parsed
Expand Down Expand Up @@ -1544,8 +1560,9 @@ platforms = ["linux-64", "win-64"]
priority: None,
};
manifest
.add_channels([custom_channel.clone()], &FeatureName::Default)
.add_channels([custom_channel.clone()], &FeatureName::Default, false)
.unwrap();

assert!(manifest
.parsed
.project
Expand All @@ -1559,8 +1576,9 @@ platforms = ["linux-64", "win-64"]
priority: Some(12i32),
};
manifest
.add_channels([prioritized_channel1.clone()], &FeatureName::Default)
.add_channels([prioritized_channel1.clone()], &FeatureName::Default, false)
.unwrap();

assert!(manifest
.parsed
.project
Expand All @@ -1573,8 +1591,9 @@ platforms = ["linux-64", "win-64"]
priority: Some(-12i32),
};
manifest
.add_channels([prioritized_channel2.clone()], &FeatureName::Default)
.add_channels([prioritized_channel2.clone()], &FeatureName::Default, false)
.unwrap();

assert!(manifest
.parsed
.project
Expand Down Expand Up @@ -2257,4 +2276,50 @@ bar = "*"
}
}
}

#[test]
fn test_prepend_channels() {
let contents = r#"
[project]
name = "foo"
channels = ["conda-forge"]
platforms = []
"#;
let mut manifest = Manifest::from_str(Path::new("pixi.toml"), contents).unwrap();

// Add pytorch channel with prepend=true
let pytorch = PrioritizedChannel::from(NamedChannelOrUrl::Name(String::from("pytorch")));
manifest
.add_channels([pytorch.clone()], &FeatureName::Default, true)
.unwrap();

// Verify pytorch is first in the list
assert_eq!(
manifest
.parsed
.project
.channels
.iter()
.next()
.unwrap()
.channel,
pytorch.channel
);

// Add another channel without prepend
let bioconda = PrioritizedChannel::from(NamedChannelOrUrl::Name(String::from("bioconda")));
manifest
.add_channels([bioconda.clone()], &FeatureName::Default, false)
.unwrap();

// Verify order is still pytorch, conda-forge, bioconda
let channels: Vec<_> = manifest
.parsed
.project
.channels
.iter()
.map(|c| c.channel.to_string())
.collect();
assert_eq!(channels, vec!["pytorch", "conda-forge", "bioconda"]);
}
}
2 changes: 2 additions & 0 deletions docs/reference/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,7 @@ When you add channels, the channels are tested for existence, added to the lock

- `--no-install`: do not update the environment, only add changed packages to the lock-file.
- `--feature <FEATURE> (-f)`: The feature for which the channel is added.
- `--prepend`: Prepend the channel to the list of channels.

```
pixi project channel add robostack
Expand All @@ -1218,6 +1219,7 @@ pixi project channel add file:///home/user/local_channel
pixi project channel add https://repo.prefix.dev/conda-forge
pixi project channel add --no-install robostack
pixi project channel add --feature cuda nvidia
pixi project channel add --prepend pytorch
```

### `project channel list`
Expand Down
8 changes: 5 additions & 3 deletions src/cli/project/channel/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ use super::AddRemoveArgs;

pub async fn execute(mut project: Project, args: AddRemoveArgs) -> miette::Result<()> {
// Add the channels to the manifest
project
.manifest
.add_channels(args.prioritized_channels(), &args.feature_name())?;
project.manifest.add_channels(
args.prioritized_channels(),
&args.feature_name(),
args.prepend,
)?;

// TODO: Update all environments touched by the features defined.
update_prefix(
Expand Down
4 changes: 4 additions & 0 deletions src/cli/project/channel/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ pub struct AddRemoveArgs {
#[clap(long, num_args = 1)]
pub priority: Option<i32>,

/// Add the channel(s) to the beginning of the channels list, making them highest priority
#[clap(long)]
pub prepend: bool,

/// Don't update the environment, only modify the manifest and the
/// lock-file.
#[clap(long)]
Expand Down
2 changes: 2 additions & 0 deletions tests/integration_rust/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,7 @@ impl PixiControl {
no_install: true,
feature: None,
priority: None,
prepend: false,
},
}
}
Expand All @@ -373,6 +374,7 @@ impl PixiControl {
no_install: true,
feature: None,
priority: None,
prepend: false,
},
}
}
Expand Down

0 comments on commit 689cf9d

Please sign in to comment.