Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

install silently prunes misnamed git dependencies #10401

Closed
olizilla opened this issue Nov 15, 2015 · 4 comments
Closed

install silently prunes misnamed git dependencies #10401

olizilla opened this issue Nov 15, 2015 · 4 comments

Comments

@olizilla
Copy link
Contributor

Adding a dependency on a git url fails silently if the name given doesn't match the name property of the remote package.json

This is an error on the users part, but the result is the remote dep is resolved correctly, but then pruned:

npm sill install loadAllDepsIntoIdealTree
npm sill fetchOtherPackageData x@github:select2/select2#6be96cfaa175448ef70b978648de7d6cb5822

...

verb addRemoteGit data._resolved: git://github.com/select2/select2.git#6be96cfaa175448ef70b978648de7d6cb5822e89
npm sill cache afterAdd Select2@4.0.0
npm verb afterAdd /Users/oli/.npm/Select2/4.0.0/package/package.json not in flight; writing
npm verb afterAdd /Users/oli/.npm/Select2/4.0.0/package/package.json written
npm sill resolveWithNewModule Select2@4.0.0 checking installable status
npm sill loadAllDepsIntoIdealTree Finishing
npm sill idealTree:prePrune some-whizzy-app-built-with-npm3@1.0.0
npm sill idealTree:prePrune └── Select2@4.0.0
npm sill loadIdealTree Finishing
npm sill currentTree some-whizzy-app-built-with-npm3@1.0.0
npm sill idealTree some-whizzy-app-built-with-npm3@1.0.0

...

npm verb exit [ 0, true ]
npm info ok

The user gets no warning of their mistake, and the dep is not installed.

This is most noticeable when coupled with casing issues. Given a dependency declaration like:

"dependencies": {
  "select2": "select2/select2#6be96cf"
}

...where we've accidentally lowercased the dep name to "select2" (in savant-like readiness for the new way) but the remote is actually uppity cased as "Select2";
in npm@2 the dep is installed regardless.

In npm@3 the dep is silently pruned as if you never declared it, which is surprising when transitioning an old project.

Digging around, I can see that it's because the resolved dep name doesn't match any listed deps, and is correctly pruned, so I reckon this sort of error should be caught earlier, in a similar way to how tarballs are verified

@olizilla
Copy link
Contributor Author

olizilla added a commit to olizilla/npm that referenced this issue Nov 16, 2015
Where the name given for a git url dep doesn't match the name given
in the remote package.json, fail the install and let the user know.

Existing `npm@3.4.0` behaviour is to siltenly prune the dep from the tree.
with no warning.

- Minimal fix for npm#10401
- Resuses the error message from [tarball verification](https://github.com/npm/npm/blob/65a64c9277184b1a0665d78fce0a9b00f930d9bc/lib/cache/add-local-tarball.js#L124)
@olizilla
Copy link
Contributor Author

Minimal fix here: #10410

@iarna
Copy link
Contributor

iarna commented Jan 6, 2016

See also: #8588

@npm-robot
Copy link

We're closing this issue as it has gone seven days without activity and without being labeled. If we haven't even labeled in issue in seven days then we're unlikely to ever read it.

If you are still experiencing the issue that led to you opening this or this is a feature request you're still interested in then we encourage you to open a new issue. If this was a support issue, you may be better served by joining package.communty and asking your question there.

For more information about our new issue aging policies and why we've instituted them please see our blog post.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants