From 1b55ab71b7ff81f1b8f9b3b4b942fc1be2f6b07d Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 09:09:14 -0700 Subject: [PATCH 1/8] fix: remove metric-registry config BREAKING CHANGE: the hard-coded "metrics-registry" config has been removed. This is not used by npm at all, and is also not used by any of the npm modules that consume config. npm does not send data to any metrics server. --- workspaces/config/lib/index.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/workspaces/config/lib/index.js b/workspaces/config/lib/index.js index 0e19d32e3f8b4..fda049fe08535 100644 --- a/workspaces/config/lib/index.js +++ b/workspaces/config/lib/index.js @@ -322,10 +322,6 @@ class Config { const { data } = this.data.get('default') - // the metrics-registry defaults to the current resolved value of - // the registry, unless overridden somewhere else. - settableGetter(data, 'metrics-registry', () => this.#get('registry')) - // if the prefix is set on cli, env, or userconfig, then we need to // default the globalconfig file to that location, instead of the default // global prefix. It's weird that `npm get globalconfig --prefix=/foo` From 0270cfefbedd7c683b294e77d76a404614c0334f Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 09:11:45 -0700 Subject: [PATCH 2/8] fix: remove "tmp" config BREAKING CHANGE: the unused "tmp" config has been removed --- .../config/lib/definitions/definitions.js | 20 +------------------ .../test/type-description.js.test.cjs | 3 --- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index fe5cafa1922d9..6eb845c7e4bae 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -64,7 +64,7 @@ const editor = process.env.EDITOR || const shell = isWindows ? process.env.ComSpec || 'cmd' : process.env.SHELL || 'sh' -const { tmpdir, networkInterfaces } = require('os') +const { networkInterfaces } = require('os') const getLocalAddresses = () => { try { return Object.values(networkInterfaces()).map( @@ -2127,24 +2127,6 @@ define('timing', { `, }) -define('tmp', { - default: tmpdir(), - defaultDescription: ` - The value returned by the Node.js \`os.tmpdir()\` method - - `, - type: path, - deprecated: ` - This setting is no longer used. npm stores temporary files in a special - location in the cache, and they are managed by - [\`cacache\`](http://npm.im/cacache). - `, - description: ` - Historically, the location where temporary files were stored. No longer - relevant. - `, -}) - define('umask', { default: 0, type: Umask, diff --git a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs index 5157d6708b0a4..4ef28584bf993 100644 --- a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs +++ b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs @@ -477,9 +477,6 @@ Object { "timing": Array [ "boolean value (true or false)", ], - "tmp": Array [ - "valid filesystem path", - ], "umask": Array [ "octal number in range 0o000..0o777 (0..511)", ], From f8d305b3be7a04d2dcd9661c120e56df55ddf12f Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 09:23:03 -0700 Subject: [PATCH 3/8] fix: remove "hashAlgorithm" from flatOptions BREAKING CHANGE: the hard-coded "hashAlgorithm" value is no longer being passed through flatOptions Nothing in npm is using this. --- workspaces/config/lib/definitions/index.js | 3 --- workspaces/config/test/definitions/index.js | 2 -- 2 files changed, 5 deletions(-) diff --git a/workspaces/config/lib/definitions/index.js b/workspaces/config/lib/definitions/index.js index 748f306bd2ce3..51c7aa7c352cf 100644 --- a/workspaces/config/lib/definitions/index.js +++ b/workspaces/config/lib/definitions/index.js @@ -25,9 +25,6 @@ const flatten = (obj, flat = {}) => { : /* istanbul ignore next - not configurable property */ undefined flat.nodeBin = process.env.NODE || process.execPath - // XXX should this be sha512? is it even relevant? - flat.hashAlgorithm = 'sha1' - return flat } diff --git a/workspaces/config/test/definitions/index.js b/workspaces/config/test/definitions/index.js index 26c2a7fdf84e2..35bf4bc6885fd 100644 --- a/workspaces/config/test/definitions/index.js +++ b/workspaces/config/test/definitions/index.js @@ -36,7 +36,6 @@ t.test('flatten', t => { '//foo.bar.com:_authToken': 'foobarbazquuxasdf', npmBin: '/path/to/npm', nodeBin: '/path/to/node', - hashAlgorithm: 'sha1', }) mockGlobals(t, { @@ -52,7 +51,6 @@ t.test('flatten', t => { '//foo.bar.com:_authToken': 'foobarbazquuxasdf', npmBin: '/path/to/npm', nodeBin: '/usr/local/bin/node.exe', - hashAlgorithm: 'sha1', }) t.end() From 4f73fff0175d9261fa598de7c682c528876b4d74 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 10:36:07 -0700 Subject: [PATCH 4/8] chore: test fixes for breaking changes in workspaces --- .../test/lib/commands/config.js.test.cjs | 8 +---- tap-snapshots/test/lib/docs.js.test.cjs | 34 ------------------- test/fixtures/sandbox.js | 11 ------ 3 files changed, 1 insertion(+), 52 deletions(-) diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index 7c590b1cf3fb4..a7c71e72de74b 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -30,7 +30,6 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "cafile": null, "call": "", "cert": null, - "ci-name": null, "cidr": null, "color": true, "commit-hooks": true, @@ -147,7 +146,6 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "tag": "latest", "tag-version-prefix": "v", "timing": false, - "tmp": "{TMP}", "umask": 0, "unicode": false, "update-notifier": true, @@ -161,8 +159,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "workspaces": null, "workspaces-update": true, "yes": null, - "npm-version": "{NPM-VERSION}", - "metrics-registry": "https://registry.npmjs.org/" + "npm-version": "{NPM-VERSION}" } ` @@ -187,7 +184,6 @@ cache-min = 0 cafile = null call = "" cert = null -ci-name = null cidr = null color = true commit-hooks = true @@ -254,7 +250,6 @@ logs-max = 10 ; long = false ; overridden by cli maxsockets = 15 message = "%s" -metrics-registry = "https://registry.npmjs.org/" node-options = null noproxy = [""] npm-version = "{NPM-VERSION}" @@ -306,7 +301,6 @@ strict-ssl = true tag = "latest" tag-version-prefix = "v" timing = false -tmp = "{TMP}" umask = 0 unicode = false update-notifier = true diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 99ddff6e150b3..2c8f8124235b6 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -1822,20 +1822,6 @@ registry-scoped "certfile" path like -#### \`ci-name\` - -* Default: The name of the current CI system, or \`null\` when not on a known CI - platform. -* Type: null or String -* DEPRECATED: This config is deprecated and will not be changeable in future - version of npm. - -The name of a continuous integration system. If not set explicitly, npm will -detect the current CI environment using the -[\`ci-info\`](http://npm.im/ci-info) module. - - - #### \`dev\` * Default: false @@ -1995,20 +1981,6 @@ Alias for \`--omit=dev\` Alias for --package-lock - -#### \`tmp\` - -* Default: The value returned by the Node.js \`os.tmpdir()\` method - -* Type: Path -* DEPRECATED: This setting is no longer used. npm stores temporary files in a - special location in the cache, and they are managed by - [\`cacache\`](http://npm.im/cacache). - -Historically, the location where temporary files were stored. No longer -relevant. - - ` exports[`test/lib/docs.js TAP config > all keys 1`] = ` @@ -2031,7 +2003,6 @@ Array [ "cafile", "call", "cert", - "ci-name", "cidr", "color", "commit-hooks", @@ -2148,7 +2119,6 @@ Array [ "tag", "tag-version-prefix", "timing", - "tmp", "umask", "unicode", "update-notifier", @@ -2186,7 +2156,6 @@ Array [ "cafile", "call", "cert", - "ci-name", "cidr", "color", "commit-hooks", @@ -2314,7 +2283,6 @@ Array [ "node-options", "prefix", "timing", - "tmp", "update-notifier", "usage", "userconfig", @@ -2343,7 +2311,6 @@ Object { "call": "", "cert": null, "cidr": null, - "ciName": "{ci}", "color": false, "commitHooks": true, "defaultTag": "latest", @@ -2367,7 +2334,6 @@ Object { "gitTagVersion": true, "global": false, "globalconfig": "{CWD}/global/etc/npmrc", - "hashAlgorithm": "sha1", "heading": "npm", "httpsProxy": null, "ifPresent": false, diff --git a/test/fixtures/sandbox.js b/test/fixtures/sandbox.js index 2c4e5c2968a38..5be02fcf80c1e 100644 --- a/test/fixtures/sandbox.js +++ b/test/fixtures/sandbox.js @@ -42,11 +42,6 @@ const _get = Symbol('sandbox.proxy.get') const _set = Symbol('sandbox.proxy.set') const _logs = Symbol('sandbox.logs') -// these config keys can be redacted widely -const redactedDefaults = [ - 'tmp', -] - // we can't just replace these values everywhere because they're known to be // very short strings that could be present all over the place, so we only // replace them if they're located within quotes for now @@ -161,12 +156,6 @@ class Sandbox extends EventEmitter { // and we replaced the node version first, the real execPath we're trying // to replace would no longer be represented, and be missed. if (this[_npm]) { - // replace default config values with placeholders - for (const name of redactedDefaults) { - const value = this[_npm].config.defaults[name] - clean = clean.split(normalize(value)).join(`{${name.toUpperCase()}}`) - } - // replace vague default config values that are present within quotes // with placeholders for (const name of vagueRedactedDefaults) { From 548548f08a85064393a887c3f0872972c832e371 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 12:28:54 -0700 Subject: [PATCH 5/8] fix: stop retrying on 409 conflict BREAKING CHANGE: libnpmpublish will no longer attempt a single automatic retry on 409 responses during publish. --- workspaces/libnpmpublish/lib/publish.js | 89 +------- workspaces/libnpmpublish/test/publish.js | 261 ----------------------- 2 files changed, 9 insertions(+), 341 deletions(-) diff --git a/workspaces/libnpmpublish/lib/publish.js b/workspaces/libnpmpublish/lib/publish.js index 554eb9bec46f8..b0ef782a166c6 100644 --- a/workspaces/libnpmpublish/lib/publish.js +++ b/workspaces/libnpmpublish/lib/publish.js @@ -50,42 +50,16 @@ Remove the 'private' field from the package.json to publish it.`), opts ) - try { - const res = await npmFetch(spec.escapedName, { - ...opts, - method: 'PUT', - body: metadata, - ignoreBody: true, - }) - if (transparencyLogUrl) { - res.transparencyLogUrl = transparencyLogUrl - } - return res - } catch (err) { - if (err.code !== 'E409') { - throw err - } - // if E409, we attempt exactly ONE retry, to protect us - // against malicious activity like trying to publish - // a bunch of new versions of a package at the same time - // and/or spamming the registry - const current = await npmFetch.json(spec.escapedName, { - ...opts, - query: { write: true }, - }) - const newMetadata = patchMetadata(current, metadata) - const res = await npmFetch(spec.escapedName, { - ...opts, - method: 'PUT', - body: newMetadata, - ignoreBody: true, - }) - /* istanbul ignore next */ - if (transparencyLogUrl) { - res.transparencyLogUrl = transparencyLogUrl - } - return res + const res = await npmFetch(spec.escapedName, { + ...opts, + method: 'PUT', + body: metadata, + ignoreBody: true, + }) + if (transparencyLogUrl) { + res.transparencyLogUrl = transparencyLogUrl } + return res } const patchManifest = (_manifest, opts) => { @@ -195,51 +169,6 @@ const buildMetadata = async (registry, manifest, tarballData, spec, opts) => { } } -const patchMetadata = (current, newData) => { - const curVers = Object.keys(current.versions || {}) - .map(v => semver.clean(v, true)) - .concat(Object.keys(current.time || {}) - .map(v => semver.valid(v, true) && semver.clean(v, true)) - .filter(v => v)) - - const newVersion = Object.keys(newData.versions)[0] - - if (curVers.indexOf(newVersion) !== -1) { - const { name: pkgid, version } = newData - throw Object.assign( - new Error( - `Cannot publish ${pkgid}@${version} over existing version.` - ), { - code: 'EPUBLISHCONFLICT', - pkgid, - version, - }) - } - - current.versions = current.versions || {} - current.versions[newVersion] = newData.versions[newVersion] - for (const i in newData) { - switch (i) { - // objects that copy over the new stuffs - case 'dist-tags': - case 'versions': - case '_attachments': - for (const j in newData[i]) { - current[i] = current[i] || {} - current[i][j] = newData[i][j] - } - break - - // copy - default: - current[i] = newData[i] - break - } - } - - return current -} - // Check that all the prereqs are met for provenance generation const ensureProvenanceGeneration = async (registry, spec, opts) => { if (ciInfo.GITHUB_ACTIONS) { diff --git a/workspaces/libnpmpublish/test/publish.js b/workspaces/libnpmpublish/test/publish.js index 6744443e6843c..f7e824a3805c8 100644 --- a/workspaces/libnpmpublish/test/publish.js +++ b/workspaces/libnpmpublish/test/publish.js @@ -1,6 +1,5 @@ 'use strict' -const cloneDeep = require('lodash.clonedeep') const crypto = require('crypto') const fs = require('fs') const npa = require('npm-package-arg') @@ -180,266 +179,6 @@ t.test('scoped publish - restricted access', async t => { t.ok(ret, 'publish succeeded') }) -t.test('retry after a conflict', async t => { - const { publish } = t.mock('..') - const registry = new MockRegistry({ - tap: t, - registry: opts.registry, - authorization: token, - }) - const REV = '72-47f2986bfd8e8b55068b204588bbf484' - const manifest = { - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - } - - const basePackument = { - name: 'libnpmpublish', - description: 'some stuff', - access: 'public', - _id: 'libnpmpublish', - 'dist-tags': {}, - versions: {}, - _attachments: {}, - } - const currentPackument = cloneDeep({ - ...basePackument, - time: { - modified: new Date().toISOString(), - created: new Date().toISOString(), - '1.0.1': new Date().toISOString(), - }, - 'dist-tags': { latest: '1.0.1' }, - versions: { - '1.0.1': { - _id: 'libnpmpublish@1.0.1', - _nodeVersion: process.versions.node, - _npmVersion: opts.npmVersion, - name: 'libnpmpublish', - version: '1.0.1', - description: 'some stuff', - dist: { - shasum, - integrity: integrity.toString(), - tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.1.tgz', - }, - }, - }, - _attachments: { - 'libnpmpublish-1.0.1.tgz': { - content_type: 'application/octet-stream', - data: tarData.toString('base64'), - length: tarData.length, - }, - }, - }) - const newPackument = cloneDeep({ - ...basePackument, - 'dist-tags': { latest: '1.0.0' }, - versions: { - '1.0.0': { - _id: 'libnpmpublish@1.0.0', - _nodeVersion: process.versions.node, - _npmVersion: opts.npmVersion, - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - dist: { - shasum, - integrity: integrity.toString(), - tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz', - }, - }, - }, - _attachments: { - 'libnpmpublish-1.0.0.tgz': { - content_type: 'application/octet-stream', - data: tarData.toString('base64'), - length: tarData.length, - }, - }, - }) - const mergedPackument = cloneDeep({ - ...basePackument, - time: currentPackument.time, - 'dist-tags': { latest: '1.0.0' }, - versions: { ...currentPackument.versions, ...newPackument.versions }, - _attachments: { ...currentPackument._attachments, ...newPackument._attachments }, - }) - - registry.nock.put('/libnpmpublish', body => { - t.notOk(body._rev, 'no _rev in initial post') - t.same(body, newPackument, 'got conflicting packument') - return true - }).reply(409, { error: 'gimme _rev plz' }) - - registry.nock.get('/libnpmpublish?write=true').reply(200, { - _rev: REV, - ...currentPackument, - }) - - registry.nock.put('/libnpmpublish', body => { - t.same(body, { - _rev: REV, - ...mergedPackument, - }, 'posted packument includes _rev and a merged version') - return true - }).reply(201, {}) - - const ret = await publish(manifest, tarData, opts) - t.ok(ret, 'publish succeeded') -}) - -t.test('retry after a conflict -- no versions on remote', async t => { - const { publish } = t.mock('..') - const registry = new MockRegistry({ - tap: t, - registry: opts.registry, - authorization: token, - }) - const REV = '72-47f2986bfd8e8b55068b204588bbf484' - const manifest = { - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - } - - const basePackument = { - name: 'libnpmpublish', - description: 'some stuff', - access: 'public', - _id: 'libnpmpublish', - } - const currentPackument = { ...basePackument } - const newPackument = cloneDeep({ - ...basePackument, - 'dist-tags': { latest: '1.0.0' }, - versions: { - '1.0.0': { - _id: 'libnpmpublish@1.0.0', - _nodeVersion: process.versions.node, - _npmVersion: opts.npmVersion, - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - dist: { - shasum, - integrity: integrity.toString(), - tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz', - }, - }, - }, - _attachments: { - 'libnpmpublish-1.0.0.tgz': { - content_type: 'application/octet-stream', - data: tarData.toString('base64'), - length: tarData.length, - }, - }, - }) - const mergedPackument = cloneDeep({ - ...basePackument, - 'dist-tags': { latest: '1.0.0' }, - versions: { ...newPackument.versions }, - _attachments: { ...newPackument._attachments }, - }) - - registry.nock.put('/libnpmpublish', body => { - t.notOk(body._rev, 'no _rev in initial post') - t.same(body, newPackument, 'got conflicting packument') - return true - }).reply(409, { error: 'gimme _rev plz' }) - - registry.nock.get('/libnpmpublish?write=true').reply(200, { - _rev: REV, - ...currentPackument, - }) - - registry.nock.put('/libnpmpublish', body => { - t.same(body, { - _rev: REV, - ...mergedPackument, - }, 'posted packument includes _rev and a merged version') - return true - }).reply(201, {}) - - const ret = await publish(manifest, tarData, opts) - - t.ok(ret, 'publish succeeded') -}) - -t.test('version conflict', async t => { - const { publish } = t.mock('..') - const registry = new MockRegistry({ - tap: t, - registry: opts.registry, - authorization: token, - }) - const REV = '72-47f2986bfd8e8b55068b204588bbf484' - const manifest = { - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - } - - const basePackument = { - name: 'libnpmpublish', - description: 'some stuff', - access: 'public', - _id: 'libnpmpublish', - 'dist-tags': {}, - versions: {}, - _attachments: {}, - } - const newPackument = cloneDeep(Object.assign({}, basePackument, { - 'dist-tags': { latest: '1.0.0' }, - versions: { - '1.0.0': { - _id: 'libnpmpublish@1.0.0', - _nodeVersion: process.versions.node, - _npmVersion: '6.13.7', - name: 'libnpmpublish', - version: '1.0.0', - description: 'some stuff', - dist: { - shasum, - integrity: integrity.toString(), - tarball: 'http://mock.reg/libnpmpublish/-/libnpmpublish-1.0.0.tgz', - }, - }, - }, - _attachments: { - 'libnpmpublish-1.0.0.tgz': { - content_type: 'application/octet-stream', - data: tarData.toString('base64'), - length: tarData.length, - }, - }, - })) - - registry.nock.put('/libnpmpublish', body => { - t.notOk(body._rev, 'no _rev in initial post') - t.same(body, newPackument, 'got conflicting packument') - return true - }).reply(409, { error: 'gimme _rev plz' }) - - registry.nock.get('/libnpmpublish?write=true').reply(200, { - _rev: REV, - ...newPackument, - }) - - try { - await publish(manifest, tarData, { - ...opts, - npmVersion: '6.13.7', - token: 'deadbeef', - }) - } catch (err) { - t.equal(err.code, 'EPUBLISHCONFLICT', 'got publish conflict code') - } -}) - t.test('refuse if package marked private', async t => { const { publish } = t.mock('..') const registry = new MockRegistry({ From 882f2c9852a11f708c31f2427c364f022dafd2d7 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 12:30:27 -0700 Subject: [PATCH 6/8] chore: remove lodash.clonedeep --- DEPENDENCIES.md | 1 - package-lock.json | 7 ------- workspaces/libnpmpublish/package.json | 1 - 3 files changed, 9 deletions(-) diff --git a/DEPENDENCIES.md b/DEPENDENCIES.md index 0146c658fd828..c2f7890be1363 100644 --- a/DEPENDENCIES.md +++ b/DEPENDENCIES.md @@ -415,7 +415,6 @@ graph LR; libnpmpack-->spawk; libnpmpack-->tap; libnpmpublish-->ci-info; - libnpmpublish-->lodash.clonedeep; libnpmpublish-->nock; libnpmpublish-->normalize-package-data; libnpmpublish-->npm-package-arg; diff --git a/package-lock.json b/package-lock.json index ec5488b896ae3..36fe7d7729f71 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7762,12 +7762,6 @@ "integrity": "sha512-TwuEnCnxbc3rAvhf/LbG7tJUDzhqXyFnv3dtzLOPgCG/hODL7WFnsbwktkD7yUV0RrreP/l1PALq/YSg6VvjlA==", "dev": true }, - "node_modules/lodash.clonedeep": { - "version": "4.5.0", - "resolved": "https://registry.npmjs.org/lodash.clonedeep/-/lodash.clonedeep-4.5.0.tgz", - "integrity": "sha512-H5ZhCF25riFd9uB5UCkVKo61m3S/xZk1x4wA6yp/L3RFP6Z/eHH1ymQcGLo7J3GMPfm0V/7m1tryHuGVxpqEBQ==", - "dev": true - }, "node_modules/lodash.flattendeep": { "version": "4.4.0", "resolved": "https://registry.npmjs.org/lodash.flattendeep/-/lodash.flattendeep-4.4.0.tgz", @@ -15850,7 +15844,6 @@ "@npmcli/mock-globals": "^1.0.0", "@npmcli/mock-registry": "^1.0.0", "@npmcli/template-oss": "4.18.0", - "lodash.clonedeep": "^4.5.0", "nock": "^13.3.0", "tap": "^16.3.4" }, diff --git a/workspaces/libnpmpublish/package.json b/workspaces/libnpmpublish/package.json index 7c7533a82c735..5b58b3da9e428 100644 --- a/workspaces/libnpmpublish/package.json +++ b/workspaces/libnpmpublish/package.json @@ -27,7 +27,6 @@ "@npmcli/mock-globals": "^1.0.0", "@npmcli/mock-registry": "^1.0.0", "@npmcli/template-oss": "4.18.0", - "lodash.clonedeep": "^4.5.0", "nock": "^13.3.0", "tap": "^16.3.4" }, From 099ba91cee669540a199f9bf7620216dd04a5ae7 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 12:48:17 -0700 Subject: [PATCH 7/8] fix: remove implicit if-present logic from run-script workspaces BREAKING CHANGE: npm no longer treats missing scripts as a special case in workspace mode. Use `if-present` to ignore missing scripts. --- lib/commands/run-script.js | 16 +--------------- test/lib/commands/run-script.js | 7 +------ 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/lib/commands/run-script.js b/lib/commands/run-script.js index 13efdde750a82..75f00a46b84e9 100644 --- a/lib/commands/run-script.js +++ b/lib/commands/run-script.js @@ -207,24 +207,10 @@ class RunScript extends BaseCommand { log.error(err) log.error(` in workspace: ${pkg._id || pkg.name}`) log.error(` at location: ${workspacePath}`) - - const scriptMissing = err.message.startsWith('Missing script') - - // avoids exiting with error code in case there's scripts missing - // in some workspaces since other scripts might have succeeded - if (!scriptMissing) { - process.exitCode = 1 - } - - return scriptMissing + process.exitCode = 1 }) res.push(runResult) } - - // in case **all** tests are missing, then it should exit with error code - if (res.every(Boolean)) { - throw new Error(`Missing script: ${args[0]}`) - } } async listWorkspaces (args, filters) { diff --git a/test/lib/commands/run-script.js b/test/lib/commands/run-script.js index cb54a7f51e900..24f51400e8dfc 100644 --- a/test/lib/commands/run-script.js +++ b/test/lib/commands/run-script.js @@ -781,12 +781,7 @@ t.test('workspaces', async t => { t.test('missing scripts in all workspaces', async t => { const { runScript, RUN_SCRIPTS, cleanLogs } = await mockWorkspaces(t, { exec: null }) - await t.rejects( - runScript.exec(['missing-script']), - /Missing script: missing-script/, - 'should throw missing script error' - ) - + await runScript.exec(['missing-script']) t.match(RUN_SCRIPTS(), []) t.strictSame( cleanLogs(), From afd3601a8c387dc1fcc349f181e997f7be1051c5 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 11 Jul 2023 09:16:55 -0700 Subject: [PATCH 8/8] fix: remove "ci-name" config BREAKING CHANGE: the "ci-name" config has been removed The "ci" portion of the default "user-agent" will now only be derived from the environment and can not be manually overridden. --- .../config/lib/definitions/definitions.js | 20 +--------- .../test/type-description.js.test.cjs | 4 -- .../config/test/definitions/definitions.js | 38 +++++++++---------- 3 files changed, 19 insertions(+), 43 deletions(-) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index 6eb845c7e4bae..7f0edc7167a42 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -429,24 +429,6 @@ define('cert', { flatten, }) -define('ci-name', { - default: ciInfo.name ? ciInfo.name.toLowerCase().split(' ').join('-') : null, - defaultDescription: ` - The name of the current CI system, or \`null\` when not on a known CI - platform. - `, - type: [null, String], - deprecated: ` - This config is deprecated and will not be changeable in future version of npm. - `, - description: ` - The name of a continuous integration system. If not set explicitly, npm - will detect the current CI environment using the - [\`ci-info\`](http://npm.im/ci-info) module. - `, - flatten, -}) - define('cidr', { default: null, type: [null, String, Array], @@ -2204,7 +2186,7 @@ define('user-agent', { `, flatten (key, obj, flatOptions) { const value = obj[key] - const ciName = obj['ci-name'] + const ciName = ciInfo.name?.toLowerCase().split(' ').join('-') || null let inWorkspaces = false if (obj.workspaces || obj.workspace && obj.workspace.length) { inWorkspaces = true diff --git a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs index 4ef28584bf993..8c93f851f94b5 100644 --- a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs +++ b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs @@ -79,10 +79,6 @@ Object { null, Function String(), ], - "ci-name": Array [ - null, - Function String(), - ], "cidr": Array [ null, Function String(), diff --git a/workspaces/config/test/definitions/definitions.js b/workspaces/config/test/definitions/definitions.js index 2c608870b51f1..a832f7da0b6cb 100644 --- a/workspaces/config/test/definitions/definitions.js +++ b/workspaces/config/test/definitions/definitions.js @@ -730,45 +730,41 @@ YYYY\r t.end() }) -t.test('detect CI', t => { - const defnNoCI = mockDefs({ - 'ci-info': { isCI: false, name: null }, - }) - const defnCIFoo = mockDefs({ - 'ci-info': { isCI: false, name: 'foo' }, - }) - t.equal(defnNoCI['ci-name'].default, null, 'null when not in CI') - t.equal(defnCIFoo['ci-name'].default, 'foo', 'name of CI when in CI') - t.end() -}) - t.test('user-agent', t => { const npmVersion = '1.2.3' const obj = { 'npm-version': npmVersion, - 'user-agent': mockDefs()['user-agent'].default, + 'user-agent': mockDefs({ + 'ci-info': { isCi: false, name: null }, + })['user-agent'].default, } const flat = {} const expectNoCI = `npm/${npmVersion} node/${process.version} ` + `${process.platform} ${process.arch} workspaces/false` - mockDefs()['user-agent'].flatten('user-agent', obj, flat) + mockDefs({ + 'ci-info': { isCi: false, name: null }, + })['user-agent'].flatten('user-agent', obj, flat) t.equal(flat.userAgent, expectNoCI) t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set') t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated') - obj['ci-name'] = 'foo' - obj['user-agent'] = mockDefs()['user-agent'].default + obj['user-agent'] = mockDefs({ + 'ci-info': { isCi: true, name: 'foo' }, + })['user-agent'].default const expectCI = `${expectNoCI} ci/foo` - mockDefs()['user-agent'].flatten('user-agent', obj, flat) + mockDefs({ + 'ci-info': { isCi: true, name: 'foo' }, + })['user-agent'].flatten('user-agent', obj, flat) t.equal(flat.userAgent, expectCI) t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set') t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated') - delete obj['ci-name'] obj.workspaces = true obj['user-agent'] = mockDefs()['user-agent'].default const expectWorkspaces = expectNoCI.replace('workspaces/false', 'workspaces/true') - mockDefs()['user-agent'].flatten('user-agent', obj, flat) + mockDefs({ + 'ci-info': { isCi: false, name: null }, + })['user-agent'].flatten('user-agent', obj, flat) t.equal(flat.userAgent, expectWorkspaces) t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set') t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated') @@ -776,7 +772,9 @@ t.test('user-agent', t => { delete obj.workspaces obj.workspace = ['foo'] obj['user-agent'] = mockDefs()['user-agent'].default - mockDefs()['user-agent'].flatten('user-agent', obj, flat) + mockDefs({ + 'ci-info': { isCi: false, name: null }, + })['user-agent'].flatten('user-agent', obj, flat) t.equal(flat.userAgent, expectWorkspaces) t.equal(process.env.npm_config_user_agent, flat.userAgent, 'npm_user_config environment is set') t.not(obj['user-agent'], flat.userAgent, 'config user-agent template is not translated')