Skip to content

Commit

Permalink
fix: Don't auto-link dependency when a root-level project. (#1114)
Browse files Browse the repository at this point in the history
* Handle root.

* Update changelog.

* Bump version.

* Polish.
  • Loading branch information
milesj authored Oct 10, 2023
1 parent 966c7ee commit aafc6ef
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 5 deletions.
9 changes: 9 additions & 0 deletions .yarn/versions/5f1de95c.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
releases:
"@moonrepo/cli": patch
"@moonrepo/core-linux-arm64-gnu": patch
"@moonrepo/core-linux-arm64-musl": patch
"@moonrepo/core-linux-x64-gnu": patch
"@moonrepo/core-linux-x64-musl": patch
"@moonrepo/core-macos-arm64": patch
"@moonrepo/core-macos-x64": patch
"@moonrepo/core-windows-x64-msvc": patch
7 changes: 5 additions & 2 deletions crates/node/platform/src/actions/sync_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ pub async fn sync_project(
continue;
};

if dep_project.is_root_level() || matches!(dep_cfg.scope, DependencyScope::Root) {
continue;
}

let dep_relative_path =
path::relative_from(&dep_project.root, &project.root).unwrap_or_default();
let is_dep_typescript_enabled = dep_project.config.toolchain.is_typescript_enabled();
Expand Down Expand Up @@ -66,9 +70,8 @@ pub async fn sync_project(
};

