Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npm ci erroneously installs optional OS-constrained transitive dependency through direct shrinkwrap dependency #7622

Open
2 tasks done
restjohn opened this issue Jul 2, 2024 · 2 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps

Comments

@restjohn
Copy link

restjohn commented Jul 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Consider a package, lib1, with the following characteristics.

  • has an optionalDependency, with an OS constraint, such as fsevents
    package.json
    { ... "os": [ "darwin" ] ... }
    
  • published with a shrinkwrap package descriptor
  • shrinkwrap was generated on the platform compatible with the OS-constrained dependency

Developer A creates another package, app1, which depends on lib1, and generates app1's package-lock.json with npm install also on the platform matching said OS constraint.

Developer B, OR a CI process, on a different platform from said OS constraint, runs npm ci to install app1's dependencies. npm ci produces an error like the following.

[.../app1> npm ci
npm error code EBADPLATFORM
npm error notsup Unsupported platform for [email protected]: wanted {"os":"darwin"} (current: {"os":"linux"})
npm error notsup Valid os:  darwin
npm error notsup Actual os: linux
npm error A complete log of this run can be found in: /root/.npm/_logs/2024-07-02T12_41_06_204Z-debug-0.log

Examining app1's package-lock.json reveals that npm does not include an "optional": true entry in the package-lock block for lib1's fsevents dependency.

Expected Behavior

npm ci should retain the optional nature of the platform-specific dependency and proceed with a successful clean install of app1's dependencies regardless of the dependencies.

Steps To Reproduce

  1. On a linux platform, clone the demo repository: https://github.com/restjohn/issues.npm.shrinkwrap_optional_dep.
  2. Use npm 10.8.1, latest at the time of this writing.
  3. cd app1
  4. npm ci
  5. npm should produce an error as in the Current Behavior section above.

Please see the README in the demo repository for quite a bit more detail about the nuances of this behavior.

Also note that the demo repository package lib1.shrinkwrap references the fsevents package through a devDependency, and npm should not be attempting to install lib1.shrinkwrap devDependencies from the app1 package anyway.

Environment

  • npm: 10.8.1
  • Node.js: 20.15.0
  • OS Name: Debian GNU/Linux 12 (bookworm)
  • System Model Name: Node 20 Slim Docker image
  • npm config:
; node bin location = /usr/local/bin/node
; node version = v20.15.0
; npm local prefix = /npm_transitive_os_dep/app1
; npm version = 10.8.1
; cwd = /npm_transitive_os_dep/app1
; HOME = /root
; Run `npm config ls -l` to show all defaults.
@restjohn restjohn added Bug thing that needs fixing Needs Triage needs review for next steps labels Jul 2, 2024
@restjohn restjohn changed the title [BUG] npm ci erroneously installs optional OS-constrained transitive dependency [BUG] npm ci erroneously installs optional OS-constrained transitive dependency through direct shrinkwrap dependency Jul 2, 2024
restjohn added a commit to ngageoint/mage-server that referenced this issue Jul 3, 2024
@restjohn
Copy link
Author

restjohn commented Jul 4, 2024

Aside from the problem of forcing installation of an optional dependency, another cause of this bug is that npm is installing the dev dependencies from the shrinkwrapped package. Apparently this is quite a long-standing issue.

restjohn added a commit to ngageoint/mage-server that referenced this issue Jul 4, 2024
* [ci] remove obsolete workflows

* [ci] remove unused pre-latest node versions list; enforce ordering on the list; create an explicit LTS version to use for packaging

* [ci] wip: separate plugin builds

* [service] upgrade better-sqlite3 dep to build with node 22

* [ci] fix triggering repo paths in workflows

* [ci] fix typos in workflow files

* [ci] add arcgis service plugin workflow

* [plugins/image] exclude spec dir with large test images from package

* bump core beta version to 6.3.0-beta.5

* [ci] remove plugin publishing from core release workflow

* [ci] fix artifact name typos
[skip ci]

* [plugins/arcgis] update mage.service dep in package-lock

* [service] brand new shrinkwrap to attempt to correct os-specific fsevents dep erroneous installation in plugin projects; upgrade typescript to 4.9 to accommodate @types/lodash upgrade

* bump core packages versions to 6.3.0-beta.6

* [plugins/arcgis] move @types/geojson dep to dev dependencies

* [plugins/arcgis] manually add optional and dev flags to fsevents dep in package-lock to work around npm issue npm/cli#7622

* [service] minor shrinkwrap update on qs dep

* [plugins/arcgis] add a test file to get the ball rolling and make ci pass running the test command

* [plugins/arcgis] wip: plugin naming: rename service package

* [plugins/arcgis] wip: plugin naming: move web artifacts to consistent project structure

* [plugins/arcgis] wip: plugin naming: fix references to old project structure names

* [plugins/arcgis] remove unused index file in web-app

* [plugins/arcgis] add test config to web-app

* [plugins/arcgis] add actions ci workflow for web-app

* [plugins/arcgis] add a dummy test in web-app to pass build

* [plugins/image] constrain mage core dep to 6.3.0-beta+

* [plugins/image] manually add dev and optional flags to fsevents in service package-lock so build does not fail on non-darwin platforms

* [ci] add image service plugin release workflow

* [plugins/image] update typescript and mongoose deps to match core mage

* [ci] rename image plugin release workflow

* [ci] rename arcgis web-app artifacts for consistency

* [ci] remove obsolete env var

* [ci] add arcgis plugin release workflow

* [ci] add nga-msi plugin release workflow

* [instance] fix references to renamed arcgis packages
@restjohn
Copy link
Author

Others are having issues with shrinkwrap as well: #4323.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps
Projects
None yet
Development

No branches or pull requests

1 participant