Don't resolve node_remote_module targets by themselves and modify how REPL works

Review Request #2997 - Created Oct. 19, 2015 and submitted

Information
Chris Pesto
pants
2402
f1d4fd8...
Reviewers
pants-reviews
jsirois, stuhood

We currently resolve 3rd party node_remote_module targets by themselves and throw that work away. Unfortunately there isn't really such a thing as an individually-resolved 3rd party module in NPM, especially NPM 3, so there isn't an obvious way to use them. Right now, the plan is to at least get Pants working with shrinkwrap in some way to get reliable builds, rather than cached individually-resolved 3rd party targets.

I do the following things in this diff:

  • Don't resolve 3rd party node_remote_module targets individually. They just get put in each target's package.json normally and installed via the "npm install" under each target.
  • Remove the "npm dedupe" in the resolve task - since we are just doing normal npm installs under each target (rather than copying/symlinking node_remote_module target trees into place, for example) we don't have any unusual duplication, so this is just slowing things down. Furthermore, "npm dedupe" in NPM 2 can actually go to the network and install new packages, which can make the build unreliable/unrepeatable. NPM 3 does the dedupe automatically as part of the "npm install", and it doesn't go to the network.
  • Change the REPL to construct a synthetic package.json in a temp directory with dependencies on all target_roots and "npm install". Since we aren't resolving node_remote_module targets individually we can't just construct a NODE_PATH with all of the targets and run Node, as before. (We still would have wanted to modify the previous implementation to use target_roots instead of targets, anyway). This makes for a slower REPL startup, but we are optimizing for the resolve which is more important. And installing rather than constructing a NODE_PATH target makes a more realistic environment.
  • Small test changes to reflect the other changes. Removing test_resolve_remote and adding test_run_repl_multiple_targets. Also switched the typ version numbers in test_resolve_simple so the natural deduping of NPM 2's install algorithm would dedupe typ, as we're now not explicitly calling "npm dedupe".

Existing tests should generally cover this, and its mainly a change to underlying behavior. I made the small test changes noted above, though.

PR is here: https://github.com/pantsbuild/pants/pull/2402. Waiting for CI to finish.

Issues

  • 0
  • 1
  • 0
  • 1
Description From Last Updated
Chris Pesto
Stu Hood
Chris Pesto
Chris Pesto
Chris Pesto
Chris Pesto
Chris Pesto
Stu Hood
Chris Pesto
Review request changed

Status: Closed (submitted)

Change Summary:

Merged as a662dd94472acfc77a03c3ad82079ddff991a889

Loading...