From ec57f5f0831e6e82b87b9ed9ebdfa9fc3d5ba1ee Mon Sep 17 00:00:00 2001 From: Gar Date: Mon, 18 Nov 2024 11:17:06 -0800 Subject: [PATCH] chore: fix dependencies script for circular workspace deps --- DEPENDENCIES.json | 10 ++++------ DEPENDENCIES.md | 9 +++++---- scripts/dependency-graph.js | 36 ++++++++++++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/DEPENDENCIES.json b/DEPENDENCIES.json index 72337999ad90b..dc8ab0751f6ff 100644 --- a/DEPENDENCIES.json +++ b/DEPENDENCIES.json @@ -2,15 +2,10 @@ [ "npm" ], - [ - "@npmcli/smoke-tests", - "libnpmaccess", - "libnpmexec", - "libnpmpublish" - ], [ "@npmcli/mock-registry", "libnpmdiff", + "libnpmexec", "libnpmfund", "libnpmpack" ], @@ -28,7 +23,9 @@ [ "@npmcli/map-workspaces", "@npmcli/run-script", + "libnpmaccess", "libnpmorg", + "libnpmpublish", "libnpmsearch", "libnpmteam", "init-package-json", @@ -50,6 +47,7 @@ ], [ "@npmcli/docs", + "@npmcli/smoke-tests", "@npmcli/fs", "npm-bundled", "npm-install-checks", diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index d35baf4ce93d2..51a390150b471 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -166,6 +166,7 @@ graph LR; npmcli-arborist-->npmcli-installed-package-contents["@npmcli/installed-package-contents"]; npmcli-arborist-->npmcli-map-workspaces["@npmcli/map-workspaces"]; npmcli-arborist-->npmcli-metavuln-calculator["@npmcli/metavuln-calculator"]; + npmcli-arborist-->npmcli-mock-registry["@npmcli/mock-registry"]; npmcli-arborist-->npmcli-name-from-folder["@npmcli/name-from-folder"]; npmcli-arborist-->npmcli-node-gyp["@npmcli/node-gyp"]; npmcli-arborist-->npmcli-package-json["@npmcli/package-json"]; @@ -578,6 +579,7 @@ graph LR; npmcli-arborist-->npmcli-installed-package-contents["@npmcli/installed-package-contents"]; npmcli-arborist-->npmcli-map-workspaces["@npmcli/map-workspaces"]; npmcli-arborist-->npmcli-metavuln-calculator["@npmcli/metavuln-calculator"]; + npmcli-arborist-->npmcli-mock-registry["@npmcli/mock-registry"]; npmcli-arborist-->npmcli-name-from-folder["@npmcli/name-from-folder"]; npmcli-arborist-->npmcli-node-gyp["@npmcli/node-gyp"]; npmcli-arborist-->npmcli-package-json["@npmcli/package-json"]; @@ -777,14 +779,13 @@ Each group depends on packages lower down the chain, nothing depends on packages higher up the chain. - npm - - @npmcli/smoke-tests, libnpmaccess, libnpmexec, libnpmpublish - - @npmcli/mock-registry, libnpmdiff, libnpmfund, libnpmpack + - @npmcli/mock-registry, libnpmdiff, libnpmexec, libnpmfund, libnpmpack - @npmcli/arborist - @npmcli/metavuln-calculator - pacote, @npmcli/config, libnpmversion - - @npmcli/map-workspaces, @npmcli/run-script, libnpmorg, libnpmsearch, libnpmteam, init-package-json, npm-profile + - @npmcli/map-workspaces, @npmcli/run-script, libnpmaccess, libnpmorg, libnpmpublish, libnpmsearch, libnpmteam, init-package-json, npm-profile - @npmcli/package-json, npm-registry-fetch - @npmcli/git, make-fetch-happen - @npmcli/installed-package-contents, npm-pick-manifest, cacache, promzard - - @npmcli/docs, @npmcli/fs, npm-bundled, npm-install-checks, npm-package-arg, normalize-package-data, unique-filename, npm-packlist, bin-links, nopt, parse-conflict-json, read-package-json-fast, @npmcli/mock-globals, read + - @npmcli/docs, @npmcli/smoke-tests, @npmcli/fs, npm-bundled, npm-install-checks, npm-package-arg, normalize-package-data, unique-filename, npm-packlist, bin-links, nopt, parse-conflict-json, read-package-json-fast, @npmcli/mock-globals, read - @npmcli/eslint-config, @npmcli/template-oss, ignore-walk, semver, npm-normalize-package-bin, @npmcli/name-from-folder, @npmcli/promise-spawn, ini, hosted-git-info, proc-log, validate-npm-package-name, json-parse-even-better-errors, fs-minipass, ssri, unique-slug, @npmcli/node-gyp, @npmcli/redact, @npmcli/agent, minipass-fetch, @npmcli/query, cmd-shim, read-cmd-shim, write-file-atomic, abbrev, proggy, minify-registry-metadata, mute-stream, npm-audit-report, npm-user-validate diff --git a/scripts/dependency-graph.js b/scripts/dependency-graph.js index 95c313f5838ed..5a946f0a88558 100644 --- a/scripts/dependency-graph.js +++ b/scripts/dependency-graph.js @@ -11,6 +11,31 @@ const { run, CWD, pkg, fs, EOL } = require('./util.js') // npx -p @npmcli/stafftools gh repos --json | json -a name | sort > scripts/npm-cli-repos.txt const repos = readFileSync(join(CWD, 'scripts', 'npm-cli-repos.txt'), 'utf-8').trim().split(os.EOL) +// Packages with known circular dependencies. This is typically something with arborist as a dependency which is also in arborist's dev dependencies. Not a problem if they're workspaces so we ignore repeats +const circular = new Set(['@npmcli/mock-registry']) + +// TODO Set.intersection/difference was added in node 22.11.0, once we're above that line we can use the builtin +// https://node.green/#ES2025-features-Set-methods-Set-prototype-intersection-- +function intersection (set1, set2) { + const result = new Set() + for (const item of set1) { + if (set2.has(item)) { + result.add(item) + } + } + return result +} + +function difference (set1, set2) { + const result = new Set() + for (const item of set1) { + if (!set2.has(item)) { + result.add(item) + } + } + return result +} + // these have a different package name than the repo name, and are ours. const aliases = { semver: 'node-semver', @@ -29,6 +54,7 @@ const namespaced = [ 'git', 'installed-package-contents', 'lint', + 'mock-registry', 'map-workspaces', 'metavuln-calculator', 'move-file', @@ -140,7 +166,11 @@ const walk = function (tree, onlyOurs) { log.silly(dep, '::', [...dependedBy[dep]].join(', ')) log.silly('-'.repeat(80)) - if (!dependedBy[dep].size) { + // things that depend on us that are at the same level + const both = intersection(allDeps, dependedBy[dep]) + // ... minus the known circular dependencies + const neither = difference(both, circular) + if (!dependedBy[dep].size || !neither.size) { level.push(dep) foundDeps.add(dep) } @@ -177,9 +207,7 @@ const iterate = function (node, dependedBy, annotations, onlyOurs) { dependedBy[node.packageName] = new Set() } for (const [name, edge] of node.edgesOut) { - if ( - (!onlyOurs || isOurs(name)) && !node.dev - ) { + if ((!onlyOurs || isOurs(name)) && !node.dev) { if (!dependedBy[node.packageName].has(edge.name)) { dependedBy[node.packageName].add(edge.name) annotations.push(` ${stripName(node.packageName)}-->${escapeName(edge.name)};`)