From 068ebd12a4c27e0f5668fd6a9bf5c0cd3917d720 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 15 Dec 2023 15:08:45 +0100 Subject: [PATCH 1/5] refactor: move manifest specifics to the manifest.rs file under Manifest struct --- src/cli/add.rs | 26 +- src/cli/info.rs | 1 + src/cli/project/channel/add.rs | 4 +- src/cli/remove.rs | 6 +- src/cli/run.rs | 1 + src/cli/task.rs | 13 +- src/environment.rs | 4 +- src/lock_file/python.rs | 2 +- src/project/manifest.rs | 729 +++++++++++- src/project/mod.rs | 1021 +++-------------- ...__manifest__test__remove_dependencies.snap | 66 ++ ...st__test__remove_target_dependencies.snap} | 4 +- src/task/executable_task.rs | 13 +- src/virtual_packages.rs | 2 +- tests/add_tests.rs | 13 +- tests/common/mod.rs | 4 +- tests/task_tests.rs | 21 +- 17 files changed, 1005 insertions(+), 925 deletions(-) create mode 100644 src/project/snapshots/pixi__project__manifest__test__remove_dependencies.snap rename src/project/snapshots/{pixi__project__tests__remove_target_dependencies.snap => pixi__project__manifest__test__remove_target_dependencies.snap} (97%) diff --git a/src/cli/add.rs b/src/cli/add.rs index 90e16aa56..a960fc5a5 100644 --- a/src/cli/add.rs +++ b/src/cli/add.rs @@ -1,6 +1,5 @@ use crate::environment::{update_prefix, verify_prefix_location_unchanged}; use crate::prefix::Prefix; -use crate::project::DependencyType::CondaDependency; use crate::project::{DependencyType, SpecType}; use crate::{ consts, @@ -94,11 +93,11 @@ impl DependencyType { if args.pypi { Self::PypiDependency } else if args.host { - CondaDependency(SpecType::Host) + DependencyType::CondaDependency(SpecType::Host) } else if args.build { - CondaDependency(SpecType::Build) + DependencyType::CondaDependency(SpecType::Build) } else { - CondaDependency(SpecType::Run) + DependencyType::CondaDependency(SpecType::Run) } } } @@ -122,7 +121,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { .filter(|p| !project.platforms().contains(p)) .cloned() .collect::>(); - project.add_platforms(platforms_to_add.iter())?; + project.manifest.add_platforms(platforms_to_add.iter())?; match dependency_type { DependencyType::CondaDependency(spec_type) => { @@ -183,7 +182,10 @@ pub async fn execute(args: Args) -> miette::Result<()> { } // Print if it is something different from host and dep - if !matches!(dependency_type, CondaDependency(SpecType::Run)) { + if !matches!( + dependency_type, + DependencyType::CondaDependency(SpecType::Run) + ) { eprintln!( "Added these as {}.", console::style(dependency_type.name()).bold() @@ -212,10 +214,12 @@ pub async fn add_pypi_specs_to_project( // TODO: Get best version // Add the dependency to the project if specs_platforms.is_empty() { - project.add_pypi_dependency(name, spec)?; + project.manifest.add_pypi_dependency(name, spec)?; } else { for platform in specs_platforms.iter() { - project.add_target_pypi_dependency(*platform, name.clone(), spec)?; + project + .manifest + .add_target_pypi_dependency(*platform, name.clone(), spec)?; } } } @@ -309,10 +313,12 @@ pub async fn add_conda_specs_to_project( // Add the dependency to the project if specs_platforms.is_empty() { - project.add_dependency(&spec, spec_type)?; + project.manifest.add_dependency(&spec, spec_type)?; } else { for platform in specs_platforms.iter() { - project.add_target_dependency(*platform, &spec, spec_type)?; + project + .manifest + .add_target_dependency(*platform, &spec, spec_type)?; } } } diff --git a/src/cli/info.rs b/src/cli/info.rs index b10c09c74..00fefe939 100644 --- a/src/cli/info.rs +++ b/src/cli/info.rs @@ -197,6 +197,7 @@ pub async fn execute(args: Args) -> miette::Result<()> { let project_info = project.map(|p| ProjectInfo { manifest_path: p.root().to_path_buf().join("pixi.toml"), tasks: p + .manifest .tasks(Some(Platform::current())) .into_keys() .map(|k| k.to_string()) diff --git a/src/cli/project/channel/add.rs b/src/cli/project/channel/add.rs index 2a3b2afd0..8beccaafb 100644 --- a/src/cli/project/channel/add.rs +++ b/src/cli/project/channel/add.rs @@ -48,7 +48,9 @@ pub async fn execute(mut project: Project, args: Args) -> miette::Result<()> { let lock_file = load_lock_file(&project).await?; // Add the channels to the lock-file - project.add_channels(missing_channels.iter().map(|(name, _channel)| name))?; + project + .manifest + .add_channels(missing_channels.iter().map(|(name, _channel)| name))?; // Try to update the lock-file with the new channels let lock_file = update_lock_file(&project, lock_file, None).await?; diff --git a/src/cli/remove.rs b/src/cli/remove.rs index 383da6904..7186b8752 100644 --- a/src/cli/remove.rs +++ b/src/cli/remove.rs @@ -44,9 +44,11 @@ pub async fn execute(args: Args) -> miette::Result<()> { .iter() .map(|dep| { if let Some(p) = &args.platform { - project.remove_target_dependency(dep, &spec_type, p) + project + .manifest + .remove_target_dependency(dep, &spec_type, p) } else { - project.remove_dependency(dep, &spec_type) + project.manifest.remove_dependency(dep, &spec_type) } }) .collect::>(); diff --git a/src/cli/run.rs b/src/cli/run.rs index 172bef3e5..146541b34 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -135,6 +135,7 @@ async fn execute_task<'p>( if status_code == 127 { let available_tasks = task .project() + .manifest .tasks(Some(Platform::current())) .into_keys() .sorted() diff --git a/src/cli/task.rs b/src/cli/task.rs index eda037cb9..471a61527 100644 --- a/src/cli/task.rs +++ b/src/cli/task.rs @@ -139,7 +139,9 @@ pub fn execute(args: Args) -> miette::Result<()> { Operation::Add(args) => { let name = &args.name; let task: Task = args.clone().into(); - project.add_task(name, task.clone(), args.platform)?; + project + .manifest + .add_task(name, task.clone(), args.platform)?; eprintln!( "{}Added task {}: {}", console::style(console::Emoji("✔ ", "+")).green(), @@ -152,6 +154,7 @@ pub fn execute(args: Args) -> miette::Result<()> { for name in args.names.iter() { if let Some(platform) = args.platform { if !project + .manifest .target_specific_tasks(platform) .contains_key(name.as_str()) { @@ -163,7 +166,7 @@ pub fn execute(args: Args) -> miette::Result<()> { ); continue; } - } else if !project.tasks(None).contains_key(name.as_str()) { + } else if !project.manifest.tasks(None).contains_key(name.as_str()) { eprintln!( "{}Task {} does not exist", console::style(console::Emoji("❌ ", "X")).red(), @@ -191,7 +194,7 @@ pub fn execute(args: Args) -> miette::Result<()> { } for (name, platform) in to_remove { - project.remove_task(name, platform)?; + project.manifest.remove_task(name, platform)?; eprintln!( "{}Removed task {} ", console::style(console::Emoji("✔ ", "+")).green(), @@ -202,7 +205,9 @@ pub fn execute(args: Args) -> miette::Result<()> { Operation::Alias(args) => { let name = &args.alias; let task: Task = args.clone().into(); - project.add_task(name, task.clone(), args.platform)?; + project + .manifest + .add_task(name, task.clone(), args.platform)?; eprintln!( "{} Added alias {}: {}", console::style("@").blue(), diff --git a/src/environment.rs b/src/environment.rs index 6930a3ec3..a15be8032 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -92,7 +92,7 @@ pub async fn get_up_to_date_prefix( // Make sure the project supports the current platform let platform = Platform::current(); if !project.platforms().contains(&platform) { - let span = project.manifest.project.platforms.span(); + let span = project.manifest.manifest.project.platforms.span(); return Err(miette::miette!( help = format!( "The project needs to be configured to support your platform ({platform})." @@ -103,7 +103,7 @@ pub async fn get_up_to_date_prefix( )], "the project is not configured for your current platform" ) - .with_source_code(project.source())); + .with_source_code(project.manifest_named_source())); } // Make sure the system requirements are met diff --git a/src/lock_file/python.rs b/src/lock_file/python.rs index 7f72f5a2f..bea7c5adc 100644 --- a/src/lock_file/python.rs +++ b/src/lock_file/python.rs @@ -67,7 +67,7 @@ pub async fn resolve_pypi_dependencies<'p>( // Determine the compatible tags let compatible_tags = project_platform_tags( platform, - &project.manifest.system_requirements, + project.system_requirements(), python_record.as_ref(), ); diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 1245a9f1e..42e0078bd 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -1,11 +1,14 @@ use crate::project::python::PyPiRequirement; -use crate::project::SpecType; +use crate::project::{DependencyType, SpecType}; +use crate::task::CmdArgs; use crate::utils::spanned::PixiSpanned; use crate::{consts, task::Task}; use ::serde::Deserialize; use indexmap::IndexMap; use miette::{Context, IntoDiagnostic, LabeledSpan, NamedSource, Report}; -use rattler_conda_types::{Channel, NamelessMatchSpec, Platform, Version}; +use rattler_conda_types::{ + Channel, ChannelConfig, MatchSpec, NamelessMatchSpec, Platform, Version, +}; use rattler_virtual_packages::{Archspec, Cuda, LibC, Linux, Osx, VirtualPackage}; use rip::types::PackageName; use serde::Deserializer; @@ -14,8 +17,554 @@ use serde_with::{serde_as, DeserializeAs, DisplayFromStr, PickFirst}; use std::collections::HashMap; use std::ops::Range; use std::path::{Path, PathBuf}; +use toml_edit::{Array, Document, Item, Table, TomlError, Value}; use url::Url; +/// Handles the project's manifest file. +/// This struct is responsible for reading, parsing, editing, and saving the manifest. +/// It encapsulates all logic related to the manifest's TOML format and structure. +/// The manifest data is represented as a [`ProjectManifest`] struct for easy manipulation. +/// Owned by the [`Project`] struct, which governs its usage. +/// +#[derive(Debug, Clone)] +pub struct Manifest { + /// The path to the manifest file + pub path: PathBuf, + + /// The raw contents of the manifest file + pub contents: String, + + /// Editable toml document + pub document: toml_edit::Document, + + /// The parsed manifest + pub manifest: ProjectManifest, +} + +impl Manifest { + /// Create a new manifest from a path + pub fn from_path(path: PathBuf) -> miette::Result { + let contents = std::fs::read_to_string(&path).into_diagnostic()?; + Self::from_str( + path.parent().expect("Path should always have a parent"), + contents, + ) + } + + /// Create a new manifest from a string + pub fn from_str(root: &Path, contents: impl Into) -> miette::Result { + let contents = contents.into(); + let (manifest, document) = match toml_edit::de::from_str::(&contents) + .map_err(TomlError::from) + .and_then(|manifest| contents.parse::().map(|doc| (manifest, doc))) + { + Ok(result) => result, + Err(e) => { + if let Some(span) = e.span() { + return Err(miette::miette!( + labels = vec![LabeledSpan::at(span, e.message())], + "failed to parse project manifest" + ) + .with_source_code(NamedSource::new(consts::PROJECT_MANIFEST, contents))); + } else { + return Err(e).into_diagnostic(); + } + } + }; + + // Validate the contents of the manifest + manifest.validate( + NamedSource::new(consts::PROJECT_MANIFEST, contents.to_owned()), + root, + )?; + + // Notify the user that pypi-dependencies are still experimental + if manifest + .pypi_dependencies + .as_ref() + .map_or(false, |deps| !deps.is_empty()) + { + tracing::warn!("ALPHA feature enabled!\n\nIt looks like your project contains `[pypi-dependencies]`. This feature is currently still in an ALPHA state!\n\nYou may encounter bugs or weird behavior. Please report any and all issues you encounter on our github repository:\n\n\thttps://github.com/prefix-dev/pixi.\n"); + } + + Ok(Self { + path: root.join(consts::PROJECT_MANIFEST), + contents, + document, + manifest, + }) + } + + /// Save the manifest to the file and update the contents + pub fn save(&mut self) -> miette::Result<()> { + self.contents = self.document.to_string(); + std::fs::write(&self.path, self.contents.clone()).into_diagnostic()?; + Ok(()) + } + + /// Returns all the targets specific metadata that apply with the given context. + /// TODO: Add more context here? + /// TODO: Should we return the selector too to provide better diagnostics later? + pub fn target_specific_metadata( + &self, + platform: Platform, + ) -> impl Iterator + '_ { + self.manifest + .target + .iter() + .filter_map(move |(selector, manifest)| match selector.as_ref() { + TargetSelector::Platform(p) if p == &platform => Some(manifest), + _ => None, + }) + } + + /// Returns a hashmap of the tasks that should run only the given platform. + pub fn target_specific_tasks(&self, platform: Platform) -> HashMap<&str, &Task> { + let mut tasks = HashMap::default(); + // Gather platform-specific tasks and overwrite them if they're double. + for target_metadata in self.target_specific_metadata(platform) { + tasks.extend(target_metadata.tasks.iter().map(|(k, v)| (k.as_str(), v))); + } + + tasks + } + + /// Add a task to the project + pub fn add_task( + &mut self, + name: impl AsRef, + task: Task, + platform: Option, + ) -> miette::Result<()> { + let table = if let Some(platform) = platform { + if self + .target_specific_tasks(platform) + .contains_key(name.as_ref()) + { + miette::bail!("task {} already exists", name.as_ref()); + } + ensure_toml_target_table(&mut self.document, platform, "tasks")? + } else { + self.document["tasks"] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) + })? + }; + let depends_on = task.depends_on(); + + for depends in depends_on { + if !self.manifest.tasks.contains_key(depends) { + miette::bail!( + "task '{}' for the depends on for '{}' does not exist", + depends, + name.as_ref(), + ); + } + } + + // Add the task to the table + table.insert(name.as_ref(), task_as_toml(task.clone())); + + self.manifest.tasks.insert(name.as_ref().to_string(), task); + + self.save()?; + + Ok(()) + } + + /// Returns a hashmap of the tasks that should run on the given platform. + pub fn tasks(&self, platform: Option) -> HashMap<&str, &Task> { + let mut all_tasks = HashMap::default(); + + // Gather non-target specific tasks + all_tasks.extend(self.manifest.tasks.iter().map(|(k, v)| (k.as_str(), v))); + + // Gather platform-specific tasks and overwrite them if they're double. + if let Some(platform) = platform { + for target_metadata in self.target_specific_metadata(platform) { + all_tasks.extend(target_metadata.tasks.iter().map(|(k, v)| (k.as_str(), v))); + } + } + + all_tasks + } + + /// Remove a task from the project, and the tasks that depend on it + pub fn remove_task( + &mut self, + name: impl AsRef, + platform: Option, + ) -> miette::Result<()> { + self.tasks(platform) + .get(name.as_ref()) + .ok_or_else(|| miette::miette!("task {} does not exist", name.as_ref()))?; + + // Get the task table either from the target platform or the default tasks. + let tasks_table = if let Some(platform) = platform { + ensure_toml_target_table(&mut self.document, platform, "tasks")? + } else { + let tasks_table = &mut self.document["tasks"]; + if tasks_table.is_none() { + miette::bail!("internal data-structure inconsistent with toml"); + } + tasks_table.as_table_like_mut().ok_or_else(|| { + miette::miette!("tasks in {} are malformed", consts::PROJECT_MANIFEST) + })? + }; + + // If it does not exist in toml, consider this ok as we want to remove it anyways + tasks_table.remove(name.as_ref()); + + self.save()?; + + Ok(()) + } + + /// Add a platform to the project + pub fn add_platforms<'a>( + &mut self, + platforms: impl Iterator + Clone, + ) -> miette::Result<()> { + // Add to platform table + let platform_array = &mut self.document["project"]["platforms"]; + let platform_array = platform_array + .as_array_mut() + .expect("platforms should be an array"); + + for platform in platforms.clone() { + platform_array.push(platform.to_string()); + } + + // Add to manifest + self.manifest.project.platforms.value.extend(platforms); + Ok(()) + } + + /// Add match spec to the specified table + fn add_to_deps_table( + deps_table: &mut Item, + spec: &MatchSpec, + ) -> miette::Result<(rattler_conda_types::PackageName, NamelessMatchSpec)> { + // If it doesn't exist create a proper table + if deps_table.is_none() { + *deps_table = Item::Table(Table::new()); + } + + // Cast the item into a table + let deps_table = deps_table.as_table_like_mut().ok_or_else(|| { + miette::miette!("dependencies in {} are malformed", consts::PROJECT_MANIFEST) + })?; + + // Determine the name of the package to add + let name = spec + .name + .clone() + .ok_or_else(|| miette::miette!("* package specifier is not supported"))?; + + // Format the requirement + // TODO: Do this smarter. E.g.: + // - split this into an object if exotic properties (like channel) are specified. + // - split the name from the rest of the requirement. + let nameless = NamelessMatchSpec::from(spec.to_owned()); + + // Store (or replace) in the document + deps_table.insert(name.as_source(), Item::Value(nameless.to_string().into())); + + Ok((name, nameless)) + } + + /// Add PyPI requirement to the specified table + fn add_pypi_dep_to_table( + deps_table: &mut Item, + name: &rip::types::PackageName, + requirement: &PyPiRequirement, + ) -> miette::Result<()> { + // If it doesn't exist create a proper table + if deps_table.is_none() { + *deps_table = Item::Table(Table::new()); + } + + // Cast the item into a table + let deps_table = deps_table.as_table_like_mut().ok_or_else(|| { + miette::miette!("dependencies in {} are malformed", consts::PROJECT_MANIFEST) + })?; + + deps_table.insert(name.as_str(), (*requirement).clone().into()); + Ok(()) + } + + /// Add match spec to the target specific table + fn add_dep_to_target_table( + &mut self, + platform: Platform, + dep_type: String, + spec: &MatchSpec, + ) -> miette::Result<(rattler_conda_types::PackageName, NamelessMatchSpec)> { + let target = self.document["target"] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) + })?; + target.set_dotted(true); + let platform_table = self.document["target"][platform.as_str()] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!( + "platform table in {} is malformed", + consts::PROJECT_MANIFEST + ) + })?; + platform_table.set_dotted(true); + + let dependencies = platform_table[dep_type.as_str()].or_insert(Item::Table(Table::new())); + + Self::add_to_deps_table(dependencies, spec) + } + + /// Add PyPI requirement to the target specific table + fn add_pypi_dep_to_target_table( + &mut self, + platform: Platform, + name: &rip::types::PackageName, + requirement: &PyPiRequirement, + ) -> miette::Result<()> { + let target = self.document["target"] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) + })?; + target.set_dotted(true); + let platform_table = self.document["target"][platform.as_str()] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!( + "platform table in {} is malformed", + consts::PROJECT_MANIFEST + ) + })?; + platform_table.set_dotted(true); + + let dependencies = platform_table[DependencyType::PypiDependency.name()] + .or_insert(Item::Table(Table::new())); + + Self::add_pypi_dep_to_table(dependencies, name, requirement) + } + + pub fn add_target_dependency( + &mut self, + platform: Platform, + spec: &MatchSpec, + spec_type: SpecType, + ) -> miette::Result<()> { + let toml_name = spec_type.name(); + // Add to target table toml + let (name, nameless) = + self.add_dep_to_target_table(platform, toml_name.to_string(), spec)?; + // Add to manifest + self.manifest + .target + .entry(TargetSelector::Platform(platform).into()) + .or_insert(TargetMetadata::default()) + .dependencies + .insert(name.as_source().into(), nameless); + Ok(()) + } + /// Add target specific PyPI requirement to the manifest + pub fn add_target_pypi_dependency( + &mut self, + platform: Platform, + name: rip::types::PackageName, + requirement: &PyPiRequirement, + ) -> miette::Result<()> { + // Add to target table toml + self.add_pypi_dep_to_target_table(platform, &name.clone(), requirement)?; + // Add to manifest + self.manifest + .target + .entry(TargetSelector::Platform(platform).into()) + .or_insert(TargetMetadata::default()) + .pypi_dependencies + .get_or_insert(IndexMap::new()) + .insert(name, requirement.clone()); + Ok(()) + } + + /// Add a matchspec to the manifest + pub fn add_dependency(&mut self, spec: &MatchSpec, spec_type: SpecType) -> miette::Result<()> { + // Find the dependencies table + let deps = &mut self.document[spec_type.name()]; + let (name, nameless) = Manifest::add_to_deps_table(deps, spec)?; + + self.manifest + .create_or_get_dependencies(spec_type) + .insert(name.as_source().into(), nameless); + + Ok(()) + } + + pub fn add_pypi_dependency( + &mut self, + name: &rip::types::PackageName, + requirement: &PyPiRequirement, + ) -> miette::Result<()> { + // Find the dependencies table + let deps = &mut self.document[DependencyType::PypiDependency.name()]; + Manifest::add_pypi_dep_to_table(deps, name, requirement)?; + + self.manifest + .create_or_get_pypi_dependencies() + .insert(name.clone(), requirement.clone()); + + Ok(()) + } + + /// Removes a dependency from `pixi.toml` based on `SpecType`. + pub fn remove_dependency( + &mut self, + dep: &rattler_conda_types::PackageName, + spec_type: &SpecType, + ) -> miette::Result<(String, NamelessMatchSpec)> { + if let Item::Table(ref mut t) = self.document[spec_type.name()] { + if t.contains_key(dep.as_normalized()) && t.remove(dep.as_normalized()).is_some() { + return self + .manifest + .remove_dependency(dep.as_normalized(), spec_type); + } + } + + Err(miette::miette!( + "Couldn't find {} in [{}]", + console::style(dep.as_normalized()).bold(), + console::style(spec_type.name()).bold(), + )) + } + + /// Removes a target specific dependency from `pixi.toml` based on `SpecType`. + pub fn remove_target_dependency( + &mut self, + dep: &rattler_conda_types::PackageName, + spec_type: &SpecType, + platform: &Platform, + ) -> miette::Result<(String, NamelessMatchSpec)> { + let table = get_toml_target_table(&mut self.document, platform, spec_type.name())?; + table.remove(dep.as_normalized()); + self.manifest + .remove_target_dependency(dep.as_normalized(), spec_type, platform) + } + + /// Returns a mutable reference to the channels array. + fn channels_array_mut(&mut self) -> miette::Result<&mut Array> { + let project = &mut self.document["project"]; + if project.is_none() { + *project = Item::Table(Table::new()); + } + + let channels = &mut project["channels"]; + if channels.is_none() { + *channels = Item::Value(Value::Array(Array::new())) + } + + channels + .as_array_mut() + .ok_or_else(|| miette::miette!("malformed channels array")) + } + + /// Adds the specified channels to the manifest. + pub fn add_channels( + &mut self, + channels: impl IntoIterator>, + ) -> miette::Result<()> { + let mut stored_channels = Vec::new(); + for channel in channels { + self.manifest.project.channels.push( + Channel::from_str(channel.as_ref(), &ChannelConfig::default()).into_diagnostic()?, + ); + stored_channels.push(channel.as_ref().to_owned()); + } + + let channels_array = self.channels_array_mut()?; + for channel in stored_channels { + channels_array.push(channel); + } + + Ok(()) + } +} + +/// Ensures that the specified TOML target table exists within a given document, +/// and inserts it if not. +/// Returns the final target table (`table_name`) inside the platform-specific table if everything +/// goes as expected. +pub fn ensure_toml_target_table<'a>( + doc: &'a mut Document, + platform: Platform, + table_name: &str, +) -> miette::Result<&'a mut Table> { + // Create target table + let target = doc["target"] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) + })?; + target.set_dotted(true); + + // Create platform table on target table + let platform_table = doc["target"][platform.as_str()] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!( + "platform table in {} is malformed", + consts::PROJECT_MANIFEST + ) + })?; + platform_table.set_dotted(true); + + // Return final table on target platform table. + platform_table[table_name] + .or_insert(Item::Table(Table::new())) + .as_table_mut() + .ok_or_else(|| { + miette::miette!( + "platform table in {} is malformed", + consts::PROJECT_MANIFEST + ) + }) +} + +/// Retrieve a mutable reference to a target table `table_name` +/// for a specific platform. +fn get_toml_target_table<'a>( + doc: &'a mut Document, + platform: &Platform, + table_name: &str, +) -> miette::Result<&'a mut Table> { + let platform_table = doc["target"][platform.as_str()] + .as_table_mut() + .ok_or(miette::miette!( + "could not find {} in {}", + console::style(platform.as_str()).bold(), + consts::PROJECT_MANIFEST, + ))?; + + platform_table.set_dotted(true); + + platform_table[table_name] + .as_table_mut() + .ok_or(miette::miette!( + "could not find {} in {}", + console::style(format!("[target.{}.{}]", platform.as_str(), table_name)).bold(), + consts::PROJECT_MANIFEST, + )) +} + /// Describes the contents of a project manifest. #[serde_as] #[derive(Debug, Clone, Deserialize)] @@ -463,10 +1012,49 @@ fn create_unsupported_platform_report( .with_source_code(source) } +/// Returns a task as a toml item +fn task_as_toml(task: Task) -> Item { + match task { + Task::Plain(str) => Item::Value(str.into()), + Task::Execute(process) => { + let mut table = Table::new().into_inline_table(); + match process.cmd { + CmdArgs::Single(cmd_str) => { + table.insert("cmd", cmd_str.into()); + } + CmdArgs::Multiple(cmd_strs) => { + table.insert("cmd", Value::Array(Array::from_iter(cmd_strs))); + } + } + if !process.depends_on.is_empty() { + table.insert( + "depends_on", + Value::Array(Array::from_iter(process.depends_on)), + ); + } + if let Some(cwd) = process.cwd { + table.insert("cwd", cwd.to_string_lossy().to_string().into()); + } + Item::Value(Value::InlineTable(table)) + } + Task::Alias(alias) => { + let mut table = Table::new().into_inline_table(); + table.insert( + "depends_on", + Value::Array(Array::from_iter(alias.depends_on)), + ); + Item::Value(Value::InlineTable(table)) + } + _ => Item::None, + } +} + #[cfg(test)] mod test { - use super::ProjectManifest; + use super::*; use insta::{assert_debug_snapshot, assert_display_snapshot}; + use std::str::FromStr; + use tempfile::tempdir; const PROJECT_BOILERPLATE: &str = r#" [project] @@ -605,4 +1193,139 @@ mod test { toml_edit::de::from_str::(&contents).expect("parsing should succeed!") ); } + + #[test] + fn system_requirements_works() { + let file_content = r#" + windows = true + unix = true + linux = "5.11" + cuda = "12.2" + macos = "10.15" + archspec = "arm64" + libc = { family = "glibc", version = "2.12" } + "#; + + let system_requirements: SystemRequirements = + toml_edit::de::from_str(file_content).unwrap(); + + let expected_requirements: Vec = vec![ + VirtualPackage::Win, + VirtualPackage::Unix, + VirtualPackage::Linux(Linux { + version: Version::from_str("5.11").unwrap(), + }), + VirtualPackage::Cuda(Cuda { + version: Version::from_str("12.2").unwrap(), + }), + VirtualPackage::Osx(Osx { + version: Version::from_str("10.15").unwrap(), + }), + VirtualPackage::Archspec(Archspec { + spec: "arm64".to_string(), + }), + VirtualPackage::LibC(LibC { + version: Version::from_str("2.12").unwrap(), + family: "glibc".to_string(), + }), + ]; + + assert_eq!( + system_requirements.virtual_packages(), + expected_requirements + ); + } + + #[test] + fn test_system_requirements_failing_edge_cases() { + let file_contents = [ + r#" + [system-requirements] + libc = { verion = "2.12" } + "#, + r#" + [system-requirements] + lib = "2.12" + "#, + r#" + [system-requirements.libc] + version = "2.12" + fam = "glibc" + "#, + r#" + [system-requirements.lic] + version = "2.12" + family = "glibc" + "#, + ]; + + for file_content in file_contents { + let file_content = format!("{PROJECT_BOILERPLATE}\n{file_content}"); + assert!(toml_edit::de::from_str::(&file_content).is_err()); + } + } + + #[test] + fn test_remove_target_dependencies() { + // Using known files in the project so the test succeed including the file check. + let file_contents = r#" + [project] + name = "foo" + version = "0.1.0" + channels = [] + platforms = ["linux-64", "win-64"] + + [dependencies] + fooz = "*" + + [target.win-64.dependencies] + bar = "*" + + [target.linux-64.build-dependencies] + baz = "*" + "#; + + let tmpdir = tempdir().unwrap(); + + let mut manifest = Manifest::from_str(tmpdir.path(), file_contents).unwrap(); + + manifest + .manifest + .remove_target_dependency("baz", &SpecType::Build, &Platform::Linux64) + .unwrap(); + assert_debug_snapshot!(manifest.manifest); + } + + #[test] + fn test_remove_dependencies() { + // Using known files in the project so the test succeed including the file check. + let file_contents = r#" + [project] + name = "foo" + version = "0.1.0" + channels = [] + platforms = ["linux-64", "win-64"] + + [dependencies] + fooz = "*" + + [target.win-64.dependencies] + fooz = "*" + + [target.linux-64.build-dependencies] + fooz = "*" + "#; + + let mut manifest = Manifest::from_str(Path::new(""), file_contents).unwrap(); + + manifest + .remove_dependency( + &rattler_conda_types::PackageName::try_from("fooz").unwrap(), + &SpecType::Run, + ) + .unwrap(); + assert!(manifest.manifest.dependencies.is_empty()); + // Should still contain the fooz dependency in the target table + assert_debug_snapshot!(manifest.manifest.target); + } } diff --git a/src/project/mod.rs b/src/project/mod.rs index 20d452f45..5c125a6c9 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -4,15 +4,14 @@ pub(crate) mod python; mod serde; use indexmap::IndexMap; -use miette::{IntoDiagnostic, LabeledSpan, NamedSource, WrapErr}; +use miette::{IntoDiagnostic, NamedSource, WrapErr}; use once_cell::sync::OnceCell; -use rattler_conda_types::{ - Channel, ChannelConfig, MatchSpec, NamelessMatchSpec, PackageName, Platform, Version, -}; +use rattler_conda_types::{Channel, NamelessMatchSpec, PackageName, Platform, Version}; use rattler_virtual_packages::VirtualPackage; use rip::{index::PackageDb, normalize_index_url}; +use std::collections::HashMap; use std::{ - collections::{HashMap, HashSet}, + collections::HashSet, env, ffi::OsStr, fs, @@ -20,15 +19,14 @@ use std::{ sync::Arc, }; +use crate::project::manifest::{Manifest, SystemRequirements}; use crate::project::python::PyPiRequirement; use crate::{ consts::{self, PROJECT_MANIFEST}, default_client, - project::manifest::{ProjectManifest, TargetMetadata, TargetSelector}, - task::{CmdArgs, Task}, + task::Task, virtual_packages::non_relevant_virtual_packages_for_platform, }; -use toml_edit::{Array, Document, Item, Table, TomlError, Value}; use url::Url; /// The dependency types we support @@ -69,60 +67,23 @@ impl SpecType { } } -/// A project represented by a pixi.toml file. +/// The pixi project, this main struct to interact with the project. This struct holds the [`Manifest`] and has functions to modify or request information from it. +/// This allows in the future to have multiple environments or manifests linked to a project. #[derive(Clone)] pub struct Project { + /// Root folder of the project root: PathBuf, - source: String, - doc: Document, + /// The PyPI package db for this project package_db: OnceCell>, - pub manifest: ProjectManifest, -} - -/// Returns a task as a toml item -fn task_as_toml(task: Task) -> Item { - match task { - Task::Plain(str) => Item::Value(str.into()), - Task::Execute(process) => { - let mut table = Table::new().into_inline_table(); - match process.cmd { - CmdArgs::Single(cmd_str) => { - table.insert("cmd", cmd_str.into()); - } - CmdArgs::Multiple(cmd_strs) => { - table.insert("cmd", Value::Array(Array::from_iter(cmd_strs))); - } - } - if !process.depends_on.is_empty() { - table.insert( - "depends_on", - Value::Array(Array::from_iter(process.depends_on)), - ); - } - if let Some(cwd) = process.cwd { - table.insert("cwd", cwd.to_string_lossy().to_string().into()); - } - Item::Value(Value::InlineTable(table)) - } - Task::Alias(alias) => { - let mut table = Table::new().into_inline_table(); - table.insert( - "depends_on", - Value::Array(Array::from_iter(alias.depends_on)), - ); - Item::Value(Value::InlineTable(table)) - } - _ => Item::None, - } + /// The manifest for the project + pub(crate) manifest: Manifest, } impl Project { /// Constructs a new instance from an internal manifest representation - pub fn from_manifest(manifest: ProjectManifest) -> Self { + pub fn from_manifest(manifest: Manifest) -> Self { Self { root: Default::default(), - source: "".to_string(), - doc: Default::default(), package_db: Default::default(), manifest, } @@ -133,89 +94,139 @@ impl Project { /// This will also set the current working directory to the project root. pub fn discover() -> miette::Result { let project_toml = match find_project_root() { - Some(root) => root.join(consts::PROJECT_MANIFEST), - None => miette::bail!("could not find {}", consts::PROJECT_MANIFEST), + Some(root) => root.join(PROJECT_MANIFEST), + None => miette::bail!("could not find {}", PROJECT_MANIFEST), }; Self::load(&project_toml) } /// Returns the source code of the project as [`NamedSource`]. - pub fn source(&self) -> NamedSource { - NamedSource::new(consts::PROJECT_MANIFEST, self.source.clone()) + /// Used in error reporting. + pub fn manifest_named_source(&self) -> NamedSource { + NamedSource::new(PROJECT_MANIFEST, self.manifest.contents.clone()) } - /// Loads a project manifest file. - pub fn load(filename: &Path) -> miette::Result { + /// Loads a project from manifest file. + fn load(manifest_path: &Path) -> miette::Result { // Determine the parent directory of the manifest file - let full_path = dunce::canonicalize(filename).into_diagnostic()?; + let full_path = dunce::canonicalize(manifest_path).into_diagnostic()?; if full_path.file_name().and_then(OsStr::to_str) != Some(PROJECT_MANIFEST) { miette::bail!("the manifest-path must point to a {PROJECT_MANIFEST} file"); } let root = full_path .parent() - .ok_or_else(|| miette::miette!("can not find parent of {}", filename.display()))?; + .ok_or_else(|| miette::miette!("can not find parent of {}", manifest_path.display()))?; // Load the TOML document - fs::read_to_string(filename) + let manifest = fs::read_to_string(manifest_path) .into_diagnostic() - .and_then(|content| Self::from_manifest_str(root, content)) + .and_then(|content| Manifest::from_str(root, content)) .wrap_err_with(|| { format!( "failed to parse {} from {}", consts::PROJECT_MANIFEST, root.display() ) - }) + }); + + Ok(Self { + root: root.to_owned(), + package_db: Default::default(), + manifest: manifest?, + }) } - /// Returns all tasks defined in the project for the given platform - pub fn task_names(&self, platform: Option) -> Vec<&String> { - let mut all_tasks = HashSet::new(); + /// Loads a project manifest file or discovers it in the current directory or any of the parent + pub fn load_or_else_discover(manifest_path: Option<&Path>) -> miette::Result { + let project = match manifest_path { + Some(path) => Project::load(path)?, + None => Project::discover()?, + }; + Ok(project) + } - // Get all non-target specific tasks - all_tasks.extend(self.manifest.tasks.keys()); + /// Returns the name of the project + pub fn name(&self) -> &str { + &self.manifest.manifest.project.name + } - // Gather platform-specific tasks and overwrite the keys if they're double. - if let Some(platform) = platform { - for target_metadata in self.target_specific_metadata(platform) { - all_tasks.extend(target_metadata.tasks.keys()); - } - } - Vec::from_iter(all_tasks) + /// Returns the version of the project + pub fn version(&self) -> &Option { + &self.manifest.manifest.project.version } - /// Returns a hashmap of the tasks that should run on the given platform. - pub fn tasks(&self, platform: Option) -> HashMap<&str, &Task> { - let mut all_tasks = HashMap::default(); + /// Returns the root directory of the project + pub fn root(&self) -> &Path { + &self.root + } + + /// Returns the pixi directory + pub fn environment_dir(&self) -> PathBuf { + self.root.join(consts::PIXI_DIR) + } + + /// Returns the path to the manifest file. + pub fn manifest_path(&self) -> PathBuf { + self.manifest.path.clone() + } + + /// Returns the path to the lock file of the project + pub fn lock_file_path(&self) -> PathBuf { + self.root.join(consts::PROJECT_LOCK_FILE) + } + + /// Save back changes + pub fn save(&mut self) -> miette::Result<()> { + self.manifest.save() + } + + /// Returns the channels used by this project + pub fn channels(&self) -> &[Channel] { + &self.manifest.manifest.project.channels + } - // Gather non-target specific tasks - all_tasks.extend(self.manifest.tasks.iter().map(|(k, v)| (k.as_str(), v))); + /// Returns the platforms this project targets + pub fn platforms(&self) -> &[Platform] { + self.manifest.manifest.project.platforms.as_ref().as_slice() + } - // Gather platform-specific tasks and overwrite them if they're double. + /// Get the tasks of this project + pub fn tasks(&self, platform: Option) -> HashMap<&str, &Task> { + self.manifest.tasks(platform) + } + /// Get the task with the specified `name` or `None` if no such task exists. If `platform` is + /// specified then the task will first be looked up in the target specific tasks for the given + /// platform. + pub fn task_opt(&self, name: &str, platform: Option) -> Option<&Task> { if let Some(platform) = platform { - for target_metadata in self.target_specific_metadata(platform) { - all_tasks.extend(target_metadata.tasks.iter().map(|(k, v)| (k.as_str(), v))); + if let Some(task) = self.manifest.target_specific_tasks(platform).get(name) { + return Some(*task); } } - all_tasks + self.manifest.manifest.tasks.get(name) } - /// Returns a hashmap of the tasks that should run only the given platform. - pub fn target_specific_tasks(&self, platform: Platform) -> HashMap<&str, &Task> { - let mut tasks = HashMap::default(); - // Gather platform-specific tasks and overwrite them if they're double. - for target_metadata in self.target_specific_metadata(platform) { - tasks.extend(target_metadata.tasks.iter().map(|(k, v)| (k.as_str(), v))); - } + /// Returns all tasks defined in the project for the given platform + pub fn task_names(&self, platform: Option) -> Vec<&String> { + let mut all_tasks = HashSet::new(); - tasks + // Get all non-target specific tasks + all_tasks.extend(self.manifest.manifest.tasks.keys()); + + // Gather platform-specific tasks and overwrite the keys if they're double. + if let Some(platform) = platform { + for target_metadata in self.manifest.target_specific_metadata(platform) { + all_tasks.extend(target_metadata.tasks.keys()); + } + } + Vec::from_iter(all_tasks) } /// Returns names of the tasks that depend on the given task. pub fn task_names_depending_on(&self, name: impl AsRef) -> Vec<&str> { - let mut tasks = self.tasks(Some(Platform::current())); + let mut tasks = self.manifest.tasks(Some(Platform::current())); let task = tasks.remove(name.as_ref()); if task.is_some() { tasks @@ -228,174 +239,17 @@ impl Project { } } - /// Add a task to the project - pub fn add_task( - &mut self, - name: impl AsRef, - task: Task, - platform: Option, - ) -> miette::Result<()> { - let table = if let Some(platform) = platform { - if self - .target_specific_tasks(platform) - .contains_key(name.as_ref()) - { - miette::bail!("task {} already exists", name.as_ref()); - } - ensure_toml_target_table(&mut self.doc, platform, "tasks")? - } else { - self.doc["tasks"] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) - })? - }; - let depends_on = task.depends_on(); - - for depends in depends_on { - if !self.manifest.tasks.contains_key(depends) { - miette::bail!( - "task '{}' for the depends on for '{}' does not exist", - depends, - name.as_ref(), - ); - } - } - - // Add the task to the table - table.insert(name.as_ref(), task_as_toml(task.clone())); - - self.manifest.tasks.insert(name.as_ref().to_string(), task); - - self.save()?; - - Ok(()) - } - - /// Remove a task from the project, and the tasks that depend on it - pub fn remove_task( - &mut self, - name: impl AsRef, - platform: Option, - ) -> miette::Result<()> { - self.tasks(platform) - .get(name.as_ref()) - .ok_or_else(|| miette::miette!("task {} does not exist", name.as_ref()))?; - - // Get the task table either from the target platform or the default tasks. - let tasks_table = if let Some(platform) = platform { - ensure_toml_target_table(&mut self.doc, platform, "tasks")? - } else { - let tasks_table = &mut self.doc["tasks"]; - if tasks_table.is_none() { - miette::bail!("internal data-structure inconsistent with toml"); - } - tasks_table.as_table_like_mut().ok_or_else(|| { - miette::miette!("tasks in {} are malformed", consts::PROJECT_MANIFEST) - })? - }; - - // If it does not exist in toml, consider this ok as we want to remove it anyways - tasks_table.remove(name.as_ref()); - - self.save()?; - - Ok(()) - } - - pub fn load_or_else_discover(manifest_path: Option<&Path>) -> miette::Result { - let project = match manifest_path { - Some(path) => Project::load(path)?, - None => Project::discover()?, - }; - Ok(project) - } - - pub fn reload(&mut self) -> miette::Result<()> { - let project = Self::load(self.root().join(consts::PROJECT_MANIFEST).as_path())?; - self.root = project.root; - self.doc = project.doc; - self.manifest = project.manifest; - self.package_db = OnceCell::new(); - Ok(()) - } - - /// Loads a project manifest. - pub fn from_manifest_str(root: &Path, contents: impl Into) -> miette::Result { - let contents = contents.into(); - let (manifest, doc) = match toml_edit::de::from_str::(&contents) - .map_err(TomlError::from) - .and_then(|manifest| contents.parse::().map(|doc| (manifest, doc))) - { - Ok(result) => result, - Err(e) => { - if let Some(span) = e.span() { - return Err(miette::miette!( - labels = vec![LabeledSpan::at(span, e.message())], - "failed to parse project manifest" - ) - .with_source_code(NamedSource::new(consts::PROJECT_MANIFEST, contents))); - } else { - return Err(e).into_diagnostic(); - } - } - }; - - // Validate the contents of the manifest - manifest.validate( - NamedSource::new(consts::PROJECT_MANIFEST, contents.to_owned()), - root, - )?; - - // Notify the user that pypi-dependencies are still experimental - if manifest - .pypi_dependencies - .as_ref() - .map_or(false, |deps| !deps.is_empty()) - { - tracing::warn!("ALPHA feature enabled!\n\nIt looks like your project contains `[pypi-dependencies]`. This feature is currently still in an ALPHA state!\n\nYou may encounter bugs or weird behavior. Please report any and all issues you encounter on our github repository:\n\n\thttps://github.com/prefix-dev/pixi.\n"); - } - - Ok(Self { - root: root.to_path_buf(), - source: contents, - doc, - manifest, - package_db: OnceCell::new(), - }) - } - - /// Add a platform to the project - pub fn add_platforms<'a>( - &mut self, - platforms: impl Iterator + Clone, - ) -> miette::Result<()> { - // Add to platform table - let platform_array = &mut self.doc["project"]["platforms"]; - let platform_array = platform_array - .as_array_mut() - .expect("platforms should be an array"); - - for platform in platforms.clone() { - platform_array.push(platform.to_string()); - } - - // Add to manifest - self.manifest.project.platforms.value.extend(platforms); - Ok(()) - } - /// Returns the dependencies of the project. pub fn dependencies( &self, platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[dependencies]` section) - let base_dependencies = self.manifest.dependencies.iter(); + let base_dependencies = self.manifest.manifest.dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self + .manifest .target_specific_metadata(platform) .flat_map(|target| target.dependencies.iter()); @@ -415,10 +269,11 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[build-dependencies]` section) - let base_dependencies = self.manifest.build_dependencies.iter(); + let base_dependencies = self.manifest.manifest.build_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self + .manifest .target_specific_metadata(platform) .flat_map(|target| target.build_dependencies.iter()); @@ -439,10 +294,11 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[host-dependencies]` section) - let base_dependencies = self.manifest.host_dependencies.iter(); + let base_dependencies = self.manifest.manifest.host_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self + .manifest .target_specific_metadata(platform) .flat_map(|target| target.host_dependencies.iter()); @@ -473,10 +329,11 @@ impl Project { platform: Platform, ) -> IndexMap<&rip::types::PackageName, &PyPiRequirement> { // Get the base pypi dependencies (defined in the `[pypi-dependencies]` section) - let base_pypi_dependencies = self.manifest.pypi_dependencies.iter(); + let base_pypi_dependencies = self.manifest.manifest.pypi_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self + .manifest .target_specific_metadata(platform) .flat_map(|target| target.pypi_dependencies.iter()); @@ -516,351 +373,13 @@ impl Project { .as_ref()) } - /// Returns all the targets specific metadata that apply with the given context. - /// TODO: Add more context here? - /// TODO: Should we return the selector too to provide better diagnostics later? - pub fn target_specific_metadata( - &self, - platform: Platform, - ) -> impl Iterator + '_ { - self.manifest - .target - .iter() - .filter_map(move |(selector, manifest)| match selector.as_ref() { - TargetSelector::Platform(p) if p == &platform => Some(manifest), - _ => None, - }) - } - - /// Returns the name of the project - pub fn name(&self) -> &str { - &self.manifest.project.name - } - - /// Returns the version of the project - pub fn version(&self) -> &Option { - &self.manifest.project.version - } - - fn add_to_deps_table( - deps_table: &mut Item, - spec: &MatchSpec, - ) -> miette::Result<(PackageName, NamelessMatchSpec)> { - // If it doesn't exist create a proper table - if deps_table.is_none() { - *deps_table = Item::Table(Table::new()); - } - - // Cast the item into a table - let deps_table = deps_table.as_table_like_mut().ok_or_else(|| { - miette::miette!("dependencies in {} are malformed", consts::PROJECT_MANIFEST) - })?; - - // Determine the name of the package to add - let name = spec - .name - .clone() - .ok_or_else(|| miette::miette!("* package specifier is not supported"))?; - - // Format the requirement - // TODO: Do this smarter. E.g.: - // - split this into an object if exotic properties (like channel) are specified. - // - split the name from the rest of the requirement. - let nameless = NamelessMatchSpec::from(spec.to_owned()); - - // Store (or replace) in the document - deps_table.insert(name.as_source(), Item::Value(nameless.to_string().into())); - - Ok((name, nameless)) - } - - fn add_pypi_dep_to_table( - deps_table: &mut Item, - name: &rip::types::PackageName, - requirement: &PyPiRequirement, - ) -> miette::Result<()> { - // If it doesn't exist create a proper table - if deps_table.is_none() { - *deps_table = Item::Table(Table::new()); - } - - // Cast the item into a table - let deps_table = deps_table.as_table_like_mut().ok_or_else(|| { - miette::miette!("dependencies in {} are malformed", consts::PROJECT_MANIFEST) - })?; - - deps_table.insert(name.as_str(), (*requirement).clone().into()); - Ok(()) - } - - fn add_dep_to_target_table( - &mut self, - platform: Platform, - dep_type: String, - spec: &MatchSpec, - ) -> miette::Result<(PackageName, NamelessMatchSpec)> { - let target = self.doc["target"] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) - })?; - target.set_dotted(true); - let platform_table = self.doc["target"][platform.as_str()] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!( - "platform table in {} is malformed", - consts::PROJECT_MANIFEST - ) - })?; - platform_table.set_dotted(true); - - let dependencies = platform_table[dep_type.as_str()].or_insert(Item::Table(Table::new())); - - Self::add_to_deps_table(dependencies, spec) - } - fn add_pypi_dep_to_target_table( - &mut self, - platform: Platform, - name: &rip::types::PackageName, - requirement: &PyPiRequirement, - ) -> miette::Result<()> { - let target = self.doc["target"] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) - })?; - target.set_dotted(true); - let platform_table = self.doc["target"][platform.as_str()] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!( - "platform table in {} is malformed", - consts::PROJECT_MANIFEST - ) - })?; - platform_table.set_dotted(true); - - let dependencies = platform_table[DependencyType::PypiDependency.name()] - .or_insert(Item::Table(Table::new())); - - Self::add_pypi_dep_to_table(dependencies, name, requirement) - } - pub fn add_target_dependency( - &mut self, - platform: Platform, - spec: &MatchSpec, - spec_type: SpecType, - ) -> miette::Result<()> { - let toml_name = spec_type.name(); - // Add to target table toml - let (name, nameless) = - self.add_dep_to_target_table(platform, toml_name.to_string(), spec)?; - // Add to manifest - self.manifest - .target - .entry(TargetSelector::Platform(platform).into()) - .or_insert(TargetMetadata::default()) - .dependencies - .insert(name.as_source().into(), nameless); - Ok(()) - } - - pub fn add_target_pypi_dependency( - &mut self, - platform: Platform, - name: rip::types::PackageName, - requirement: &PyPiRequirement, - ) -> miette::Result<()> { - // Add to target table toml - self.add_pypi_dep_to_target_table(platform, &name.clone(), requirement)?; - // Add to manifest - self.manifest - .target - .entry(TargetSelector::Platform(platform).into()) - .or_insert(TargetMetadata::default()) - .pypi_dependencies - .get_or_insert(IndexMap::new()) - .insert(name, requirement.clone()); - Ok(()) - } - - pub fn add_dependency(&mut self, spec: &MatchSpec, spec_type: SpecType) -> miette::Result<()> { - // Find the dependencies table - let deps = &mut self.doc[spec_type.name()]; - let (name, nameless) = Project::add_to_deps_table(deps, spec)?; - - self.manifest - .create_or_get_dependencies(spec_type) - .insert(name.as_source().into(), nameless); - - Ok(()) - } - - pub fn add_pypi_dependency( - &mut self, - name: &rip::types::PackageName, - requirement: &PyPiRequirement, - ) -> miette::Result<()> { - // Find the dependencies table - let deps = &mut self.doc[DependencyType::PypiDependency.name()]; - Project::add_pypi_dep_to_table(deps, name, requirement)?; - - self.manifest - .create_or_get_pypi_dependencies() - .insert(name.clone(), requirement.clone()); - - Ok(()) - } - - /// Removes a dependency from `pixi.toml` based on `SpecType`. - pub fn remove_dependency( - &mut self, - dep: &PackageName, - spec_type: &SpecType, - ) -> miette::Result<(String, NamelessMatchSpec)> { - if let Item::Table(ref mut t) = self.doc[spec_type.name()] { - if t.contains_key(dep.as_normalized()) && t.remove(dep.as_normalized()).is_some() { - return self - .manifest - .remove_dependency(dep.as_normalized(), spec_type); - } - } - - Err(miette::miette!( - "Couldn't find {} in [{}]", - console::style(dep.as_normalized()).bold(), - console::style(spec_type.name()).bold(), - )) - } - - /// Removes a target specific dependency from `pixi.toml` based on `SpecType`. - pub fn remove_target_dependency( - &mut self, - dep: &PackageName, - spec_type: &SpecType, - platform: &Platform, - ) -> miette::Result<(String, NamelessMatchSpec)> { - let table = get_toml_target_table(&mut self.doc, platform, spec_type.name())?; - table.remove(dep.as_normalized()); - self.manifest - .remove_target_dependency(dep.as_normalized(), spec_type, platform) - } - - /// Returns the root directory of the project - pub fn root(&self) -> &Path { - &self.root - } - - /// Returns the pixi directory - pub fn environment_dir(&self) -> PathBuf { - self.root.join(consts::PIXI_DIR) - } - - /// Returns the path to the manifest file. - pub fn manifest_path(&self) -> PathBuf { - self.root.join(consts::PROJECT_MANIFEST) - } - - /// Returns the path to the lock file of the project - pub fn lock_file_path(&self) -> PathBuf { - self.root.join(consts::PROJECT_LOCK_FILE) - } - - /// Save back changes - pub fn save(&self) -> miette::Result<()> { - fs::write(self.manifest_path(), self.doc.to_string()) - .into_diagnostic() - .wrap_err_with(|| { - format!( - "unable to write changes to {}", - self.manifest_path().display() - ) - })?; - Ok(()) - } - - /// Returns the channels used by this project - pub fn channels(&self) -> &[Channel] { - &self.manifest.project.channels - } - - /// Adds the specified channels to the project. - pub fn add_channels( - &mut self, - channels: impl IntoIterator>, - ) -> miette::Result<()> { - let mut stored_channels = Vec::new(); - for channel in channels { - self.manifest.project.channels.push( - Channel::from_str(channel.as_ref(), &ChannelConfig::default()).into_diagnostic()?, - ); - stored_channels.push(channel.as_ref().to_owned()); - } - - let channels_array = self.channels_array_mut()?; - for channel in stored_channels { - channels_array.push(channel); - } - - Ok(()) - } - - /// Replaces all the channels in the project with the specified channels. - pub fn set_channels( - &mut self, - channels: impl IntoIterator>, - ) -> miette::Result<()> { - self.manifest.project.channels.clear(); - let mut stored_channels = Vec::new(); - for channel in channels { - self.manifest.project.channels.push( - Channel::from_str(channel.as_ref(), &ChannelConfig::default()).into_diagnostic()?, - ); - stored_channels.push(channel.as_ref().to_owned()); - } - - let channels_array = self.channels_array_mut()?; - channels_array.clear(); - for channel in stored_channels { - channels_array.push(channel); - } - Ok(()) - } - - /// Returns a mutable reference to the channels array. - fn channels_array_mut(&mut self) -> miette::Result<&mut Array> { - let project = &mut self.doc["project"]; - if project.is_none() { - *project = Item::Table(Table::new()); - } - - let channels = &mut project["channels"]; - if channels.is_none() { - *channels = Item::Value(Value::Array(Array::new())) - } - - channels - .as_array_mut() - .ok_or_else(|| miette::miette!("malformed channels array")) - } - - /// Returns the platforms this project targets - pub fn platforms(&self) -> &[Platform] { - self.manifest.project.platforms.as_ref().as_slice() - } - /// Returns the all specified activation scripts that are used in the current platform. pub fn activation_scripts(&self, platform: Platform) -> miette::Result> { let mut full_paths = Vec::new(); let mut all_scripts = Vec::new(); // Gather platform-specific activation scripts - for target_metadata in self.target_specific_metadata(platform) { + for target_metadata in self.manifest.target_specific_metadata(platform) { if let Some(activation) = &target_metadata.activation { if let Some(scripts) = &activation.scripts { all_scripts.extend(scripts.clone()); @@ -870,7 +389,7 @@ impl Project { // Gather the main activation scripts if there are no target scripts defined. if all_scripts.is_empty() { - if let Some(activation) = &self.manifest.activation { + if let Some(activation) = &self.manifest.manifest.activation { if let Some(scripts) = &activation.scripts { all_scripts.extend(scripts.clone()); } @@ -896,32 +415,17 @@ impl Project { Ok(full_paths) } - /// Get the task with the specified `name` or `None` if no such task exists. If `platform` is - /// specified then the task will first be looked up in the target specific tasks for the given - /// platform. - pub fn task_opt(&self, name: &str, platform: Option) -> Option<&Task> { - if let Some(platform) = platform { - if let Some(task) = self.target_specific_tasks(platform).get(name) { - return Some(*task); - } - } - - self.manifest.tasks.get(name) - } - /// Get the system requirements defined under the `system-requirements` section of the project manifest. - /// These get turned into virtual packages which are used in the solve. /// They will act as the description of a reference machine which is minimally needed for this package to be run. - pub fn system_requirements(&self) -> Vec { - self.manifest.system_requirements.virtual_packages() + pub fn system_requirements(&self) -> &SystemRequirements { + &self.manifest.manifest.system_requirements } /// Get the system requirements defined under the `system-requirements` section of the project manifest. /// Excluding packages that are not relevant for the specified platform. - pub fn system_requirements_for_platform(&self, platform: Platform) -> Vec { + pub fn virtual_packages_for_platform(&self, platform: Platform) -> Vec { // Filter system requirements based on the relevant packages for the current OS. - self.manifest - .system_requirements + self.system_requirements() .virtual_packages() .iter() .filter(|requirement| { @@ -941,167 +445,21 @@ pub fn find_project_root() -> Option { .map(Path::to_path_buf) } -/// Ensures that the specified TOML target table exists within a given document, -/// and inserts it if not. -/// Returns the final target table (`table_name`) inside the platform-specific table if everything -/// goes as expected. -pub fn ensure_toml_target_table<'a>( - doc: &'a mut Document, - platform: Platform, - table_name: &str, -) -> miette::Result<&'a mut Table> { - // Create target table - let target = doc["target"] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!("target table in {} is malformed", consts::PROJECT_MANIFEST) - })?; - target.set_dotted(true); - - // Create platform table on target table - let platform_table = doc["target"][platform.as_str()] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!( - "platform table in {} is malformed", - consts::PROJECT_MANIFEST - ) - })?; - platform_table.set_dotted(true); - - // Return final table on target platform table. - platform_table[table_name] - .or_insert(Item::Table(Table::new())) - .as_table_mut() - .ok_or_else(|| { - miette::miette!( - "platform table in {} is malformed", - consts::PROJECT_MANIFEST - ) - }) -} - -/// Retrieve a mutable reference to a target table `table_name` -/// for a specific platform. -fn get_toml_target_table<'a>( - doc: &'a mut Document, - platform: &Platform, - table_name: &str, -) -> miette::Result<&'a mut Table> { - let platform_table = doc["target"][platform.as_str()] - .as_table_mut() - .ok_or(miette::miette!( - "could not find {} in {}", - console::style(platform.as_str()).bold(), - consts::PROJECT_MANIFEST, - ))?; - - platform_table.set_dotted(true); - - platform_table[table_name] - .as_table_mut() - .ok_or(miette::miette!( - "could not find {} in {}", - console::style(format!("[target.{}.{}]", platform.as_str(), table_name)).bold(), - consts::PROJECT_MANIFEST, - )) -} - #[cfg(test)] mod tests { use super::*; - use crate::project::manifest::SystemRequirements; use insta::assert_debug_snapshot; - use rattler_conda_types::ChannelConfig; - use rattler_virtual_packages::{Archspec, Cuda, LibC, Linux, Osx, VirtualPackage}; + use rattler_virtual_packages::{LibC, VirtualPackage}; use std::str::FromStr; - use tempfile::tempdir; const PROJECT_BOILERPLATE: &str = r#" [project] name = "foo" version = "0.1.0" channels = [] - platforms = [] - "#; - - #[test] - fn test_main_project_config() { - let file_content = r#" - [project] - name = "pixi" - version = "0.0.2" - channels = ["conda-forge"] - platforms = ["linux-64", "win-64"] - "#; - - let project = Project::from_manifest_str(Path::new(""), file_content.to_string()).unwrap(); - - assert_eq!(project.name(), "pixi"); - assert_eq!( - project.version().as_ref().unwrap(), - &Version::from_str("0.0.2").unwrap() - ); - assert_eq!( - project.channels(), - [Channel::from_name( - "conda-forge", - None, - &ChannelConfig::default() - )] - ); - assert_eq!( - project.platforms(), - [ - Platform::from_str("linux-64").unwrap(), - Platform::from_str("win-64").unwrap() - ] - ); - } - #[test] - fn system_requirements_works() { - let file_content = r#" - windows = true - unix = true - linux = "5.11" - cuda = "12.2" - macos = "10.15" - archspec = "arm64" - libc = { family = "glibc", version = "2.12" } + platforms = ["linux-64", "win-64"] "#; - let system_requirements: SystemRequirements = - toml_edit::de::from_str(file_content).unwrap(); - - let expected_requirements: Vec = vec![ - VirtualPackage::Win, - VirtualPackage::Unix, - VirtualPackage::Linux(Linux { - version: Version::from_str("5.11").unwrap(), - }), - VirtualPackage::Cuda(Cuda { - version: Version::from_str("12.2").unwrap(), - }), - VirtualPackage::Osx(Osx { - version: Version::from_str("10.15").unwrap(), - }), - VirtualPackage::Archspec(Archspec { - spec: "arm64".to_string(), - }), - VirtualPackage::LibC(LibC { - version: Version::from_str("2.12").unwrap(), - family: "glibc".to_string(), - }), - ]; - - assert_eq!( - system_requirements.virtual_packages(), - expected_requirements - ); - } - #[test] fn test_system_requirements_edge_cases() { let file_contents = [ @@ -1127,45 +485,16 @@ mod tests { for file_content in file_contents { let file_content = format!("{PROJECT_BOILERPLATE}\n{file_content}"); - let project = Project::from_manifest_str(Path::new(""), &file_content).unwrap(); - + let manifest = Manifest::from_str(Path::new(""), &file_content).unwrap(); + let project = Project::from_manifest(manifest); let expected_result = vec![VirtualPackage::LibC(LibC { family: "glibc".to_string(), version: Version::from_str("2.12").unwrap(), })]; - let system_requirements = project.system_requirements(); - - assert_eq!(system_requirements, expected_result); - } - } - - #[test] - fn test_system_requirements_failing_edge_cases() { - let file_contents = [ - r#" - [system-requirements] - libc = { verion = "2.12" } - "#, - r#" - [system-requirements] - lib = "2.12" - "#, - r#" - [system-requirements.libc] - version = "2.12" - fam = "glibc" - "#, - r#" - [system-requirements.lic] - version = "2.12" - family = "glibc" - "#, - ]; + let virtual_packages = project.system_requirements().virtual_packages(); - for file_content in file_contents { - let file_content = format!("{PROJECT_BOILERPLATE}\n{file_content}"); - assert!(toml_edit::de::from_str::(&file_content).is_err()); + assert_eq!(virtual_packages, expected_result); } } @@ -1182,9 +511,10 @@ mod tests { bar = "1.0" "#; - let manifest = toml_edit::de::from_str::(&format!( - "{PROJECT_BOILERPLATE}\n{file_contents}" - )) + let manifest = Manifest::from_str( + Path::new(""), + format!("{PROJECT_BOILERPLATE}\n{file_contents}").as_str(), + ) .unwrap(); let project = Project::from_manifest(manifest); @@ -1212,10 +542,10 @@ mod tests { [target.linux-64.dependencies] wolflib = "1.0" "#; - - let manifest = toml_edit::de::from_str::(&format!( - "{PROJECT_BOILERPLATE}\n{file_contents}" - )) + let manifest = Manifest::from_str( + Path::new(""), + format!("{PROJECT_BOILERPLATE}\n{file_contents}").as_str(), + ) .unwrap(); let project = Project::from_manifest(manifest); @@ -1234,10 +564,10 @@ mod tests { [activation] scripts = ["pixi.toml", "pixi.lock"] "#; - - let manifest = toml_edit::de::from_str::(&format!( - "{PROJECT_BOILERPLATE}\n{file_contents}" - )) + let manifest = Manifest::from_str( + Path::new(""), + format!("{PROJECT_BOILERPLATE}\n{file_contents}").as_str(), + ) .unwrap(); let project = Project::from_manifest(manifest); @@ -1246,70 +576,6 @@ mod tests { assert_debug_snapshot!(project.activation_scripts(Platform::OsxArm64).unwrap()); } - #[test] - fn test_remove_target_dependencies() { - // Using known files in the project so the test succeed including the file check. - let file_contents = r#" - [project] - name = "foo" - version = "0.1.0" - channels = [] - platforms = ["linux-64", "win-64"] - - [dependencies] - fooz = "*" - - [target.win-64.dependencies] - bar = "*" - - [target.linux-64.build-dependencies] - baz = "*" - "#; - - let tmpdir = tempdir().unwrap(); - - let mut project = Project::from_manifest_str(tmpdir.path(), file_contents).unwrap(); - - project - .remove_target_dependency( - &PackageName::try_from("baz").unwrap(), - &SpecType::Build, - &Platform::Linux64, - ) - .unwrap(); - assert_debug_snapshot!(project.manifest); - } - - #[test] - fn test_remove_dependencies() { - // Using known files in the project so the test succeed including the file check. - let file_contents = r#" - [project] - name = "foo" - version = "0.1.0" - channels = [] - platforms = ["linux-64", "win-64"] - - [dependencies] - fooz = "*" - - [target.win-64.dependencies] - bar = "*" - - [target.linux-64.build-dependencies] - baz = "*" - "#; - - let tmpdir = tempdir().unwrap(); - - let mut project = Project::from_manifest_str(tmpdir.path(), file_contents).unwrap(); - - project - .remove_dependency(&PackageName::try_from("fooz").unwrap(), &SpecType::Run) - .unwrap(); - assert_debug_snapshot!(project.manifest); - } - #[test] fn test_target_specific_tasks() { // Using known files in the project so the test succeed including the file check. @@ -1323,15 +589,16 @@ mod tests { [target.linux-64.tasks] test = "test linux" "#; - - let manifest = toml_edit::de::from_str::(&format!( - "{PROJECT_BOILERPLATE}\n{file_contents}" - )) + let manifest = Manifest::from_str( + Path::new(""), + format!("{PROJECT_BOILERPLATE}\n{file_contents}").as_str(), + ) .unwrap(); + let project = Project::from_manifest(manifest); - assert_debug_snapshot!(project.tasks(Some(Platform::Osx64))); - assert_debug_snapshot!(project.tasks(Some(Platform::Win64))); - assert_debug_snapshot!(project.tasks(Some(Platform::Linux64))); + assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Osx64))); + assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Win64))); + assert_debug_snapshot!(project.manifest.tasks(Some(Platform::Linux64))); } } diff --git a/src/project/snapshots/pixi__project__manifest__test__remove_dependencies.snap b/src/project/snapshots/pixi__project__manifest__test__remove_dependencies.snap new file mode 100644 index 000000000..893233a8d --- /dev/null +++ b/src/project/snapshots/pixi__project__manifest__test__remove_dependencies.snap @@ -0,0 +1,66 @@ +--- +source: src/project/manifest.rs +expression: manifest.manifest.target +--- +{ + PixiSpanned { + span: Some( + 223..229, + ), + value: Platform( + Win64, + ), + }: TargetMetadata { + dependencies: { + "fooz": NamelessMatchSpec { + version: Some( + Any, + ), + build: None, + build_number: None, + file_name: None, + channel: None, + subdir: None, + namespace: None, + md5: None, + sha256: None, + }, + }, + host_dependencies: None, + build_dependencies: None, + pypi_dependencies: None, + activation: None, + tasks: {}, + }, + PixiSpanned { + span: Some( + 288..296, + ), + value: Platform( + Linux64, + ), + }: TargetMetadata { + dependencies: {}, + host_dependencies: None, + build_dependencies: Some( + { + "fooz": NamelessMatchSpec { + version: Some( + Any, + ), + build: None, + build_number: None, + file_name: None, + channel: None, + subdir: None, + namespace: None, + md5: None, + sha256: None, + }, + }, + ), + pypi_dependencies: None, + activation: None, + tasks: {}, + }, +} diff --git a/src/project/snapshots/pixi__project__tests__remove_target_dependencies.snap b/src/project/snapshots/pixi__project__manifest__test__remove_target_dependencies.snap similarity index 97% rename from src/project/snapshots/pixi__project__tests__remove_target_dependencies.snap rename to src/project/snapshots/pixi__project__manifest__test__remove_target_dependencies.snap index 73f613ce8..2fee4e008 100644 --- a/src/project/snapshots/pixi__project__tests__remove_target_dependencies.snap +++ b/src/project/snapshots/pixi__project__manifest__test__remove_target_dependencies.snap @@ -1,6 +1,6 @@ --- -source: src/project/mod.rs -expression: project.manifest +source: src/project/manifest.rs +expression: manifest.manifest --- ProjectManifest { project: ProjectMetadata { diff --git a/src/task/executable_task.rs b/src/task/executable_task.rs index 716a5cb48..63b8e49db 100644 --- a/src/task/executable_task.rs +++ b/src/task/executable_task.rs @@ -232,6 +232,7 @@ fn get_output_writer_and_handle() -> (ShellPipeWriter, JoinHandle) { #[cfg(test)] mod tests { use super::*; + use crate::project::manifest::Manifest; use std::path::Path; #[tokio::test] @@ -247,7 +248,8 @@ mod tests { task2 = {cmd="echo task2", depends_on=["root"]} top = {cmd="echo top", depends_on=["task1","task2"]} "#; - let project = Project::from_manifest_str(Path::new(""), file_content.to_string()).unwrap(); + let manifest = Manifest::from_str(Path::new(""), file_content.to_string()).unwrap(); + let project = Project::from_manifest(manifest); let executable_tasks = ExecutableTask::from_cmd_args( &project, @@ -288,7 +290,8 @@ mod tests { task2 = {cmd="echo task2", depends_on=["root"]} top = {cmd="echo top", depends_on=["task1","task2"]} "#; - let project = Project::from_manifest_str(Path::new(""), file_content.to_string()).unwrap(); + let manifest = Manifest::from_str(Path::new(""), file_content.to_string()).unwrap(); + let project = Project::from_manifest(manifest); let executable_tasks = ExecutableTask::from_cmd_args( &project, @@ -325,7 +328,8 @@ mod tests { [target.linux-64.tasks] root = {cmd="echo linux", depends_on=["task1"]} "#; - let project = Project::from_manifest_str(Path::new(""), file_content.to_string()).unwrap(); + let manifest = Manifest::from_str(Path::new(""), file_content.to_string()).unwrap(); + let project = Project::from_manifest(manifest); let executable_tasks = ExecutableTask::from_cmd_args( &project, @@ -355,7 +359,8 @@ mod tests { channels = ["conda-forge"] platforms = ["linux-64"] "#; - let project = Project::from_manifest_str(Path::new(""), file_content.to_string()).unwrap(); + let manifest = Manifest::from_str(Path::new(""), file_content.to_string()).unwrap(); + let project = Project::from_manifest(manifest); let executable_tasks = ExecutableTask::from_cmd_args( &project, diff --git a/src/virtual_packages.rs b/src/virtual_packages.rs index 81f763a32..f153aa6cb 100644 --- a/src/virtual_packages.rs +++ b/src/virtual_packages.rs @@ -112,7 +112,7 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the system requirements from the project manifest - let system_requirements = self.system_requirements_for_platform(platform); + let system_requirements = self.virtual_packages_for_platform(platform); // Combine the requirements, allowing the system requirements to overwrite the reference // virtual packages. diff --git a/tests/add_tests.rs b/tests/add_tests.rs index 1bd036dfa..4069419dc 100644 --- a/tests/add_tests.rs +++ b/tests/add_tests.rs @@ -93,14 +93,15 @@ async fn add_functionality_union() { let project = pixi.project().unwrap(); // Should contain all added dependencies - let (name, _) = project.manifest.dependencies.first().unwrap(); - assert_eq!(name, "rattler"); - let host_deps = project.manifest.host_dependencies.unwrap(); + let dependencies = project.dependencies(Platform::current()).unwrap(); + let (name, _) = dependencies.first().unwrap(); + assert_eq!(name, &PackageName::try_from("rattler").unwrap()); + let host_deps = project.host_dependencies(Platform::current()).unwrap(); let (name, _) = host_deps.first().unwrap(); - assert_eq!(name, "libcomputer"); - let build_deps = project.manifest.build_dependencies.unwrap(); + assert_eq!(name, &PackageName::try_from("libcomputer").unwrap()); + let build_deps = project.build_dependencies(Platform::current()).unwrap(); let (name, _) = build_deps.first().unwrap(); - assert_eq!(name, "libidk"); + assert_eq!(name, &PackageName::try_from("libidk").unwrap()); // Lock file should contain all packages as well let lock = pixi.lock_file().await.unwrap(); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index ff510a858..5b2bf113a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -170,7 +170,7 @@ impl PixiControl { /// Loads the project manifest and returns it. pub fn project(&self) -> miette::Result { - Project::load(&self.manifest_path()) + Project::load_or_else_discover(Some(&self.manifest_path())) } /// Get the path to the project @@ -287,7 +287,7 @@ impl PixiControl { /// Get the associated lock file pub async fn lock_file(&self) -> miette::Result { - let project = Project::load(&self.manifest_path())?; + let project = Project::load_or_else_discover(Some(&self.manifest_path()))?; pixi::lock_file::load_lock_file(&project).await } diff --git a/tests/task_tests.rs b/tests/task_tests.rs index 7b3625553..ab1a9146d 100644 --- a/tests/task_tests.rs +++ b/tests/task_tests.rs @@ -20,12 +20,13 @@ pub async fn add_remove_task() { .unwrap(); let project = pixi.project().unwrap(); - let task = project.manifest.tasks.get("test").unwrap(); + let tasks = project.tasks(None); + let task = tasks.get("test").unwrap(); assert!(matches!(task, Task::Plain(s) if s == "echo hello")); // Remove the task pixi.tasks().remove("test", None).await.unwrap(); - assert_eq!(pixi.project().unwrap().manifest.tasks.len(), 0); + assert_eq!(pixi.project().unwrap().tasks(None).len(), 0); } #[tokio::test] @@ -47,8 +48,9 @@ pub async fn add_command_types() { .unwrap(); let project = pixi.project().unwrap(); - let task2 = project.manifest.tasks.get("test2").unwrap(); - let task = project.manifest.tasks.get("test").unwrap(); + let tasks = project.tasks(None); + let task2 = tasks.get("test2").unwrap(); + let task = tasks.get("test").unwrap(); assert!(matches!(task2, Task::Execute(cmd) if matches!(cmd.cmd, CmdArgs::Single(_)))); assert!(matches!(task2, Task::Execute(cmd) if !cmd.depends_on.is_empty())); @@ -65,7 +67,8 @@ pub async fn add_command_types() { .execute() .unwrap(); let project = pixi.project().unwrap(); - let task = project.manifest.tasks.get("testing").unwrap(); + let tasks = project.tasks(None); + let task = tasks.get("testing").unwrap(); assert!(matches!(task, Task::Alias(a) if a.depends_on.get(0).unwrap() == "test")); } @@ -137,11 +140,9 @@ pub async fn add_remove_target_specific_task() { .await .unwrap(); assert_eq!( - pixi.project() - .unwrap() - .target_specific_tasks(Platform::Win64) - .len(), - 0 + project.tasks(Some(Platform::Win64)).len(), + // The default task is still there + 1 ); } From 0429c28b4d747cf71b4311f7045d127fecc1d6c5 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Sun, 17 Dec 2023 16:25:02 +0100 Subject: [PATCH 2/5] rename: manifest.manifest > manifest.parsed --- src/environment.rs | 2 +- src/project/manifest.rs | 36 ++++++++++++++++++------------------ src/project/mod.rs | 24 ++++++++++++------------ 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/environment.rs b/src/environment.rs index a15be8032..4fcc3a04c 100644 --- a/src/environment.rs +++ b/src/environment.rs @@ -92,7 +92,7 @@ pub async fn get_up_to_date_prefix( // Make sure the project supports the current platform let platform = Platform::current(); if !project.platforms().contains(&platform) { - let span = project.manifest.manifest.project.platforms.span(); + let span = project.manifest.parsed.project.platforms.span(); return Err(miette::miette!( help = format!( "The project needs to be configured to support your platform ({platform})." diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 42e0078bd..3b99c8139 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -38,7 +38,7 @@ pub struct Manifest { pub document: toml_edit::Document, /// The parsed manifest - pub manifest: ProjectManifest, + pub parsed: ProjectManifest, } impl Manifest { @@ -91,7 +91,7 @@ impl Manifest { path: root.join(consts::PROJECT_MANIFEST), contents, document, - manifest, + parsed: manifest, }) } @@ -109,7 +109,7 @@ impl Manifest { &self, platform: Platform, ) -> impl Iterator + '_ { - self.manifest + self.parsed .target .iter() .filter_map(move |(selector, manifest)| match selector.as_ref() { @@ -155,7 +155,7 @@ impl Manifest { let depends_on = task.depends_on(); for depends in depends_on { - if !self.manifest.tasks.contains_key(depends) { + if !self.parsed.tasks.contains_key(depends) { miette::bail!( "task '{}' for the depends on for '{}' does not exist", depends, @@ -167,7 +167,7 @@ impl Manifest { // Add the task to the table table.insert(name.as_ref(), task_as_toml(task.clone())); - self.manifest.tasks.insert(name.as_ref().to_string(), task); + self.parsed.tasks.insert(name.as_ref().to_string(), task); self.save()?; @@ -179,7 +179,7 @@ impl Manifest { let mut all_tasks = HashMap::default(); // Gather non-target specific tasks - all_tasks.extend(self.manifest.tasks.iter().map(|(k, v)| (k.as_str(), v))); + all_tasks.extend(self.parsed.tasks.iter().map(|(k, v)| (k.as_str(), v))); // Gather platform-specific tasks and overwrite them if they're double. if let Some(platform) = platform { @@ -238,7 +238,7 @@ impl Manifest { } // Add to manifest - self.manifest.project.platforms.value.extend(platforms); + self.parsed.project.platforms.value.extend(platforms); Ok(()) } @@ -367,7 +367,7 @@ impl Manifest { let (name, nameless) = self.add_dep_to_target_table(platform, toml_name.to_string(), spec)?; // Add to manifest - self.manifest + self.parsed .target .entry(TargetSelector::Platform(platform).into()) .or_insert(TargetMetadata::default()) @@ -385,7 +385,7 @@ impl Manifest { // Add to target table toml self.add_pypi_dep_to_target_table(platform, &name.clone(), requirement)?; // Add to manifest - self.manifest + self.parsed .target .entry(TargetSelector::Platform(platform).into()) .or_insert(TargetMetadata::default()) @@ -401,7 +401,7 @@ impl Manifest { let deps = &mut self.document[spec_type.name()]; let (name, nameless) = Manifest::add_to_deps_table(deps, spec)?; - self.manifest + self.parsed .create_or_get_dependencies(spec_type) .insert(name.as_source().into(), nameless); @@ -417,7 +417,7 @@ impl Manifest { let deps = &mut self.document[DependencyType::PypiDependency.name()]; Manifest::add_pypi_dep_to_table(deps, name, requirement)?; - self.manifest + self.parsed .create_or_get_pypi_dependencies() .insert(name.clone(), requirement.clone()); @@ -433,7 +433,7 @@ impl Manifest { if let Item::Table(ref mut t) = self.document[spec_type.name()] { if t.contains_key(dep.as_normalized()) && t.remove(dep.as_normalized()).is_some() { return self - .manifest + .parsed .remove_dependency(dep.as_normalized(), spec_type); } } @@ -454,7 +454,7 @@ impl Manifest { ) -> miette::Result<(String, NamelessMatchSpec)> { let table = get_toml_target_table(&mut self.document, platform, spec_type.name())?; table.remove(dep.as_normalized()); - self.manifest + self.parsed .remove_target_dependency(dep.as_normalized(), spec_type, platform) } @@ -482,7 +482,7 @@ impl Manifest { ) -> miette::Result<()> { let mut stored_channels = Vec::new(); for channel in channels { - self.manifest.project.channels.push( + self.parsed.project.channels.push( Channel::from_str(channel.as_ref(), &ChannelConfig::default()).into_diagnostic()?, ); stored_channels.push(channel.as_ref().to_owned()); @@ -1290,10 +1290,10 @@ mod test { let mut manifest = Manifest::from_str(tmpdir.path(), file_contents).unwrap(); manifest - .manifest + .parsed .remove_target_dependency("baz", &SpecType::Build, &Platform::Linux64) .unwrap(); - assert_debug_snapshot!(manifest.manifest); + assert_debug_snapshot!(manifest.parsed); } #[test] @@ -1324,8 +1324,8 @@ mod test { &SpecType::Run, ) .unwrap(); - assert!(manifest.manifest.dependencies.is_empty()); + assert!(manifest.parsed.dependencies.is_empty()); // Should still contain the fooz dependency in the target table - assert_debug_snapshot!(manifest.manifest.target); + assert_debug_snapshot!(manifest.parsed.target); } } diff --git a/src/project/mod.rs b/src/project/mod.rs index 5c125a6c9..e6612f821 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -148,12 +148,12 @@ impl Project { /// Returns the name of the project pub fn name(&self) -> &str { - &self.manifest.manifest.project.name + &self.manifest.parsed.project.name } /// Returns the version of the project pub fn version(&self) -> &Option { - &self.manifest.manifest.project.version + &self.manifest.parsed.project.version } /// Returns the root directory of the project @@ -183,12 +183,12 @@ impl Project { /// Returns the channels used by this project pub fn channels(&self) -> &[Channel] { - &self.manifest.manifest.project.channels + &self.manifest.parsed.project.channels } /// Returns the platforms this project targets pub fn platforms(&self) -> &[Platform] { - self.manifest.manifest.project.platforms.as_ref().as_slice() + self.manifest.parsed.project.platforms.as_ref().as_slice() } /// Get the tasks of this project @@ -205,7 +205,7 @@ impl Project { } } - self.manifest.manifest.tasks.get(name) + self.manifest.parsed.tasks.get(name) } /// Returns all tasks defined in the project for the given platform @@ -213,7 +213,7 @@ impl Project { let mut all_tasks = HashSet::new(); // Get all non-target specific tasks - all_tasks.extend(self.manifest.manifest.tasks.keys()); + all_tasks.extend(self.manifest.parsed.tasks.keys()); // Gather platform-specific tasks and overwrite the keys if they're double. if let Some(platform) = platform { @@ -245,7 +245,7 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[dependencies]` section) - let base_dependencies = self.manifest.manifest.dependencies.iter(); + let base_dependencies = self.manifest.parsed.dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self @@ -269,7 +269,7 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[build-dependencies]` section) - let base_dependencies = self.manifest.manifest.build_dependencies.iter(); + let base_dependencies = self.manifest.parsed.build_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self @@ -294,7 +294,7 @@ impl Project { platform: Platform, ) -> miette::Result> { // Get the base dependencies (defined in the `[host-dependencies]` section) - let base_dependencies = self.manifest.manifest.host_dependencies.iter(); + let base_dependencies = self.manifest.parsed.host_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self @@ -329,7 +329,7 @@ impl Project { platform: Platform, ) -> IndexMap<&rip::types::PackageName, &PyPiRequirement> { // Get the base pypi dependencies (defined in the `[pypi-dependencies]` section) - let base_pypi_dependencies = self.manifest.manifest.pypi_dependencies.iter(); + let base_pypi_dependencies = self.manifest.parsed.pypi_dependencies.iter(); // Get the platform specific dependencies in the order they were defined. let platform_specific = self @@ -389,7 +389,7 @@ impl Project { // Gather the main activation scripts if there are no target scripts defined. if all_scripts.is_empty() { - if let Some(activation) = &self.manifest.manifest.activation { + if let Some(activation) = &self.manifest.parsed.activation { if let Some(scripts) = &activation.scripts { all_scripts.extend(scripts.clone()); } @@ -418,7 +418,7 @@ impl Project { /// Get the system requirements defined under the `system-requirements` section of the project manifest. /// They will act as the description of a reference machine which is minimally needed for this package to be run. pub fn system_requirements(&self) -> &SystemRequirements { - &self.manifest.manifest.system_requirements + &self.manifest.parsed.system_requirements } /// Get the system requirements defined under the `system-requirements` section of the project manifest. From 937f2c8f6e99d2488f8b2ab74d9c1bc75e078f3f Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 18 Dec 2023 11:03:22 +0100 Subject: [PATCH 3/5] fix: use correct path to Project in documentation --- src/project/manifest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 3b99c8139..f60090fc4 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -24,7 +24,7 @@ use url::Url; /// This struct is responsible for reading, parsing, editing, and saving the manifest. /// It encapsulates all logic related to the manifest's TOML format and structure. /// The manifest data is represented as a [`ProjectManifest`] struct for easy manipulation. -/// Owned by the [`Project`] struct, which governs its usage. +/// Owned by the [`crate::project::Project`] struct, which governs its usage. /// #[derive(Debug, Clone)] pub struct Manifest { From dcfe2b16389b186b4152aaea4b0613fc5d5668a9 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 18 Dec 2023 11:04:34 +0100 Subject: [PATCH 4/5] fix: use &Path instead of PathBuf --- src/project/manifest.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/project/manifest.rs b/src/project/manifest.rs index f60090fc4..4650d1be2 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -43,8 +43,8 @@ pub struct Manifest { impl Manifest { /// Create a new manifest from a path - pub fn from_path(path: PathBuf) -> miette::Result { - let contents = std::fs::read_to_string(&path).into_diagnostic()?; + pub fn from_path(path: &Path) -> miette::Result { + let contents = std::fs::read_to_string(path).into_diagnostic()?; Self::from_str( path.parent().expect("Path should always have a parent"), contents, From 24976351f7fb7c00c6c9f9602549911a6c861266 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Mon, 18 Dec 2023 11:29:04 +0100 Subject: [PATCH 5/5] fix: use AsRef --- src/project/manifest.rs | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 4650d1be2..a020d448c 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -43,12 +43,13 @@ pub struct Manifest { impl Manifest { /// Create a new manifest from a path - pub fn from_path(path: &Path) -> miette::Result { - let contents = std::fs::read_to_string(path).into_diagnostic()?; - Self::from_str( - path.parent().expect("Path should always have a parent"), - contents, - ) + pub fn from_path(path: impl AsRef) -> miette::Result { + let contents = std::fs::read_to_string(path.as_ref()).into_diagnostic()?; + let parent = path + .as_ref() + .parent() + .expect("Path should always have a parent"); + Self::from_str(parent, contents) } /// Create a new manifest from a string @@ -1064,6 +1065,26 @@ mod test { platforms = [] "#; + #[test] + fn test_from_path() { + // Test the toml from a path + let dir = tempdir().unwrap(); + let path = dir.path().join("pixi.toml"); + std::fs::write(&path, PROJECT_BOILERPLATE).unwrap(); + // From &PathBuf + let _manifest = Manifest::from_path(&path).unwrap(); + // From &Path + let _manifest = Manifest::from_path(path.as_path()).unwrap(); + // From PathBuf + let manifest = Manifest::from_path(path).unwrap(); + + assert_eq!(manifest.parsed.project.name, "foo"); + assert_eq!( + manifest.parsed.project.version, + Some(Version::from_str("0.1.0").unwrap()) + ); + } + #[test] fn test_target_specific() { let contents = format!(