match dep_cfg.scope {
DependencyScope::Build => {
DependencyScope::Build | DependencyScope::Root => {
// Not supported by Node.js
unimplemented!();
}
DependencyScope::Production => {
package_prod_deps.insert(dep_package_name.to_owned(), dep_version);
Expand Down
3 changes: 3 additions & 0 deletions nextgen/config/src/project/dep_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ derive_enum!(
Peer,
#[default]
Production,

// Special case when depending on the root-level project
Root,
}
);

Expand Down
7 changes: 6 additions & 1 deletion nextgen/project-builder/src/project_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ impl Event for DetectLanguageEvent {
pub struct ProjectBuilderContext<'app> {
pub detect_language: &'app Emitter<DetectLanguageEvent>,
pub detect_platform: &'app Emitter<DetectPlatformEvent>,
pub root_project_id: Option<&'app Id>,
pub toolchain_config: &'app ToolchainConfig,
pub workspace_root: &'app Path,
}
Expand Down Expand Up @@ -268,7 +269,11 @@ impl<'app> ProjectBuilder<'app> {
dep_id.to_owned(),
DependencyConfig {
id: dep_id.to_owned(),
scope: DependencyScope::Peer,
scope: if self.context.root_project_id.is_some_and(|id| id == dep_id) {
DependencyScope::Root
} else {
DependencyScope::Peer
},
source: DependencySource::Implicit,
via: Some(task_config.target.to_string()),
},
Expand Down
1 change: 1 addition & 0 deletions nextgen/project-builder/tests/project_builder_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl Stub {
ProjectBuilderContext {
detect_language: &self.detect_language,
detect_platform: &self.detect_platform,
root_project_id: None,
toolchain_config: &self.toolchain_config,
workspace_root: &self.workspace_root,
},
Expand Down
25 changes: 24 additions & 1 deletion nextgen/project-graph/src/project_graph_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use async_recursion::async_recursion;
use moon_cache::CacheEngine;
use moon_common::path::{to_virtual_string, WorkspaceRelativePath, WorkspaceRelativePathBuf};
use moon_common::{color, consts, is_test_env, Id};
use moon_config::{InheritedTasksManager, ToolchainConfig, WorkspaceConfig, WorkspaceProjects};
use moon_config::{
DependencyScope, InheritedTasksManager, ToolchainConfig, WorkspaceConfig, WorkspaceProjects,
};
use moon_hash::HashEngine;
use moon_project::Project;
use moon_project_builder::{DetectLanguageEvent, ProjectBuilder, ProjectBuilderContext};
Expand Down Expand Up @@ -57,6 +59,10 @@ pub struct ProjectGraphBuilder<'app> {
/// Nodes (projects) inserted into the graph.
nodes: FxHashMap<Id, NodeIndex>,

/// The root project ID.
#[serde(skip)]
root_project_id: Option<Id>,

/// Mapping of project IDs to file system sources,
/// derived from the `workspace.projects` setting.
sources: FxHashMap<Id, WorkspaceRelativePathBuf>,
Expand All @@ -75,6 +81,7 @@ impl<'app> ProjectGraphBuilder<'app> {
aliases: FxHashMap::default(),
graph: DiGraph::new(),
nodes: FxHashMap::default(),
root_project_id: None,
sources: FxHashMap::default(),
};

Expand Down Expand Up @@ -246,6 +253,12 @@ impl<'app> ProjectGraphBuilder<'app> {
dependency_id = dep_id.as_str(),
"Encountered a dependency cycle (from project); will disconnect nodes to avoid recursion",
);

// Don't link the root project to any project, but still load it
} else if matches!(dep_config.scope, DependencyScope::Root) {
self.internal_load(dep_id, cycle).await?;

// Otherwise link projects
} else {
edges.push((self.internal_load(dep_id, cycle).await?, dep_config.scope));
}
Expand Down Expand Up @@ -285,6 +298,7 @@ impl<'app> ProjectGraphBuilder<'app> {
ProjectBuilderContext {
detect_language: &context.detect_language,
detect_platform: &context.detect_platform,
root_project_id: self.root_project_id.as_ref(),
toolchain_config: context.toolchain_config,
workspace_root: context.workspace_root,
},
Expand Down Expand Up @@ -451,6 +465,15 @@ impl<'app> ProjectGraphBuilder<'app> {
})
.await?;

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

self.sources = sources;
self.aliases = extended_data.aliases;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
tasks:
build:
command: 'build'
deps: ['root:noop']
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
tasks:
noop:
command: 'noop'
21 changes: 21 additions & 0 deletions nextgen/project-graph/tests/project_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,27 @@ mod project_graph {
.await;

assert_eq!(map_ids(graph.ids()), ["b", "c", "from-task-deps"]);

let deps = &graph.get("from-task-deps").unwrap().dependencies;

assert_eq!(deps.get("b").unwrap().scope, DependencyScope::Peer);
assert_eq!(deps.get("c").unwrap().scope, DependencyScope::Peer);
}

#[tokio::test]
async fn from_root_task_deps() {
let sandbox = create_sandbox("dependency-types");
let container = ProjectGraphContainer::new(sandbox.path());
let context = container.create_context();
let graph = container
.build_graph_for(context, &["from-root-task-deps"])
.await;

assert_eq!(map_ids(graph.ids()), ["root", "from-root-task-deps"]);

let deps = &graph.get("from-root-task-deps").unwrap().dependencies;

assert_eq!(deps.get("root").unwrap().scope, DependencyScope::Root);
}

#[tokio::test]
Expand Down
5 changes: 5 additions & 0 deletions nextgen/project/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ impl Project {
.any(|file| file.starts_with(&self.source))
}

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

/// Return true if the provided locator string (an ID or alias) matches the
/// current project.
pub fn matches_locator(&self, locator: &str) -> bool {
Expand Down
12 changes: 11 additions & 1 deletion nextgen/test-utils/src/project_graph.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use moon_config::{
InheritedTasksEntry, InheritedTasksManager, NodeConfig, PartialInheritedTasksConfig,
PartialTaskConfig, ToolchainConfig, ToolsConfig, WorkspaceConfig, WorkspaceProjects,
WorkspaceProjectsConfig,
};
use moon_project_graph::{
DetectLanguageEvent, DetectPlatformEvent, ExtendProjectEvent, ExtendProjectGraphEvent,
Expand Down Expand Up @@ -52,7 +53,16 @@ impl ProjectGraphContainer {
}

// Use folders as project names
graph.workspace_config.projects = WorkspaceProjects::Globs(vec!["*".into()]);
let mut projects = WorkspaceProjectsConfig {
globs: vec!["*".into()],
..WorkspaceProjectsConfig::default()
};

if root.join("moon.yml").exists() {
projects.sources.insert("root".into(), ".".into());
}

graph.workspace_config.projects = WorkspaceProjects::Both(projects);

graph
}
Expand Down
12 changes: 12 additions & 0 deletions packages/cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@
- More accurately monitors signals (ctrl+c) and shutdowns.
- Tasks can now be configured with a timeout.

## Unreleased

#### 🚀 Updates

- Based on feedback, we've updated the automatic dependency linking to _not apply_ when the target
is the root-level project. This should alleviate all unwanted cycles.

#### 🐞 Fixes

- Fixed an issue where Node.js dependency syncing would fail on `build` dependencies, and be over
zealous with root-level projects.

## 1.15.0

#### 💥 Breaking
Expand Down

0 comments on commit aafc6ef

Please sign in to comment.