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 dedupe -g removes some globally installed packages #2620

Closed
markSmurphy opened this issue Feb 4, 2021 · 8 comments · Fixed by #2716
Closed

[BUG] npm dedupe -g removes some globally installed packages #2620

markSmurphy opened this issue Feb 4, 2021 · 8 comments · Fixed by #2716
Assignees
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release

Comments

@markSmurphy
Copy link

Current Behaviour:

I have some npm packages installed globally, which are command line utilities (create-react-app, distributed-dig, snyk`, and others). I recently updated to npm version 7 (7.5.2) with node 14 (14.15.4) on Windows 10 (version: 2004 || build: 19041.746).

If I perform a global dedupe ...

npm dedupe -g

... a number of the globally installed packages are removed. Curiously not all of them. In the example below eight of the 11 are removed.

Incidentally, I couldn't reproduce this behaviour with npm v7.5.2 & node v15.5.1 on Linux Mint v19

Expected Behaviour:

I'd expect the same behaviour as npm v6 where each top-level globally installed package persists after the global dedupe. In the example steps below I'd expect all 11 globally installed packages to still be installed after the dedupe.

Steps To Reproduce:

Report current versions: npm -v ; node -v

PS C:\Users> npm -v;node -v
7.5.2
v14.15.4

List currently installed global packages (and report a count): npm ls -g --depth 0;npm ls -g --depth 0|find "--" /c

PS C:\Users> npm ls -g --depth 0;npm ls -g --depth 0|find `"--`" /c
C:\Users\markm\AppData\Roaming\npm
+-- akamai-error-lookup@1.2.8
+-- akamai-staging@1.1.8
+-- ascii-art@2.5.0
+-- cdn-cache-check@1.6.0
+-- codename-generator@1.0.15
+-- create-react-app@4.0.2
+-- depcheck@1.3.1
+-- distributed-dig@1.8.3
+-- eslint@7.19.0
+-- jshint@2.12.0
+-- npm@7.5.2
`-- snyk@1.441.0

12

Perform a global dedupe: npm dedupe -g

PS C:\Users> npm dedupe -g

added 1 package, removed 1334 packages, and audited 256 packages in 6s

11 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

Notice that removed 1334 packages

Re-list globally installed packages (and a new count): npm ls -g --depth 0;npm ls -g --depth 0|find "--" /c

PS C:\Users> npm ls -g --depth 0;npm ls -g --depth 0|find `"--`" /c
C:\Users\markm\AppData\Roaming\npm
+-- acorn-jsx@5.3.1
+-- npm@7.5.2
`-- prettyjson@1.2.1

3

Eight of the the initial 11 global packages are now removed
2021-02-04_12h19_39

Environment:

  • OS: Windows 10 (version: 2004 || build: 19041.746)
  • Node: 14.15.4
  • npm: 7.5.2
  • PowerShell 7.1.1 (although I still see this behaviour when using cmd.exe instead:
    2021-02-04_13h27_40
@markSmurphy markSmurphy added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Feb 4, 2021
@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2021

What are you expecting to happen with a global dedupe? (certainly not remove packages, this sounds like a bug)

@markSmurphy
Copy link
Author

markSmurphy commented Feb 4, 2021

I'd expect the same behaviour as npm v6 where each top-level globally installed package persists after the global dedupe. In the example I've given I'd expect all 12 globally installed packages to still be installed after the dedupe.

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2021

Right, but i mean, what do you expect it to change - to deduplicate?

@markSmurphy
Copy link
Author

oh, I see what you're asking.
I'd expect the dedupe to look at each packages' dependencies for duplicates and where they are found to reference the same one on disk rather than have multiple copies.
So, for example, if more than one of the global packages references yargs in its respective packahe.json, then I'd expect dedupe to ensure they both use the same copy rather than have it twice (or more) on the disk.

From the documentation this is:

Searches the local package tree and attempts to simplify the overall structure by moving dependencies further up the tree, where they can be more effectively shared by multiple dependent packages.

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2021

That's not what dedupe does tho - it hoists everything farther up the filesystem so they can be shared. However, in the case of global packages, nothing can be hoisted up to the global root (or it'd mess with npm ls -g, and what's in your PATH), so there's no possible deduplication to be had.

In other words, I'd expect npm dedupe -g to fail immediately, without achieving anything or deleting anything.

@markSmurphy
Copy link
Author

That's what I meant, even if I didn't explain it very clearly or gave a duff example.

Here's a better (real world) example.

I install npm install -g akamai-error-lookup which references Akamai's edgegrid (used to authenticate with, and call, Akamai's APIs).
edgegrid references request which in turn references combined-stream & form-data. But form-data also references combined-stream; so there's two copies of it on disk. After a dedupe -g I would expect form-data to uses the hoisted (as you say) copy of combined-stream.

If I look on my linux system (where I don't see this problem) and examine the output of npm ls -g --depth 9 I see the form-data reference of combined-stream is deduped (line 24) along with >20 other packages.

...
├─┬ akamai-error-lookup@1.2.8
│ ├── colors@1.4.0
│ ├─┬ debug@4.3.1
│ │ └── ms@2.1.2
│ ├─┬ edgegrid@3.0.8
│ │ ├─┬ log4js@0.6.38
│ │ │ ├─┬ readable-stream@1.0.34
│ │ │ │ ├── core-util-is@1.0.2
│ │ │ │ ├── inherits@2.0.4
│ │ │ │ ├── isarray@0.0.1
│ │ │ │ └── string_decoder@0.10.31
│ │ │ └── semver@4.3.6
│ │ ├── moment@2.29.1
│ │ ├─┬ request@2.88.2
│ │ │ ├── aws-sign2@0.7.0
│ │ │ ├── aws4@1.11.0
│ │ │ ├── caseless@0.12.0
│ │ │ ├─┬ combined-stream@1.0.8
│ │ │ │ └── delayed-stream@1.0.0
│ │ │ ├── extend@3.0.2
│ │ │ ├── forever-agent@0.6.1
│ │ │ ├─┬ form-data@2.3.3
│ │ │ │ ├── asynckit@0.4.0
│ │ │ │ ├── combined-stream@1.0.8 deduped
│ │ │ │ └── mime-types@2.1.28 deduped
│ │ │ ├─┬ har-validator@5.1.5
│ │ │ │ ├─┬ ajv@6.12.6
│ │ │ │ │ ├── fast-deep-equal@3.1.3
│ │ │ │ │ ├── fast-json-stable-stringify@2.1.0
│ │ │ │ │ ├── json-schema-traverse@0.4.1
│ │ │ │ │ └─┬ uri-js@4.4.1
│ │ │ │ │   └── punycode@2.1.1 deduped
│ │ │ │ └── har-schema@2.0.0
│ │ │ ├─┬ http-signature@1.2.0
│ │ │ │ ├── assert-plus@1.0.0
│ │ │ │ ├─┬ jsprim@1.4.1
│ │ │ │ │ ├── assert-plus@1.0.0 deduped
│ │ │ │ │ ├── extsprintf@1.3.0
│ │ │ │ │ ├── json-schema@0.2.3
│ │ │ │ │ └─┬ verror@1.10.0
│ │ │ │ │   ├── assert-plus@1.0.0 deduped
│ │ │ │ │   ├── core-util-is@1.0.2 deduped
│ │ │ │ │   └── extsprintf@1.3.0 deduped
│ │ │ │ └─┬ sshpk@1.16.1
│ │ │ │   ├─┬ asn1@0.2.4
│ │ │ │   │ └── safer-buffer@2.1.2 deduped
│ │ │ │   ├── assert-plus@1.0.0 deduped
│ │ │ │   ├─┬ bcrypt-pbkdf@1.0.2
│ │ │ │   │ └── tweetnacl@0.14.5 deduped
│ │ │ │   ├─┬ dashdash@1.14.1
│ │ │ │   │ └── assert-plus@1.0.0 deduped
│ │ │ │   ├─┬ ecc-jsbn@0.1.2
│ │ │ │   │ ├── jsbn@0.1.1 deduped
│ │ │ │   │ └── safer-buffer@2.1.2 deduped
│ │ │ │   ├─┬ getpass@0.1.7
│ │ │ │   │ └── assert-plus@1.0.0 deduped
│ │ │ │   ├── jsbn@0.1.1
│ │ │ │   ├── safer-buffer@2.1.2
│ │ │ │   └── tweetnacl@0.14.5
│ │ │ ├── is-typedarray@1.0.0
│ │ │ ├── isstream@0.1.2
│ │ │ ├── json-stringify-safe@5.0.1
│ │ │ ├─┬ mime-types@2.1.28
│ │ │ │ └── mime-db@1.45.0
│ │ │ ├── oauth-sign@0.9.0
│ │ │ ├── performance-now@2.1.0
│ │ │ ├── qs@6.5.2
│ │ │ ├── safe-buffer@5.2.1
│ │ │ ├─┬ tough-cookie@2.5.0
│ │ │ │ ├── psl@1.8.0
│ │ │ │ └── punycode@2.1.1
│ │ │ ├─┬ tunnel-agent@0.6.0
│ │ │ │ └── safe-buffer@5.2.1 deduped
│ │ │ └── uuid@3.4.0 deduped
│ │ └── uuid@3.4.0
│ ├── html-entities@2.1.0
│ ├─┬ prettyjson@1.2.1
│ │ ├── colors@1.4.0 deduped
│ │ └── minimist@1.2.5
│ ├─┬ supports-color@8.1.1
│ │ └── has-flag@4.0.0
│ └─┬ yargs@16.2.0
│   ├─┬ cliui@7.0.4
│   │ ├── string-width@4.2.0 deduped
│   │ ├─┬ strip-ansi@6.0.0
│   │ │ └── ansi-regex@5.0.0
│   │ └─┬ wrap-ansi@7.0.0
│   │   ├─┬ ansi-styles@4.3.0
│   │   │ └─┬ color-convert@2.0.1
│   │   │   └── color-name@1.1.4
│   │   ├── string-width@4.2.0 deduped
│   │   └── strip-ansi@6.0.0 deduped
│   ├── escalade@3.1.1
│   ├── get-caller-file@2.0.5
│   ├── require-directory@2.1.1
│   ├─┬ string-width@4.2.0
│   │ ├── emoji-regex@8.0.0
│   │ ├── is-fullwidth-code-point@3.0.0
│   │ └── strip-ansi@6.0.0 deduped
│   ├── y18n@5.0.5
│   └── yargs-parser@20.2.4
...

I think we can agree that dedupe -g shouldn't remove top level packages in their entirety though, so this is probably a bug. And since npm v6 doesn't behave this way, it's probably a bug introduced in npm v7

@ljharb
Copy link
Contributor

ljharb commented Feb 4, 2021

Agreed. I think the correct behavior would be to error immediately with a message that deduping global packages doesn’t make sense.

@nlf nlf removed the Needs Triage needs review for next steps label Feb 17, 2021
@nlf nlf self-assigned this Feb 17, 2021
@nlf
Copy link
Contributor

nlf commented Feb 22, 2021

this is fixed in npm 7.5.5 (we now throw an error to prevent you from doing something that won't work how you expect anyway)

@nlf nlf closed this as completed Feb 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Release 7.x work is associated with a specific npm 7 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants