Skip to content

Commit

Permalink
new: Final polish before release. (#1157)
Browse files Browse the repository at this point in the history
* Better cycle.

* Update blog post.

* Test interweave.
  • Loading branch information
milesj authored Oct 29, 2023
1 parent 33a7640 commit 2ef0752
Show file tree
Hide file tree
Showing 13 changed files with 130 additions and 56 deletions.
12 changes: 12 additions & 0 deletions Cargo.lock

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

Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ expression: assert.output()
---
Error: action_graph::cycle_detected

× A dependency cycle has been detected for RunTask(depsC:taskCycle).
× A dependency cycle has been detected for RunTask(depsC:taskCycle) →
RunTask(depsA:taskCycle) → RunTask(depsB:taskCycle).



1 change: 1 addition & 0 deletions nextgen/action-graph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ moon_project = { path = "../project" }
moon_project_graph = { path = "../project-graph" }
moon_task = { path = "../task" }
moon_query = { path = "../query" }
graph-cycles = "0.1.0"
miette = { workspace = true }
petgraph = { workspace = true }
rustc-hash = { workspace = true }
Expand Down
28 changes: 28 additions & 0 deletions nextgen/action-graph/src/action_graph.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::action_graph_error::ActionGraphError;
use crate::action_node::ActionNode;
use graph_cycles::Cycles;
use moon_common::is_test_env;
use petgraph::dot::{Config, Dot};
use petgraph::prelude::*;
Expand Down Expand Up @@ -92,6 +93,31 @@ pub struct ActionGraphIter<'graph> {

impl<'graph> ActionGraphIter<'graph> {
pub fn new(graph: &'graph GraphType) -> miette::Result<Self> {
// Detect any cycles first
let mut cycle: Vec<NodeIndex> = vec![];

graph.visit_cycles(|_, c| {
cycle.extend(c);
std::ops::ControlFlow::Break(())
});

if !cycle.is_empty() {
return Err(ActionGraphError::CycleDetected(
cycle
.into_iter()
.map(|index| {
graph
.node_weight(index)
.map(|n| n.label())
.unwrap_or_else(|| "(unknown)".into())
})
.collect::<Vec<_>>()
.join(" → "),
)
.into());
}

// Then sort topologically
match petgraph::algo::toposort(graph, None) {
Ok(mut indices) => {
indices.reverse();
Expand All @@ -112,6 +138,8 @@ impl<'graph> ActionGraphIter<'graph> {
sender,
})
}
// For some reason the topo sort can detect a cycle,
// that wasn't previously detected, so error...
Err(cycle) => Err(ActionGraphError::CycleDetected(
graph
.node_weight(cycle.node_id())
Expand Down
2 changes: 1 addition & 1 deletion nextgen/action-graph/tests/action_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ mod action_graph {
use super::*;

#[tokio::test]
#[should_panic(expected = "A dependency cycle has been detected for RunTask(deps:cycle2).")]
#[should_panic(expected = "A dependency cycle has been detected for RunTask(deps:cycle2)")]
async fn errors_on_cycle() {
let sandbox = create_sandbox("tasks");
let container = ActionGraphContainer::new(sandbox.path()).await;
Expand Down
72 changes: 25 additions & 47 deletions nextgen/task-builder/src/tasks_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,17 @@ fn extract_config<'builder, 'proj>(
task_id: &'builder Id,
local_tasks: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>,
global_tasks: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>,
global_only: bool,
) -> miette::Result<Vec<ConfigChain<'proj>>> {
let mut local_stack = vec![];
let mut global_stack = vec![];
let mut stack = vec![];

let mut extract = |tasks: &'builder FxHashMap<&'proj Id, &'proj TaskConfig>,
inherited: bool|
-> miette::Result<()> {
if let Some(config) = tasks.get(task_id) {
stack.push(ConfigChain { config, inherited });

if !global_only {
if let Some(config) = local_tasks.get(task_id) {
if let Some(extend_task_id) = &config.extends {
let extended_stack =
extract_config(extend_task_id, local_tasks, global_tasks, false)?;
let extended_stack = extract_config(extend_task_id, local_tasks, global_tasks)?;

if extended_stack.is_empty() {
return Err(TasksBuilderError::UnknownExtendsSource {
Expand All @@ -85,43 +86,18 @@ fn extract_config<'builder, 'proj>(
}
.into());
} else {
local_stack.extend(extended_stack);
}
}

local_stack.push(ConfigChain {
config,
inherited: false,
});
}
}

if let Some(config) = global_tasks.get(task_id) {
if let Some(extend_task_id) = &config.extends {
let extended_stack = extract_config(extend_task_id, local_tasks, global_tasks, true)?;

if extended_stack.is_empty() {
return Err(TasksBuilderError::UnknownExtendsSource {
source_id: task_id.to_owned(),
target_id: extend_task_id.to_owned(),
stack.extend(extended_stack);
}
.into());
} else {
global_stack.extend(extended_stack);
}
}

global_stack.push(ConfigChain {
config,
inherited: true,
});
}
Ok(())
};

let mut configs = vec![];
configs.extend(global_stack);
configs.extend(local_stack);
extract(local_tasks, false)?;
extract(global_tasks, true)?;

Ok(configs)
Ok(stack)
}

#[derive(Debug)]
Expand Down Expand Up @@ -315,7 +291,7 @@ impl<'proj> TasksBuilder<'proj> {
);

let mut task = Task::default();
let chain = self.get_config_inherit_chain(id)?;
let chain = self.get_config_inherit_chain(id, true)?;

// Determine command and args before building options and the task,
// as we need to figure out if we're running in local mode or not.
Expand Down Expand Up @@ -499,7 +475,7 @@ impl<'proj> TasksBuilder<'proj> {
};

let configs = self
.get_config_inherit_chain(id)?
.get_config_inherit_chain(id, false)?
.iter()
.map(|link| &link.config.options)
.collect::<Vec<_>>();
Expand Down Expand Up @@ -675,7 +651,11 @@ impl<'proj> TasksBuilder<'proj> {
Ok((command, args))
}

fn get_config_inherit_chain(&self, id: &Id) -> miette::Result<Vec<ConfigChain>> {
fn get_config_inherit_chain(
&self,
id: &Id,
_inspect: bool,
) -> miette::Result<Vec<ConfigChain>> {
let mut configs = vec![];

if self.context.legacy_task_inheritance {
Expand Down Expand Up @@ -703,12 +683,10 @@ impl<'proj> TasksBuilder<'proj> {
false,
)?;
} else {
configs.extend(extract_config(
id,
&self.local_tasks,
&self.global_tasks,
false,
)?);
let mut stack = extract_config(id, &self.local_tasks, &self.global_tasks)?;
stack.reverse();

configs.extend(stack);
}

Ok(configs)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
tasks:
parent:
env:
VAR1: 'local-parent'

child:
env:
VAR3: 'local-child'
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
tasks:
parent:
command: 'global-parent'
env:
VAR1: 'global-parent'

child:
extends: 'parent'
env:
VAR2: 'global-child'
28 changes: 28 additions & 0 deletions nextgen/task-builder/tests/tasks_builder_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,5 +1351,33 @@ mod tasks_builder {
assert_eq!(task.command, "lint");
assert_eq!(task.args, vec!["./src", "--fix", "--bail"]);
}

#[tokio::test]
async fn can_interweave_global_and_local_extends() {
let sandbox = create_sandbox("builder");
let tasks = build_tasks_with_config(
sandbox.path(),
"extends-interweave",
ProjectConfig::load(
sandbox.path(),
sandbox.path().join("extends-interweave/moon.yml"),
)
.unwrap(),
ToolchainConfig::default(),
Some("global-interweave"),
)
.await;
let task = tasks.get("child").unwrap();

assert_eq!(task.command, "global-parent");
assert_eq!(
task.env,
FxHashMap::from_iter([
("VAR2".into(), "global-child".into()),
("VAR3".into(), "local-child".into()),
("VAR1".into(), "local-parent".into()),
])
)
}
}
}
5 changes: 5 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@
- Added components and targets support for the Rust toolchain.
- Added `rust.components` and `rust.targets` settings to `.moon/toolchain.yml`.
- Will automatically be installed with `rustup` when the pipeline is ran.
- Added a `MOON_TOOLCHAIN_FORCE_GLOBALS` environment variable, that will force all toolchain tools
to use the global binary available on `PATH`, instead of downloading and installing.
- Added an improved task inheritance chain resolver.
- Global and local tasks are now interweaved within the chain, where as previously global was
built first, then local.
- To fallback to the previous behavior, set `experiments.interweavedTaskInheritance: false` in
`.moon/workspace.yml`.
- Added a new project type `automation`, for projects like E2E and integration testing.
- Updated action graph cycle detection to list all nodes in the cycle (when detectable).
- Updated all npx calls to use a package manager equivalent. For example: `yarn dlx`, `pnpm dlx`,
`bunx`.
- Updated to support Yarn v4.
Expand Down
12 changes: 7 additions & 5 deletions website/blog/2023-10-30_moon-v1.16.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ slug: moon-v1.16
title: moon v1.16 - Bun support, Rust improvements, and more!
authors: [milesj]
tags: [bun, rust, toolchain]
# image: ./img/moon/v1.15.png
image: ./img/moon/v1.15.png
---

With this release, we've focused on requests from the community, instead of migration work or new
features.
With this release, we've focused on requests from the community, instead of internal migration work
or new features.

<!--truncate-->

Expand Down Expand Up @@ -65,8 +65,9 @@ In this release, we've added support for both!
Simply configure the [`rust.components`](/docs/config/toolchain#components) or
[`rust.targets`](/docs/config/toolchain#targets) settings in
[`.moon/toolchain.yml`](/docs/config/toolchain), and moon will automatically install them when the
pipeline is ran. This functionality uses the same hashing implementation as `rust.bins`, so will
only install the first time, and again if the configuration changes (or the cache is removed).
pipeline is ran. This functionality uses the same hashing implementation as
[`rust.bins`](/docs/config/toolchain#bins-1), so will only install the first time, and again if the
configuration changes (or the cache is removed).

```yaml title=".moon/toolchain.yml"
rust:
Expand Down Expand Up @@ -106,6 +107,7 @@ of changes.
`experiments.interweavedTaskInheritance: false` in `.moon/workspace.yml`.
- Added a new [project type `automation`](/docs/config/project#type), for projects like E2E and
integration testing.
- Updated action graph cycle detection to list all nodes in the cycle (when detectable).
- Updated all npx calls to use a package manager equivalent. For example: `yarn dlx`, `pnpm dlx`,
`bunx`.
- Updated to support Yarn v4.
Binary file added website/blog/img/moon/v1.16.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 3 additions & 2 deletions website/docs/config/workspace.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,9 @@ Enable or disable experiments that alter core functionality.

<HeadingApiLink to="/api/types/interface/ExperimentsConfig#interweavedTaskInheritance" />

Enables legacy task inheritance behavior where global and local tasks were built separately before
merging. The modern behavior builds them in sequence as an interweaved chain.
Enables the new interweaved task inheritance behavior where global and local tasks are interweaved
throughout the inheritance and extends chain. The legacy behavior would build global and local tasks
separately, then merge them. Defaults to `true`.

```yaml title=".moon/workspace.yml" {2}
experiments:
Expand Down

0 comments on commit 2ef0752

Please sign in to comment.