From a1f9be8a7f9b7a3a813fc3e5e705bc982470b0e2 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 27 Oct 2020 15:25:20 -0700 Subject: [PATCH] Support publishing any kind of spec, not just directories Credit: @isaacs Close: #2074 Reviewed-by: @nlf --- 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') }) })