Skip to content

Commit

Permalink
fix: Audit 02/20 (#1838)
Browse files Browse the repository at this point in the history
* Update aliases.

* Add tests.

* Fix lints.

* Handle git bash.

* Fix windows.

* Fix.
  • Loading branch information
milesj authored Feb 21, 2025
1 parent 79250fd commit 2863ebc
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 26 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## Unreleased

#### 🐞 Fixes

- Fixed an issue where aliases would not be inherited for some toolchains.
- Fixed some token variable interpolation for paths on Windows.

## 1.32.5

#### 🚀 Updates
Expand Down
13 changes: 12 additions & 1 deletion crates/project-graph/tests/project_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,18 @@ mod project_graph {
let graph = generate_workspace_graph("expansion").await;
let task = graph.get_task_from_project("tasks", "build").unwrap();

assert_eq!(task.args, string_vec!["a", "../other.yaml", "b"]);
assert_eq!(
task.args,
string_vec![
"a",
if cfg!(windows) {
"..\\other.yaml"
} else {
"../other.yaml"
},
"b"
]
);

assert_eq!(
task.input_files,
Expand Down
13 changes: 11 additions & 2 deletions crates/task-expander/src/task_expander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,17 @@ impl<'graph> TaskExpander<'graph> {
"Loading environment variables from .env files",
);

let mut missing_paths = vec![];
let mut merged_env_vars = FxHashMap::default();

// The file may not have been committed, so avoid crashing
for env_path in env_paths {
if env_path.exists() {
trace!(
task_target = task.target.as_str(),
env_file = ?env_path,
"Loading .env file",
);

let handle_error = |error: dotenvy::Error| TasksExpanderError::InvalidEnvFile {
path: env_path.to_path_buf(),
error: Box::new(error),
Expand All @@ -179,7 +184,11 @@ impl<'graph> TaskExpander<'graph> {
merged_env_vars.insert(key, val);
}
} else {
missing_paths.push(env_path);
trace!(
task_target = task.target.as_str(),
env_file = ?env_path,
"Skipping .env file because it doesn't exist",
);
}
}

Expand Down
32 changes: 28 additions & 4 deletions crates/task-expander/src/token_expander.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use std::borrow::Cow;
use std::env;
use std::mem;
use std::path::Path;
use tracing::{debug, instrument, warn};

#[derive(Debug, Default, PartialEq)]
Expand Down Expand Up @@ -580,8 +581,8 @@ impl<'graph> TokenExpander<'graph> {
"arch" => Cow::Borrowed(env::consts::ARCH),
"os" => Cow::Borrowed(env::consts::OS),
"osFamily" => Cow::Borrowed(env::consts::FAMILY),
"workingDir" => Cow::Owned(path::to_string(&self.context.working_dir)?),
"workspaceRoot" => Cow::Owned(path::to_string(&self.context.workspace_root)?),
"workingDir" => Cow::Owned(self.stringify_path(&self.context.working_dir)?),
"workspaceRoot" => Cow::Owned(self.stringify_path(&self.context.workspace_root)?),
// Project
"language" => Cow::Owned(project.language.to_string()),
"project" => Cow::Borrowed(project.id.as_str()),
Expand All @@ -592,7 +593,7 @@ impl<'graph> TokenExpander<'graph> {
"projectChannel" => get_metadata(|md| md.channel.as_deref()),
"projectName" => get_metadata(|md| md.name.as_deref()),
"projectOwner" => get_metadata(|md| md.owner.as_deref()),
"projectRoot" => Cow::Owned(path::to_string(&project.root)?),
"projectRoot" => Cow::Owned(self.stringify_path(&project.root)?),
"projectSource" => Cow::Borrowed(project.source.as_str()),
"projectStack" => Cow::Owned(project.stack.to_string()),
"projectType" => Cow::Owned(project.type_of.to_string()),
Expand Down Expand Up @@ -738,10 +739,33 @@ impl<'graph> TokenExpander<'graph> {
} else {
let abs_path = path.to_logical_path(&self.context.workspace_root);

path::to_virtual_string(diff_paths(&abs_path, &self.project.root).unwrap_or(abs_path))
self.stringify_path(&diff_paths(&abs_path, &self.project.root).unwrap_or(abs_path))
}
}

fn stringify_path(&self, orig_value: &Path) -> miette::Result<String> {
let value = path::to_string(orig_value)?;

// https://cygwin.com/cygwin-ug-net/cygpath.html
#[cfg(windows)]
if env::var("MSYSTEM").is_ok_and(|value| value == "MINGW32" || value == "MINGW64") {
let mut value = moon_common::path::standardize_separators(value);

if orig_value.is_absolute() {
for drive in 'A'..='Z' {
if let Some(suffix) = value.strip_prefix(&format!("{drive}:/")) {
value = format!("/{}/{suffix}", drive.to_ascii_lowercase());
break;
}
}
}

return Ok(value);
}

Ok(value)
}

fn infer_inputs(&self, task: &mut Task, result: &ExpandedResult) {
if task.options.infer_inputs {
task.input_files.extend(result.files.clone());
Expand Down
30 changes: 24 additions & 6 deletions crates/task-expander/tests/task_expander_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,16 +245,36 @@ mod task_expander {
env::set_var("FOO_BAR", "foo-bar");

let context = create_context(sandbox.path());
TaskExpander::new(&project, &context)
.expand_args(&mut task)
.unwrap();
let task = TaskExpander::new(&project, &context).expand(&task).unwrap();

env::remove_var("FOO_BAR");
env::remove_var("BAR_BAZ");

assert_eq!(task.args, ["a", "foo-bar", "b", "c/bar-baz/d"]);
}

#[test]
fn replaces_env_vars_from_file() {
let sandbox = create_empty_sandbox();
sandbox.create_file(".env", "FOO_BAR=foo-bar\nBAR_BAZ=bar-baz");

let project = create_project(sandbox.path());

let mut task = create_task();
task.options.env_files = Some(vec![InputPath::WorkspaceFile(".env".into())]);
task.args = vec![
"a".into(),
"$FOO_BAR".into(),
"b".into(),
"c/${BAR_BAZ}/d".into(),
];

let context = create_context(sandbox.path());
let task = TaskExpander::new(&project, &context).expand(&task).unwrap();

assert_eq!(task.args, ["a", "foo-bar", "b", "c/bar-baz/d"]);
}

#[test]
fn replaces_env_var_from_self() {
let sandbox = create_empty_sandbox();
Expand All @@ -265,9 +285,7 @@ mod task_expander {
task.env.insert("FOO_BAR".into(), "foo-bar-self".into());

let context = create_context(sandbox.path());
TaskExpander::new(&project, &context)
.expand_args(&mut task)
.unwrap();
let task = TaskExpander::new(&project, &context).expand(&task).unwrap();

assert_eq!(task.args, ["a", "foo-bar-self", "b"]);
}
Expand Down
8 changes: 4 additions & 4 deletions crates/task-expander/tests/token_expander_test.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
mod utils;

use moon_common::path::WorkspaceRelativePathBuf;
use moon_common::path::{self, WorkspaceRelativePathBuf};
use moon_config::{InputPath, LanguageType, OutputPath, ProjectType};
use moon_task_expander::{ExpandedResult, TokenExpander};
use rustc_hash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -277,7 +277,7 @@ mod token_expander {
expander
.replace_variable(&task, Cow::Borrowed("$projectRoot"))
.unwrap(),
project.root.to_string_lossy()
path::to_string(&project.root).unwrap()
);
assert_eq!(
expander
Expand Down Expand Up @@ -313,13 +313,13 @@ mod token_expander {
expander
.replace_variable(&task, Cow::Borrowed("$workingDir"))
.unwrap(),
sandbox.path().to_string_lossy()
path::to_string(sandbox.path()).unwrap()
);
assert_eq!(
expander
.replace_variable(&task, Cow::Borrowed("$workspaceRoot"))
.unwrap(),
sandbox.path().to_string_lossy()
path::to_string(sandbox.path()).unwrap()
);

assert!(predicate::str::is_match("[0-9]{4}-[0-9]{2}-[0-9]{2}")
Expand Down
4 changes: 3 additions & 1 deletion legacy/bun/platform/src/bun_platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ impl Platform for BunPlatform {
self.package_names
.insert(package_name.clone(), project_id.to_owned());

aliases_list.push((project_id.to_owned(), package_name.clone()));
if package_name != project_id.as_str() {
aliases_list.push((project_id.to_owned(), package_name.clone()));
}
}
}
}
Expand Down
11 changes: 3 additions & 8 deletions legacy/node/platform/src/node_platform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,17 +181,12 @@ impl Platform for NodePlatform {
PackageJsonCache::read(project_source.to_path(&self.workspace_root))?
{
if let Some(package_name) = package_json.data.name {
debug!(
target: LOG_TARGET,
"Inheriting alias {} for project {}",
color::label(&package_name),
color::id(project_id)
);

self.package_names
.insert(package_name.clone(), project_id.to_owned());

aliases_list.push((project_id.to_owned(), package_name.clone()));
if package_name != project_id.as_str() {
aliases_list.push((project_id.to_owned(), package_name.clone()));
}
}
}
}
Expand Down

0 comments on commit 2863ebc

Please sign in to comment.