From 9adb9e00beee38ba1f41a4a3e10a7f8a599e5b96 Mon Sep 17 00:00:00 2001 From: Emily Xiong Date: Thu, 31 Oct 2024 23:33:04 -0400 Subject: [PATCH] fix(core): fix dependency with multiple dependent packages (#28669) ## Current Behavior - we introduce a feature to allow circular project dependencies to execute tasks in pr https://github.com/nrwl/nx/pull/28227/files - for this feature, we introduce the concept of dummy task as a temporary placeholder so we can still generate a task graph even if dependencies contain circular dependencies - however, it does not handle a task graph of multiple dummy task deps. for example: lib1 -> lib2 -> lib3 -> lib4 if we got task graph like: lib1:build -> lib2:dummy lib2:dummy -> lib3:dummy lib3:dummy -> lib4:rebuild currently, it does not create a dependency between lib1:build->lib4:prebuild ## Expected Behavior it should link the dependency when it contains multiple level of depending on dummy tasks ## Related Issue(s) Fixes https://github.com/nrwl/nx/issues/28599 and https://github.com/nrwl/nx/issues/28380 --- .../tasks-runner/create-task-graph.spec.ts | 175 +++++++++++++++++- .../nx/src/tasks-runner/create-task-graph.ts | 62 +++++-- .../nx/src/tasks-runner/task-graph-utils.ts | 6 +- 3 files changed, 220 insertions(+), 23 deletions(-) diff --git a/packages/nx/src/tasks-runner/create-task-graph.spec.ts b/packages/nx/src/tasks-runner/create-task-graph.spec.ts index 9e5a6fc195e35..a327f484919a5 100644 --- a/packages/nx/src/tasks-runner/create-task-graph.spec.ts +++ b/packages/nx/src/tasks-runner/create-task-graph.spec.ts @@ -1624,7 +1624,7 @@ describe('createTaskGraph', () => { } ); expect(taskGraph).toEqual({ - roots: [], + roots: ['lib2:build'], tasks: { 'lib1:build': expect.objectContaining({ id: 'lib1:build', @@ -1668,7 +1668,7 @@ describe('createTaskGraph', () => { }, dependencies: { 'lib1:build': ['lib2:build'], - 'lib2:build': ['lib4:build'], + 'lib2:build': [], 'lib4:build': ['lib1:build'], }, }); @@ -1870,7 +1870,7 @@ describe('createTaskGraph', () => { }); }); - it('should handle cycles between projects that do not create cycles between tasks and not contain the same task target (app1:build -> app2 <-> app3:build)``', () => { + it('should handle cycles between projects that do not create cycles between tasks and not contain the same task target (app1:build -> app2 <-> app3:build)', () => { projectGraph = { nodes: { app1: { @@ -2488,6 +2488,175 @@ describe('createTaskGraph', () => { } `); }); + + it('should handle mulitple projects that are dependent on each other (app1->app2->app3->app4)', () => { + projectGraph = { + nodes: { + app1: { + name: 'app1', + type: 'app', + data: { + root: 'app1-root', + targets: { + compile: { + executor: 'nx:run-commands', + dependsOn: ['precompiple', '^precompile'], + }, + }, + }, + }, + app2: { + name: 'app2', + type: 'app', + data: { + root: 'app2-root', + targets: { + compile: { + executor: 'nx:run-commands', + dependsOn: ['precompiple', '^precompile'], + }, + }, + }, + }, + app3: { + name: 'app3', + type: 'app', + data: { + root: 'app3-root', + targets: { + compile: { + executor: 'nx:run-commands', + dependsOn: ['precompiple', '^precompile'], + }, + }, + }, + }, + app4: { + name: 'app4', + type: 'app', + data: { + root: 'app4-root', + targets: { + precompile: { + executor: 'nx:run-commands', + }, + }, + }, + }, + }, + dependencies: { + app1: [{ source: 'app1', target: 'app2', type: 'implicit' }], + app2: [{ source: 'app2', target: 'app3', type: 'implicit' }], + app3: [{ source: 'app3', target: 'app4', type: 'implicit' }], + }, + }; + + let taskGraph = createTaskGraph( + projectGraph, + {}, + ['app1'], + ['compile'], + 'development', + { + __overrides_unparsed__: [], + } + ); + expect(taskGraph).toEqual({ + roots: ['app4:precompile'], + tasks: { + 'app1:compile': { + id: 'app1:compile', + target: { + project: 'app1', + target: 'compile', + }, + outputs: [], + overrides: { + __overrides_unparsed__: [], + }, + projectRoot: 'app1-root', + parallelism: true, + }, + 'app4:precompile': { + id: 'app4:precompile', + target: { + project: 'app4', + target: 'precompile', + }, + outputs: [], + overrides: { + __overrides_unparsed__: [], + }, + projectRoot: 'app4-root', + parallelism: true, + }, + }, + dependencies: { + 'app1:compile': ['app4:precompile'], + 'app4:precompile': [], + }, + }); + + taskGraph = createTaskGraph( + projectGraph, + {}, + ['app2', 'app3'], + ['compile'], + 'development', + { + __overrides_unparsed__: [], + } + ); + expect(taskGraph).toEqual({ + roots: ['app4:precompile'], + tasks: { + 'app2:compile': { + id: 'app2:compile', + target: { + project: 'app2', + target: 'compile', + }, + outputs: [], + overrides: { + __overrides_unparsed__: [], + }, + projectRoot: 'app2-root', + parallelism: true, + }, + 'app3:compile': { + id: 'app3:compile', + target: { + project: 'app3', + target: 'compile', + }, + outputs: [], + overrides: { + __overrides_unparsed__: [], + }, + projectRoot: 'app3-root', + parallelism: true, + }, + 'app4:precompile': { + id: 'app4:precompile', + target: { + project: 'app4', + target: 'precompile', + }, + outputs: [], + overrides: { + __overrides_unparsed__: [], + }, + projectRoot: 'app4-root', + parallelism: true, + }, + }, + dependencies: { + 'app2:compile': ['app4:precompile'], + 'app3:compile': ['app4:precompile'], + 'app4:precompile': [], + }, + }); + }); }); class GraphBuilder { diff --git a/packages/nx/src/tasks-runner/create-task-graph.ts b/packages/nx/src/tasks-runner/create-task-graph.ts index 62d3b695e12ad..45347a086722f 100644 --- a/packages/nx/src/tasks-runner/create-task-graph.ts +++ b/packages/nx/src/tasks-runner/create-task-graph.ts @@ -8,6 +8,7 @@ import { Task, TaskGraph } from '../config/task-graph'; import { TargetDefaults, TargetDependencies } from '../config/nx-json'; import { output } from '../utils/output'; import { TargetDependencyConfig } from '../config/workspace-json-project-json'; +import { findCycle } from './task-graph-utils'; const DUMMY_TASK_TARGET = '__nx_dummy_task__'; @@ -105,7 +106,7 @@ export class ProcessTasks { projectUsedToDeriveDependencies: string, configuration: string, overrides: Object - ) { + ): void { const seenKey = `${task.id}-${projectUsedToDeriveDependencies}`; if (this.seen.has(seenKey)) { return; @@ -233,7 +234,7 @@ export class ProcessTasks { task: Task, taskOverrides: Object | { __overrides_unparsed__: any[] }, overrides: Object - ) { + ): void { if ( !this.projectGraph.dependencies.hasOwnProperty( projectUsedToDeriveDependencies @@ -292,7 +293,7 @@ export class ProcessTasks { undefined ); this.dependencies[task.id].push(dummyId); - this.dependencies[dummyId] = []; + this.dependencies[dummyId] ??= []; const noopTask = this.createDummyTask(dummyId, task); this.processTask(noopTask, depProject.name, configuration, overrides); } @@ -377,22 +378,53 @@ export class ProcessTasks { return id; } + /** + * this function is used to get the non dummy dependencies of a task recursively + * For example, when we have the following dependencies: + * { + * 'app1:compile': [ 'app2:__nx_dummy_task__' ], + * 'app2:__nx_dummy_task__': [ 'app3:__nx_dummy_task__' ], + * 'app3:__nx_dummy_task__': [ 'app4:precompile' ], + * 'app4:precompile': [] + * } + * getNonDummyDeps('app1:compile') will return ['app1:compile'] + * getNonDummyDeps('app2:__nx_dummy_task__') will return ['app4:precompile'] + * getNonDummyDeps('app3:__nx_dummy_task__') will return ['app4:precompile'] + * getNonDummyDeps('app4:precompile') will return ['app4:precompile'] + */ + private getNonDummyDeps( + currentTask: string, + originalTask: string, + cycle?: string[] + ): string[] { + if (currentTask === originalTask) { + return []; + } else if (currentTask.endsWith(DUMMY_TASK_TARGET)) { + if (cycle?.length && cycle?.includes(currentTask)) { + return []; + } + // if not a cycle, recursively get the non dummy dependencies + return ( + this.dependencies[currentTask]?.flatMap((dep) => + this.getNonDummyDeps(dep, originalTask, cycle) + ) ?? [] + ); + } else { + return [currentTask]; + } + } + private filterDummyTasks() { + const cycle = findCycle({ dependencies: this.dependencies }); for (const [key, deps] of Object.entries(this.dependencies)) { - const normalizedDeps = []; - for (const dep of deps) { - if (dep.endsWith(DUMMY_TASK_TARGET)) { - normalizedDeps.push( - ...this.dependencies[dep].filter( - (d) => !d.endsWith(DUMMY_TASK_TARGET) - ) - ); - } else { - normalizedDeps.push(dep); + if (!key.endsWith(DUMMY_TASK_TARGET)) { + const normalizedDeps = []; + for (const dep of deps) { + normalizedDeps.push(...this.getNonDummyDeps(dep, key, cycle)); } - } - this.dependencies[key] = normalizedDeps; + this.dependencies[key] = normalizedDeps; + } } for (const key of Object.keys(this.dependencies)) { diff --git a/packages/nx/src/tasks-runner/task-graph-utils.ts b/packages/nx/src/tasks-runner/task-graph-utils.ts index e73afe5dba2ee..fe9a0e72984cf 100644 --- a/packages/nx/src/tasks-runner/task-graph-utils.ts +++ b/packages/nx/src/tasks-runner/task-graph-utils.ts @@ -1,10 +1,6 @@ -import { readNxJson } from '../config/configuration'; import { ProjectGraph } from '../config/project-graph'; -import { Task, TaskGraph } from '../config/task-graph'; -import { isNxCloudUsed } from '../utils/nx-cloud-utils'; +import { TaskGraph } from '../config/task-graph'; import { output } from '../utils/output'; -import { serializeTarget } from '../utils/serialize-target'; -import chalk = require('chalk'); function _findCycle( graph: { dependencies: Record },