From 8f9752feed4ea84fb7d9f500d292303e98113d55 Mon Sep 17 00:00:00 2001 From: FrozenPandaz Date: Thu, 27 Jul 2023 15:43:33 -0400 Subject: [PATCH] fix(core): deterministically hash external nodes --- .../__snapshots__/task-hasher.spec.ts.snap | 25 ++- packages/nx/src/hasher/task-hasher.spec.ts | 150 +++++++++++++++--- packages/nx/src/hasher/task-hasher.ts | 77 +++++---- 3 files changed, 187 insertions(+), 65 deletions(-) diff --git a/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap b/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap index d3702d19e0b556..6855021bb11831 100644 --- a/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap +++ b/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap @@ -53,14 +53,14 @@ exports[`TaskHasher hashTarget should hash entire subtree of dependencies 1`] = "ProjectConfiguration": "12026883044296863450", "TsConfig": "8767608672024750088", "app:{projectRoot}/**/*": "3244421341483603138", - "target": "3789300870433976270", + "target": "11811695043583341473", "{workspaceRoot}/.gitignore": "3244421341483603138", "{workspaceRoot}/.nxignore": "3244421341483603138", "{workspaceRoot}/nx.json": "8942239360311677987", }, "runtime": {}, }, - "value": "11829832011053499600", + "value": "17607022607820563118", } `; @@ -219,6 +219,27 @@ exports[`TaskHasher should create task hash 1`] = ` } `; +exports[`TaskHasher should hash missing dependent npm project versions 1`] = ` +{ + "details": { + "command": "14389236043839781668", + "implicitDeps": {}, + "nodes": { + "ProjectConfiguration": "8128657069648957137", + "TsConfig": "8767608672024750088", + "app:{projectRoot}/**/*": "9104199730100321982", + "npm:react": "4468841026152585217", + "target": "14358315432887000841", + "{workspaceRoot}/.gitignore": "3244421341483603138", + "{workspaceRoot}/.nxignore": "3244421341483603138", + "{workspaceRoot}/nx.json": "8942239360311677987", + }, + "runtime": {}, + }, + "value": "3668827038634092448", +} +`; + exports[`TaskHasher should hash multiple filesets of a project 1`] = ` { "details": { diff --git a/packages/nx/src/hasher/task-hasher.spec.ts b/packages/nx/src/hasher/task-hasher.spec.ts index 802f2403d3b7b5..81d9dbd72dc6c9 100644 --- a/packages/nx/src/hasher/task-hasher.spec.ts +++ b/packages/nx/src/hasher/task-hasher.spec.ts @@ -681,6 +681,7 @@ describe('TaskHasher', () => { }, }, }, + externalNodes: {}, dependencies: { parent: [], }, @@ -794,7 +795,16 @@ describe('TaskHasher', () => { }, }, }, - externalNodes: {}, + externalNodes: { + 'npm:react': { + name: 'npm:react', + type: 'npm', + data: { + version: '17.0.0', + packageName: 'react', + }, + }, + }, dependencies: { 'npm:react': [], app: [ @@ -829,9 +839,7 @@ describe('TaskHasher', () => { }); // note that the parent hash is based on parent source files only! - assertFilesets(hash, { - 'npm:react': { contains: '__npm:react__' }, - }); + expect(hash).toMatchSnapshot(); }); describe('hashTarget', () => { @@ -887,6 +895,113 @@ describe('TaskHasher', () => { expect(hash).toMatchSnapshot(); }); + it('should hash entire subtree of dependencies deterministically', async () => { + function createHasher() { + return new InProcessTaskHasher( + { + a: [{ file: 'a/filea.ts', hash: 'a.hash' }], + b: [{ file: 'b/fileb.ts', hash: 'b.hash' }], + }, + allWorkspaceFiles, + { + nodes: { + a: { + name: 'a', + type: 'lib', + data: { + root: 'a', + targets: { build: { executor: '@nx/webpack:webpack' } }, + }, + }, + b: { + name: 'b', + type: 'lib', + data: { + root: 'b', + targets: { build: { executor: '@nx/webpack:webpack' } }, + }, + }, + }, + externalNodes: { + 'npm:@nx/webpack': { + name: 'npm:@nx/webpack', + type: 'npm', + data: { + packageName: '@nx/webpack', + version: '16.0.0', + hash: '$nx/webpack16$', + }, + }, + }, + dependencies: { + a: [ + { + source: 'a', + target: 'b', + type: DependencyType.static, + }, + ], + b: [ + { + source: 'b', + target: 'a', + type: DependencyType.static, + }, + ], + 'npm:@nx/webpack': [], + }, + }, + { + roots: [], + tasks: { + 'a-build': { + id: 'a-build', + target: { project: 'a', target: 'build' }, + overrides: {}, + }, + 'b-build': { + id: 'b-build', + target: { project: 'b', target: 'build' }, + overrides: {}, + }, + }, + dependencies: {}, + }, + {} as any, + {}, + fileHasher + ); + } + + const hasher1 = createHasher(); + const hasher2 = createHasher(); + + const hashA1 = hasher1.hashTask({ + id: 'a-build', + target: { project: 'a', target: 'build' }, + overrides: {}, + }); + const hashB1 = hasher1.hashTask({ + id: 'b-build', + target: { project: 'b', target: 'build' }, + overrides: {}, + }); + + const hashB2 = hasher2.hashTask({ + id: 'b-build', + target: { project: 'b', target: 'build' }, + overrides: {}, + }); + const hashA2 = hasher2.hashTask({ + id: 'a-build', + target: { project: 'a', target: 'build' }, + overrides: {}, + }); + + expect(hashA1).toEqual(hashA2); + expect(hashB1).toEqual(hashB2); + }); + it('should hash entire subtree of dependencies', async () => { const hasher = new InProcessTaskHasher( {}, @@ -1043,6 +1158,15 @@ describe('TaskHasher', () => { hash: '$packageC0.0.0$', }, }, + 'npm:@nx/webpack': { + name: 'npm:@nx/webpack', + type: 'npm', + data: { + packageName: '@nx/webpack', + version: '0.0.0', + hash: '$@nx/webpack0.0.0$', + }, + }, }, dependencies: { appA: [ @@ -1716,21 +1840,3 @@ describe('TaskHasher', () => { }); }); }); - -function assertFilesets( - hash: Hash, - assertions: { [name: string]: { contains?: string; excludes?: string } } -) { - const nodes = hash.details.nodes; - for (let k of Object.keys(assertions)) { - expect(nodes[k]).toBeDefined(); - if (assertions[k].contains) { - expect(nodes[k]).toContain(assertions[k].contains); - } - if (assertions[k].excludes) { - expect(nodes[k]).not.toContain(assertions[k].excludes); - } - } -} - -//{ [name: string]: string } diff --git a/packages/nx/src/hasher/task-hasher.ts b/packages/nx/src/hasher/task-hasher.ts index acc8e5de56c7c5..c2be78da58adb1 100644 --- a/packages/nx/src/hasher/task-hasher.ts +++ b/packages/nx/src/hasher/task-hasher.ts @@ -186,7 +186,7 @@ class TaskHasherImpl { private runtimeHashes: { [runtime: string]: Promise; } = {}; - private externalDepsHashCache: { [packageName: string]: string } = {}; + private externalDependencyHashes: { [packageName: string]: string } = {}; private projectRootMappings = createProjectRootMappings( this.projectGraph.nodes ); @@ -201,7 +201,10 @@ class TaskHasherImpl { private readonly taskGraph: TaskGraph, private readonly fileHasher: FileHasher, private readonly options: { selectivelyHashTsConfig: boolean } - ) {} + ) { + // External Dependencies are all calculated up front in a deterministic order + this.calculateExternalDependencyHashes(); + } async hashTask(task: Task, visited: string[]): Promise { return Promise.resolve().then(async () => { @@ -334,7 +337,7 @@ class TaskHasherImpl { visited ); } else { - const { hash } = this.hashExternalDependency(d.target); + const hash = this.getExternalDependencyHash(d.target); return { value: hash, details: { @@ -423,21 +426,21 @@ class TaskHasherImpl { return partialHashes; } + private getExternalDependencyHash(externalNodeName: string) { + return this.externalDependencyHashes[externalNodeName]; + } + private hashExternalDependency( - projectName: string, - parentProjects = new Set() - ): { fullyResolved: boolean; hash: string } { + externalNodeName: string, + visited: Set + ): string { // try to retrieve the hash from cache - if (this.externalDepsHashCache[projectName]) { - return { - fullyResolved: true, - hash: this.externalDepsHashCache[projectName], - }; + if (this.externalDependencyHashes[externalNodeName]) { + return this.externalDependencyHashes[externalNodeName]; } - parentProjects.add(projectName); - const node = this.projectGraph.externalNodes[projectName]; + visited.add(externalNodeName); + const node = this.projectGraph.externalNodes[externalNodeName]; let partialHash: string; - let fullyResolved = true; if (node) { const partialHashes: string[] = []; if (node.data.hash) { @@ -448,38 +451,23 @@ class TaskHasherImpl { partialHashes.push(node.data.version); } // we want to calculate the hash of the entire dependency tree - if (this.projectGraph.dependencies[projectName]) { - this.projectGraph.dependencies[projectName].forEach((d) => { - if (!parentProjects.has(d.target)) { - const hashResult = this.hashExternalDependency( - d.target, - new Set(parentProjects) - ); - partialHashes.push(hashResult.hash); - if (!hashResult.fullyResolved) { - fullyResolved = false; - } - } else { - // NOTE: do not store hash to cache since it is only a partial hash - fullyResolved = false; + if (this.projectGraph.dependencies[externalNodeName]) { + this.projectGraph.dependencies[externalNodeName].forEach((d) => { + if (!visited.has(d.target)) { + partialHashes.push(this.hashExternalDependency(d.target, visited)); } }); } - partialHash = hashArray(partialHashes); } else { // unknown dependency // this may occur if dependency is not an npm package // but rather symlinked in node_modules or it's pointing to a remote git repo // in this case we have no information about the versioning of the given package - partialHash = `__${projectName}__`; - } - - if (fullyResolved) { - this.externalDepsHashCache[projectName] = partialHash; + partialHash = `__${externalNodeName}__`; } - - return { fullyResolved, hash: partialHash }; + this.externalDependencyHashes[externalNodeName] = partialHash; + return partialHash; } private hashTarget( @@ -504,7 +492,7 @@ class TaskHasherImpl { const executorPackage = target.executor.split(':')[0]; const executorNodeName = this.findExternalDependencyNodeName(executorPackage); - hash = this.hashExternalDependency(executorNodeName).hash; + hash = this.getExternalDependencyHash(executorNodeName); } else { // use command external dependencies if available to construct the hash const partialHashes: string[] = []; @@ -516,7 +504,7 @@ class TaskHasherImpl { const externalDependencies = input['externalDependencies']; for (let dep of externalDependencies) { dep = this.findExternalDependencyNodeName(dep); - partialHashes.push(this.hashExternalDependency(dep).hash); + partialHashes.push(this.getExternalDependencyHash(dep)); } } } @@ -524,11 +512,11 @@ class TaskHasherImpl { hash = hashArray(partialHashes); } else { // cache the hash of the entire external dependencies tree - if (this.externalDepsHashCache['']) { - hash = this.externalDepsHashCache['']; + if (this.externalDependencyHashes['']) { + hash = this.externalDependencyHashes['']; } else { hash = hashArray([JSON.stringify(this.projectGraph.externalNodes)]); - this.externalDepsHashCache[''] = hash; + this.externalDependencyHashes[''] = hash; } } } @@ -762,6 +750,13 @@ class TaskHasherImpl { value, }; } + + private calculateExternalDependencyHashes() { + const keys = Object.keys(this.projectGraph.externalNodes); + for (const externalNodeName of keys) { + this.hashExternalDependency(externalNodeName, new Set()); + } + } } export function getNamedInputs(