From b9397c0ecd6cb2656837cf05ad8acc8fb94715ba Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 27 Oct 2020 15:25:20 -0700 Subject: [PATCH 1/3] Support *all* conf keys in publishConfig This adds a flatOptions.flatten() method, which takes an object full of config keys, and turns it into an options object. This method expects an object that already inherits from npm's defaults, and is thus expected to be internal only. This commit also removes some config keys which were used by npm dependencies at the start of the v7 beta process, but are no longer: - all lockfile configs (since we don't use lockfiles any more! for anything! and good riddance, they're a rats' nest of race conditions) - cacheMax/cacheMin (we only use preferOffline/offline/online now, so these are strictly legacy support as input and never shared with deps) Once this lands in cli, we can remove the publishConfig handling logic in npm-registry-fetch, as it will be redundant. --- lib/publish.js | 28 +- lib/utils/flat-options.js | 314 +++++++++--------- ...test-lib-utils-flat-options.js-TAP.test.js | 10 - test/lib/publish.js | 181 +++++++--- test/lib/utils/flat-options.js | 4 +- 5 files changed, 306 insertions(+), 231 deletions(-) diff --git a/lib/publish.js b/lib/publish.js index 0ad5204c9624b..78f6507a02dd0 100644 --- a/lib/publish.js +++ b/lib/publish.js @@ -12,6 +12,9 @@ const output = require('./utils/output.js') const otplease = require('./utils/otplease.js') const { getContents, logTar } = require('./utils/tar.js') +// this is the only case in the CLI where we use the old full slow +// 'read-package-json' module, because we want to pull in all the +// defaults and metadata, like git sha's and default scripts and all that. const readJson = util.promisify(require('read-package-json')) const completion = require('./utils/completion/none.js') @@ -46,9 +49,22 @@ const publish = async args => { return tarball } +// for historical reasons, publishConfig in package.json can contain +// ANY config keys that npm supports in .npmrc files and elsewhere. +// We *may* want to revisit this at some point, and have a minimal set +// that's a SemVer-major change that ought to get a RFC written on it. +const { flatten } = require('./utils/flat-options.js') +const publishConfigToOpts = publishConfig => + // create a new object that inherits from the config stack + // then squash the css-case into camelCase opts, like we do + flatten(Object.assign(Object.create(npm.config.list[0]), publishConfig)) + const publish_ = async (arg, opts) => { const { unicode, dryRun, json } = opts - let manifest = await readJson(`${arg}/package.json`) + const manifest = await readJson(`${arg}/package.json`) + + if (manifest.publishConfig) + Object.assign(opts, publishConfigToOpts(manifest.publishConfig)) // prepublishOnly await runScript({ @@ -58,7 +74,7 @@ const publish_ = async (arg, opts) => { pkg: manifest, }) - const tarballData = await pack(arg) + const tarballData = await pack(arg, opts) const pkgContents = await getContents(manifest, tarballData) if (!json) @@ -67,9 +83,11 @@ const publish_ = async (arg, opts) => { if (!dryRun) { // The purpose of re-reading the manifest is in case it changed, // so that we send the latest and greatest thing to the registry - manifest = await readJson(`${arg}/package.json`) - const { publishConfig } = manifest - await otplease(opts, opts => libpub(arg, manifest, { ...opts, publishConfig })) + // note that publishConfig might have changed as well! + const manifest = await readJson(`${arg}/package.json`, opts) + if (manifest.publishConfig) + Object.assign(opts, publishConfigToOpts(manifest.publishConfig)) + await otplease(opts, opts => libpub(arg, manifest, opts)) } // publish diff --git a/lib/utils/flat-options.js b/lib/utils/flat-options.js index d7713ae13122f..be62c5a4cdb58 100644 --- a/lib/utils/flat-options.js +++ b/lib/utils/flat-options.js @@ -7,241 +7,231 @@ const npmSession = crypto.randomBytes(8).toString('hex') log.verbose('npm-session', npmSession) const { join } = require('path') -const buildOmitList = npm => { - const include = npm.config.get('include') || [] - const omit = new Set((npm.config.get('omit') || []) +const buildOmitList = obj => { + const include = obj.include || [] + const omit = new Set((obj.omit || []) .filter(type => !include.includes(type))) - const only = npm.config.get('only') + const only = obj.only - if (/^prod(uction)?$/.test(only) || npm.config.get('production')) + if (/^prod(uction)?$/.test(only) || obj.production) omit.add('dev') - if (/dev/.test(npm.config.get('also'))) + if (/dev/.test(obj.also)) omit.delete('dev') - if (npm.config.get('dev')) + if (obj.dev) omit.delete('dev') - if (npm.config.get('optional') === false) + if (obj.optional === false) omit.add('optional') - npm.config.set('omit', [...omit]) + obj.omit = [...omit] return [...omit] } -const flatOptions = npm => npm.flatOptions || Object.freeze({ - // Note that many of these do not come from configs or cli flags - // per se, though they may be implied or defined by them. - log, - npmSession, - dmode: npm.modes.exec, - fmode: npm.modes.file, - umask: npm.modes.umask, - hashAlgorithm: 'sha1', // XXX should this be sha512? - color: !!npm.color, - includeStaged: npm.config.get('include-staged'), - - preferDedupe: npm.config.get('prefer-dedupe'), - ignoreScripts: npm.config.get('ignore-scripts'), - - projectScope: npm.projectScope, - npmVersion: npm.version, - nodeVersion: npm.config.get('node-version'), - // npm.command is not set until AFTER flatOptions are defined - // so we need to make this a getter. - get npmCommand () { - return npm.command - }, - - tmp: npm.tmp, - cache: join(npm.config.get('cache'), '_cacache'), - prefix: npm.prefix, - globalPrefix: npm.globalPrefix, - localPrefix: npm.localPrefix, - global: npm.config.get('global'), - - metricsRegistry: npm.config.get('metrics-registry') || - npm.config.get('registry'), - sendMetrics: npm.config.get('send-metrics'), - registry: npm.config.get('registry'), - scope: npm.config.get('scope'), - access: npm.config.get('access'), - alwaysAuth: npm.config.get('always-auth'), - audit: npm.config.get('audit'), - auditLevel: npm.config.get('audit-level'), - authType: npm.config.get('auth-type'), - ssoType: npm.config.get('sso-type'), - ssoPollFrequency: npm.config.get('sso-poll-frequency'), - before: npm.config.get('before'), - browser: npm.config.get('browser'), - ca: npm.config.get('ca'), - cafile: npm.config.get('cafile'), - cert: npm.config.get('cert'), - key: npm.config.get('key'), - - // XXX remove these when we don't use lockfile any more, once - // arborist is handling the installation process - cacheLockRetries: npm.config.get('cache-lock-retries'), - cacheLockStale: npm.config.get('cache-lock-stale'), - cacheLockWait: npm.config.get('cache-lock-wait'), - lockFile: { - retries: npm.config.get('cache-lock-retries'), - stale: npm.config.get('cache-lock-stale'), - wait: npm.config.get('cache-lock-wait'), - }, - - // XXX remove these once no longer used - get cacheMax () { - return npm.config.get('cache-max') - }, - get cacheMin () { - return npm.config.get('cache-min') - }, +// turn an object with npm-config style keys into an options object +// with camelCase values. This doesn't account for the stuff that is +// not pulled from the config keys, that's all handled only for the +// main function which acts on the npm object itself. Used by the +// flatOptions generator, and by the publishConfig handling logic. +const flatten = obj => ({ + includeStaged: obj['include-staged'], + preferDedupe: obj['prefer-dedupe'], + ignoreScripts: obj['ignore-scripts'], + nodeVersion: obj['node-version'], + cache: join(obj.cache, '_cacache'), + global: obj.global, + + metricsRegistry: obj['metrics-registry'] || obj.registry, + sendMetrics: obj['send-metrics'], + registry: obj.registry, + scope: obj.scope, + access: obj.access, + alwaysAuth: obj['always-auth'], + audit: obj.audit, + auditLevel: obj['audit-level'], + authType: obj['auth-type'], + ssoType: obj['sso-type'], + ssoPollFrequency: obj['sso-poll-frequency'], + before: obj.before, + browser: obj.browser, + ca: obj.ca, + cafile: obj.cafile, + cert: obj.cert, + key: obj.key, // token creation options - cidr: npm.config.get('cidr'), - readOnly: npm.config.get('read-only'), + cidr: obj.cidr, + readOnly: obj['read-only'], // npm version options - preid: npm.config.get('preid'), - tagVersionPrefix: npm.config.get('tag-version-prefix'), - allowSameVersion: npm.config.get('allow-same-version'), + preid: obj.preid, + tagVersionPrefix: obj['tag-version-prefix'], + allowSameVersion: obj['allow-same-version'], // npm version git options - message: npm.config.get('message'), - commitHooks: npm.config.get('commit-hooks'), - gitTagVersion: npm.config.get('git-tag-version'), - signGitCommit: npm.config.get('sign-git-commit'), - signGitTag: npm.config.get('sign-git-tag'), + message: obj.message, + commitHooks: obj['commit-hooks'], + gitTagVersion: obj['git-tag-version'], + signGitCommit: obj['sign-git-commit'], + signGitTag: obj['sign-git-tag'], // only used for npm ls in v7, not update - depth: npm.config.get('depth'), - all: npm.config.get('all'), + depth: obj.depth, + all: obj.all, // Output configs - unicode: npm.config.get('unicode'), - json: npm.config.get('json'), - long: npm.config.get('long'), - parseable: npm.config.get('parseable'), + unicode: obj.unicode, + json: obj.json, + long: obj.long, + parseable: obj.parseable, // options for npm search search: { - description: npm.config.get('description'), - exclude: npm.config.get('searchexclude'), - limit: npm.config.get('searchlimit') || 20, - opts: npm.config.get('searchopts'), - staleness: npm.config.get('searchstaleness'), + description: obj.description, + exclude: obj.searchexclude, + limit: obj.searchlimit || 20, + opts: obj.searchopts, + staleness: obj.searchstaleness, }, - dryRun: npm.config.get('dry-run'), - engineStrict: npm.config.get('engine-strict'), + dryRun: obj['dry-run'], + engineStrict: obj['engine-strict'], retry: { - retries: npm.config.get('fetch-retries'), - factor: npm.config.get('fetch-retry-factor'), - maxTimeout: npm.config.get('fetch-retry-maxtimeout'), - minTimeout: npm.config.get('fetch-retry-mintimeout'), + retries: obj['fetch-retries'], + factor: obj['fetch-retry-factor'], + maxTimeout: obj['fetch-retry-maxtimeout'], + minTimeout: obj['fetch-retry-mintimeout'], }, - timeout: npm.config.get('fetch-timeout'), + timeout: obj['fetch-timeout'], - force: npm.config.get('force'), + force: obj.force, - formatPackageLock: npm.config.get('format-package-lock'), - fund: npm.config.get('fund'), + formatPackageLock: obj['format-package-lock'], + fund: obj.fund, // binary locators - git: npm.config.get('git'), - npmBin: require.main.filename, - nodeBin: process.env.NODE || process.execPath, - viewer: npm.config.get('viewer'), - editor: npm.config.get('editor'), + git: obj.git, + viewer: obj.viewer, + editor: obj.editor, // configs that affect how we build trees - binLinks: npm.config.get('bin-links'), - rebuildBundle: npm.config.get('rebuild-bundle'), + binLinks: obj['bin-links'], + rebuildBundle: obj['rebuild-bundle'], // --no-shrinkwrap is the same as --no-package-lock - packageLock: !(npm.config.get('package-lock') === false || - npm.config.get('shrinkwrap') === false), - packageLockOnly: npm.config.get('package-lock-only'), - globalStyle: npm.config.get('global-style'), - legacyBundling: npm.config.get('legacy-bundling'), - scriptShell: npm.config.get('script-shell') || undefined, - shell: npm.config.get('shell'), - omit: buildOmitList(npm), - legacyPeerDeps: npm.config.get('legacy-peer-deps'), - strictPeerDeps: npm.config.get('strict-peer-deps'), + packageLock: !(obj['package-lock'] === false || + obj.shrinkwrap === false), + packageLockOnly: obj['package-lock-only'], + globalStyle: obj['global-style'], + legacyBundling: obj['legacy-bundling'], + scriptShell: obj['script-shell'] || undefined, + shell: obj.shell, + omit: buildOmitList(obj), + legacyPeerDeps: obj['legacy-peer-deps'], + strictPeerDeps: obj['strict-peer-deps'], // npx stuff - call: npm.config.get('call'), - package: npm.config.get('package'), + call: obj.call, + package: obj.package, // used to build up the appropriate {add:{...}} options to Arborist.reify - save: npm.config.get('save'), - saveBundle: npm.config.get('save-bundle') && !npm.config.get('save-peer'), - saveType: npm.config.get('save-optional') && npm.config.get('save-peer') + save: obj.save, + saveBundle: obj['save-bundle'] && !obj['save-peer'], + saveType: obj['save-optional'] && obj['save-peer'] ? 'peerOptional' - : npm.config.get('save-optional') ? 'optional' - : npm.config.get('save-dev') ? 'dev' - : npm.config.get('save-peer') ? 'peer' - : npm.config.get('save-prod') ? 'prod' + : obj['save-optional'] ? 'optional' + : obj['save-dev'] ? 'dev' + : obj['save-peer'] ? 'peer' + : obj['save-prod'] ? 'prod' : null, - savePrefix: npm.config.get('save-exact') ? '' - : npm.config.get('save-prefix'), + savePrefix: obj['save-exact'] ? '' + : obj['save-prefix'], // configs for npm-registry-fetch - otp: npm.config.get('otp'), - offline: npm.config.get('offline'), - preferOffline: getPreferOffline(npm), - preferOnline: getPreferOnline(npm), - strictSSL: npm.config.get('strict-ssl'), - defaultTag: npm.config.get('tag'), - get tag () { - return npm.config.get('tag') - }, - userAgent: npm.config.get('user-agent'), + otp: obj.otp, + offline: obj.offline, + preferOffline: getPreferOffline(obj), + preferOnline: getPreferOnline(obj), + strictSSL: obj['strict-ssl'], + defaultTag: obj.tag, + userAgent: obj['user-agent'], // yes, it's fine, just do it, jeez, stop asking - yes: npm.config.get('yes'), + yes: obj.yes, - ...getScopesAndAuths(npm), + ...getScopesAndAuths(obj), // npm fund exclusive option to select an item from a funding list - which: npm.config.get('which'), + which: obj.which, // socks proxy can be configured in https-proxy or proxy field - // note that the various (HTTPS_|HTTP_|)PROXY environs will be + // note that the various (HTTPS_|HTTP_|]PROXY environs will be // respected if this is not set. - proxy: npm.config.get('https-proxy') || npm.config.get('proxy'), - noProxy: npm.config.get('noproxy'), + proxy: obj['https-proxy'] || obj.proxy, + noProxy: obj.noproxy, +}) + +const flatOptions = npm => npm.flatOptions || Object.freeze({ + // flatten the config object + ...flatten(npm.config.list[0]), + + // Note that many of these do not come from configs or cli flags + // per se, though they may be implied or defined by them. + log, + npmSession, + dmode: npm.modes.exec, + fmode: npm.modes.file, + umask: npm.modes.umask, + hashAlgorithm: 'sha1', // XXX should this be sha512? + color: !!npm.color, + projectScope: npm.projectScope, + npmVersion: npm.version, + + // npm.command is not set until AFTER flatOptions are defined + // so we need to make this a getter. + get npmCommand () { + return npm.command + }, + + tmp: npm.tmp, + prefix: npm.prefix, + globalPrefix: npm.globalPrefix, + localPrefix: npm.localPrefix, + npmBin: require.main && require.main.filename, + nodeBin: process.env.NODE || process.execPath, + get tag () { + return npm.config.get('tag') + }, }) -const getPreferOnline = npm => { - const po = npm.config.get('prefer-online') +const getPreferOnline = obj => { + const po = obj['prefer-online'] if (po !== undefined) return po - return npm.config.get('cache-max') <= 0 + return obj['cache-max'] <= 0 } -const getPreferOffline = npm => { - const po = npm.config.get('prefer-offline') +const getPreferOffline = obj => { + const po = obj['prefer-offline'] if (po !== undefined) return po - return npm.config.get('cache-min') >= 9999 + return obj['cache-min'] >= 9999 } // pull out all the @scope: and //host:key config fields // these are used by npm-registry-fetch for authing against registries -const getScopesAndAuths = npm => { +const getScopesAndAuths = obj => { const scopesAndAuths = {} // pull out all the @scope:... configs into a flat object. - for (const key in npm.config.list[0]) { + for (const key in obj) { if (/@.*:registry$/i.test(key) || /^\/\//.test(key)) - scopesAndAuths[key] = npm.config.get(key) + scopesAndAuths[key] = obj[key] } return scopesAndAuths } -module.exports = flatOptions +module.exports = Object.assign(flatOptions, { flatten }) diff --git a/tap-snapshots/test-lib-utils-flat-options.js-TAP.test.js b/tap-snapshots/test-lib-utils-flat-options.js-TAP.test.js index 3522084e67c71..0e6c053b2f452 100644 --- a/tap-snapshots/test-lib-utils-flat-options.js-TAP.test.js +++ b/tap-snapshots/test-lib-utils-flat-options.js-TAP.test.js @@ -21,11 +21,6 @@ Object { "browser": "browser", "ca": "ca", "cache": "cache/_cacache", - "cacheLockRetries": "cache-lock-retries", - "cacheLockStale": "cache-lock-stale", - "cacheLockWait": "cache-lock-wait", - "cacheMax": "cache-max", - "cacheMin": "cache-min", "cafile": "cafile", "call": "call", "cert": "cert", @@ -55,11 +50,6 @@ Object { "legacyBundling": "legacy-bundling", "legacyPeerDeps": undefined, "localPrefix": "/path/to/npm/cli", - "lockFile": Object { - "retries": "cache-lock-retries", - "stale": "cache-lock-stale", - "wait": "cache-lock-wait", - }, "log": Object {}, "long": undefined, "message": "message", diff --git a/test/lib/publish.js b/test/lib/publish.js index e5e412e396c1a..65fb91ae895d7 100644 --- a/test/lib/publish.js +++ b/test/lib/publish.js @@ -1,14 +1,73 @@ -const { test, cleanSnapshot } = require('tap') +const t = require('tap') const requireInject = require('require-inject') -test('should publish with libnpmpublish', (t) => { +// mock config +const {defaults} = require('../../lib/utils/config.js') +const config = { list: [defaults] } +const fs = require('fs') + +t.test('should publish with libnpmpublish, respecting publishConfig', (t) => { + const publishConfig = { registry: 'https://some.registry' } + const testDir = t.testdir({ + 'package.json': JSON.stringify({ + name: 'my-cool-pkg', + version: '1.0.0', + publishConfig, + }, null, 2), + }) + + const publish = requireInject('../../lib/publish.js', { + '../../lib/npm.js': { + flatOptions: { + json: true, + defaultTag: 'latest', + }, + config, + }, + '../../lib/utils/tar.js': { + getContents: () => ({ + id: 'someid', + }), + logTar: () => {}, + }, + '../../lib/utils/output.js': () => {}, + '../../lib/utils/otplease.js': (opts, fn) => { + return Promise.resolve().then(() => fn(opts)) + }, + // verify that we do NOT remove publishConfig if it was there originally + // and then removed during the script/pack process + libnpmpack: async () => { + fs.writeFileSync(`${testDir}/package.json`, JSON.stringify({ + name: 'my-cool-pkg', + version: '1.0.0', + })) + return '' + }, + libnpmpublish: { + publish: (arg, manifest, opts) => { + t.ok(arg, 'gets path') + t.ok(manifest, 'gets manifest') + t.ok(opts, 'gets opts object') + t.same(opts.registry, publishConfig.registry, 'publishConfig is passed through') + t.ok(true, 'libnpmpublish is called') + }, + }, + }) + + publish([testDir], (er) => { + if (er) + throw er + t.end() + }) +}) + +t.test('re-loads publishConfig if added during script process', (t) => { const publishConfig = { registry: 'https://some.registry' } const testDir = t.testdir({ 'package.json': JSON.stringify({ name: 'my-cool-pkg', version: '1.0.0', - publishConfig - }, null, 2) + }, null, 2), }) const publish = requireInject('../../lib/publish.js', { @@ -16,41 +75,51 @@ test('should publish with libnpmpublish', (t) => { flatOptions: { json: true, defaultTag: 'latest', - } + }, + config, }, '../../lib/utils/tar.js': { - 'getContents': () => ({ - id: 'someid' + getContents: () => ({ + id: 'someid', }), - 'logTar': () => {} + logTar: () => {}, }, '../../lib/utils/output.js': () => {}, '../../lib/utils/otplease.js': (opts, fn) => { return Promise.resolve().then(() => fn(opts)) }, - 'libnpmpack': () => '', - 'libnpmpublish': { + libnpmpack: async () => { + fs.writeFileSync(`${testDir}/package.json`, JSON.stringify({ + name: 'my-cool-pkg', + version: '1.0.0', + publishConfig, + })) + return '' + }, + libnpmpublish: { publish: (arg, manifest, opts) => { t.ok(arg, 'gets path') t.ok(manifest, 'gets manifest') t.ok(opts, 'gets opts object') - t.same(opts.publishConfig, publishConfig, 'publishConfig is passed through') + t.same(opts.registry, publishConfig.registry, 'publishConfig is passed through') t.ok(true, 'libnpmpublish is called') - } + }, }, }) - publish([testDir], () => { + publish([testDir], (er) => { + if (er) + throw er t.end() }) }) -test('should not log if silent', (t) => { +t.test('should not log if silent', (t) => { const testDir = t.testdir({ 'package.json': JSON.stringify({ name: 'my-cool-pkg', - version: '1.0.0' - }, null, 2) + version: '1.0.0', + }, null, 2), }) const publish = requireInject('../../lib/publish.js', { @@ -58,37 +127,40 @@ test('should not log if silent', (t) => { flatOptions: { json: false, defaultTag: 'latest', - dryRun: true - } + dryRun: true, + }, + config, }, '../../lib/utils/tar.js': { - 'getContents': () => ({}), - 'logTar': () => {} + getContents: () => ({}), + logTar: () => {}, }, '../../lib/utils/otplease.js': (opts, fn) => { return Promise.resolve().then(() => fn(opts)) - }, - 'npmlog': { - 'verbose': () => {}, - 'level': 'silent' }, - 'libnpmpack': () => '', - 'libnpmpublish': { - publish: () => {} + npmlog: { + verbose: () => {}, + level: 'silent', + }, + libnpmpack: async () => '', + libnpmpublish: { + publish: () => {}, }, }) - publish([testDir], () => { + publish([testDir], (er) => { + if (er) + throw er t.end() }) }) -test('should log tarball contents', (t) => { +t.test('should log tarball contents', (t) => { const testDir = t.testdir({ 'package.json': JSON.stringify({ name: 'my-cool-pkg', - version: '1.0.0' - }, null, 2) + version: '1.0.0', + }, null, 2), }) const publish = requireInject('../../lib/publish.js', { @@ -96,16 +168,17 @@ test('should log tarball contents', (t) => { flatOptions: { json: false, defaultTag: 'latest', - dryRun: true - } + dryRun: true, + }, + config, }, '../../lib/utils/tar.js': { - 'getContents': () => ({ - id: 'someid' + getContents: () => ({ + id: 'someid', }), - 'logTar': () => { + logTar: () => { t.ok(true, 'logTar is called') - } + }, }, '../../lib/utils/output.js': () => { t.ok(true, 'output fn is called') @@ -113,45 +186,49 @@ test('should log tarball contents', (t) => { '../../lib/utils/otplease.js': (opts, fn) => { return Promise.resolve().then(() => fn(opts)) }, - 'libnpmpack': () => '', - 'libnpmpublish': { - publish: () => {} + libnpmpack: async () => '', + libnpmpublish: { + publish: () => {}, }, }) - publish([testDir], () => { + publish([testDir], (er) => { + if (er) + throw er t.end() }) }) -test('shows usage with wrong set of arguments', (t) => { +t.test('shows usage with wrong set of arguments', (t) => { const publish = requireInject('../../lib/publish.js', { '../../lib/npm.js': { flatOptions: { json: false, - defaultTag: '0.0.13' - } - } + defaultTag: '0.0.13', + }, + config, + }, }) - publish(['a', 'b', 'c'], (result) => { - t.matchSnapshot(result, 'should print usage') + publish(['a', 'b', 'c'], (er) => { + t.matchSnapshot(er, 'should print usage') t.end() }) }) -test('throws when invalid tag', (t) => { +t.test('throws when invalid tag', (t) => { const publish = requireInject('../../lib/publish.js', { '../../lib/npm.js': { flatOptions: { json: false, - defaultTag: '0.0.13' - } - } + defaultTag: '0.0.13', + }, + config, + }, }) publish([], (err) => { - t.ok(err, 'throws when tag name is a valid SemVer range') + t.ok(err, 'throws when tag name is a valid SemVer range') t.end() }) }) diff --git a/test/lib/utils/flat-options.js b/test/lib/utils/flat-options.js index f18ce828c3e1d..d3b8b89bc865a 100644 --- a/test/lib/utils/flat-options.js +++ b/test/lib/utils/flat-options.js @@ -163,7 +163,7 @@ t.test('get preferOffline from cache-min', t => { const opts = flatOptions(npm) t.equal(opts.preferOffline, true, 'got preferOffline from cache min') logs.length = 0 - t.equal(opts.cacheMin, 9999999, 'opts.cacheMin is set') + t.equal(opts.cacheMin, undefined, 'opts.cacheMin is not set') t.match(logs, []) logs.length = 0 t.end() @@ -177,7 +177,7 @@ t.test('get preferOnline from cache-max', t => { const opts = flatOptions(npm) t.equal(opts.preferOnline, true, 'got preferOnline from cache min') logs.length = 0 - t.equal(opts.cacheMax, -1, 'opts.cacheMax is set') + t.equal(opts.cacheMax, undefined, 'opts.cacheMax is not set') t.match(logs, []) logs.length = 0 t.end() From 3fc9d67fbeadc9f6ca32fc976fd50154ab1fac8c Mon Sep 17 00:00:00 2001 From: isaacs Date: Wed, 28 Oct 2020 16:25:51 -0700 Subject: [PATCH 2/3] TEMP: depend on github:npm/libnpmpublish#pull/16 Remove this once that PR lands --- node_modules/libnpmpublish/publish.js | 75 +++++++++++---------------- package-lock.json | 2 +- package.json | 2 +- 3 files changed, 31 insertions(+), 48 deletions(-) diff --git a/node_modules/libnpmpublish/publish.js b/node_modules/libnpmpublish/publish.js index 8a382e4ad9a9c..f342b717404e5 100644 --- a/node_modules/libnpmpublish/publish.js +++ b/node_modules/libnpmpublish/publish.js @@ -1,25 +1,15 @@ -'use strict' - const { fixer } = require('normalize-package-data') const npmFetch = require('npm-registry-fetch') -const cloneDeep = require('lodash.clonedeep') const npa = require('npm-package-arg') -const pack = require('libnpmpack') const semver = require('semver') -const { URL } = require('url') -const util = require('util') const ssri = require('ssri') +const { URL } = require('url') -const statAsync = util.promisify(require('fs').stat) - -module.exports = publish -async function publish (folder, manifest, opts) { +const publish = async (manifest, tarballData, opts) => { if (manifest.private) { throw Object.assign( - new Error( - `This package has been marked as private\n - Remove the 'private' field from the package.json to publish it.` - ), + new Error(`This package has been marked as private +Remove the 'private' field from the package.json to publish it.`), { code: 'EPRIVATE' } ) } @@ -32,16 +22,7 @@ async function publish (folder, manifest, opts) { access: spec.scope ? 'restricted' : 'public', algorithms: ['sha512'], ...opts, - spec - } - - const stat = await statAsync(folder) - // checks if it's a dir - if (!stat.isDirectory()) { - throw Object.assign( - new Error('not a directory'), - { code: 'ENOTDIR' } - ) + spec, } const reg = npmFetch.pickRegistry(spec, opts) @@ -56,7 +37,6 @@ async function publish (folder, manifest, opts) { ) } - const tarballData = await pack(`file:${folder}`, { ...opts }) const metadata = buildMetadata(reg, pubManifest, tarballData, opts) try { @@ -64,36 +44,37 @@ async function publish (folder, manifest, opts) { ...opts, method: 'PUT', body: metadata, - ignoreBody: true + ignoreBody: true, }) } catch (err) { - if (err.code !== 'E409') { throw 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 } + query: { write: true }, }) const newMetadata = patchMetadata(current, metadata, opts) return npmFetch(spec.escapedName, { ...opts, method: 'PUT', body: newMetadata, - ignoreBody: true + ignoreBody: true, }) } } -function patchManifest (_manifest, opts) { +const patchManifest = (_manifest, opts) => { const { npmVersion } = opts - const manifest = cloneDeep(_manifest) + // we only update top-level fields, so a shallow clone is fine + const manifest = { ..._manifest } manifest._nodeVersion = process.versions.node - if (npmVersion) { + if (npmVersion) manifest._npmVersion = npmVersion - } fixer.fixNameField(manifest, { strict: true, allowLegacyCase: true }) const version = semver.clean(manifest.version) @@ -107,7 +88,7 @@ function patchManifest (_manifest, opts) { return manifest } -function buildMetadata (registry, manifest, tarballData, opts) { +const buildMetadata = (registry, manifest, tarballData, opts) => { const { access, defaultTag, algorithms } = opts const root = { _id: manifest.name, @@ -116,7 +97,6 @@ function buildMetadata (registry, manifest, tarballData, opts) { 'dist-tags': {}, versions: {}, access, - readme: manifest.readme || '' } root.versions[manifest.version] = manifest @@ -126,7 +106,7 @@ function buildMetadata (registry, manifest, tarballData, opts) { const tarballName = `${manifest.name}-${manifest.version}.tgz` const tarballURI = `${manifest.name}/-/${tarballName}` const integrity = ssri.fromData(tarballData, { - algorithms: [...new Set(['sha1'].concat(algorithms))] + algorithms: [...new Set(['sha1'].concat(algorithms))], }) manifest._id = `${manifest.name}@${manifest.version}` @@ -146,18 +126,18 @@ function buildMetadata (registry, manifest, tarballData, opts) { root._attachments[tarballName] = { content_type: 'application/octet-stream', data: tarballData.toString('base64'), - length: tarballData.length + length: tarballData.length, } return root } -function patchMetadata (current, newData) { - const curVers = Object.keys(current.versions || {}).map(v => { - return semver.clean(v, true) - }).concat(Object.keys(current.time || {}).map(v => { - if (semver.valid(v, true)) { return semver.clean(v, true) } - })).filter(v => v) +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] @@ -169,19 +149,19 @@ function patchMetadata (current, newData) { ), { code: 'EPUBLISHCONFLICT', pkgid, - version + version, }) } current.versions = current.versions || {} current.versions[newVersion] = newData.versions[newVersion] - for (var i in newData) { + for (const i in newData) { switch (i) { // objects that copy over the new stuffs case 'dist-tags': case 'versions': case '_attachments': - for (var j in newData[i]) { + for (const j in newData[i]) { current[i] = current[i] || {} current[i][j] = newData[i][j] } @@ -190,8 +170,11 @@ function patchMetadata (current, newData) { // copy default: current[i] = newData[i] + break } } return current } + +module.exports = publish diff --git a/package-lock.json b/package-lock.json index b33c5011e2bc2..c5a79128736e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -109,7 +109,7 @@ "libnpmhook": "^6.0.0", "libnpmorg": "^2.0.0", "libnpmpack": "^2.0.0", - "libnpmpublish": "^3.0.2", + "libnpmpublish": "github:npm/libnpmpublish#pull/16", "libnpmsearch": "^3.0.0", "libnpmteam": "^2.0.1", "libnpmversion": "^1.0.6", diff --git a/package.json b/package.json index ed8c35d2be7f2..823080e7842f7 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,7 @@ "libnpmhook": "^6.0.0", "libnpmorg": "^2.0.0", "libnpmpack": "^2.0.0", - "libnpmpublish": "^3.0.2", + "libnpmpublish": "github:npm/libnpmpublish#pull/16", "libnpmsearch": "^3.0.0", "libnpmteam": "^2.0.1", "libnpmversion": "^1.0.6", From 0e44521eaaf2bb66898849427fa64756a3cfcc3f Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 27 Oct 2020 15:25:20 -0700 Subject: [PATCH 3/3] Support publishing any kind of spec, not just directories Depends on npm/libnpmpublish#16 --- lib/publish.js | 67 ++++++----- node_modules/@npmcli/config/lib/set-envs.js | 3 +- test/lib/publish.js | 122 ++++++++++++++++---- 3 files changed, 140 insertions(+), 52 deletions(-) diff --git a/lib/publish.js b/lib/publish.js index 78f6507a02dd0..da58134f7c250 100644 --- a/lib/publish.js +++ b/lib/publish.js @@ -6,6 +6,8 @@ const semver = require('semver') const pack = require('libnpmpack') const libpub = require('libnpmpublish').publish const runScript = require('@npmcli/run-script') +const pacote = require('pacote') +const npa = require('npm-package-arg') const npm = require('./npm.js') const output = require('./utils/output.js') @@ -49,6 +51,12 @@ const publish = async args => { return tarball } +// if it's a directory, read it from the file system +// otherwise, get the full metadata from whatever it is +const getManifest = (spec, opts) => + spec.type === 'directory' ? readJson(`${spec.fetchSpec}/package.json`) + : pacote.manifest(spec, { ...opts, fullMetadata: true }) + // for historical reasons, publishConfig in package.json can contain // ANY config keys that npm supports in .npmrc files and elsewhere. // We *may* want to revisit this at some point, and have a minimal set @@ -61,22 +69,29 @@ const publishConfigToOpts = publishConfig => const publish_ = async (arg, opts) => { const { unicode, dryRun, json } = opts - const manifest = await readJson(`${arg}/package.json`) + // you can publish name@version, ./foo.tgz, etc. + // even though the default is the 'file:.' cwd. + const spec = npa(arg) + const manifest = await getManifest(spec, opts) if (manifest.publishConfig) Object.assign(opts, publishConfigToOpts(manifest.publishConfig)) - // prepublishOnly - await runScript({ - event: 'prepublishOnly', - path: arg, - stdio: 'inherit', - pkg: manifest, - }) + // only run scripts for directory type publishes + if (spec.type === 'directory') { + await runScript({ + event: 'prepublishOnly', + path: spec.fetchSpec, + stdio: 'inherit', + pkg: manifest, + }) + } - const tarballData = await pack(arg, opts) + const tarballData = await pack(spec, opts) const pkgContents = await getContents(manifest, tarballData) + // note that logTar calls npmlog.notice(), so if we ARE in silent mode, + // this will do nothing, but we still want it in the debuglog if it fails. if (!json) logTar(pkgContents, { log, unicode }) @@ -84,27 +99,27 @@ const publish_ = async (arg, opts) => { // The purpose of re-reading the manifest is in case it changed, // so that we send the latest and greatest thing to the registry // note that publishConfig might have changed as well! - const manifest = await readJson(`${arg}/package.json`, opts) + const manifest = await getManifest(spec, opts) if (manifest.publishConfig) Object.assign(opts, publishConfigToOpts(manifest.publishConfig)) - await otplease(opts, opts => libpub(arg, manifest, opts)) + await otplease(opts, opts => libpub(manifest, tarballData, opts)) } - // publish - await runScript({ - event: 'publish', - path: arg, - stdio: 'inherit', - pkg: manifest, - }) - - // postpublish - await runScript({ - event: 'postpublish', - path: arg, - stdio: 'inherit', - pkg: manifest, - }) + if (spec.type === 'directory') { + await runScript({ + event: 'publish', + path: spec.fetchSpec, + stdio: 'inherit', + pkg: manifest, + }) + + await runScript({ + event: 'postpublish', + path: spec.fetchSpec, + stdio: 'inherit', + pkg: manifest, + }) + } return pkgContents } diff --git a/node_modules/@npmcli/config/lib/set-envs.js b/node_modules/@npmcli/config/lib/set-envs.js index d41b044ed9298..b1b1db35c5a86 100644 --- a/node_modules/@npmcli/config/lib/set-envs.js +++ b/node_modules/@npmcli/config/lib/set-envs.js @@ -92,7 +92,8 @@ const setEnvs = (config) => { if (cliConf['node-options']) env.NODE_OPTIONS = cliConf['node-options'] - env.npm_execpath = require.main.filename + if (require.main && require.main.filename) + env.npm_execpath = require.main.filename env.npm_node_execpath = config.execPath } diff --git a/test/lib/publish.js b/test/lib/publish.js index 65fb91ae895d7..14e2179816394 100644 --- a/test/lib/publish.js +++ b/test/lib/publish.js @@ -7,6 +7,8 @@ const config = { list: [defaults] } const fs = require('fs') t.test('should publish with libnpmpublish, respecting publishConfig', (t) => { + t.plan(5) + const publishConfig = { registry: 'https://some.registry' } const testDir = t.testdir({ 'package.json': JSON.stringify({ @@ -41,15 +43,14 @@ t.test('should publish with libnpmpublish, respecting publishConfig', (t) => { name: 'my-cool-pkg', version: '1.0.0', })) - return '' + return Buffer.from('') }, libnpmpublish: { - publish: (arg, manifest, opts) => { - t.ok(arg, 'gets path') - t.ok(manifest, 'gets manifest') + publish: (manifest, tarData, opts) => { + t.match(manifest, { name: 'my-cool-pkg', version: '1.0.0' }, 'gets manifest') + t.isa(tarData, Buffer, 'tarData is a buffer') t.ok(opts, 'gets opts object') t.same(opts.registry, publishConfig.registry, 'publishConfig is passed through') - t.ok(true, 'libnpmpublish is called') }, }, }) @@ -57,11 +58,12 @@ t.test('should publish with libnpmpublish, respecting publishConfig', (t) => { publish([testDir], (er) => { if (er) throw er - t.end() + t.pass('got to callback') }) }) t.test('re-loads publishConfig if added during script process', (t) => { + t.plan(5) const publishConfig = { registry: 'https://some.registry' } const testDir = t.testdir({ 'package.json': JSON.stringify({ @@ -94,15 +96,14 @@ t.test('re-loads publishConfig if added during script process', (t) => { version: '1.0.0', publishConfig, })) - return '' + return Buffer.from('') }, libnpmpublish: { - publish: (arg, manifest, opts) => { - t.ok(arg, 'gets path') - t.ok(manifest, 'gets manifest') + publish: (manifest, tarData, opts) => { + t.match(manifest, { name: 'my-cool-pkg', version: '1.0.0' }, 'gets manifest') + t.isa(tarData, Buffer, 'tarData is a buffer') t.ok(opts, 'gets opts object') t.same(opts.registry, publishConfig.registry, 'publishConfig is passed through') - t.ok(true, 'libnpmpublish is called') }, }, }) @@ -110,11 +111,13 @@ t.test('re-loads publishConfig if added during script process', (t) => { publish([testDir], (er) => { if (er) throw er - t.end() + t.pass('got to callback') }) }) t.test('should not log if silent', (t) => { + t.plan(2) + const testDir = t.testdir({ 'package.json': JSON.stringify({ name: 'my-cool-pkg', @@ -133,29 +136,38 @@ t.test('should not log if silent', (t) => { }, '../../lib/utils/tar.js': { getContents: () => ({}), - logTar: () => {}, + logTar: () => { + t.pass('called logTar (but nothing actually printed)') + }, }, '../../lib/utils/otplease.js': (opts, fn) => { return Promise.resolve().then(() => fn(opts)) }, + '../../lib/utils/output.js': () => { + throw new Error('should not output in silent mode') + }, npmlog: { verbose: () => {}, + notice: () => {}, level: 'silent', }, libnpmpack: async () => '', libnpmpublish: { - publish: () => {}, + publish: (manifest, tarData, opts) => { + throw new Error('should not call libnpmpublish!') + }, }, }) publish([testDir], (er) => { if (er) throw er - t.end() + t.pass('got to callback') }) }) t.test('should log tarball contents', (t) => { + t.plan(3) const testDir = t.testdir({ 'package.json': JSON.stringify({ name: 'my-cool-pkg', @@ -177,29 +189,32 @@ t.test('should log tarball contents', (t) => { id: 'someid', }), logTar: () => { - t.ok(true, 'logTar is called') + t.pass('logTar is called') }, }, '../../lib/utils/output.js': () => { - t.ok(true, 'output fn is called') + t.pass('output fn is called') }, '../../lib/utils/otplease.js': (opts, fn) => { return Promise.resolve().then(() => fn(opts)) }, libnpmpack: async () => '', libnpmpublish: { - publish: () => {}, + publish: () => { + throw new Error('should not call libnpmpublish!') + }, }, }) publish([testDir], (er) => { if (er) throw er - t.end() + t.pass('got to callback') }) }) t.test('shows usage with wrong set of arguments', (t) => { + t.plan(1) const publish = requireInject('../../lib/publish.js', { '../../lib/npm.js': { flatOptions: { @@ -210,13 +225,11 @@ t.test('shows usage with wrong set of arguments', (t) => { }, }) - publish(['a', 'b', 'c'], (er) => { - t.matchSnapshot(er, 'should print usage') - t.end() - }) + publish(['a', 'b', 'c'], (er) => t.matchSnapshot(er, 'should print usage')) }) t.test('throws when invalid tag', (t) => { + t.plan(1) const publish = requireInject('../../lib/publish.js', { '../../lib/npm.js': { flatOptions: { @@ -228,7 +241,66 @@ t.test('throws when invalid tag', (t) => { }) publish([], (err) => { - t.ok(err, 'throws when tag name is a valid SemVer range') - t.end() + t.match(err, { + message: /Tag name must not be a valid SemVer range: /, + }, 'throws when tag name is a valid SemVer range') + }) +}) + +t.test('can publish a tarball', t => { + t.plan(3) + const testDir = t.testdir({ + package: { + 'package.json': JSON.stringify({ + name: 'my-cool-tarball', + version: '1.2.3', + }), + }, + }) + const tar = require('tar') + tar.c({ + cwd: testDir, + file: `${testDir}/package.tgz`, + sync: true, + }, ['package']) + + // no cheating! read it from the tarball. + fs.unlinkSync(`${testDir}/package/package.json`) + fs.rmdirSync(`${testDir}/package`) + + const tarFile = fs.readFileSync(`${testDir}/package.tgz`) + const publish = requireInject('../../lib/publish.js', { + '../../lib/npm.js': { + flatOptions: { + json: true, + defaultTag: 'latest', + }, + config, + }, + '../../lib/utils/tar.js': { + getContents: () => ({ + id: 'someid', + }), + logTar: () => {}, + }, + '../../lib/utils/output.js': () => {}, + '../../lib/utils/otplease.js': (opts, fn) => { + return Promise.resolve().then(() => fn(opts)) + }, + libnpmpublish: { + publish: (manifest, tarData, opts) => { + t.match(manifest, { + name: 'my-cool-tarball', + version: '1.2.3', + }, 'sent manifest to lib pub') + t.strictSame(tarData, tarFile, 'sent the tarball data to lib pub') + }, + }, + }) + + publish([`${testDir}/package.tgz`], (er) => { + if (er) + throw er + t.pass('got to callback') }) })