medikoo / es5-ext

ECMAScript extensions (with respect to upcoming ECMAScript features)
ISC License
168 stars 81 forks source link

v0.10.54 release breaks package installation #109

Closed marshall007 closed 2 years ago

marshall007 commented 2 years ago
npm i es5-ext@0.10.54

> es5-ext@0.10.54 postinstall /handbook/node_modules/es5-ext
> node ./_postinstall.js

+ es5-ext@0.10.54

This module is a dependency for many other popular modules and has broken some of our lint jobs in CI. For example, commands like this one using npx are now failing:

npx sass-lint -c .sass-lint.yml **/*.scss -v --max-warnings 0

/cc @medikoo

marshall007 commented 2 years ago

Here is the commit that broke installs: https://github.com/medikoo/es5-ext/commit/28de285ed433b45113f01e4ce7c74e9a356b2af2

marksteele commented 2 years ago

๐Ÿ‘

Build broken. Understand and appreciate the sentiment behind what you're doing, but please test your stuff before breaking everyone's CI/CD pipelines.

medikoo commented 2 years ago

@marshall007 @marksteele can you elaborate on why exactly you assume that it breaks package installation?

You can confirm here, that v0.10.54 is perfectly fine: https://npmview.vercel.app/es5-ext@0.10.54

It has all the modules, and there's _postinstall.js included. You can confirm it installs without any issues, simply by running npm install es5-ext@0.10.54

stanhu commented 2 years ago

@medikoo The published NPM package is 0 bytes: https://github.com/medikoo/es5-ext/issues/108#issue-1161848936.

marshall007 commented 2 years ago

You can confirm it installs without any issues, simply by running npm install es5-ext@0.10.54

@medikoo that is the command I am running to reproduce the issue. For what it's worth I am on v12.21.0 of node which is fairly old but also the version you get from apt. Perhaps there is some inconsistency between node versions?

medikoo commented 2 years ago

@stanhu The published NPM package is 0 bytes: https://github.com/medikoo/es5-ext/issues/108#issue-1161848936.

It's definitely not the case. Are you sure you're relying on a legit npm registry mirror?

Again you can easily confirm on the content of published es5-ext at e.g. https://npmview.vercel.app/es5-ext@0.10.54

medikoo commented 2 years ago

Note, also that if that would be the case. It'll be mayhem already (this package has 10+ million downloads per week). It's over 1 hour after release, and you're the only reporter of the issue :)

marshall007 commented 2 years ago

@medikoo we are not relying on a registry mirror. Most folks probably have this dependency pinned in their package-lock.json. I'm sure plenty of people are wondering why their builds are failing right now, but it would be limited to those that are not pinning versions (like the example above using npx).

All of the builds on GitLab's public handbook have been failing this morning. You can see that here for example: https://gitlab.com/gitlab-com/www-gitlab-com/-/jobs/2173245711

I confirmed this was an issue locally, nothing specific to our CI environment. Please make sure you do not have a local copy of your package cached and try to run npm i es5-ext@v0.10.54.

medikoo commented 2 years ago

@marshall007 again you're the only person reporting the issue, and we can see in various remote sources, that package publication is perfectly fine.

  1. https://npmview.vercel.app/es5-ext@0.10.54 shows all the content, and it takes data simply from npm registry

  2. npm page reports that size of 0.10.54 version is 543 kB -> https://www.npmjs.com/package/es5-ext/v/0.10.54

  3. When I test installation locally in an empty folder, all goes as expected:

Screenshot 2022-03-07 at 21 54 56

It looks there's some issue with your setup, and I cannot really do much about it

marshall007 commented 2 years ago

@medikoo I haven't tested on the latest version of node, but as I mentioned in https://github.com/medikoo/es5-ext/issues/109#issuecomment-1061114429 we are on an older version. This could be part of the problem.

I can confirm the package is not zero bytes, so I agree that is not the issue. It appears to be a node version inconsistency. Try running:

$ docker run -it node:12 "/bin/bash"
root@c25867fc0684:/# npx sass-lint

internal/modules/cjs/loader.js:818
  throw err;
  ^

Error: Cannot find module '/root/.npm/_npx/8/lib/node_modules/sass-lint/node_modules/es5-ext/_postinstall.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at Function.Module._load (internal/modules/cjs/loader.js:667:27)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
npm ERR! Maximum call stack size exceeded

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2022-03-07T21_18_28_716Z-debug.log
Install for [ 'sass-lint@latest' ] failed with code 1
medikoo commented 2 years ago

@marshall007 I don't see how Error: Cannot find module '/sh -c npx sass-lint' error can be related to this package in any way.

Installations with node.js v12 also work perfectly fine:

Screenshot 2022-03-07 at 22 20 35
ci commented 2 years ago

I think the problem on older node versions is inside Docker where getuid() == 0 because of the file permissions:

3977 verbose lifecycle es5-ext@0.10.55~postinstall: unsafe-perm in lifecycle false
3978 verbose lifecycle es5-ext@0.10.55~postinstall: PATH: /usr/lib/node_modules/npm/node_modules/npm-lifecycle/node-gyp-bin:/root/.npm/_npx/1653/lib/node_modules/sass-lint/node_modules/es5-ext/node_modules/.bin:/root/.npm/_npx/1653/lib/node_modules/sass-lint/node_modules/.bin:/root/.npm/_npx/1653/lib/node_modules/.bin:/asd/asd/node_modules/.bin:/usr/local/bundle/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
3979 verbose lifecycle es5-ext@0.10.55~postinstall: CWD: /root/.npm/_npx/1653/lib/node_modules/sass-lint/node_modules/es5-ext
3980 silly lifecycle es5-ext@0.10.55~postinstall: Args: [ '-c', 'node ./_postinstall.js' ]
3981 silly lifecycle es5-ext@0.10.55~postinstall: Returned: code: 1  signal: null
3982 info lifecycle es5-ext@0.10.55~postinstall: Failed to exec postinstall script

With a node:12 docker image :

โ•ฐโ”€>$ docker run --rm -it node:12 bash
root@a068395a1032:/# npm -v
6.14.4
root@a068395a1032:/# npx sass-lint -V
internal/modules/cjs/loader.js:983
  throw err;
  ^

Error: Cannot find module '/root/.npm/_npx/46/lib/node_modules/sass-lint/node_modules/es5-ext/_postinstall.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:980:15)
    at Function.Module._load (internal/modules/cjs/loader.js:862:27)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
    at internal/main/run_main_module.js:18:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
npm ERR! Maximum call stack size exceeded

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2022-03-07T21_20_21_093Z-debug.log
Install for [ 'sass-lint@latest' ] failed with code 1

root@a068395a1032:/# npm config set unsafe-perm true
root@a068395a1032:/# npx sass-lint -V
1.13.1

Since we do node ./_postinstall.js instead of ./_postinstall.js in the postinstall section, maybe a chown -x postinstall.js and publishing that would work?

Note: newer versions work ~fine:

โ•ฐโ”€>$ docker run --rm -it node:16 bash
root@9fb9485b56a7:/# npm -v
8.3.1
root@9fb9485b56a7:/# npx sass-lint -V
Need to install the following packages:
  sass-lint
Ok to proceed? (y) y
npm WARN deprecated circular-json@0.3.3: CircularJSON is in maintenance only, flatted is its successor.
1.13.1
medikoo commented 2 years ago

Since we do node ./_postinstall.js instead of ./_postinstall.js in the postinstall section, maybe a chown -x postinstall.js and publishing that would work?

It's published as executable file, but still that's not needed for node command (it would be if command would be just ./postinstall.js)

One thing it misses (for an executable file) is shebang (I'll add it now), but as it's invoked via node ./_postinstall.js it shouldn't be an issue for npm installation

marshall007 commented 2 years ago

@medikoo I agree that the file permissions being executable should not matter. I think it might be that the postinstall script being set to node ./_postinstall.js vs simply ./_postinstall.js may result in differing behavior between versions (or maybe even npm i vs npx).

Notice that in the npm error logs you see this line indicating the arguments that were passed when executing the postinstall script:

3979 verbose lifecycle es5-ext@0.10.55~postinstall: CWD: /root/.npm/_npx/1653/lib/node_modules/sass-lint/node_modules/es5-ext
3980 silly lifecycle es5-ext@0.10.55~postinstall: Args: [ '-c', 'node ./_postinstall.js' ]
3981 silly lifecycle es5-ext@0.10.55~postinstall: Returned: code: 1  signal: null
3982 info lifecycle es5-ext@0.10.55~postinstall: Failed to exec postinstall script

So if we try to run this directly from within the node:12 image:

/ # touch _postinstall.js
/ # node -c "_postinstall.js"          # works
/ # node -c "node _postinstall.js" # fails
internal/modules/cjs/loader.js:818
  throw err;
  ^

Error: Cannot find module '/node _postinstall.js'
    at Module._resolveFilename (internal/modules/cjs/loader.js:815:15)
    at internal/main/check_syntax.js:32:20 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

In other words, I think it seems to be interpreting the entire string node _postinstall.js as the argument that should be passed to the node binary. Still very unclear what the problem is here, but perhaps try adding the shebang, making the file executable and setting the postinstall script to just _postinstall.js?

medikoo commented 2 years ago

node -c "node _postinstall.js" will fail in any case, and it's expected. Argument for -c should be a path to script file, and not a CLI command.

Issue lies in node -c usage and not in this package setup.

gilelias commented 2 years ago

workaround for npx scripts: disable post install scripts npm config set ignore-scripts true before running npx.

medikoo commented 2 years ago

@marshall007 I've looked more closely to silly lifecycle es5-ext@0.10.55~postinstall: Args: [ '-c', 'node ./_postinstall.js' ] log, and those are args for shell, not for node.

See: https://github.com/npm/npm-lifecycle/blob/v3.1.5/index.js#L322-L324

By default in non windows environment it's sh, so executed command is sh -c "node ./_postinstall.js", and it works without issues.

It appears in your case shell is customized (see https://github.com/npm/npm-lifecycle/blob/v3.1.5/index.js#L291-L304) and the issue is that it doesn't handle -c "node ./_postinstall.js" args as expected.

gilelias commented 2 years ago

@medikoo even with sh we got error. try docker run node:14.0.0-alpine npx sequelize-cli --version to reproduce.

medikoo commented 2 years ago

@gilelias if you observe issue via docker run node:14.0.0-alpine npx sequelize-cli --version and not via npx sequelize-cli --version, it's clear it's an issue with docker container. Have you tried to report it over there?

gilelias commented 2 years ago

@medikoo The issue is npx. (https://npm.community/t/npx-crashes-on-postinstall-script/9360.html). npx is deprecated. the solution is to use npm cli instead.

medikoo commented 2 years ago

@gilelias great thanks for clarifying this. @marshall007 can we close this report?

kinueng commented 2 years ago

I am experiencing a build break in my CI/CD environment due to the new node call in the postinstall step of es5-ext.

Our builds fail when trying to install gulp 4.0.2 using command npm install gulp -g. npm version 8.3.1. I am going to try the suggested npm install --ignore-scripts option.

The error in our builds:

> es5-ext@0.10.54 postinstall /root/.nvm/versions/node/v12.13.0/lib/node_modules/gulp/node_modules/es5-ext
> node ./_postinstall.js

sh: 1: node: Permission denied

Here is the dependency tree for gulp on my local system

$ npm list es5-ext -g
...
โ””โ”€โ”ฌ gulp@4.0.2
  โ””โ”€โ”ฌ undertaker@1.3.0
    โ””โ”€โ”ฌ es6-weak-map@2.0.3
      โ”œโ”€โ”ฌ d@1.0.1
      โ”‚ โ””โ”€โ”€ es5-ext@0.10.56  deduped
      โ”œโ”€โ”€ es5-ext@0.10.56 
      โ””โ”€โ”ฌ es6-iterator@2.0.3
        โ””โ”€โ”€ es5-ext@0.10.56  deduped

Update: The workaround to use --ignore-scripts worked for my scenario.

martindrq commented 2 years ago

One quick solution is to use $ npm shrinkwrap and change the es5-next version to 0.10.53 this works momently for me. ( iยดm running a gulp task with npx and docker with node:14 in a jenkins job )

martindrq commented 2 years ago

I found that core-js had the same problem in the past:

https://github.com/zloirock/core-js/issues/551

I created a PR with the same approach to prevent npx fails.

medikoo commented 2 years ago

Thanks to @martindrq finding, the issue should be gone starting with v0.10.57 version ๐ŸŽ‰