Pnpm: scripts not called on `install <git dep>`

[andreineculau]

pnpm version: 1.7.1

Code to reproduce the issue:

see https://github.com/andreineculau/package-json-scripts

cd $(mktemp -d)
mkdir node_modules
pnpm install git://github.com/andreineculau/package-json-scripts.git # lists preinstall, install, postinstall

cd $(mktemp -d)
mkdir node_modules
npm install git://github.com/andreineculau/package-json-scripts.git # lists preinstall, install, postinstall, prepare, prepack, postpack, preinstall, install, postinstall

Expected behavior:

same scripts as current npm version (5) intends to run

Actual behavior:

only same scripts as npm@4 are run

Additional information:

• npm -v prints: 5.3.0
• node -v prints: v8.2.1
• Windows, OS X, or Linux?: OSX

[andreineculau]

I should note that I didn't expect pnpm to perform so well re:scripts :clap: https://docs.google.com/spreadsheets/d/1XH8iDyOmNf-ZH3A2SMV5YTDcT2YPXmhFcaUPsIyYEu8/edit#gid=0

[zkochan]

Documentation of npm says that prepack should be executed when installing git dependencies. Not a word about prepare, postpack and second execution of install

[andreineculau]

We could look at it from multiple angles. One being that documentation is not up-to-date with the code or that it's not clear enough (by all means, that's why I started the package-json-scripts repo, but maybe that's just me). Another is the the docs hide implementation detail i.e. we may run other things, but we tell users to consider only that prepare is run, because that's what we run on npm install (no args). And yet another being that I messed up my test (though I recently double-checked my results).

When looking at pacote though, you find this https://github.com/zkat/pacote/blob/latest/lib/fetchers/git.js#L20

I didn't look in super in depth, but my take is that the flow is as follows:

• if the git dep is already in the local cache, and we're in the cache window, the cache will be used
• if not, the git dep is cloned to a temp folder, where the {pre,,post}install and prepare scripts runs
  -- npm then packs it and puts the tarball in the cache
  -- npm then unpacks it from the cache in the "expected" node_modules location
  -- npm runs the {pre,,post}install scripts

That would simulate a full publishing chain (except for the {pre,,post}publish scripts ofc).

[3cp]

@zkochan is there a timeline on improving compatibility on lifecycle scripts calls?

@andreineculau there is another aspect you could add to your valuable matrix. When npm installs from git repo, it runs all scripts with NODE_ENV=production, but yarn doesn't force NODE_ENV. I don't know whether npm use production env on all cases, I didn't test enough.

[zkochan]

It shouldn't be too hard to do for a first-time contributor. I will probably take it not earlier than 5th of May

[andreineculau]

I need to test this myself but setting NODE_ENV to prod is counter intuitive since you need dev deps to run 'npm prepare'

[3cp]

@zkochan I considered for few minutes by peeking through source code. But I don’t use typescript, so I hesitated.

I like typescript but never had strong enough motivation to invest my time on it. I can see pnpm attracted lots of users but not many non-trivial contributors. Nothing against typescript, but it could be barrier of many potential contributors.

I want to contribute when I had more spare time in future. Some yarn issues start to annoy me, I want to contribute to pnpm because technically pnpm makes more sense to me.

[zkochan]

@huochunpeng thanks. Our usage of TypeScript is not very complex. We use really simple code. Just functions, no classes. No nesting. So you should be fine if you know just how to declare a type or interface in TypeScript. On the other hand, it gives you autocompletion and other nice things (especially if you use VS Code).

This particular issue seems a bit more complex now. I didn't know that dev dependencies should be installed for git-hosted dependencies. If so, then we'll also need to save those dev deps to shrinkwrap.yaml

[3cp]

@andreineculau I think npm think all build scripts should be executed to make a production build not development (default env) build.

I was caught by it, wondering why npm failed to install my private bitbucket repo. I use babelrc environment config for development and test env, but not production env. So npm failed but yarn did not.

[andreineculau]

I suggest you create a minimal github repo with a package.json that surfaces the problems you describe with your private repo, including specific npm/pnpm/yarn versions.

[3cp]

@andreineculau here you are. yarn currently has bug that bypasses prepare script on github repos, so I created one on bitbucket https://bitbucket.org/huochunpeng/debug-npm.

It has a prepare script:

"scripts": {
  "prepare": "echo NODE_ENV=$NODE_ENV; babel src -d dist"
}

And babelrc

{
  "env": {
    "development": {
      "presets": ["env", "stage-1"]
    }
  }
}

To test against it, create a local package.json then run npm/yarn/pnpm

{
  "dependencies": {
    "debug-npm": "https://bitbucket.org/huochunpeng/debug-npm.git"
  }
}

npm 5.8.0, note it prints NODE_ENV=production

⋊> ~/p/dt npm i
npm ERR! prepareGitDep 1> 
npm ERR! prepareGitDep > [email protected] install /Users/huocp/.npm/_cacache/tmp/git-clone-83fac8cf/node_modules/fsevents
npm ERR! prepareGitDep > node install
npm ERR! prepareGitDep 
npm ERR! prepareGitDep [fsevents] Success: "/Users/huocp/.npm/_cacache/tmp/git-clone-83fac8cf/node_modules/fsevents/lib/binding/Release/node-v57-darwin-x64/fse.node" already installed
npm ERR! prepareGitDep Pass --update-binary to reinstall or --build-from-source to recompile
npm ERR! prepareGitDep 
npm ERR! prepareGitDep > [email protected] prepare /Users/huocp/.npm/_cacache/tmp/git-clone-83fac8cf
npm ERR! prepareGitDep > echo NODE_ENV=$NODE_ENV; babel src -d dist
npm ERR! prepareGitDep 
npm ERR! prepareGitDep NODE_ENV=production
npm ERR! prepareGitDep 
npm ERR! prepareGitDep 2> npm WARN install Usage of the `--dev` option is deprecated. Use `--only=dev` instead.
npm ERR! prepareGitDep SyntaxError: src/index.js: Unexpected token (2:6)
npm ERR! prepareGitDep   1 | export class Debug {
npm ERR! prepareGitDep > 2 |   foo = 1;
npm ERR! prepareGitDep     |       ^
npm ERR! prepareGitDep   3 | }
npm ERR! prepareGitDep   4 | 
npm ERR! prepareGitDep npm ERR! code ELIFECYCLE
npm ERR! prepareGitDep npm ERR! errno 1
npm ERR! prepareGitDep npm ERR! [email protected] prepare: `echo NODE_ENV=$NODE_ENV; babel src -d dist`
npm ERR! prepareGitDep npm ERR! Exit status 1
npm ERR! prepareGitDep npm ERR! 
npm ERR! prepareGitDep npm ERR! Failed at the [email protected] prepare script.
npm ERR! prepareGitDep npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! prepareGitDep 
npm ERR! prepareGitDep npm ERR! A complete log of this run can be found in:
npm ERR! prepareGitDep npm ERR!     /Users/huocp/.npm/_logs/2018-04-22T01_54_35_515Z-debug.log
npm ERR! prepareGitDep 
npm ERR! code ENOPACKAGEJSON
npm ERR! package.json Non-registry package missing package.json: debug-npm@git+https://bitbucket.org/huochunpeng/debug-npm.git.
npm ERR! package.json npm can't find a package.json file in your current directory.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/huocp/.npm/_logs/2018-04-22T01_54_35_594Z-debug.log

yarn 1.5.1, note it prints NODE_ENV=

⋊> ~/p/dt yarn
yarn install v1.5.1
warning package.json: No license field
info No lockfile found.
warning No license field
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
info No lockfile found.
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
$ echo NODE_ENV=$NODE_ENV; babel src -d dist
NODE_ENV=
src/index.js -> dist/index.js
[3/4] 🔗  Linking dependencies...
[4/4] 📃  Building fresh packages...
success Saved lockfile.
✨  Done in 19.41s.

pnpm 1.41.1, of course, no script run.

⋊> ~/p/dt pnpm i
Packages: +1
+
Resolving: total 1, reused 0, downloaded 1, done
dependencies:
+ debug-npm 1.0.0

[zkochan]

Recently the shrinkwrap.yaml format has been updated. Now package that should be built have a requiresBuild: true property. For instance,

https://github.com/pnpm/pnpm/blob/fa98b4eae401cc0fca43c7e2e524b3d9bc4fcdc6/shrinkwrap.yaml#L907-L917

but now we also have to know if the package's prepare script has to be executed. Currently, there is no way to tell whether a package is a git-hosted package (from info in the shrinkwrap.yaml).

I suggest adding a new boolean called prepare. So for a git dep the shrinkwrap.yaml entry would look like this:

  github.com/pnpm/supi/6f64f8d2248e84a6c642eeb9b5546006e040898e:
    dependencies:
      '@pnpm/check-package': 1.0.0
      '@pnpm/fs-locker': 1.0.1
      '@pnpm/headless': 0.2.6
      '@pnpm/lifecycle': 2.1.0
    engines:
      node: '>=4'
    name: supi
    peerDependencies:
      '@pnpm/logger': ^1.0.0
    prepare: true
    requiresBuild: true
    resolution:
      tarball: 'https://codeload.github.com/pnpm/supi/tar.gz/6f64f8d2248e84a6c642eeb9b5546006e040898e'
    version: 0.16.8

[3cp]

Currently, there is no way to tell whether a package is a git-hosted package (from info in the shrinkwrap.yaml).

@zkochan the version is in url format, not semver for any git-hosted package, isn't it?

devDependencies:
  aurelia-cli: github.com/huochunpeng/cli/10edaef0ca222ab61efcaf141b28c8f8df268967

[zkochan]

Not always. registry-hosted packages also have the whole URL if they are
from the non-default registry.

On Thu, Apr 26, 2018 at 1:13 AM huochunpeng [e-mail]
wrote:

Currently, there is no way to tell whether a package is a git-hosted
package (from info in the shrinkwrap.yaml).

@zkochan https://github.com/zkochan the version are in url format, not
semver for any git-hosted package, isn't it?

devDependencies:
aurelia-cli: github.com/huochunpeng/cli/10edaef0ca222ab61efcaf141b28c8f8df268967

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/pnpm/pnpm/issues/855#issuecomment-384450675, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AB1pm5KtQkRA9iyDPABXg3Wt_3XDDr3iks5tsPT0gaJpZM4Ogepx
.

[vjpr]

I suggest adding a new boolean called prepare. So for a git dep the shrinkwrap.yaml entry would look like this:

Why not add an entry explicitly stating its from git?

Also, it might be useful to be able to know this in the pnpmfile#addPackage hook. If you know its a git dep, you can tweak the install scripts if necessary. This is probably already possible right?

[zkochan]

Also, it might be useful to be able to know this in the pnpmfile#addPackage hook. If you know its a git dep, you can tweak the install scripts if necessary. This is probably already possible right?

not possible currently, here's an issue to allow it: https://github.com/pnpm/supi/issues/55

Why not add an entry explicitly stating its from git?

I suppose it will be more efficient to have the prepare: true entry. When doing pnpm rebuild for instance, there would be no need to read the package.json files of the git dependencies. A simple filter by x.prepare && x.requiresBuild would return everything that should be built.

[vjpr]

not possible currently, here's an issue to allow it: pnpm/supi#55

Excellent, I had this same need.

[zkochan]

@huochunpeng running "prepare" in git-hosted deps landed in [email protected]

[3cp]

@zhochan sorry for late feedback. Could you check how prepare script was executed?

