Skip to content

Commit

Permalink
fix(core): fix dependency with multiple dependent packages (#28669)
Browse files Browse the repository at this point in the history
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
- 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
<!-- This is the behavior we should expect with the changes in this PR
-->
it should link the dependency when it contains multiple level of
depending on dummy tasks

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28599 and
#28380
  • Loading branch information
xiongemi authored and jaysoo committed Nov 1, 2024
1 parent 3a34241 commit c1e77a1
Show file tree
Hide file tree
Showing 3 changed files with 220 additions and 23 deletions.
175 changes: 172 additions & 3 deletions packages/nx/src/tasks-runner/create-task-graph.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1624,7 +1624,7 @@ describe('createTaskGraph', () => {
}
);
expect(taskGraph).toEqual({
roots: [],
roots: ['lib2:build'],
tasks: {
'lib1:build': expect.objectContaining({
id: 'lib1:build',
Expand Down Expand Up @@ -1668,7 +1668,7 @@ describe('createTaskGraph', () => {
},
dependencies: {
'lib1:build': ['lib2:build'],
'lib2:build': ['lib4:build'],
'lib2:build': [],
'lib4:build': ['lib1:build'],
},
});
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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 {
Expand Down
62 changes: 47 additions & 15 deletions packages/nx/src/tasks-runner/create-task-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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__';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -233,7 +234,7 @@ export class ProcessTasks {
task: Task,
taskOverrides: Object | { __overrides_unparsed__: any[] },
overrides: Object
) {
): void {
if (
!this.projectGraph.dependencies.hasOwnProperty(
projectUsedToDeriveDependencies
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)) {
Expand Down
6 changes: 1 addition & 5 deletions packages/nx/src/tasks-runner/task-graph-utils.ts
Original file line number Diff line number Diff line change
@@ -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<string, string[]> },
Expand Down

0 comments on commit c1e77a1

Please sign in to comment.