From ac4b3a55117b96110fcfd60b2df849f29e824960 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Wed, 12 Jul 2023 15:29:46 +0200 Subject: [PATCH 1/7] wip: first try at adding additional activation scripts. --- src/cli/global/install.rs | 1 + src/cli/run.rs | 23 +++++++++++++++++++---- src/cli/shell.rs | 7 +++++++ src/project/manifest.rs | 3 +++ src/project/mod.rs | 15 +++++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/cli/global/install.rs b/src/cli/global/install.rs index 31364a314..7145c22f3 100644 --- a/src/cli/global/install.rs +++ b/src/cli/global/install.rs @@ -98,6 +98,7 @@ fn create_activation_script(prefix: &Prefix, shell: ShellEnum) -> anyhow::Result conda_prefix: None, path: None, path_modification_behaviour: PathModificationBehaviour::Prepend, + additional_activation_scripts: None, })?; let script = format!("#!/bin/sh\n{}", result.script); Ok(script) diff --git a/src/cli/run.rs b/src/cli/run.rs index d495d3e22..0fd2ffe39 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -221,9 +221,19 @@ pub async fn get_task_env(project: &Project) -> anyhow::Result anyhow::Result anyhow::Result> { +async fn run_activation( + prefix: Prefix, + additional_activation_scripts: Vec, +) -> anyhow::Result> { let activator_result = tokio::task::spawn_blocking(move || { // Run and cache the activation script let shell: ShellEnum = ShellEnum::default(); @@ -254,6 +267,8 @@ async fn run_activation(prefix: Prefix) -> anyhow::Result anyhow::Result<()> { // Construct an activator so we can run commands from the environment let prefix = get_up_to_date_prefix(&project).await?; + let activation_scripts = project + .activation_scripts() + .into_iter() + .map(|p| p.clone()) + .collect(); let activator = Activator::from_path(prefix.root(), shell.clone(), Platform::current())?; let activator_result = activator.activation(ActivationVariables { @@ -34,6 +39,8 @@ pub async fn execute(args: Args) -> anyhow::Result<()> { // Prepending environment paths so they get found first. path_modification_behaviour: PathModificationBehaviour::Prepend, + + additional_activation_scripts: Some(activation_scripts), })?; // Generate a temporary file with the script to execute. This includes the activation of the diff --git a/src/project/manifest.rs b/src/project/manifest.rs index ddd723cb9..a8d6708bf 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -12,6 +12,7 @@ use serde_with::de::DeserializeAsWrap; use serde_with::{serde_as, DeserializeAs, DisplayFromStr, PickFirst}; use std::collections::HashMap; use std::ops::Range; +use std::path::PathBuf; /// Describes the contents of a project manifest. #[serde_as] @@ -162,6 +163,8 @@ pub struct ProjectMetadata { // TODO: This is actually slightly different from the rattler_conda_types::Platform because it // should not include noarch. pub platforms: Spanned>, + + pub activation_scripts: Option>, } #[serde_as] diff --git a/src/project/mod.rs b/src/project/mod.rs index f0b965942..c7245ec75 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -500,6 +500,21 @@ impl Project { self.manifest.project.platforms.as_ref().as_slice() } + pub fn activation_scripts(&self) -> Vec { + let mut full_paths = vec![]; + match &self.manifest.project.activation_scripts { + None => full_paths, + Some(scripts) => { + if !scripts.is_empty() { + for path in scripts { + full_paths.push(self.root.join(path)); + } + } + full_paths + } + } + } + /// Get the task with the specified name or `None` if no such task exists. pub fn task_opt(&self, name: &str) -> Option<&Task> { self.manifest.tasks.get(name) From 339a2fabf50d813e22ece01d4690cc5f74a2226c Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 13 Jul 2023 09:25:52 +0200 Subject: [PATCH 2/7] wip: start adding result to the additional scripts --- src/project/mod.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/project/mod.rs b/src/project/mod.rs index c7245ec75..a6d47ce6b 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -7,7 +7,7 @@ use crate::consts::PROJECT_MANIFEST; use crate::project::manifest::{ProjectManifest, TargetMetadata, TargetSelector}; use crate::report_error::ReportError; use crate::task::{CmdArgs, Task}; -use anyhow::Context; +use anyhow::{anyhow, Context}; use ariadne::{Label, Report, ReportKind, Source}; use indexmap::IndexMap; use rattler_conda_types::{ @@ -500,17 +500,21 @@ impl Project { self.manifest.project.platforms.as_ref().as_slice() } - pub fn activation_scripts(&self) -> Vec { + pub fn activation_scripts(&self) -> anyhow::Result> { let mut full_paths = vec![]; match &self.manifest.project.activation_scripts { - None => full_paths, + None => Ok(full_paths), Some(scripts) => { if !scripts.is_empty() { for path in scripts { - full_paths.push(self.root.join(path)); + if self.root().join(path).exists() { + println!("{:?} exists", path); + full_paths.push(self.root.join(path)); + } else { + } } } - full_paths + Ok(full_paths) } } } From 32dcf18ba4cc166db82bbe2ee167dfe903846656 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 13 Jul 2023 14:53:30 +0200 Subject: [PATCH 3/7] fix: result is checked --- src/cli/run.rs | 2 +- src/cli/shell.rs | 2 +- src/project/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/run.rs b/src/cli/run.rs index 0fd2ffe39..d2a8df325 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -221,7 +221,7 @@ pub async fn get_task_env(project: &Project) -> anyhow::Result anyhow::Result<()> { // Construct an activator so we can run commands from the environment let prefix = get_up_to_date_prefix(&project).await?; let activation_scripts = project - .activation_scripts() + .activation_scripts()? .into_iter() .map(|p| p.clone()) .collect(); diff --git a/src/project/mod.rs b/src/project/mod.rs index a6d47ce6b..6abfccc46 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -7,7 +7,7 @@ use crate::consts::PROJECT_MANIFEST; use crate::project::manifest::{ProjectManifest, TargetMetadata, TargetSelector}; use crate::report_error::ReportError; use crate::task::{CmdArgs, Task}; -use anyhow::{anyhow, Context}; +use anyhow::Context; use ariadne::{Label, Report, ReportKind, Source}; use indexmap::IndexMap; use rattler_conda_types::{ From a6b92711edb63ec4fec952e69c703dadf4c60b43 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Thu, 13 Jul 2023 19:31:48 +0200 Subject: [PATCH 4/7] fix: error when activation script is not found --- src/cli/run.rs | 10 +-- src/cli/shell.rs | 3 +- src/project/manifest.rs | 39 ++++++++- src/project/mod.rs | 80 ++++++++++++++---- ...t__manifest__test__activation_scripts.snap | 83 +++++++++++++++++++ ...ect__manifest__test__dependency_types.snap | 1 + ...ject__manifest__test__target_specific.snap | 3 + ..._project__tests__activation_scripts-2.snap | 7 ++ ..._project__tests__activation_scripts-3.snap | 8 ++ ...i__project__tests__activation_scripts.snap | 7 ++ 10 files changed, 214 insertions(+), 27 deletions(-) create mode 100644 src/project/snapshots/pixi__project__manifest__test__activation_scripts.snap create mode 100644 src/project/snapshots/pixi__project__tests__activation_scripts-2.snap create mode 100644 src/project/snapshots/pixi__project__tests__activation_scripts-3.snap create mode 100644 src/project/snapshots/pixi__project__tests__activation_scripts.snap diff --git a/src/cli/run.rs b/src/cli/run.rs index f457fda35..fdc8dda97 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -221,16 +221,10 @@ pub async fn get_task_env(project: &Project) -> miette::Result miette::Result<()> { // Construct an activator so we can run commands from the environment let prefix = get_up_to_date_prefix(&project).await?; let activation_scripts = project - .activation_scripts()? + .activation_scripts(Platform::current())? .into_iter() - .map(|p| p.clone()) .collect(); let activator = Activator::from_path(prefix.root(), shell.clone(), Platform::current()) .into_diagnostic()?; diff --git a/src/project/manifest.rs b/src/project/manifest.rs index 1411e97b7..7e09bcc20 100644 --- a/src/project/manifest.rs +++ b/src/project/manifest.rs @@ -10,7 +10,6 @@ use serde_with::de::DeserializeAsWrap; use serde_with::{serde_as, DeserializeAs, DisplayFromStr, PickFirst}; use std::collections::HashMap; use std::ops::Range; -use std::path::PathBuf; /// Describes the contents of a project manifest. #[serde_as] @@ -57,6 +56,12 @@ pub struct ProjectManifest { /// manifest. #[serde(default)] pub target: IndexMap, TargetMetadata>, + + /// Environment activation information. + /// + /// We use an [`IndexMap`] to preserve the order in which the items where defined in the + /// manifest. + pub activation: Option, } impl ProjectManifest { @@ -132,6 +137,10 @@ pub struct TargetMetadata { #[serde(default, rename = "build-dependencies")] #[serde_as(as = "Option>")] pub build_dependencies: Option>, + + /// Additional information to activate an environment. + #[serde(default)] + pub activation: Option, } /// Describes the contents of the `[package]` section of the project manifest. @@ -160,8 +169,6 @@ pub struct ProjectMetadata { // TODO: This is actually slightly different from the rattler_conda_types::Platform because it // should not include noarch. pub platforms: Spanned>, - - pub activation_scripts: Option>, } #[serde_as] @@ -258,6 +265,11 @@ impl From for LibC { } } } +#[derive(Default, Clone, Deserialize, Debug)] +#[serde(deny_unknown_fields)] +pub struct Activation { + pub scripts: Option>, +} // Create an error report for usign a platform that is not supported by the project. fn create_unsupported_platform_report( @@ -348,4 +360,25 @@ mod test { .collect::>() .join("\n")) } + + #[test] + fn test_activation_scripts() { + let contents = format!( + r#" + {PROJECT_BOILERPLATE} + [activation] + scripts = [".pixi/install/setup.sh"] + + [target.win-64.activation] + scripts = [".pixi/install/setup.ps1"] + + [target.linux-64.activation] + scripts = [".pixi/install/setup.sh", "test"] + "# + ); + + assert_debug_snapshot!( + toml_edit::de::from_str::(&contents).expect("parsing should succeed!") + ); + } } diff --git a/src/project/mod.rs b/src/project/mod.rs index 576966ee1..5c385f06e 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -503,23 +503,46 @@ impl Project { self.manifest.project.platforms.as_ref().as_slice() } - pub fn activation_scripts(&self) -> miette::Result> { - let mut full_paths = vec![]; - match &self.manifest.project.activation_scripts { - None => Ok(full_paths), - Some(scripts) => { - if !scripts.is_empty() { - for path in scripts { - if self.root().join(path).exists() { - println!("{:?} exists", path); - full_paths.push(self.root.join(path)); - } else { - } - } + /// 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) { + if let Some(activation) = &target_metadata.activation { + if let Some(scripts) = &activation.scripts { + all_scripts.extend(scripts.clone()); } - Ok(full_paths) } } + + // 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(scripts) = &activation.scripts { + all_scripts.extend(scripts.clone()); + } + } + } + + // Check if scripts exist + let mut missing_scripts = Vec::new(); + for script_name in &all_scripts { + let script_path = self.root().join(script_name); + if script_path.exists() { + full_paths.push(script_path); + tracing::debug!("Found activation script: {:?}", script_name); + } else { + missing_scripts.push(script_name); + } + } + + if !missing_scripts.is_empty() { + miette::bail!("Can't find activation scripts: {:?}", missing_scripts); + } + + Ok(full_paths) } /// Get the task with the specified name or `None` if no such task exists. @@ -761,4 +784,33 @@ mod tests { assert_debug_snapshot!(project.all_dependencies(Platform::Linux64).unwrap()); } + #[test] + fn test_activation_scripts() { + // Using known files in the project so the test succeed including the file check. + let file_contents = r#" + [target.linux-64.activation] + scripts = ["Cargo.toml"] + + [target.win-64.activation] + scripts = ["Cargo.lock"] + + [activation] + scripts = ["pixi.toml", "pixi.lock"] + "#; + + let manifest = toml_edit::de::from_str::(&format!( + "{PROJECT_BOILERPLATE}\n{file_contents}" + )) + .unwrap(); + let project = Project { + root: Default::default(), + source: "".to_string(), + doc: Default::default(), + manifest, + }; + + assert_debug_snapshot!(project.activation_scripts(Platform::Linux64).unwrap()); + assert_debug_snapshot!(project.activation_scripts(Platform::Win64).unwrap()); + assert_debug_snapshot!(project.activation_scripts(Platform::OsxArm64).unwrap()); + } } diff --git a/src/project/snapshots/pixi__project__manifest__test__activation_scripts.snap b/src/project/snapshots/pixi__project__manifest__test__activation_scripts.snap new file mode 100644 index 000000000..a9248025f --- /dev/null +++ b/src/project/snapshots/pixi__project__manifest__test__activation_scripts.snap @@ -0,0 +1,83 @@ +--- +source: src/project/manifest.rs +expression: "toml_edit::de::from_str::(&contents).expect(\"parsing should succeed!\")" +--- +ProjectManifest { + project: ProjectMetadata { + name: "foo", + version: Version { + version: [[0], [0], [1], [0]], + local: [], + }, + description: None, + authors: [], + channels: [], + platforms: Spanned { + span: 121..123, + value: [], + }, + }, + tasks: {}, + system_requirements: SystemRequirements { + windows: None, + unix: None, + macos: None, + linux: None, + cuda: None, + libc: None, + archspec: None, + }, + dependencies: {}, + host_dependencies: None, + build_dependencies: None, + target: { + Spanned { + span: 228..234, + value: Platform( + Win64, + ), + }: TargetMetadata { + dependencies: {}, + host_dependencies: None, + build_dependencies: None, + activation: Some( + Activation { + scripts: Some( + [ + ".pixi/install/setup.ps1", + ], + ), + }, + ), + }, + Spanned { + span: 318..326, + value: Platform( + Linux64, + ), + }: TargetMetadata { + dependencies: {}, + host_dependencies: None, + build_dependencies: None, + activation: Some( + Activation { + scripts: Some( + [ + ".pixi/install/setup.sh", + "test", + ], + ), + }, + ), + }, + }, + activation: Some( + Activation { + scripts: Some( + [ + ".pixi/install/setup.sh", + ], + ), + }, + ), +} diff --git a/src/project/snapshots/pixi__project__manifest__test__dependency_types.snap b/src/project/snapshots/pixi__project__manifest__test__dependency_types.snap index 6a366ef24..88c125325 100644 --- a/src/project/snapshots/pixi__project__manifest__test__dependency_types.snap +++ b/src/project/snapshots/pixi__project__manifest__test__dependency_types.snap @@ -83,4 +83,5 @@ ProjectManifest { }, ), target: {}, + activation: None, } diff --git a/src/project/snapshots/pixi__project__manifest__test__target_specific.snap b/src/project/snapshots/pixi__project__manifest__test__target_specific.snap index 4771ff41d..17cd18a35 100644 --- a/src/project/snapshots/pixi__project__manifest__test__target_specific.snap +++ b/src/project/snapshots/pixi__project__manifest__test__target_specific.snap @@ -60,6 +60,7 @@ ProjectManifest { }, host_dependencies: None, build_dependencies: None, + activation: None, }, Spanned { span: 206..212, @@ -90,6 +91,8 @@ ProjectManifest { }, host_dependencies: None, build_dependencies: None, + activation: None, }, }, + activation: None, } diff --git a/src/project/snapshots/pixi__project__tests__activation_scripts-2.snap b/src/project/snapshots/pixi__project__tests__activation_scripts-2.snap new file mode 100644 index 000000000..6cae18912 --- /dev/null +++ b/src/project/snapshots/pixi__project__tests__activation_scripts-2.snap @@ -0,0 +1,7 @@ +--- +source: src/project/mod.rs +expression: "project.activation_scripts(Platform::Win64).unwrap()" +--- +[ + "Cargo.lock", +] diff --git a/src/project/snapshots/pixi__project__tests__activation_scripts-3.snap b/src/project/snapshots/pixi__project__tests__activation_scripts-3.snap new file mode 100644 index 000000000..c89eb90fe --- /dev/null +++ b/src/project/snapshots/pixi__project__tests__activation_scripts-3.snap @@ -0,0 +1,8 @@ +--- +source: src/project/mod.rs +expression: "project.activation_scripts(Platform::OsxArm64).unwrap()" +--- +[ + "pixi.toml", + "pixi.lock", +] diff --git a/src/project/snapshots/pixi__project__tests__activation_scripts.snap b/src/project/snapshots/pixi__project__tests__activation_scripts.snap new file mode 100644 index 000000000..662c5c370 --- /dev/null +++ b/src/project/snapshots/pixi__project__tests__activation_scripts.snap @@ -0,0 +1,7 @@ +--- +source: src/project/mod.rs +expression: "project.activation_scripts(Platform::Linux64).unwrap()" +--- +[ + "Cargo.toml", +] From c105fc60027dd3ac92635dff03592dffc927de4c Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 14 Jul 2023 10:52:27 +0200 Subject: [PATCH 5/7] fix: just extend the activation script list instead of adding it to the activation variables --- src/cli/global/install.rs | 1 - src/cli/run.rs | 7 ++++--- src/cli/shell.rs | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/cli/global/install.rs b/src/cli/global/install.rs index 28d266322..03871baf3 100644 --- a/src/cli/global/install.rs +++ b/src/cli/global/install.rs @@ -105,7 +105,6 @@ fn create_activation_script(prefix: &Prefix, shell: ShellEnum) -> miette::Result conda_prefix: None, path: None, path_modification_behaviour: PathModificationBehaviour::Prepend, - additional_activation_scripts: None, }) .into_diagnostic()?; let script = format!("#!/bin/sh\n{}", result.script); diff --git a/src/cli/run.rs b/src/cli/run.rs index fdc8dda97..a7c4aefa7 100644 --- a/src/cli/run.rs +++ b/src/cli/run.rs @@ -249,7 +249,10 @@ async fn run_activation( let shell: ShellEnum = ShellEnum::default(); // Construct an activator for the script - let activator = Activator::from_path(prefix.root(), shell, Platform::current())?; + let mut activator = Activator::from_path(prefix.root(), shell, Platform::current())?; + activator + .activation_scripts + .extend(additional_activation_scripts); // Run the activation activator.run_activation(ActivationVariables { @@ -261,8 +264,6 @@ async fn run_activation( // Prepending environment paths so they get found first. path_modification_behaviour: PathModificationBehaviour::Prepend, - - additional_activation_scripts: Some(additional_activation_scripts), }) }) .await diff --git a/src/cli/shell.rs b/src/cli/shell.rs index b71c0eed5..5fea8e23f 100644 --- a/src/cli/shell.rs +++ b/src/cli/shell.rs @@ -24,13 +24,15 @@ pub async fn execute(args: Args) -> miette::Result<()> { // Construct an activator so we can run commands from the environment let prefix = get_up_to_date_prefix(&project).await?; - let activation_scripts = project + let activation_scripts: Vec<_> = project .activation_scripts(Platform::current())? .into_iter() .collect(); - let activator = Activator::from_path(prefix.root(), shell.clone(), Platform::current()) + let mut activator = Activator::from_path(prefix.root(), shell.clone(), Platform::current()) .into_diagnostic()?; + activator.activation_scripts.extend(activation_scripts); + let activator_result = activator .activation(ActivationVariables { // Get the current PATH variable @@ -41,8 +43,6 @@ pub async fn execute(args: Args) -> miette::Result<()> { // Prepending environment paths so they get found first. path_modification_behaviour: PathModificationBehaviour::Prepend, - - additional_activation_scripts: Some(activation_scripts), }) .into_diagnostic()?; From fea6552cfbfff49031df3aa0898573bfbc3789cf Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 14 Jul 2023 10:53:06 +0200 Subject: [PATCH 6/7] fix: just throw warning instead of error when there is no activation script --- src/project/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/mod.rs b/src/project/mod.rs index 5c385f06e..1f5e9ce3e 100644 --- a/src/project/mod.rs +++ b/src/project/mod.rs @@ -539,7 +539,7 @@ impl Project { } if !missing_scripts.is_empty() { - miette::bail!("Can't find activation scripts: {:?}", missing_scripts); + tracing::warn!("can't find activation scripts: {:?}", missing_scripts); } Ok(full_paths) From f42b92c88990174230d014cb5c2f0afb44ea00d5 Mon Sep 17 00:00:00 2001 From: Ruben Arts Date: Fri, 14 Jul 2023 11:02:53 +0200 Subject: [PATCH 7/7] misc: set pixi's default tracing level to warn --- src/cli/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index a3a4d1115..7dbf22bc6 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -85,7 +85,9 @@ pub async fn execute() -> miette::Result<()> { .from_env() .into_diagnostic()? // filter logs from apple codesign because they are very noisy - .add_directive("apple_codesign=off".parse().into_diagnostic()?); + .add_directive("apple_codesign=off".parse().into_diagnostic()?) + // set pixi's tracing level to warn + .add_directive("pixi=warn".parse().into_diagnostic()?); // Setup the tracing subscriber tracing_subscriber::fmt()