From 2d20ad819b2d80112e9a198e0771d3b0df645c43 Mon Sep 17 00:00:00 2001 From: Jason Jean Date: Thu, 18 Apr 2024 09:36:17 -0400 Subject: [PATCH] fix(core): fix hashing of external dependencies (#22865) --- .prettierignore | 2 +- packages/nx/src/native/hasher.rs | 4 +- packages/nx/src/native/index.d.ts | 5 +- packages/nx/src/native/index.js | 11 +- packages/nx/src/native/native-bindings.js | 244 ++++++++---------- packages/nx/src/native/project_graph/types.rs | 7 +- packages/nx/src/native/tasks/hash_planner.rs | 59 ++--- .../src/native/tasks/hashers/hash_external.rs | 2 + packages/nx/src/native/tasks/utils.rs | 15 +- packages/nx/src/native/transform-objects.ts | 1 + 10 files changed, 162 insertions(+), 188 deletions(-) diff --git a/.prettierignore b/.prettierignore index f104106c26..233f7bb441 100644 --- a/.prettierignore +++ b/.prettierignore @@ -17,7 +17,7 @@ packages/nx/src/plugins/js/lock-file/__fixtures__/**/*.* packages/**/schematics/**/files/**/*.html packages/**/generators/**/files/**/*.html packages/nx/src/native/**/*.rs -packages/nx/src/native/index.js +packages/nx/src/native/native-bindings.js packages/nx/src/native/index.d.ts nx-dev/nx-dev/.next/ nx-dev/nx-dev/public/documentation diff --git a/packages/nx/src/native/hasher.rs b/packages/nx/src/native/hasher.rs index 3ef50f3a1d..91a3cb0145 100644 --- a/packages/nx/src/native/hasher.rs +++ b/packages/nx/src/native/hasher.rs @@ -26,8 +26,10 @@ pub fn hash_file_path>(path: P) -> Option { trace!("Failed to read file: {:?}", path); return None; }; + let hash = hash(&content); + trace!("Hashed file {:?} - {:?}", path, hash); - Some(hash(&content)) + Some(hash) } #[cfg(test)] diff --git a/packages/nx/src/native/index.d.ts b/packages/nx/src/native/index.d.ts index c5469a3c9c..d05d51b4a0 100644 --- a/packages/nx/src/native/index.d.ts +++ b/packages/nx/src/native/index.d.ts @@ -29,11 +29,8 @@ export function findImports(projectFileMap: Record>): Arra * This wont be needed once the project graph is created in Rust */ export function transferProjectGraph(projectGraph: ProjectGraph): ExternalObject -export interface ExternalNodeData { - version: string - hash?: string -} export interface ExternalNode { + packageName?: string version: string hash?: string } diff --git a/packages/nx/src/native/index.js b/packages/nx/src/native/index.js index 7f0ce5277f..eef8087714 100644 --- a/packages/nx/src/native/index.js +++ b/packages/nx/src/native/index.js @@ -1,8 +1,8 @@ -const { join, basename } = require('path'); +const { join, basename } = require('path'); const { copyFileSync, existsSync, mkdirSync } = require('fs'); const Module = require('module'); -const { nxVersion} = require("../utils/versions") -const { cacheDir} = require("../utils/cache-directory") +const { nxVersion } = require('../utils/versions'); +const { cacheDir } = require('../utils/cache-directory'); const nxPackages = new Set([ '@nx/nx-android-arm64', @@ -40,7 +40,7 @@ const localNodeFiles = [ const originalLoad = Module._load; -// We override the _load function so that when a native file is required, +// We override the _load function so that when a native file is required, // we copy it to a cache directory and require it from there. // This prevents the file being loaded from node_modules and causing file locking issues. // Will only be called once because the require cache takes over afterwards. @@ -51,7 +51,7 @@ Module._load = function (request, parent, isMain) { localNodeFiles.some((f) => modulePath.endsWith(f)) ) { const nativeLocation = require.resolve(modulePath); - const fileName = basename(nativeLocation) + const fileName = basename(nativeLocation); // we copy the file to the cache directory (.nx/cache by default) and prefix with nxVersion to avoid stale files being loaded const tmpFile = join(cacheDir, nxVersion + '-' + fileName); if (existsSync(tmpFile)) { @@ -72,6 +72,5 @@ const indexModulePath = require.resolve('./native-bindings.js'); delete require.cache[indexModulePath]; const indexModule = require('./native-bindings.js'); - module.exports = indexModule; Module._load = originalLoad; diff --git a/packages/nx/src/native/native-bindings.js b/packages/nx/src/native/native-bindings.js index 005745e916..21277c6d1e 100644 --- a/packages/nx/src/native/native-bindings.js +++ b/packages/nx/src/native/native-bindings.js @@ -1,27 +1,24 @@ -const { existsSync, readFileSync } = require('fs'); -const { join } = require('path'); +const { existsSync, readFileSync } = require('fs') +const { join } = require('path') -const { platform, arch } = process; +const { platform, arch } = process -let nativeBinding = null; -let localFileExisted = false; -let loadError = null; +let nativeBinding = null +let localFileExisted = false +let loadError = null function isMusl() { // For Node 10 if (!process.report || typeof process.report.getReport !== 'function') { try { - const lddPath = require('child_process') - .execSync('which ldd') - .toString() - .trim(); - return readFileSync(lddPath, 'utf8').includes('musl'); + const lddPath = require('child_process').execSync('which ldd').toString().trim(); + return readFileSync(lddPath, 'utf8').includes('musl') } catch (e) { - return true; + return true } } else { - const { glibcVersionRuntime } = process.report.getReport().header; - return !glibcVersionRuntime; + const { glibcVersionRuntime } = process.report.getReport().header + return !glibcVersionRuntime } } @@ -29,262 +26,243 @@ switch (platform) { case 'android': switch (arch) { case 'arm64': - localFileExisted = existsSync(join(__dirname, 'nx.android-arm64.node')); + localFileExisted = existsSync(join(__dirname, 'nx.android-arm64.node')) try { if (localFileExisted) { - nativeBinding = require('./nx.android-arm64.node'); + nativeBinding = require('./nx.android-arm64.node') } else { - nativeBinding = require('@nx/nx-android-arm64'); + nativeBinding = require('@nx/nx-android-arm64') } } catch (e) { - loadError = e; + loadError = e } - break; + break case 'arm': - localFileExisted = existsSync( - join(__dirname, 'nx.android-arm-eabi.node') - ); + localFileExisted = existsSync(join(__dirname, 'nx.android-arm-eabi.node')) try { if (localFileExisted) { - nativeBinding = require('./nx.android-arm-eabi.node'); + nativeBinding = require('./nx.android-arm-eabi.node') } else { - nativeBinding = require('@nx/nx-android-arm-eabi'); + nativeBinding = require('@nx/nx-android-arm-eabi') } } catch (e) { - loadError = e; + loadError = e } - break; + break default: - throw new Error(`Unsupported architecture on Android ${arch}`); + throw new Error(`Unsupported architecture on Android ${arch}`) } - break; + break case 'win32': switch (arch) { case 'x64': localFileExisted = existsSync( join(__dirname, 'nx.win32-x64-msvc.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.win32-x64-msvc.node'); + nativeBinding = require('./nx.win32-x64-msvc.node') } else { - nativeBinding = require('@nx/nx-win32-x64-msvc'); + nativeBinding = require('@nx/nx-win32-x64-msvc') } } catch (e) { - loadError = e; + loadError = e } - break; + break case 'ia32': localFileExisted = existsSync( join(__dirname, 'nx.win32-ia32-msvc.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.win32-ia32-msvc.node'); + nativeBinding = require('./nx.win32-ia32-msvc.node') } else { - nativeBinding = require('@nx/nx-win32-ia32-msvc'); + nativeBinding = require('@nx/nx-win32-ia32-msvc') } } catch (e) { - loadError = e; + loadError = e } - break; + break case 'arm64': localFileExisted = existsSync( join(__dirname, 'nx.win32-arm64-msvc.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.win32-arm64-msvc.node'); + nativeBinding = require('./nx.win32-arm64-msvc.node') } else { - nativeBinding = require('@nx/nx-win32-arm64-msvc'); + nativeBinding = require('@nx/nx-win32-arm64-msvc') } } catch (e) { - loadError = e; + loadError = e } - break; + break default: - throw new Error(`Unsupported architecture on Windows: ${arch}`); + throw new Error(`Unsupported architecture on Windows: ${arch}`) } - break; + break case 'darwin': - localFileExisted = existsSync(join(__dirname, 'nx.darwin-universal.node')); + localFileExisted = existsSync(join(__dirname, 'nx.darwin-universal.node')) try { if (localFileExisted) { - nativeBinding = require('./nx.darwin-universal.node'); + nativeBinding = require('./nx.darwin-universal.node') } else { - nativeBinding = require('@nx/nx-darwin-universal'); + nativeBinding = require('@nx/nx-darwin-universal') } - break; + break } catch {} switch (arch) { case 'x64': - localFileExisted = existsSync(join(__dirname, 'nx.darwin-x64.node')); + localFileExisted = existsSync(join(__dirname, 'nx.darwin-x64.node')) try { if (localFileExisted) { - nativeBinding = require('./nx.darwin-x64.node'); + nativeBinding = require('./nx.darwin-x64.node') } else { - nativeBinding = require('@nx/nx-darwin-x64'); + nativeBinding = require('@nx/nx-darwin-x64') } } catch (e) { - loadError = e; + loadError = e } - break; + break case 'arm64': - localFileExisted = existsSync(join(__dirname, 'nx.darwin-arm64.node')); + localFileExisted = existsSync( + join(__dirname, 'nx.darwin-arm64.node') + ) try { if (localFileExisted) { - nativeBinding = require('./nx.darwin-arm64.node'); + nativeBinding = require('./nx.darwin-arm64.node') } else { - nativeBinding = require('@nx/nx-darwin-arm64'); + nativeBinding = require('@nx/nx-darwin-arm64') } } catch (e) { - loadError = e; + loadError = e } - break; + break default: - throw new Error(`Unsupported architecture on macOS: ${arch}`); + throw new Error(`Unsupported architecture on macOS: ${arch}`) } - break; + break case 'freebsd': if (arch !== 'x64') { - throw new Error(`Unsupported architecture on FreeBSD: ${arch}`); + throw new Error(`Unsupported architecture on FreeBSD: ${arch}`) } - localFileExisted = existsSync(join(__dirname, 'nx.freebsd-x64.node')); + localFileExisted = existsSync(join(__dirname, 'nx.freebsd-x64.node')) try { if (localFileExisted) { - nativeBinding = require('./nx.freebsd-x64.node'); + nativeBinding = require('./nx.freebsd-x64.node') } else { - nativeBinding = require('@nx/nx-freebsd-x64'); + nativeBinding = require('@nx/nx-freebsd-x64') } } catch (e) { - loadError = e; + loadError = e } - break; + break case 'linux': switch (arch) { case 'x64': if (isMusl()) { localFileExisted = existsSync( join(__dirname, 'nx.linux-x64-musl.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.linux-x64-musl.node'); + nativeBinding = require('./nx.linux-x64-musl.node') } else { - nativeBinding = require('@nx/nx-linux-x64-musl'); + nativeBinding = require('@nx/nx-linux-x64-musl') } } catch (e) { - loadError = e; + loadError = e } } else { localFileExisted = existsSync( join(__dirname, 'nx.linux-x64-gnu.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.linux-x64-gnu.node'); + nativeBinding = require('./nx.linux-x64-gnu.node') } else { - nativeBinding = require('@nx/nx-linux-x64-gnu'); + nativeBinding = require('@nx/nx-linux-x64-gnu') } } catch (e) { - loadError = e; + loadError = e } } - break; + break case 'arm64': if (isMusl()) { localFileExisted = existsSync( join(__dirname, 'nx.linux-arm64-musl.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.linux-arm64-musl.node'); + nativeBinding = require('./nx.linux-arm64-musl.node') } else { - nativeBinding = require('@nx/nx-linux-arm64-musl'); + nativeBinding = require('@nx/nx-linux-arm64-musl') } } catch (e) { - loadError = e; + loadError = e } } else { localFileExisted = existsSync( join(__dirname, 'nx.linux-arm64-gnu.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.linux-arm64-gnu.node'); + nativeBinding = require('./nx.linux-arm64-gnu.node') } else { - nativeBinding = require('@nx/nx-linux-arm64-gnu'); + nativeBinding = require('@nx/nx-linux-arm64-gnu') } } catch (e) { - loadError = e; + loadError = e } } - break; + break case 'arm': localFileExisted = existsSync( join(__dirname, 'nx.linux-arm-gnueabihf.node') - ); + ) try { if (localFileExisted) { - nativeBinding = require('./nx.linux-arm-gnueabihf.node'); + nativeBinding = require('./nx.linux-arm-gnueabihf.node') } else { - nativeBinding = require('@nx/nx-linux-arm-gnueabihf'); + nativeBinding = require('@nx/nx-linux-arm-gnueabihf') } } catch (e) { - loadError = e; + loadError = e } - break; + break default: - throw new Error(`Unsupported architecture on Linux: ${arch}`); + throw new Error(`Unsupported architecture on Linux: ${arch}`) } - break; + break default: - throw new Error(`Unsupported OS: ${platform}, architecture: ${arch}`); + throw new Error(`Unsupported OS: ${platform}, architecture: ${arch}`) } if (!nativeBinding) { if (loadError) { - throw loadError; + throw loadError } - throw new Error(`Failed to load native binding`); + throw new Error(`Failed to load native binding`) } -const { - expandOutputs, - getFilesForOutputs, - remove, - copy, - hashArray, - hashFile, - ImportResult, - findImports, - transferProjectGraph, - ChildProcess, - RustPseudoTerminal, - HashPlanner, - TaskHasher, - EventType, - Watcher, - WorkspaceContext, - WorkspaceErrors, - testOnlyTransferFileMap, -} = nativeBinding; +const { expandOutputs, getFilesForOutputs, remove, copy, hashArray, hashFile, ImportResult, findImports, transferProjectGraph, ChildProcess, RustPseudoTerminal, HashPlanner, TaskHasher, EventType, Watcher, WorkspaceContext, WorkspaceErrors, testOnlyTransferFileMap } = nativeBinding -module.exports.expandOutputs = expandOutputs; -module.exports.getFilesForOutputs = getFilesForOutputs; -module.exports.remove = remove; -module.exports.copy = copy; -module.exports.hashArray = hashArray; -module.exports.hashFile = hashFile; -module.exports.ImportResult = ImportResult; -module.exports.findImports = findImports; -module.exports.transferProjectGraph = transferProjectGraph; -module.exports.ChildProcess = ChildProcess; -module.exports.RustPseudoTerminal = RustPseudoTerminal; -module.exports.HashPlanner = HashPlanner; -module.exports.TaskHasher = TaskHasher; -module.exports.EventType = EventType; -module.exports.Watcher = Watcher; -module.exports.WorkspaceContext = WorkspaceContext; -module.exports.WorkspaceErrors = WorkspaceErrors; -module.exports.testOnlyTransferFileMap = testOnlyTransferFileMap; +module.exports.expandOutputs = expandOutputs +module.exports.getFilesForOutputs = getFilesForOutputs +module.exports.remove = remove +module.exports.copy = copy +module.exports.hashArray = hashArray +module.exports.hashFile = hashFile +module.exports.ImportResult = ImportResult +module.exports.findImports = findImports +module.exports.transferProjectGraph = transferProjectGraph +module.exports.ChildProcess = ChildProcess +module.exports.RustPseudoTerminal = RustPseudoTerminal +module.exports.HashPlanner = HashPlanner +module.exports.TaskHasher = TaskHasher +module.exports.EventType = EventType +module.exports.Watcher = Watcher +module.exports.WorkspaceContext = WorkspaceContext +module.exports.WorkspaceErrors = WorkspaceErrors +module.exports.testOnlyTransferFileMap = testOnlyTransferFileMap diff --git a/packages/nx/src/native/project_graph/types.rs b/packages/nx/src/native/project_graph/types.rs index 11193a4429..8eb4171bc0 100644 --- a/packages/nx/src/native/project_graph/types.rs +++ b/packages/nx/src/native/project_graph/types.rs @@ -1,14 +1,9 @@ use crate::native::types::JsInputs; use std::collections::HashMap; -#[napi(object)] -pub struct ExternalNodeData { - pub version: String, - pub hash: Option, -} - #[napi(object)] pub struct ExternalNode { + pub package_name: Option, pub version: String, pub hash: Option, } diff --git a/packages/nx/src/native/tasks/hash_planner.rs b/packages/nx/src/native/tasks/hash_planner.rs index ae6db03b03..19e6944388 100644 --- a/packages/nx/src/native/tasks/hash_planner.rs +++ b/packages/nx/src/native/tasks/hash_planner.rs @@ -110,20 +110,13 @@ impl HashPlanner { project_name: &str, target_name: &str, self_inputs: &[Input], - external_deps_map: &hashbrown::HashMap<&str, Vec<&'a str>>, + external_deps_map: &hashbrown::HashMap<&String, Vec<&'a String>>, ) -> anyhow::Result>> { let project = &self.project_graph.nodes[project_name]; let Some(target) = project.targets.get(target_name) else { return Ok(None); }; - let external_nodes_keys: Vec<&str> = self - .project_graph - .external_nodes - .keys() - .map(|s| s.as_str()) - .collect(); - // we can only vouch for @nx packages's executor dependencies // if it's "run commands" or third-party we skip traversing since we have no info what this command depends on if target @@ -139,29 +132,29 @@ impl HashPlanner { .next() .expect("Executors should always have a ':'"); let Some(existing_package) = - find_external_dependency_node_name(executor_package, &external_nodes_keys) + find_external_dependency_node_name(executor_package, &self.project_graph) else { // this usually happens because the executor was a local plugin. // todo)) @Cammisuli: we need to gather the project's inputs and its dep inputs similar to how we do it in `self_and_deps_inputs` return Ok(None); }; Ok(Some(vec![HashInstruction::External( - existing_package.to_string(), + existing_package.to_owned(), )])) } else { - let mut external_deps: Vec<&str> = vec![]; + let mut external_deps: Vec<&'a String> = vec![]; for input in self_inputs { match input { Input::ExternalDependency(deps) => { for dep in deps.iter() { - let external_node_name: Option<&str> = - find_external_dependency_node_name(dep, &external_nodes_keys); + let external_node_name = + find_external_dependency_node_name(dep, &self.project_graph); let Some(external_node_name) = external_node_name else { anyhow::bail!("The externalDependency '{dep}' for '{project_name}:{target_name}' could not be found") }; - external_deps.push(external_node_name); - external_deps.extend(&external_deps_map[external_node_name]); + external_deps.push(&external_node_name); + external_deps.extend(&external_deps_map[&external_node_name]); } } _ => continue, @@ -186,12 +179,11 @@ impl HashPlanner { task: &Task, inputs: &SplitInputs, task_graph: &TaskGraph, - external_deps_mapped: &hashbrown::HashMap<&str, Vec<&str>>, + external_deps_mapped: &hashbrown::HashMap<&String, Vec<&String>>, visited: &mut Box>, ) -> anyhow::Result> { let project_deps = &self.project_graph.dependencies[project_name] .iter() - .map(|d| d.as_str()) .collect::>(); let self_inputs = self.gather_self_inputs(project_name, &inputs.self_inputs); let deps_inputs = self.gather_dependency_inputs( @@ -215,15 +207,13 @@ impl HashPlanner { .collect()) } - fn setup_external_deps(&self) -> hashbrown::HashMap<&str, Vec<&str>> { + fn setup_external_deps(&self) -> hashbrown::HashMap<&String, Vec<&String>> { self.project_graph .external_nodes .keys() - .collect::>() - .par_iter() .map(|external_node| { ( - external_node.as_str(), + external_node, utils::find_all_project_node_dependencies( external_node, &self.project_graph, @@ -240,8 +230,8 @@ impl HashPlanner { task: &Task, inputs: &[Input], task_graph: &TaskGraph, - project_deps: &[&'a str], - external_deps_mapped: &hashbrown::HashMap<&str, Vec<&'a str>>, + project_deps: &[&'a String], + external_deps_mapped: &hashbrown::HashMap<&String, Vec<&'a String>>, visited: &mut Box>, ) -> anyhow::Result> { let mut deps_inputs: Vec = vec![]; @@ -377,12 +367,23 @@ impl HashPlanner { fn find_external_dependency_node_name<'a>( package_name: &str, - external_nodes: &[&'a str], -) -> Option<&'a str> { - external_nodes - .iter() - .find(|n| **n == package_name || n.ends_with(package_name)) - .copied() + project_graph: &'a ProjectGraph, +) -> Option<&'a String> { + let npm_name = format!("npm:{}", &package_name); + if let Some((key, _)) = project_graph.external_nodes.get_key_value(package_name) { + Some(key) + } else if let Some((key, _)) = project_graph.external_nodes.get_key_value(&npm_name) { + Some(key) + } else { + for (node_name, node) in project_graph.external_nodes.iter() { + if let Some(pkg_name) = &node.package_name { + if pkg_name.as_str() == package_name { + return Some(node_name); + } + } + } + None + } } fn project_file_set_inputs(project_name: &str, file_sets: Vec<&str>) -> Vec { diff --git a/packages/nx/src/native/tasks/hashers/hash_external.rs b/packages/nx/src/native/tasks/hashers/hash_external.rs index 21105205cb..1b86e1f89c 100644 --- a/packages/nx/src/native/tasks/hashers/hash_external.rs +++ b/packages/nx/src/native/tasks/hashers/hash_external.rs @@ -54,6 +54,7 @@ mod test { ( "my_external".to_string(), ExternalNode { + package_name: Some("my_external".into()), version: "0.0.1".into(), hash: None, }, @@ -61,6 +62,7 @@ mod test { ( "my_external_with_hash".to_string(), ExternalNode { + package_name: Some("my_external_with_hash".into()), version: "0.0.1".into(), hash: Some("hashvalue".into()), }, diff --git a/packages/nx/src/native/tasks/utils.rs b/packages/nx/src/native/tasks/utils.rs index 6a97909240..0362560803 100644 --- a/packages/nx/src/native/tasks/utils.rs +++ b/packages/nx/src/native/tasks/utils.rs @@ -5,7 +5,7 @@ pub(super) fn find_all_project_node_dependencies<'a>( parent_node_name: &str, project_graph: &'a ProjectGraph, exclude_external_dependencies: bool, -) -> Vec<&'a str> { +) -> Vec<&'a String> { let mut dependent_node_names = HashSet::new(); collect_dependent_project_node_names( project_graph, @@ -19,28 +19,27 @@ pub(super) fn find_all_project_node_dependencies<'a>( fn collect_dependent_project_node_names<'a>( project_graph: &'a ProjectGraph, parent_node_name: &str, - dependent_node_names: &mut HashSet<&'a str>, + dependent_node_names: &mut HashSet<&'a String>, exclude_external_dependencies: bool, ) { - let Some( dependencies ) = project_graph.dependencies.get(parent_node_name) else { + let Some(dependencies) = project_graph.dependencies.get(parent_node_name) else { return; }; for dependency in dependencies { - let dep = dependency.as_str(); // skip dependencies already added (avoid circular dependencies) - if dependent_node_names.contains(dep) { + if dependent_node_names.contains(dependency) { continue; } - if exclude_external_dependencies && project_graph.external_nodes.contains_key(dep) { + if exclude_external_dependencies && project_graph.external_nodes.contains_key(dependency) { continue; } - dependent_node_names.insert(dep); + dependent_node_names.insert(dependency); collect_dependent_project_node_names( project_graph, - dependency.as_str(), + dependency, dependent_node_names, exclude_external_dependencies, ); diff --git a/packages/nx/src/native/transform-objects.ts b/packages/nx/src/native/transform-objects.ts index 054ea9f933..923652bf06 100644 --- a/packages/nx/src/native/transform-objects.ts +++ b/packages/nx/src/native/transform-objects.ts @@ -42,6 +42,7 @@ export function transformProjectGraphForRust( graph.externalNodes ?? {} )) { externalNodes[projectName] = { + packageName: externalNode.data.packageName, hash: externalNode.data.hash, version: externalNode.data.version, };