Skip to content

Commit

Permalink
refactor: extract activation scripts into Environment (#659)
Browse files Browse the repository at this point in the history
Final refactor of the project model. This moves the activation scripts
to `Environment` as well.
  • Loading branch information
baszalmstra authored Jan 12, 2024
1 parent 0b92af9 commit 9fbce52
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 47 deletions.
16 changes: 15 additions & 1 deletion src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,21 @@ pub async fn run_activation_async(
prefix: Prefix,
) -> miette::Result<HashMap<String, String>> {
let platform = Platform::current();
let additional_activation_scripts = project.activation_scripts(platform)?;
let additional_activation_scripts = project.activation_scripts(Some(platform));

// Make sure the scripts exists
let (additional_activation_scripts, missing_scripts): (Vec<_>, _) =
additional_activation_scripts
.into_iter()
.map(|script| project.root().join(script))
.partition(|full_path| full_path.is_file());

if !missing_scripts.is_empty() {
tracing::warn!(
"Could not find activation scripts: {}",
missing_scripts.iter().map(|p| p.display()).format(", ")
);
}

// Check if the platform and activation script extension match. For Platform::Windows the extension should be .bat and for All other platforms it should be .sh or .bash.
for script in additional_activation_scripts.iter() {
Expand Down
48 changes: 48 additions & 0 deletions src/project/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ impl<'p> Environment<'p> {
})
}

/// Returns the activation scripts that should be run when activating this environment.
///
/// The activation scripts of all features are combined in the order they are defined for the
/// environment.
pub fn activation_scripts(&self, platform: Option<Platform>) -> Vec<String> {
self.features()
.filter_map(|f| f.activation_scripts(platform))
.flatten()
.cloned()
.collect()
}

/// Validates that the given platform is supported by this environment.
fn validate_platform_support(
&self,
Expand Down Expand Up @@ -391,6 +403,42 @@ mod test {
assert_display_snapshot!(format_dependencies(deps));
}

#[test]
fn test_activation() {
let manifest = Project::from_str(
Path::new(""),
r#"
[project]
name = "foobar"
channels = []
platforms = ["linux-64", "osx-64"]
[activation]
scripts = ["default.bat"]
[target.linux-64.activation]
scripts = ["linux.bat"]
[feature.foo.activation]
scripts = ["foo.bat"]
[environments]
foo = ["foo"]
"#,
)
.unwrap();

let foo_env = manifest.environment("foo").unwrap();
assert_eq!(
foo_env.activation_scripts(None),
vec!["foo.bat".to_string(), "default.bat".to_string()]
);
assert_eq!(
foo_env.activation_scripts(Some(Platform::Linux64)),
vec!["foo.bat".to_string(), "linux.bat".to_string()]
);
}

#[test]
fn test_channel_priorities() {
let manifest = Project::from_str(
Expand Down
46 changes: 46 additions & 0 deletions src/project/manifest/feature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,18 @@ impl Feature {
}
})
}

/// Returns the activation scripts for the most specific target that matches the given
/// `platform`.
///
/// Returns `None` if this feature does not define any target with an activation.
pub fn activation_scripts(&self, platform: Option<Platform>) -> Option<&Vec<String>> {
self.targets
.resolve(platform)
.filter_map(|t| t.activation.as_ref())
.filter_map(|a| a.scripts.as_ref())
.next()
}
}

impl<'de> Deserialize<'de> for Feature {
Expand Down Expand Up @@ -287,4 +299,38 @@ mod test {
"[feature.bla] combined dependencies should also be borrowed"
);
}