The local command babel (installed by debug-npm's devDependencies babel-cli) could not be found.

⋊> ~/p/tp pnpm i git+ssh://[email protected]/huochunpeng/debug-npm.git#v1.0.0
Packages: +1
+
Resolving: total 1, reused 0, downloaded 1, done

bitbucket.org/huochunpeng/debug-npm/361ff261e60f60a91da09bb259230303ccef8087 |     prepare$ echo NODE_ENV=$NODE_ENV; babel src -d dist
bitbucket.org/huochunpeng/debug-npm/361ff261e60f60a91da09bb259230303ccef8087 |     prepare: NODE_ENV=
bitbucket.org/huochunpeng/debug-npm/361ff261e60f60a91da09bb259230303ccef8087 |     prepare: sh: babel: command not found
 ERROR  [email protected] prepare: `echo NODE_ENV=$NODE_ENV; babel src -d dist`
spawn ENOENT
at Error: debug-npm                                                                                
at <anonymous>       …e_modules/npm-lifecycle/lib/spawn.js:48  const er = new Error('spawn ENOENT')
at emitTwo           events.js:126                                                                 
at emit              events.js:214                                                                 
at maybeClose        internal/child_process.js:925                                                 
at onexit            internal/child_process.js:209

[zkochan]

That is correct. devDependencies are not installed. This is how prepare of subdeps work with Yarn as well. Just install the dependencies that are needed for building the package as dev dependencies of the main app

[3cp]

@zkochan
https://github.com/pnpm/pnpm/issues/855#issuecomment-383200924
https://github.com/pnpm/pnpm/issues/855#issuecomment-383220822
I am pretty sure devDeps are needed to run prepare script, which you aknowledged before 😄 . That's how people build their package.json scripts section, babel should be in devDeps, not deps.

[zkochan]

Seems like yarn uses a temp folder or the cache folder, where it installs the package with its dev deps, builds it and then puts the result to the target node_modules. I am personally lazy to implement such complicated logic. Maybe for these packages we could just convert the prod deps to dev deps? As a temporary solution, you can do it with a pnpmfile.js (a readPackage hook)

[3cp]

Yarn must be using temp folder. Because when it failed, it doesn’t go into yarn cache, which is correct behavior.

While pnpm still cached (or sould I say store) the failed install, a repeated install command uses the malfunctioned cache, without further complaints.

[3cp]

@zkochan
I was thinking about this, it might not be too bad. What we want is not to run certain npm script, but to do a full npm pack which is dry-run of npm publish.

Instead of fetching git/tarball directly into pnpm store cache, we need

1. fetch it to a temp folder.
2. pnpm i in the temp folder.
3. run npm pack which reports the generated tar.gz file name.
4. now we can expand the tar.gz file into pnpm store cache.
5. purge temp folder.

I suspect this is what npm is doing, because I can see the final installed npm module correctly respected .npmignore or package.json "files" sections.

[3cp]

Any future plan/roadmap to support this properly? This is the missing feature that prevents me from fully migrating from npm to pnpm.

[zkochan]

I don't have this in my near future roadmap.

We can offer a $100 bounty from our opencollective to anyone who implements this (you should have a PayPal account).

Please discuss the solution before implementing it.

[3cp]

@zkochan what about the steps I proposed in 3 comments above? I am not sure what's the right choice for step2.

[zkochan]

Seems OK. I edited step 2

[andykais]

Hi, I just want to make sure I have all the facts right. This doc shows
all the scripts that pnpm calls right? https://docs.google.com/spreadsheets/d/1XH8iDyOmNf-ZH3A2SMV5YTDcT2YPXmhFcaUPsIyYEu8/edit#gid=0

I have noticed that the package pre-push does not seem to call their "install" script (though npm does). Am I misinterpreting the doc? If not though, I can make a separate issue describing the problem if that is better.

 pnpm --version
3.8.1

[trusktr]

Documentation of npm says that prepack should be executed when installing git dependencies. Not a word about prepare, postpack and second execution of install

The latest docs now mention that both prepare and prepack are ran for git dependencies. https://docs.npmjs.com/misc/scripts

[trusktr]

Another important note:

With NPM, in a package being installed from git prepack scripts do not have access to dev dependencies while prepare scripts do.

This means we can not rely on prepack as a way to have git dependencies build themselves during install.