This repository has been archived by the owner. It is now read-only.

When shrinkwrap is used npm install won't update git dependencies #12718

Closed
kossnocorp opened this Issue May 16, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@kossnocorp
Contributor

kossnocorp commented May 16, 2016

See the problem:

To reproduce locally, run (see https://github.com/kossnocorp/npm-shrinkwrap-bug):

git clone git@github.com:kossnocorp/npm-shrinkwrap-bug.git
cd npm-shrinkwrap-bug
npm install
@philberg

This comment has been minimized.

philberg commented May 16, 2016

If you run shrinkwrap on this, it will also fail.

I am also facing this issue for the following reason.

My top level package "A" has a dep "B" which is installed via git. Dep "B" has a dep on the chai package (v3.4.) and package "A" has a devDep on chai (v3.3.). This is breaking shrinkwrap

npm ERR! missing: chai@3.4.*, required by package B

@iarna

This comment has been minimized.

Member

iarna commented May 17, 2016

For reference:

$ npm ls
npm-shrinkwrap-bug@ /Users/rebecca/code/npmtest/12718/npm-shrinkwrap-bug
└── left-pad@1.1.0 invalid (git://github.com/stevemao/left-pad.git#37723f75f3a2cf99ce0b64bd14de6c2818cfb506)

npm ERR! invalid: left-pad@1.1.0 /Users/rebecca/code/npmtest/12718/npm-shrinkwrap-bug/node_modules/left-pad

Obviously, it should be seeing that the installed version doesn't match and trying to install from the shrinkwrap (which would then fail, because that committish doesn't exist).

@kossnocorp

This comment has been minimized.

Contributor

kossnocorp commented May 19, 2016

I've successfully wrote a fix:

diff --git a/lib/install/inflate-shrinkwrap.js b/lib/install/inflate-shrinkwrap.js
index f5f53f1..dfc12fc 100644
--- a/lib/install/inflate-shrinkwrap.js
+++ b/lib/install/inflate-shrinkwrap.js
@@ -29,7 +29,7 @@ var inflateShrinkwrap = module.exports = function (tree, swdeps, finishInflating
     if (child && (child.fromShrinkwrap ||
                   (sw.resolved && child.package._resolved === sw.resolved) ||
                   (sw.from && url.parse(sw.from).protocol && child.package._from === sw.from) ||
-                  child.package.version === sw.version)) {
+                  (!child.package.gitHead && child.package.version === sw.version))) {
       if (!child.fromShrinkwrap) child.fromShrinkwrap = spec
       tree.children.push(child)
       return inflateShrinkwrap(child, sw.dependencies || {}, next)

It fixes the both problems (missing update and failure during npm shrinkwrap run). Now I need to add tests, but I'm not sure where to start. It would be cool if someone could help me with that.

kossnocorp added a commit to kossnocorp/npm that referenced this issue May 19, 2016

Fix missing update action for git deps when shrinkwrap is used
Prevent inflateShrinkwrap function from relying on package version
for git dependencies. That was the cause of two problems:

1. When node_modules dir contains an outdated git dependency,
   it won't be updated during npm install.

2. npm shrinkwrap command fails.

For more details about the bug,
see npm#12718.
@kossnocorp

This comment has been minimized.

Contributor

kossnocorp commented May 19, 2016

See PR: #12770

kossnocorp added a commit to kossnocorp/npm that referenced this issue May 19, 2016

Fix missing update action for git deps when shrinkwrap is used
Prevent inflateShrinkwrap function from relying on a package version
for git dependencies. That was the cause of two problems:

1. When node_modules dir contains an outdated git dependency,
   it won't be updated during npm install.

2. npm shrinkwrap command fails.

For more details about the bug,
see npm#12718.

kossnocorp added a commit to kossnocorp/npm that referenced this issue Jun 1, 2016

Fix missing update action for git deps when shrinkwrap is used
Prevent inflateShrinkwrap function from relying on a package version
for git dependencies. That was the cause of two problems:

1. When node_modules dir contains an outdated git dependency,
   it won't be updated during npm install.

2. npm shrinkwrap command fails.

For more details about the bug,
see npm#12718.

@iarna iarna self-assigned this Jun 13, 2016

@codykrainock

This comment has been minimized.

codykrainock commented Jul 8, 2016

I described the issue in vivid detail here: https://github.com/codykrainock/use-sw-bug

Can we please have the fix merged? This bug broke our site today. 😁

iarna added a commit that referenced this issue Aug 11, 2016

Fix missing update action for git deps when shrinkwrap is used
Prevent inflateShrinkwrap function from relying on a package version
for git dependencies. That was the cause of two problems:

1. When node_modules dir contains an outdated git dependency,
   it won't be updated during npm install.

2. npm shrinkwrap command fails.

For more details about the bug,
see #12718.

@iarna iarna closed this in 0f7e319 Aug 12, 2016

@zkat zkat referenced this issue Sep 22, 2016

Closed

deps: upgrade npm to 3.10.8 #8706

4 of 4 tasks complete
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.