#[test]
fn test_activation() {
let manifest = Manifest::from_str(
Path::new(""),
r#"
[project]
name = "foo"
platforms = ["linux-64", "osx-64", "win-64"]
channels = []
[activation]
scripts = ["run.bat"]
[target.linux-64.activation]
scripts = ["linux-64.bat"]
"#,
)
.unwrap();

assert_eq!(
manifest.default_feature().activation_scripts(None).unwrap(),
&vec!["run.bat".to_string()],
"should have selected the activation from the [activation] section"
);
assert_eq!(
manifest
.default_feature()
.activation_scripts(Some(Platform::Linux64))
.unwrap(),
&vec!["linux-64.bat".to_string()],
"should have selected the activation from the [linux-64] section"
);
}
}
59 changes: 13 additions & 46 deletions src/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pub mod metadata;
pub mod virtual_packages;

use indexmap::{Equivalent, IndexMap, IndexSet};
use itertools::Itertools;
use miette::{IntoDiagnostic, NamedSource, WrapErr};
use once_cell::sync::OnceCell;
use rattler_conda_types::{
Expand Down Expand Up @@ -299,6 +298,13 @@ impl Project {
self.default_environment().pypi_dependencies(platform)
}

/// Returns the all specified activation scripts that are used in the current platform.
///
/// TODO: Remove this function and use the `activation_scripts function from the default environment instead.
pub fn activation_scripts(&self, platform: Option<Platform>) -> Vec<String> {
self.default_environment().activation_scripts(platform)
}

/// Returns true if the project contains any reference pypi dependencies. Even if just
/// `[pypi-dependencies]` is specified without any requirements this will return true.
pub fn has_pypi_dependencies(&self) -> bool {
Expand Down Expand Up @@ -331,43 +337,6 @@ impl Project {
})?
.as_ref())
}

/// Returns the all specified activation scripts that are used in the current platform.
pub fn activation_scripts(&self, platform: Platform) -> miette::Result<Vec<PathBuf>> {
let feature = self.manifest.default_feature();

// Select the platform-specific activation scripts that is most specific
let activation = feature
.targets
.resolve(Some(platform))
.filter_map(|target| target.activation.as_ref())
.next();

// Get the activation scripts
let all_scripts = activation
.into_iter()
.flat_map(|activation| activation.scripts.iter().flatten())
.collect_vec();

// Check if scripts exist
let mut full_paths = Vec::new();
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() {
tracing::warn!("can't find activation scripts: {:?}", missing_scripts);
}

Ok(full_paths)
}
}

/// Iterates over the current directory and all its parent directories and returns the first
Expand Down Expand Up @@ -404,6 +373,7 @@ impl Display for DependencyKind {
mod tests {
use super::*;
use insta::{assert_debug_snapshot, assert_display_snapshot};
use itertools::Itertools;
use rattler_virtual_packages::{LibC, VirtualPackage};
use std::str::FromStr;

Expand Down Expand Up @@ -519,11 +489,8 @@ mod tests {

#[test]
fn test_activation_scripts() {
fn fmt_activation_scripts(scripts: Vec<PathBuf>) -> String {
scripts
.iter()
.format_with("\n", |p, f| f(&format_args!("{}", p.display())))
.to_string()
fn fmt_activation_scripts(scripts: Vec<String>) -> String {
scripts.iter().join("\n")
}

// Using known files in the project so the test succeed including the file check.
Expand All @@ -546,9 +513,9 @@ mod tests {

assert_display_snapshot!(format!(
"= Linux64\n{}\n\n= Win64\n{}\n\n= OsxArm64\n{}",
fmt_activation_scripts(project.activation_scripts(Platform::Linux64).unwrap()),
fmt_activation_scripts(project.activation_scripts(Platform::Win64).unwrap()),
fmt_activation_scripts(project.activation_scripts(Platform::OsxArm64).unwrap())
fmt_activation_scripts(project.activation_scripts(Some(Platform::Linux64))),
fmt_activation_scripts(project.activation_scripts(Some(Platform::Win64))),
fmt_activation_scripts(project.activation_scripts(Some(Platform::OsxArm64)))
));
}

Expand Down

0 comments on commit 9fbce52

Please sign in to comment.