Skip to content

Commit

Permalink
internal: Add graph utils and rework project/task graphs. (#1711)
Browse files Browse the repository at this point in the history
* Add graph utils.

* Polish.

* Add utils to tg.

* Clean up builders.

* Add task data.

* Add deps resolving.

* Add to session.

* Polish.

* Fix ID resolving.

* Add changelog.

* Fix lints.

* Fix tests.

* Start on tests.

* Add final tests.
  • Loading branch information
milesj authored Nov 10, 2024
1 parent aa65069 commit ff38f23
Show file tree
Hide file tree
Showing 53 changed files with 1,954 additions and 499 deletions.
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Changelog

## Unreleased

#### 💥 Breaking

- If you renamed a project using the `id` setting in `moon.yml`, you can no longer reference that
project in dependencies and targets using its original ID.

#### 🚀 Updates

- Resolved the `strictProjectIds` experiment and you can no longer reference the original ID.
- Resolved the `disallowRunInCiMismatch` experiment and you can no longer have a CI based task
depend on a non-CI based task.
- Added a new task graph, that enables new granular based functionality for task related features.

## 1.29.4

#### 🚀 Updates
Expand Down
21 changes: 21 additions & 0 deletions Cargo.lock

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

12 changes: 6 additions & 6 deletions crates/action-graph/src/action_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use moon_common::{color, Id};
use moon_config::{PlatformType, TaskDependencyConfig};
use moon_platform::{PlatformManager, Runtime};
use moon_project::{Project, ProjectError};
use moon_project_graph::ProjectGraph;
use moon_project_graph::{GraphConnections, ProjectGraph};
use moon_query::{build_query, Criteria};
use moon_task::{Target, TargetError, TargetLocator, TargetScope, Task};
use moon_task_args::parse_task_args;
Expand Down Expand Up @@ -458,8 +458,8 @@ impl<'app> ActionGraphBuilder<'app> {
projects_to_build.push(self_project.clone());

// From other projects
for dependent_id in self.project_graph.dependents_of(&self_project)? {
projects_to_build.push(self.project_graph.get(dependent_id)?);
for dependent_id in self.project_graph.dependents_of(&self_project) {
projects_to_build.push(self.project_graph.get(&dependent_id)?);
}

for project in projects_to_build {
Expand Down Expand Up @@ -672,12 +672,12 @@ impl<'app> ActionGraphBuilder<'app> {
let mut edges = vec![setup_tool_index];

// And we should also depend on other projects
for dep_project_id in self.project_graph.dependencies_of(project)? {
if cycle.contains(dep_project_id) {
for dep_project_id in self.project_graph.dependencies_of(project) {
if cycle.contains(&dep_project_id) {
continue;
}

let dep_project = self.project_graph.get(dep_project_id)?;
let dep_project = self.project_graph.get(&dep_project_id)?;
let dep_project_index = self.internal_sync_project(&dep_project, cycle)?;

if index != dep_project_index {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tasks:
# Note: No longer allowed!
ci1-dependency:
inputs:
- 'input.txt'
options:
runInCI: false
ci1-dependent:
deps:
- ci1-dependency
options:
runInCI: true
27 changes: 14 additions & 13 deletions crates/action-graph/tests/__fixtures__/tasks/ci/moon.yml
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
tasks:
ci1-dependency:
inputs:
- 'input.txt'
options:
runInCI: false
ci1-dependant:
deps:
- ci1-dependency
options:
runInCI: true
# Note: No longer allowed!
# ci1-dependency:
# inputs:
# - 'input.txt'
# options:
# runInCI: false
# ci1-dependent:
# deps:
# - ci1-dependency
# options:
# runInCI: true

ci2-dependency:
inputs:
- 'input.txt'
options:
runInCI: false
ci2-dependant:
ci2-dependent:
deps:
- ci2-dependency
options:
Expand All @@ -26,7 +27,7 @@ tasks:
- 'input.txt'
options:
runInCI: true
ci3-dependant:
ci3-dependent:
deps:
- ci2-dependency
options:
Expand All @@ -37,7 +38,7 @@ tasks:
- 'input.txt'
options:
runInCI: true
ci4-dependant:
ci4-dependent:
deps:
- ci4-dependency
options:
Expand Down
75 changes: 36 additions & 39 deletions crates/action-graph/tests/action_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1117,38 +1117,39 @@ mod action_graph {
assert!(!topo(graph).is_empty());
}

#[tokio::test]
async fn runs_dependents_if_dependency_is_ci_false_but_affected() {
let sandbox = create_sandbox("tasks");
let container = ActionGraphContainer::new(sandbox.path()).await;
let mut builder = container.create_builder();

let project = container.project_graph.get("ci").unwrap();
let task = project.get_task("ci1-dependency").unwrap();

// Must be affected to run the dependent
let touched_files =
FxHashSet::from_iter([WorkspaceRelativePathBuf::from("ci/input.txt")]);

builder.set_touched_files(&touched_files).unwrap();

builder
.run_task(
&project,
task,
&RunRequirements {
ci: true,
ci_check: true,
dependents: true,
..RunRequirements::default()
},
)
.unwrap();

let graph = builder.build();

assert_snapshot!(graph.to_dot());
}
// TODO: Enable after new task graph!
// #[tokio::test]
// async fn runs_dependents_if_dependency_is_ci_false_but_affected() {
// let sandbox = create_sandbox("tasks");
// let container = ActionGraphContainer::new(sandbox.path()).await;
// let mut builder = container.create_builder();

// let project = container.project_graph.get("ci").unwrap();
// let task = project.get_task("ci2-dependency").unwrap();

// // Must be affected to run the dependent
// let touched_files =
// FxHashSet::from_iter([WorkspaceRelativePathBuf::from("ci/input.txt")]);

// builder.set_touched_files(&touched_files).unwrap();

// builder
// .run_task(
// &project,
// task,
// &RunRequirements {
// ci: true,
// ci_check: true,
// dependents: true,
// ..RunRequirements::default()
// },
// )
// .unwrap();

// let graph = builder.build();

// assert_snapshot!(graph.to_dot());
// }

#[tokio::test]
async fn doesnt_run_dependents_if_dependency_is_ci_false_and_not_affected() {
Expand All @@ -1157,7 +1158,7 @@ mod action_graph {
let mut builder = container.create_builder();

let project = container.project_graph.get("ci").unwrap();
let task = project.get_task("ci1-dependency").unwrap();
let task = project.get_task("ci2-dependency").unwrap();

builder
.run_task(
Expand Down Expand Up @@ -1206,15 +1207,11 @@ mod action_graph {

#[tokio::test]
#[should_panic(
expected = "Task ci:ci1-dependant cannot depend on task ci:ci1-dependency"
expected = "Task ci:ci1-dependent cannot depend on task ci:ci1-dependency"
)]
async fn errors_if_dependency_is_ci_false_and_constraint_enabled() {
env::set_var("MOON_INTERNAL_CONSTRAINT_RUNINCI", "true");

let sandbox = create_sandbox("tasks");
let sandbox = create_sandbox("tasks-ci-mismatch");
ActionGraphContainer::new(sandbox.path()).await;

env::remove_var("MOON_INTERNAL_CONSTRAINT_RUNINCI");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ digraph {
1 [ label="SetupToolchain(system)" ]
2 [ label="SyncProject(system, ci)" ]
3 [ label="RunTask(ci:ci4-dependency)" ]
4 [ label="RunTask(ci:ci4-dependant)" ]
4 [ label="RunTask(ci:ci4-dependent)" ]
1 -> 0 [ ]
2 -> 1 [ ]
3 -> 2 [ ]
Expand Down
2 changes: 1 addition & 1 deletion crates/affected/src/affected.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hash::{FxHashMap, FxHashSet};
use serde::{Deserialize, Serialize};
use std::fmt;

#[derive(PartialEq)]
#[derive(Debug, PartialEq)]
pub enum AffectedBy {
AlreadyMarked,
AlwaysAffected,
Expand Down
14 changes: 7 additions & 7 deletions crates/affected/src/affected_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::affected::*;
use moon_common::path::WorkspaceRelativePathBuf;
use moon_common::{color, Id};
use moon_project::Project;
use moon_project_graph::ProjectGraph;
use moon_project_graph::{GraphConnections, ProjectGraph};
use moon_task::{Target, TargetScope, Task};
use rustc_hash::{FxHashMap, FxHashSet};
use std::env;
Expand Down Expand Up @@ -198,17 +198,17 @@ impl<'app> AffectedTracker<'app> {
}
}

for dep_id in self.project_graph.dependencies_of(project)? {
for dep_id in self.project_graph.dependencies_of(project) {
self.projects
.entry(dep_id.to_owned())
.entry(dep_id.clone())
.or_default()
.push(AffectedBy::DownstreamProject(project.id.clone()));

if depth == 0 && self.project_upstream == UpstreamScope::Direct {
continue;
}

let dep_project = self.project_graph.get(dep_id)?;
let dep_project = self.project_graph.get(&dep_id)?;

self.track_project_dependencies(&dep_project, depth + 1)?;
}
Expand Down Expand Up @@ -240,17 +240,17 @@ impl<'app> AffectedTracker<'app> {
}
}

for dep_id in self.project_graph.dependents_of(project)? {
for dep_id in self.project_graph.dependents_of(project) {
self.projects
.entry(dep_id.to_owned())
.entry(dep_id.clone())
.or_default()
.push(AffectedBy::UpstreamProject(project.id.clone()));

if depth == 0 && self.project_downstream == DownstreamScope::Direct {
continue;
}

let dep_project = self.project_graph.get(dep_id)?;
let dep_project = self.project_graph.get(&dep_id)?;

self.track_project_dependents(&dep_project, depth + 1)?;
}
Expand Down
1 change: 1 addition & 0 deletions crates/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ moon_project = { path = "../project" }
moon_project_graph = { path = "../project-graph" }
moon_query = { path = "../query" }
moon_task = { path = "../task" }
moon_task_graph = { path = "../task-graph" }
moon_toolchain = { path = "../toolchain" }
moon_toolchain_plugin = { path = "../toolchain-plugin" }
moon_vcs = { path = "../vcs" }
Expand Down
11 changes: 0 additions & 11 deletions crates/app/src/commands/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use moon_task::{Target, TargetLocator};
use rustc_hash::FxHashSet;
use starbase::AppResult;
use starbase_styles::color;
use std::env;
use std::sync::Arc;
use tracing::instrument;

Expand Down Expand Up @@ -85,16 +84,6 @@ impl CiConsole {
}

async fn generate_project_graph(session: &CliSession) -> AppResult<Arc<ProjectGraph>> {
// We have no easy way of passing this experiment boolean into the
// project graph, so use an environment variable for now...
if session
.workspace_config
.experiments
.disallow_run_in_ci_mismatch
{
env::set_var("MOON_INTERNAL_CONSTRAINT_RUNINCI", "true");
}

session.get_project_graph().await
}

Expand Down
Loading

0 comments on commit ff38f23

Please sign in to comment.