npm / cli

the package manager for JavaScript
https://docs.npmjs.com/cli/
Other
8.49k stars 3.17k forks source link

[BUG] "npm pack" ignores --silent when running "prepack" scripts #4121

Closed fregante closed 2 years ago

fregante commented 2 years ago

Is there an existing issue for this?

This issue exists in the latest npm version

Current Behavior

I noticed this change when GitHub Actions bumped Node 12 from v12.13.1 to 12.22.7 and I can replicate it locally.

npm pack --silent should only output the generated archive name, but it ends up also printing the scripts information:

❯ npm pack --silent

> content-scripts-register-polyfill@3.1.0 prepack
> tsc --sourceMap false

content-scripts-register-polyfill-3.1.0.tgz

Note: This also happens when .npmrc includes the silent log level (which works correctly for the regular npm pack output)

I pinpointed it to this line:

https://github.com/npm/cli/blob/4dbeb007d0d6350284c7b1edbf4d5b0030c67c66/lib/commands/pack.js#L53

The function expects a log object to appear in that options object, but it's not there. Here's that "flatOptions" object (without private information):

{
  _auth: null,
  access: null,
  all: false,
  allowSameVersion: false,
  omit: [],
  audit: true,
  auditLevel: null,
  authType: 'legacy',
  before: null,
  binLinks: true,
  browser: null,
  ca: null,
  call: '',
  cert: null,
  ciName: null,
  cidr: null,
  color: true,
  commitHooks: true,
  depth: null,
  search: {
    limit: 20,
    description: true,
    exclude: '',
    opts: [Object: null prototype] {},
    staleness: 900
  },
  diff: [],
  diffIgnoreAllSpace: false,
  diffNameOnly: false,
  diffNoPrefix: false,
  diffDstPrefix: 'b/',
  diffSrcPrefix: 'a/',
  diffText: false,
  diffUnified: 3,
  dryRun: false,
  editor: 'vi',
  engineStrict: false,
  retry: { retries: 2, factor: 10, maxTimeout: 60000, minTimeout: 10000 },
  timeout: 300000,
  force: false,
  foregroundScripts: false,
  formatPackageLock: true,
  fund: true,
  git: 'git',
  gitTagVersion: true,
  global: false,
  globalStyle: false,
  globalconfig: '/usr/local/etc/npmrc',
  heading: 'npm',
  httpsProxy: null,
  ifPresent: false,
  ignoreScripts: false,
  includeStaged: false,
  json: false,
  key: null,
  legacyBundling: false,
  legacyPeerDeps: false,
  localAddress: null,
  location: 'user',
  maxSockets: 15,
  message: '%s',
  nodeVersion: 'v16.4.2',
  noProxy: '',
  npmVersion: '8.2.0',
  offline: false,
  otp: null,
  package: [],
  packageLock: true,
  packageLockOnly: false,
  parseable: false,
  preferOffline: false,
  preferOnline: false,
  preid: '',
  proxy: null,
  readOnly: false,
  rebuildBundle: true,
  registry: 'https://registry.npmjs.org/',
  save: true,
  saveBundle: false,
  savePrefix: '^',
  projectScope: '',
  scriptShell: undefined,
  shell: '/usr/local/bin/fish',
  signGitCommit: false,
  signGitTag: false,
  ssoPollFrequency: 500,
  ssoType: 'oauth',
  strictPeerDeps: false,
  strictSSL: true,
  defaultTag: 'latest',
  tagVersionPrefix: 'v',
  umask: 0,
  npmCommand: 'pack'
}

Expected Behavior

❯ npm pack --silent

content-scripts-register-polyfill-3.1.0.tgz

Steps To Reproduce

echo '{"name":"name", "version": "1", "scripts": {"prepack":"true"}}' > package.json
npm pack --silent

Environment

wraithgar commented 2 years ago

silent is a directive to logging, not to script execution. --foreground-scripts is the thing that is supposed to silence script output.

Having said that, the --foreground-scripts param is not being obeyed properly by libnpmpack, so that's a bug.

winterqt commented 2 years ago

--foreground-scripts is the thing that is supposed to silence script output.

Doesn't --foreground-scripts do the exact opposite of silencing the output?

Run all build scripts (ie, preinstall, install, and postinstall) scripts for installed packages in the foreground process, sharing standard input, output, and error with the main npm process.

I assume you mean it's not obeying the option no matter if it's true or false, right?

winterqt commented 2 years ago

I wonder why silent ever worked for this, stdio was always set to "inherit" in libnpmpack.

wraithgar commented 2 years ago

fixed by https://github.com/npm/cli/pull/5645, thank you @winterqt