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

npm-shrinkwrap.json not included in published hapi package #3338

Closed
iarna opened this issue Sep 7, 2016 · 11 comments
Closed

npm-shrinkwrap.json not included in published hapi package #3338

iarna opened this issue Sep 7, 2016 · 11 comments
Assignees
Labels
Milestone

Comments

@iarna
Copy link

@iarna iarna commented Sep 7, 2016

So I was poking around inside the distributed version of hapi for various reasons and noticed the npm-shrinkwrap.json had gone missing. You can see that it's missing by installing it and looking in node_modules or by fetching down https://registry.npmjs.org/hapi/-/hapi-15.0.3.tgz and looking inside the tarball.

hapi has a pretty atypical shrinkwrap so I've often used it as a very basic canary for shrinkwrap changes.

The reason it's not being packaged is the .npmignore that was added in 84288ec. From the commit I can't tell if this was intentional.

Now... don't get me wrong, I would actually be quite pleased to see fewer unusual shrinkwraps on the registry. So if removing it was intentional, don't put it back on my account! But if it wasn't, I figured you all would want to be aware that it had gone missing.

@devinivy
Copy link
Member

@devinivy devinivy commented Sep 7, 2016

Wow! I feel sure this was just an oversight. Thanks very much for letting us know. :)

@hueniverse hueniverse added the bug label Sep 7, 2016
@hueniverse hueniverse closed this in 7633977 Sep 7, 2016
hueniverse added a commit that referenced this issue Sep 7, 2016
Restore npm-shrinkwrap.json to package.  Closes #3338
@hueniverse hueniverse added this to the 15.0.4 milestone Sep 7, 2016
@AdriVanHoudt
Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Sep 7, 2016

@iarna just out of curiosity what is so special about hapi's shrinkwrap?

@iarna
Copy link
Author

@iarna iarna commented Sep 7, 2016

@AdriVanHoudt It's incomplete– it does not include all the modules necessary to use Hapi, so npm has to fill those in from the package.jsons.

(It also only provides versions, which is unusual, but isn't a corner case the way an incomplete shrinkwrap is.)

@iarna
Copy link
Author

@iarna iarna commented Sep 7, 2016

If I flip off the bit in npm that makes it fill in gaps in npm-shrinkwrap.json I get a tree that looks like this:

🕟  rebecca@Caldina:~/code/npmtest/hapitest$ npm ls
hapitest@1.0.0 /Users/rebecca/code/npmtest/hapitest
└─┬ hapi@15.0.3
  ├─┬ accept@2.1.2
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├─┬ ammo@2.0.2
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├── boom@4.0.0
  ├─┬ call@3.0.3
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├─┬ catbox@7.1.2
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├── catbox-memory@2.0.3
  ├─┬ cryptiles@3.0.2
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├─┬ heavy@4.0.2
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├── hoek@4.0.2
  ├─┬ iron@4.0.3
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├── items@2.1.1
  ├─┬ joi@9.0.4
  │ ├── isemail@2.2.1
  │ └── moment@2.14.1
  ├─┬ mimos@3.0.3
  │ └── mime-db@1.23.0
  ├── podium@1.2.3
  ├── shot@3.3.1
  ├─┬ statehood@5.0.0
  │ └── UNMET DEPENDENCY boom@3.x.x
  ├─┬ subtext@4.2.1
  │ ├── UNMET DEPENDENCY boom@3.x.x
  │ ├─┬ content@3.0.2
  │ │ └── UNMET DEPENDENCY boom@3.x.x
  │ ├─┬ pez@2.1.2
  │ │ ├── b64@3.0.2
  │ │ ├── UNMET DEPENDENCY boom@3.x.x
  │ │ └─┬ nigel@2.0.2
  │ │   └── vise@2.0.2
  │ └─┬ wreck@9.0.0 invalid
  │   └── UNMET DEPENDENCY boom@3.x.x
  └── topo@2.0.2

npm ERR! missing: boom@3.x.x, required by accept@2.1.2
npm ERR! missing: boom@3.x.x, required by ammo@2.0.2
npm ERR! missing: boom@3.x.x, required by call@3.0.3
npm ERR! missing: boom@3.x.x, required by catbox@7.1.2
npm ERR! missing: boom@3.x.x, required by cryptiles@3.0.2
npm ERR! missing: boom@3.x.x, required by heavy@4.0.2
npm ERR! missing: boom@3.x.x, required by iron@4.0.3
npm ERR! missing: boom@3.x.x, required by statehood@5.0.0
npm ERR! missing: boom@3.x.x, required by subtext@4.2.1
npm ERR! invalid: wreck@9.0.0 /Users/rebecca/code/npmtest/hapitest/node_modules/hapi/node_modules/subtext/node_modules/wreck
npm ERR! missing: boom@3.x.x, required by wreck@9.0.0
npm ERR! missing: boom@3.x.x, required by content@3.0.2
npm ERR! missing: boom@3.x.x, required by pez@2.1.2

@iarna
Copy link
Author

@iarna iarna commented Sep 7, 2016

Looking over this again, at this point the weirdness is more specific. And actually I don't think your shrinkwrap is doing what you want. You seem to be trying to do things with it that we unfortunately do not support.

So the boom errors are happening because your deps require boom@3 but you forced boom@4 to be installed at the top level. When npm@3 goes to install hapi it's going to give each of those modules its own copy of boom@3 to fulfill that requirement. There is no way to force the subdeps to use boom@4 short of including it in the shrinkwrap under each of them. (Or, you know, updating the subdeps package.json's to permit boom@4)

