Skip to content

Commit

Permalink
fix: Final round of fixes. (#1676)
Browse files Browse the repository at this point in the history
* Fix polyrepo.

* Add toolchain static locks.

* Add hook format.

* Add tests.
  • Loading branch information
milesj authored Oct 7, 2024
1 parent 4f20a8f commit 3a13ce8
Show file tree
Hide file tree
Showing 32 changed files with 302 additions and 66 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,21 @@
- Pkl is programmable, allowing for variables, loops, conditionals, and more.
- Added a new task option, `cacheLifetime`, that controls how long a task will be cached for.
- Added a new task merge strategy, `preserve`, that preserves the original inherited value.
- Added a new setting `vcs.hookFormat` to `.moon/workspace.yml`, that can customize the shell/file
format for hooks.
- Added "sync workspace action" support to toolchain plugins. This is our first step in supporting
toolchains via WASM plugins.
- Updated task `outputs` to support token and environment variables.
- Updated `moon query projects` to include the project description as a trailing value.
- Updated `moon query tasks` to include the task type and platform, and the task description as a
trailing value.

#### 🐞 Fixes

- Fixed an issue where a root project in a polyrepo would not default to `**/*` inputs for tasks.
- Potential fix for an issue that occurs when multiple toolchains of the same type (2 different
Node.js versions for example) would fail in weird ways when installing in parallel.

#### ⚙️ Internal

- Updated identifiers and targets to use [compact strings](https://crates.io/crates/compact_str).
Expand Down
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/actions/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ moon_vcs_hooks = { path = "../vcs-hooks" }
miette = { workspace = true }
proto_core = { workspace = true }
rustc-hash = { workspace = true }
scc = { workspace = true }
serde = { workspace = true }
starbase_utils = { workspace = true }
tokio = { workspace = true }
Expand Down
12 changes: 11 additions & 1 deletion crates/actions/src/actions/setup_toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,14 @@ use moon_config::UnresolvedVersionSpec;
use moon_platform::PlatformManager;
use moon_time::now_millis;
use rustc_hash::FxHashMap;
use std::sync::Arc;
use std::sync::{Arc, OnceLock};
use tokio::sync::Mutex;
use tracing::{debug, instrument};

// Avoid the same tool running in parallel causing issues,
// so use a global lock keyed by tool ID.
static LOCKS: OnceLock<scc::HashMap<String, Mutex<()>>> = OnceLock::new();

cache_item!(
pub struct ToolCacheState {
pub last_versions: FxHashMap<String, UnresolvedVersionSpec>,
Expand Down Expand Up @@ -51,6 +56,11 @@ pub async fn setup_toolchain(
hash_component(node.runtime.requirement.to_string()),
))?;

// Acquire a lock for the toolchain ID
let locks = LOCKS.get_or_init(|| scc::HashMap::default());
let entry = locks.entry(node.runtime.id()).or_default();
let _lock = entry.lock().await;

// Install and setup the specific tool + version in the toolchain!
let installed_count = PlatformManager::write()
.get_mut(&node.runtime)?
Expand Down
2 changes: 1 addition & 1 deletion crates/app/src/commands/docker/scaffold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ async fn scaffold_workspace(
let mut has_root_project = false;

for project in projects {
if project.source.as_str() == "." {
if path::is_root_level_source(&project.source) {
has_root_project = true;
}

Expand Down
8 changes: 7 additions & 1 deletion crates/common/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ pub type ProjectRelativePathBuf = RelativePathBuf;
pub type WorkspaceRelativePath = RelativePath;
pub type WorkspaceRelativePathBuf = RelativePathBuf;

#[inline]
pub fn is_root_level_source<T: AsRef<str>>(source: T) -> bool {
let source = source.as_ref();
source.is_empty() || source == "."
}

#[cfg(unix)]
#[inline]
pub fn normalize_separators<T: AsRef<str>>(path: T) -> String {
Expand Down Expand Up @@ -41,7 +47,7 @@ pub fn expand_to_workspace_relative<P: AsRef<str>>(
match from_format {
RelativeFrom::Project(source) => {
// Root-level project
if source.is_empty() || source == "." {
if is_root_level_source(source) {
WorkspaceRelativePathBuf::from(path)

// Project-level, prefix with source path
Expand Down
2 changes: 1 addition & 1 deletion crates/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ indexmap = { workspace = true }
miette = { workspace = true }
once_cell = { workspace = true }
regex = { workspace = true }
rpkl = "0.3.3"
rpkl = "0.3.4"
rustc-hash = { workspace = true }
schematic = { workspace = true, features = [
"config",
Expand Down
13 changes: 13 additions & 0 deletions crates/config/src/workspace/vcs_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ derive_enum!(
}
);

derive_enum!(
/// The format to use for generated VCS hook files.
#[derive(ConfigEnum, Copy, Default)]
pub enum VcsHookFormat {
Bash,
#[default]
Native,
}
);

/// Configures the version control system (VCS).
#[derive(Clone, Config, Debug, PartialEq)]
pub struct VcsConfig {
Expand All @@ -38,6 +48,9 @@ pub struct VcsConfig {
/// A mapping of hooks to commands to run when the hook is triggered.
pub hooks: FxHashMap<String, Vec<String>>,

/// The format to use for generated VCS hook files.
pub hook_format: VcsHookFormat,

/// The VCS client being utilized by the repository.
pub manager: VcsManager,

Expand Down
3 changes: 2 additions & 1 deletion crates/config/tests/workspace_config_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -938,10 +938,11 @@ extensions:
"moon run :pre-commit".into()
]
)]),
hook_format: VcsHookFormat::Native,
manager: VcsManager::Git,
provider: VcsProvider::GitLab,
remote_candidates: vec!["main".into(), "origin/main".into()],
sync_hooks: true
sync_hooks: true,
}
);
}
Expand Down
2 changes: 2 additions & 0 deletions crates/project-builder/src/project_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use tracing::{debug, instrument, trace};

pub struct ProjectBuilderContext<'app> {
pub config_loader: &'app ConfigLoader,
pub monorepo: bool,
pub root_project_id: Option<&'app Id>,
pub toolchain_config: &'app ToolchainConfig,
pub workspace_root: &'app Path,
Expand Down Expand Up @@ -367,6 +368,7 @@ impl<'app> ProjectBuilder<'app> {
self.source.as_str(),
&self.platform,
TasksBuilderContext {
monorepo: self.context.monorepo,
toolchain_config: self.context.toolchain_config,
workspace_root: self.context.workspace_root,
},
Expand Down
1 change: 1 addition & 0 deletions crates/project-builder/tests/project_builder_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ impl Stub {
&self.source,
ProjectBuilderContext {
config_loader: &self.config_loader,
monorepo: true,
root_project_id: None,
toolchain_config: &self.toolchain_config,
workspace_root: &self.workspace_root,
Expand Down
36 changes: 27 additions & 9 deletions crates/project-graph/src/project_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::project_graph_error::ProjectGraphError;
use crate::project_graph_hash::ProjectGraphHash;
use crate::projects_locator::locate_projects_with_globs;
use moon_cache::CacheEngine;
use moon_common::path::{to_virtual_string, WorkspaceRelativePathBuf};
use moon_common::path::{is_root_level_source, to_virtual_string, WorkspaceRelativePathBuf};
use moon_common::{color, consts, Id};
use moon_config::{
ConfigLoader, DependencyScope, InheritedTasksManager, ProjectConfig, ProjectsSourcesList,
Expand Down Expand Up @@ -55,14 +55,17 @@ pub struct ProjectGraphBuilder<'app> {
/// The DAG instance.
graph: GraphType,

/// Is this a monorepo or polyrepo.
monorepo: bool,

/// Nodes (projects) inserted into the graph.
nodes: FxHashMap<Id, NodeIndex>,

/// Projects that have explicitly renamed themselves.
/// Maps original ID to renamed ID.
renamed_ids: FxHashMap<Id, Id>,

/// The root project ID.
/// The root project ID (only if a monorepo).
root_id: Option<Id>,

/// Mapping of project IDs to file system sources,
Expand All @@ -83,6 +86,7 @@ impl<'app> ProjectGraphBuilder<'app> {
configs: FxHashMap::default(),
aliases: FxHashMap::default(),
graph: DiGraph::new(),
monorepo: false,
nodes: FxHashMap::default(),
renamed_ids: FxHashMap::default(),
root_id: None,
Expand Down Expand Up @@ -322,6 +326,7 @@ impl<'app> ProjectGraphBuilder<'app> {
&source,
ProjectBuilderContext {
config_loader: context.config_loader,
monorepo: self.monorepo,
root_project_id: self.root_id.as_ref(),
toolchain_config: context.toolchain_config,
workspace_root: context.workspace_root,
Expand Down Expand Up @@ -509,14 +514,27 @@ impl<'app> ProjectGraphBuilder<'app> {
.await?
.aliases;

// Determine if a polyrepo or monorepo
let polyrepo = sources.len() == 1
&& sources
.first()
.map(|(_, source)| is_root_level_source(source))
.unwrap_or_default();

self.monorepo = !polyrepo;

// Find the root project
self.root_id = sources.iter().find_map(|(id, source)| {
if source.as_str().is_empty() || source.as_str() == "." {
Some(id.to_owned())
} else {
None
}
});
self.root_id = if self.monorepo {
sources.iter().find_map(|(id, source)| {
if is_root_level_source(source) {
Some(id.to_owned())
} else {
None
}
})
} else {
None
};

// Set our data and warn/error against problems
for (id, source) in sources {
Expand Down
6 changes: 4 additions & 2 deletions crates/project-graph/src/projects_locator.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::project_graph_builder::ProjectGraphBuilderContext;
use moon_common::path::{to_virtual_string, WorkspaceRelativePathBuf};
use moon_common::path::{is_root_level_source, to_virtual_string, WorkspaceRelativePathBuf};
use moon_common::{color, consts, Id};
use moon_config::{ProjectSourceEntry, ProjectsSourcesList};
use starbase_utils::{fs, glob};
Expand Down Expand Up @@ -41,7 +41,9 @@ where
V: AsRef<str> + 'glob,
{
let mut locate_globs = vec![];
let mut has_root_level = sources.iter().any(|(_, source)| source == ".");
let mut has_root_level = sources
.iter()
.any(|(_, source)| is_root_level_source(source));

// Root-level project has special handling
for glob in globs.into_iter() {
Expand Down
9 changes: 7 additions & 2 deletions crates/project/src/project.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use crate::project_error::ProjectError;
use moon_common::{cacheable, path::WorkspaceRelativePathBuf, serde::is_wasm_bridge, Id};
use moon_common::{
cacheable,
path::{is_root_level_source, WorkspaceRelativePathBuf},
serde::is_wasm_bridge,
Id,
};
use moon_config::{
DependencyConfig, InheritedTasksResult, LanguageType, PlatformType, ProjectConfig, ProjectType,
StackType,
Expand Down Expand Up @@ -117,7 +122,7 @@ impl Project {

/// Return true if the root-level project.
pub fn is_root_level(&self) -> bool {
self.source.as_str().is_empty() || self.source.as_str() == "."
is_root_level_source(&self.source)
}

/// Return true if the provided locator string (an ID or alias) matches the
Expand Down
8 changes: 5 additions & 3 deletions crates/task-builder/src/tasks_builder.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#![allow(dead_code)]

use crate::tasks_builder_error::TasksBuilderError;
use moon_common::path::is_root_level_source;
use moon_common::{color, supports_pkl_configs, Id};
use moon_config::{
is_glob_like, InheritedTasksConfig, InputPath, PlatformType, ProjectConfig,
Expand Down Expand Up @@ -62,6 +63,7 @@ fn extract_config<'builder, 'proj>(

#[derive(Debug)]
pub struct TasksBuilderContext<'proj> {
pub monorepo: bool,
pub toolchain_config: &'proj ToolchainConfig,
pub workspace_root: &'proj Path,
}
Expand Down Expand Up @@ -293,7 +295,7 @@ impl<'proj> TasksBuilder<'proj> {
task.preset = preset;
task.options = self.build_task_options(id, preset)?;
task.metadata.local_only = is_local;
task.metadata.root_level = self.project_source == ".";
task.metadata.root_level = is_root_level_source(self.project_source);

// Aggregate all values that are inherited from the global task configs,
// and should always be included in the task, regardless of merge strategy.
Expand Down Expand Up @@ -399,10 +401,10 @@ impl<'proj> TasksBuilder<'proj> {
);

task.metadata.empty_inputs = true;
} else if task.metadata.root_level {
} else if self.context.monorepo && task.metadata.root_level {
trace!(
target = target.as_str(),
"Task is in a root-level project, defaulting to no inputs",
"Task is a root-level project in a monorepo, defaulting to no inputs",
);

task.metadata.empty_inputs = true;
Expand Down
10 changes: 10 additions & 0 deletions crates/task-builder/tests/__fixtures__/builder-poly/moon.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
tasks:
no-inputs:
command: 'no-inputs'
empty-inputs:
command: 'empty-inputs'
inputs: []
with-inputs:
command: 'with-inputs'
inputs:
- 'local/*'
Loading

0 comments on commit 3a13ce8

Please sign in to comment.