Add support for npm5

[JamieMason]

npm install -g npm5 has a different cache structure so shrinkpack will need updating to support that, while being backwards compatible for older versions.

[zkat]

So, there's two ways you can go about this: The way I think I most recommend is to run npm pack pkg@1.2.3 pkg2@3.4.5 other-pkg@1 ..., according to the versions in npm-shrinkwrap.json or package-lock.json. This way is compatible with just about any version of npm I know of, and does not even require a warm cache to work: it'll use the cache if needed, but will otherwise make sure whatever you need to fetch gets fetched before it's copied to the cwd.

The other way, which will be only compatible with npm@>=5, is to use cacache in conjunction with the content addresses in package-lock.json or npm-shrinkwrap.json. That is, cacache.get.stream.byDigest('~/.npm/_cacache', locked.dependencies[pkg].integrity).pipe(fs.createWriteStream(locked.dependencies[pkg].name + '.tgz'). Obvs with whatever other stuff you wanna add to this. Note that doing it this way is not all that different from going the npm pack route, except this version allows you to use/verify integrity hashes when available.

lmk if you have any questions about the cache format or the tools around it. shrinkpack is real cool. :)

[JamieMason]

Thanks @zkat, I'll most likely go the npm pack route :+1:

[timdp]

In the meantime, it might be helpful to pass {stdio: ['pipe', 'pipe', 'inherit']} when you spawn npm so stderr doesn't get swallowed. Anyway, I appreciate the effort on this. :+1:

[JamieMason]

Thanks a lot for the tip @timdp

[JamieMason]

@zkat should I consider rewriting the paths in package-lock.json in the same way I do npm-shrinkwrap.json (from URLs to their local .tar path) or would you advise against that?

[zkat]

@JamieMason If you do that, it HAS to be: file:relative/path/to.tar, and you have to add the sha512sum of the .tar to integrity (NOT replace it -- it needs to have both sha512s in that string, separated by a space. See https://npm.im/ssri)

I haven't tried this myself yet, but I would hope it would work. It should. If not, that's a bug I gotta fix.

So, to summarize:

"foo": {
  "version": "1.2.3",
  "resolved": "https://registry.npmjs.org/mypkg/-/mypkg-1.2.3.tgz",
  "integrity": "sha512-deadbeef"
}

becomes

"foo": {
  "version": "1.2.3",
  "resolved": "file:package.mirror/mypkg-1.2.3.tar",
  "integrity": "sha512-deadbeef sha512-c0ffee"
}

If the integrity hash doesn't match resolved, you'll get an integrity error.

[pietermees]

@JamieMason just wondering, any progress on this? Thanks!

[JamieMason]

Hey @zkat, I've noticed a few mentions from npm peeps that there's a plan to ~implement shrinkpack in npm, is that right? If so, would there be any need to add npm v5 support to shrinkpack if it will be available without a 3rd party package anyway?

Thanks a lot,

Jamie

[zkat]

@JamieMason doing that is a ways away. You're free to implement it shrinkpack-side in he meantime -- it's gone pretty far down the queue in the past couple of months, and idk when we'd even get around to it, rn.

[JamieMason]

righto, thanks a lot @zkat

[cmandlbaur]

Is this something that is in progress?

[JamieMason]

Hey again @zkat, hope you like @ mentions.

If I wanted to keep the ↓ part of the +110 -0 ↓50 →60 ✓0 summary shown after shrinkpack has run, am I right that for npm >= 5 cacache.get.hasContent(cache, integrity) is the method I would want to use?

I'm thinking I would use that to report on whether the package was downloaded, but run npm pack to do the ✨🦄 ✨.

@pietermees @cmandlbaur It hasn't been possible to work on shrinkpack up until now, from next week I will be able to hopefully make a start.

If anyone knows of anything I can do to help make it easier for people to contribute, please let me know. I've spent a lot of time on the readme.md and contributing.md etc but there's probably all kinds of stuff that should be clear to people but really isn't.

[pietermees]

That's great news @JamieMason!

I'm not entirely sure what would make contributing easier. I think there's a general sense (at least on my end) that I don't fully understand all the moving parts. And seeing that even you - after all this amazing work - still seem to rely on zkat's knowledge from time to time, makes me feel like the learning curve is relatively long and steep ;)

On the one hand, having a number of smaller tasks that people can tackle to learn seems like a good idea, on the other hand people probably run into specific issues from time to time that they care about a lot (like this one) and are very happy the rest of the time. So not many people might pick up the smaller tasks.

[zkat]

@jamiemason yeah. Although note that the cacache directory is inside ~/.npm/_cacache, when you point it at a cache

[JamieMason]

Thanks again 👊

[JamieMason]

Sorry @zkat, can I nag you again please?

1

I've noticed that sometimes entries have a semver string for version and a location for resolved and other times a location for version and no resolved field, like so;

EDIT: package-lock.json on its own looks as expected (I'm on npm 5.3.0) but the following is seen if the user runs npm shrinkwrap --dev over it.

// familar, with version semver and resolved location
"enhanced-resolve": {
  "version": "3.4.1",
  "resolved": "https://registry.npmjs.org/enhanced-resolve/-/enhanced-resolve-3.4.1.tgz",
  "integrity": "sha1-BCHjOf1xQZs9oT0Smzl5BAIwR24=",
  "dev": true,
  "requires": {
    "graceful-fs": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-4.1.11.tgz",
    "memory-fs": "0.4.1",
    "object-assign": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz",
    "tapable": "0.2.8"
  }
},
// new, with location version and no resolved
"ent": {
  "version": "https://registry.npmjs.org/ent/-/ent-2.2.0.tgz",
  "integrity": "sha1-6WQhkyWiHQX0RGai9obtbOX13R0=",
  "dev": true
},

When running npm pack, should I eg. npm pack ${node.resolved || node.version}? Can you tell me anything more about the rules behind what's happening here?

2

When I come to update the lockfile, what should the resulting version be in these cases? eg.

// input
"ent": {
  "version": "https://registry.npmjs.org/ent/-/ent-2.2.0.tgz",
  "integrity": "sha1-6WQhkyWiHQX0RGai9obtbOX13R0=",
  "dev": true
},

Would npm be able to resolve the dependency if that became this?

// possible output 1
"ent": {
  "version": "file:node_shrinkwrap/some-file.tar",
  "integrity": "sha1-6WQhkyWiHQX0RGai9obtbOX13R0= some-new-sha-for-the-local-file",
  "dev": true
},

Or should it maybe become this?

// possible output 2
"ent": {
  "version": "https://registry.npmjs.org/ent/-/ent-2.2.0.tgz",
  "resolved": "file:node_shrinkwrap/some-file.tar",
  "integrity": "sha1-6WQhkyWiHQX0RGai9obtbOX13R0= some-new-sha-for-the-local-file",
  "dev": true
},

3

Can you point me to anything you'd recommend I should use to generate the sha512s please? also, what should I feed it? the contents of the new tarball? and/or the location? This isn't something I've done before.

4

EDIT: Following on from question 1, I wonder if I'll be able to maintain the file names of node_shrinkwrap/<name>-<semver>.tgz without opening the packages?

Sorry this is a huge list, thanks for all your help.

[zkat]

1. See the spec for package-lock's version field here: https://github.com/npm/npm/blob/latest/doc/spec/package-lock.md#version-changed. Also, you should keep requires in there.

2. Thinking about this makes me wonder if npm5 can actually do this sort of thing right now. URL specs don't match semver specs, so npm would (I believe) immediately NOPE out and force a full reinstallation because the package-lock would not be considered to match the package.json. I think the right thing here is to keep the original version field value and change resolved, but I think npm insists on resolved being web URLs right now. I really just don't know and you'll have to experiment here and maybe help patch npm to make it support this.

3. https://npm.im/ssri is a full toolset for working with SRI strings, and is what npm itself uses

4. I don't really understand this or why it's a problem.

[JamieMason]

Thanks a lot

[rockerest]

Just wanted to leave a note that I'm pleased that you've found time to work on this.

Like @pietermees:

there's a general sense (at least on my end) that I don't fully understand all the moving parts. And seeing that even you - after all this amazing work - still seem to rely on zkat's knowledge from time to time, makes me feel like the learning curve is relatively long and steep ;)

So I feel indebted to you for the work already done, and for the work you've now picked up again.

I'm cheering for you, and I deeply appreciate your time and energy on this project. :beers: :tada:

[pietermees]

I second that!

[JamieMason]

That's amazing, thanks so much. Just a note to people dropping in that I'm travelling now so won't be able to pick up again until mid-late next week.

😊👍

[JamieMason]

Some learnings today, I've created 4 branches which show different structures of npm-shrinkwrap.json and package.json.

• format 1 ❌
• format 2 ❌
• format 3 ✅
• format 4 ❌

Possible things to try next are updating the requires fields as follows...

"react": {
  "version": "file:node_shrinkwrap/react-15.6.1.tgz",
  "integrity": "sha1-uqhDTsZ4C96ZfNw4C3nNM7ljk98=",
  "requires": {
    "create-react-class": "file:node_shrinkwrap/create-react-class-15.6.0.tgz",
    "fbjs": "file:node_shrinkwrap/fbjs-0.8.14.tgz",
    "loose-envify": "file:node_shrinkwrap/loose-envify-1.3.1.tgz",
    "object-assign": "file:node_shrinkwrap/object-assign-4.1.1.tgz",
    "prop-types": "file:node_shrinkwrap/prop-types-15.5.10.tgz"
  }
}

...as that is what was produced when running the following to see how npm itself would generate a localised npm-shrinkwrap.json;

npm install --save ./node_shrinkwrap/angular-1.6.5.tgz
npm install --save ./node_shrinkwrap/asap-2.0.6.tgz
npm install --save ./node_shrinkwrap/core-js-1.2.7.tgz
npm install --save ./node_shrinkwrap/create-react-class-15.6.0.tgz
npm install --save ./node_shrinkwrap/encoding-0.1.12.tgz
npm install --save ./node_shrinkwrap/fbjs-0.8.14.tgz
npm install --save ./node_shrinkwrap/iconv-lite-0.4.18.tgz
npm install --save ./node_shrinkwrap/is-stream-1.1.0.tgz
npm install --save ./node_shrinkwrap/isomorphic-fetch-2.2.1.tgz
npm install --save ./node_shrinkwrap/js-tokens-3.0.2.tgz
npm install --save ./node_shrinkwrap/lodash-4.17.4.tgz
npm install --save ./node_shrinkwrap/lodash.assign-4.2.0.tgz
npm install --save ./node_shrinkwrap/loose-envify-1.3.1.tgz
npm install --save ./node_shrinkwrap/node-fetch-1.7.2.tgz
npm install --save ./node_shrinkwrap/object-assign-4.1.1.tgz
npm install --save ./node_shrinkwrap/promise-7.3.1.tgz
npm install --save ./node_shrinkwrap/prop-types-15.5.10.tgz
npm install --save ./node_shrinkwrap/react-15.6.1.tgz
npm install --save ./node_shrinkwrap/setimmediate-1.0.5.tgz
npm install --save ./node_shrinkwrap/ua-parser-js-0.7.14.tgz
npm install --save ./node_shrinkwrap/whatwg-fetch-2.0.3.tgz
npm shrinkwrap --dev

that produced format 3 which is the only format which will let you install while offline and with an empty npm/cacache cache. But it has the following in package.json which will not be acceptable;

  "dependencies": {
    "angular": "file:node_shrinkwrap/angular-1.6.5",
    "asap": "file:node_shrinkwrap/asap-2.0.6",
    "core-js": "file:node_shrinkwrap/core-js-1.2.7",
    "create-react-class": "file:node_shrinkwrap/create-react-class-15.6.0",
    "encoding": "file:node_shrinkwrap/encoding-0.1.12",
    "fbjs": "file:node_shrinkwrap/fbjs-0.8.14",
    "iconv-lite": "file:node_shrinkwrap/iconv-lite-0.4.18",
    "is-stream": "file:node_shrinkwrap/is-stream-1.1.0",
    "isomorphic-fetch": "file:node_shrinkwrap/isomorphic-fetch-2.2.1",
    "js-tokens": "file:node_shrinkwrap/js-tokens-3.0.2",
    "lodash": "file:node_shrinkwrap/lodash-4.17.4",
    "lodash.assign": "file:node_shrinkwrap/lodash.assign-4.2.0",
    "loose-envify": "file:node_shrinkwrap/loose-envify-1.3.1",
    "node-fetch": "file:node_shrinkwrap/node-fetch-1.7.2",
    "object-assign": "file:node_shrinkwrap/object-assign-4.1.1",
    "promise": "file:node_shrinkwrap/promise-7.3.1",
    "prop-types": "file:node_shrinkwrap/prop-types-15.5.10",
    "react": "file:node_shrinkwrap/react-15.6.1",
    "setimmediate": "file:node_shrinkwrap/setimmediate-1.0.5",
    "ua-parser-js": "file:node_shrinkwrap/ua-parser-js-0.7.14",
    "whatwg-fetch": "file:node_shrinkwrap/whatwg-fetch-2.0.3"
  }

• format 4 is format 3 but with the package.json put back to a more typical format;

  "dependencies": {
    "angular": "1.6.5",
    "asap": "2.0.6",
    "core-js": "1.2.7",
    "create-react-class": "15.6.0",
    "encoding": "0.1.12",
    "fbjs": "0.8.14",
    "iconv-lite": "0.4.18",
    "is-stream": "1.1.0",
    "isomorphic-fetch": "2.2.1",
    "js-tokens": "3.0.2",
    "lodash": "4.17.4",
    "lodash.assign": "4.2.0",
    "loose-envify": "1.3.1",
    "node-fetch": "1.7.2",
    "object-assign": "4.1.1",
    "promise": "7.3.1",
    "prop-types": "15.5.10",
    "react": "15.6.1",
    "setimmediate": "1.0.5",
    "ua-parser-js": "0.7.14",
    "whatwg-fetch": "2.0.3"
  }

Strangely though, this version does not work.

I'll post more as I find it, thanks all.

[rockerest]

Thanks for the continuing investigation and updates!

[JamieMason]

@iarna @zkat if you have some time I could use your help if you can please? It's most likely that I have the structure of package-lock.json slightly wrong, but there's also a slim chance this might be an npm bug? I wonder this because format 4 from https://github.com/JamieMason/shrinkpack/issues/83#issuecomment-322593863 is created by installing a local tarball using npm alone, then changing the dependency in package.json back to a semver version number. Again though, this is likely just me misunderstanding something in the new lockfiles.

Thanks as always, grateful for your time and help.

[JamieMason]

I've opened https://github.com/npm/npm/issues/18339 over on the npm repo and am available if there's anything at all I can do to help, I know the npm team are crazy busy.

[iarna]

Yeah, I'll look at this uh, probably next week, but sooner if possible.

[JamieMason]

is this something one of us could contribute to do you think @iarna? Or does it look like it could be too involved to be handled outside the core team? if there's any way I can help I'd be happy to. Thanks.

[timdp]

@iarna Do you happen to have an update? Thanks!

[JamieMason]

I wonder if cipm is an alternative route that might work? (https://twitter.com/rebeccaorg/status/902983343376961536). I am away but will try next week if no one beats me to it.

The latest shrinkpack is on the https://github.com/JamieMason/shrinkpack/tree/dev branch and can be run with npm install && npm run build && node dist/bin.js --help

[iarna]

@timdb Nope! My priorities largely got rejuggled by 2fa, but I'm making sure it's on my list of things.

@JamieMason I don't actually know what the scope of the problem is as yet. If someone wants to take a crack at it, that'd be great though. (And if someone does and has questions about the source, drop by the #npm channel on https://package.community/ and we'd be happy to answer them.)

[iarna]

Ok, I've gotten some time to look at this, in particular the aggregate-error branch:

There are two problems with the package-lock in that branch.

First, the requires section should still be by version. npm matches them up by comparing values from the requires field with the version field.

Second, the integrity field needs to have hashes from the tar files in your resolved field. Right now it has values from the tgz files that the tar files were derived from.

The actual error itself is coming from extract's subprocesses as they have problems with the shasum failures (as we now extract tarballs in parallel). The aggregate mentioned is the aggregate of the extract workers. It both needs better messaging, well, currently the real errors show up on screen but not in the logs, which is pretty awful.

[JamieMason]

Great, thanks @iarna I will take a look at those issues and get them fixed.

[iarna]

My local testing shows that fixing those won't be quite enough, there's also some underlying npm bug…

[iarna]

This is my cut-down repro: https://gist.github.com/iarna/a47991307e626ffdef0a3012ae41fae5

[JamieMason]

OK thanks for the info, I'll concentrate on correctness of the package lock and head to that channel with any questions, most likely around generating the hashes but will see when I get to it.

Grateful for your help as always.

[iarna]

ssri is a great way to go generate those hashes =)

[Fridus]

Shrinkpack for npm5 coming soon ? 😃

[JamieMason]

Second, the integrity field needs to have hashes from the tar files in your resolved field. Right now it has values from the tgz files that the tar files were derived from. – @iarna

I just want to check, right now locally I have:

    "add-matchers": {
      "version": "0.5.0",
      "resolved": "file:node_shrinkwrap/add-matchers-0.5.0.tar",
      "integrity": "sha1-UCGQ5HUM1XIWGDkyaLYaFXNm52U= sha512-WqLVrqoSZuHp2fNqq/9XTILkn8VeJCO5QdVvo+qWTAdohrUvtR7sLYA6isVLJsZ2KoU6oRhY7d2yGOdjxrBdpQ=="
    },

This looks like it should be right if I've understood https://github.com/JamieMason/shrinkpack/issues/83#issuecomment-306861678 from @zkat correctly, but is that correct or should it change to contain only the sha for the .tar like so?

    "add-matchers": {
      "version": "0.5.0",
      "resolved": "file:node_shrinkwrap/add-matchers-0.5.0.tar",
      "integrity": "sha512-WqLVrqoSZuHp2fNqq/9XTILkn8VeJCO5QdVvo+qWTAdohrUvtR7sLYA6isVLJsZ2KoU6oRhY7d2yGOdjxrBdpQ=="
    },

Thanks.

[JamieMason]

The latest implementation is as follows;

When leaving packages compressed

The integrity properties are left as "<original-tgz-hash>"

When decompressing packages

The integrity properties are set to "<original-tgz-hash> <new-tar-hash>"

Generating hashes

The <new-tar-hash> is generated using the file contents and ssri.stringify

const contents = await fs.readFile(tarPath); 
const integrity = ssri.fromData(contents); 
return ssri.stringify(integrity);

[zkat]

I just pushed a patch that might help with this -- do you wanna try using npx npmc i? I've published the patch as part of the canary so folks can try it out. It fixed @iarna's repro and a couple of other git/tar/etc-related issues, specially around checksums.

[JamieMason]

I've just given this a try @zkat, but as things are at the moment shrinkpack with npm5 does not appear to work.

The gist of the problem is that npmc is still attempting to hit the public registry instead of reading the resolved fields and hitting ./node_shrinkwrap.

The most likely cause is that I don't have the npm-shrinkwrap.json set up exactly as it needs to be.

Here is a script to reproduce:

#!/usr/bin/env bash

rm -rf ~/npm5-and-shrinkpack
mkdir ~/npm5-and-shrinkpack
cd ~/npm5-and-shrinkpack

git clone https://github.com/JamieMason/shrinkpack.git -b dev
cd ~/npm5-and-shrinkpack/shrinkpack
npmc i
npmc run build

cd ~/npm5-and-shrinkpack
npm init -y
npmc install @telerik/eslint-config
npmc install @types/core-js
npmc install @types/jasmine
npmc install ava
npmc install finch

npmc shrinkwrap
node ~/npm5-and-shrinkpack/shrinkpack/dist/bin.js --compress

read -r -p "Disconnect From The Internet Then Hit Enter To Continue" response

npmc cache clear --force
rm -rf node_modules
npmc install

The general format of the npm-shrinkwrap.json produced by shrinkpack is:

{
  "name": "mock-project",
  "version": "1.0.0",
  "lockfileVersion": 1,
  "requires": true,
  "dependencies": {
    // lots of other deps
    "ansi-align": {
      "version": "1.1.0",
      "resolved": "file:node_shrinkwrap/ansi-align-1.1.0.tgz",
      "integrity": "sha1-LwwWWIKXOa3V67FeawxuNCPwFro=",
      "requires": {
        "string-width": "1.0.2"
      }
    }
    // lots of other deps
  }
}

The error log produced by npmc is:

2017-11-16T11_00_30_255Z-debug.log

Notes:

• The value in the integrity field (sha1-LwwWWIKXOa3V67FeawxuNCPwFro=) is the original one put there by npmc for ansi-align-1.1.0.tgz, it is the exact same tarball as downloaded from the registry. I this fine or should the sha be regenerated using the tarball on disk?
• Is it correct that the requires members have their semver version as the value? or should that be eg. file:node_shrinkwrap/string-width-1.0.2.tgz? I ask because when following the steps at https://github.com/npm/npm/issues/18339 this is what npm itself does to the requires fields but in this discussion here we've talked about it needing to be the semver version.

Thanks all.

[JamieMason]

Sorry, I'm also still able to reproduce https://github.com/npm/npm/issues/18339 using npmc.

[JamieMason]

I think let's focus on getting @iarna's cut-down repro working in npmc then continue from there.

To save confusion I have left the dev branch at the commit it was at when I wrote https://github.com/JamieMason/shrinkpack/issues/83#issuecomment-344893102 and will do any new work in a new refactor branch for now.

[JamieMason]

I've added --loglevel silly to the refactor branch and an example failing test for aggregate error can be seen at https://travis-ci.org/JamieMason/shrinkpack/jobs/303640167.

The dev branch is passing, but I think an important difference between that and the test script I've been using from https://github.com/JamieMason/shrinkpack/issues/83#issuecomment-344893102 is that it does not npm cache clear --force.

I will update the tests to do that as shrinkpack would need to work offline from an empty cache.

[simllll]

npm 5.6 is out, any news on this?

[JamieMason]

This ticket is still blocked by https://github.com/npm/npm/issues/18339. The npm team are aware of the issue but have a lot on their plate.

For anyone reading this in the future: this is the highest priority issue in shrinkpack and when anything does progress you will hear about it by subscribing. You do not need to poke or nudge this thread to keep it alive (this isn't a dig at you @simllll 😃 👍 )

[rockerest]

It looks like the fix has landed but as far as I can tell it hasn't made it into an npm release yet. Fingers crossed. As always thanks for your continued work on this.

[iarna]

@rockerest That landed in 5.6.0 which is latest.

[JamieMason]

as far as I can tell https://github.com/npm/npm/issues/18339 is still present in 5.6.0, I have left a comment over there. Anyone following this issue might also want to subscribe to that one.

[JamieMason]

Spotted this PR over at https://github.com/npm/npm/pull/19423 🎉