The wreck@9.0.0 error is happening because you're forcing v9, but the package.json wants v8. While v9 will be initially installed in node_modules/subtext/node_modules/wreck, subsequent runs of npm will switch it to something matching your package.json.

This is definitely a bug–the current intended behavior (I believe!) is that the package.json defines validity and npm install should never produce invalid installs, so it should be forcing v8 even if you tried to pin it to v9 in your npm-shrinkwrap.json.

@iarna
Copy link
Author

@iarna iarna commented Sep 7, 2016

The "corrected" tree after running npm install a second time ends up looking like this:

This is a physical tree, what you see from npm ls in npm@2.

hapitest@1.0.0 /Users/rebecca/code/npmtest/hapitest
└─┬ hapi@15.0.3
  ├─┬ accept@2.1.2
  │ └── boom@3.2.2
  ├─┬ ammo@2.0.2
  │ └── boom@3.2.2
  ├── boom@4.0.0
  ├─┬ call@3.0.3
  │ └── boom@3.2.2
  ├─┬ catbox@7.1.2
  │ └── boom@3.2.2
  ├── catbox-memory@2.0.3
  ├─┬ cryptiles@3.0.2
  │ └── boom@3.2.2
  ├─┬ heavy@4.0.2
  │ └── boom@3.2.2
  ├── hoek@4.0.2
  ├─┬ iron@4.0.3
  │ └── boom@3.2.2
  ├── items@2.1.1
  ├─┬ joi@9.0.4
  │ ├── isemail@2.2.1
  │ └── moment@2.14.1
  ├─┬ mimos@3.0.3
  │ └── mime-db@1.23.0
  ├── podium@1.2.3
  ├── shot@3.3.1
  ├─┬ statehood@5.0.0
  │ └── boom@3.2.2
  ├─┬ subtext@4.2.1
  │ ├── boom@3.2.2
  │ ├── content@3.0.2
  │ ├─┬ pez@2.1.2
  │ │ ├── b64@3.0.2
  │ │ └─┬ nigel@2.0.2
  │ │   └── vise@2.0.2
  │ └── wreck@8.0.1
  └── topo@2.0.2

@AdriVanHoudt
Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Sep 8, 2016

So a few things.
The fact that wreck is in the shrinkwrap means @hueniverse generates it with the --dev flag right?
Does this mean that when I npm i hapi --production it will also install wreck? Seems unnecessary.

The corrected tree in your last comment, @iarna, does not even include wreck at the top level for hapi how's that?

@hueniverse what is the reason the hapi shrinkwrap looks this way? aka includes dev deps and is "not complete"

Btw I am asking these questions out of pure curiosity, thanks @iarna for the explanation!

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Sep 8, 2016

It was generated with an older version of npm and has been manually updated since.

@iarna
Copy link
Author

@iarna iarna commented Oct 6, 2016

@AdriVanHoudt Because wreck is a dev dependency of hapi but a regular dependency of subtext which is in turn a regular dependency of hapi. And the wreck in the shrinkwrap is explicitly nested under subtext. This tree is from me installing hapi as a dependency, so hapi's dev deps aren't installed.

If I were running npm install from a hapi clone then there'd be a second copy of wreck installed directly below hapi.

@AdriVanHoudt
Copy link
Contributor

@AdriVanHoudt AdriVanHoudt commented Oct 6, 2016

@iarna ah I see, thanks! Should hapi update its shrinkwrap?

@iarna
Copy link
Author

@iarna iarna commented Oct 6, 2016

@AdriVanHoudt If it's at all possible for hapi to switch to a fresh (and complete) shrinkwrap I would strongly encourage it. Go ahead and edit out resolved URLs, but make sure there's an entry for every package in the tree.

This will quickly become urgent in light of the soon-to-be changing shrinkwrap behavior (see #3360).

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants