feat: symlink file dependencies instead of copying them

This is roughly how npm@5 does it. However, pnpm does not try
to install dependencies of the file dependency, just a symlink
is created and the binstubs are linked into `node_modules/.bin`
if there are any.

BREAKING CHANGE:

file dependencies are symlinked instead of copied (packed/unpacked)

Ref #772

src/api/install.ts

@@ -1,6 +1,7 @@
 import path = require('path')
 import RegClient = require('npm-registry-client')
 import logger from 'pnpm-logger'
+import logStatus from '../logging/logInstallStatus'
 import pLimit = require('p-limit')
 import npa = require('npm-package-arg')
 import pFilter = require('p-filter')
@@ -11,6 +12,7 @@ import {PnpmOptions, StrictPnpmOptions, Dependencies} from '../types'
 import createGot from '../network/got'
 import getContext, {PnpmContext} from './getContext'
 import installMultiple, {InstalledPackage} from '../install/installMultiple'
+import externalLink from './link'
 import linkPackages from '../link'
 import save from '../save'
 import getSaveType from '../getSaveType'
@@ -29,7 +31,7 @@ import {save as saveModules} from '../fs/modulesController'
 import mkdirp = require('mkdirp-promise')
 import createMemoize, {MemoizedFunc} from '../memoize'
 import {Package} from '../types'
-import {PackageSpec} from '../resolve'
+import {PackageSpec, DirectoryResolution, Resolution} from '../resolve'
 import {DependencyTreeNode} from '../link/resolvePeers'
 import depsToSpecs, {similarDepsToSpecs} from '../depsToSpecs'
 
@@ -56,6 +58,15 @@ export type PackageContentInfo = {
 
 export type InstallContext = {
   installs: InstalledPackages,
+  linkedPkgs: {
+    optional: boolean,
+    dev: boolean,
+    resolution: DirectoryResolution,
+    id: string,
+    version: string,
+    name: string,
+    specRaw: string,
+  }[],
   childrenIdsByParentId: {[parentId: string]: string[]},
   nodesToBuild: {
     nodeId: string,
@@ -251,7 +262,6 @@ async function installInContext (
     nodeModules: nodeModulesPath,
     update,
     keypath: [],
-    referencedFrom: opts.prefix,
     prefix: opts.prefix,
     parentNodeId: ':/:',
     currentDepth: 0,
@@ -275,6 +285,15 @@ async function installInContext (
     }
   })
   const pkgs: InstalledPackage[] = R.props<TreeNode>(rootNodeIds, installCtx.tree).map(node => node.pkg)
+  const pkgsToSave = (pkgs as {
+    optional: boolean,
+    dev: boolean,
+    resolution: Resolution,
+    id: string,
+    version: string,
+    name: string,
+    specRaw: string,
+  }[]).concat(installCtx.linkedPkgs)
 
   let newPkg: Package | undefined = ctx.pkg
   if (installType === 'named') {
@@ -285,12 +304,12 @@ async function installInContext (
     const saveType = getSaveType(opts)
     newPkg = await save(
       pkgJsonPath,
-      <any>pkgs.map(dep => { // tslint:disable-line
+      <any>pkgsToSave.map(dep => { // tslint:disable-line
         const spec = R.find(spec => spec.raw === dep.specRaw, newSpecs)
         if (!spec) return null
         return {
           name: dep.name,
-          saveSpec: getSaveSpec(spec, dep, opts.saveExact)
+          saveSpec: getSaveSpec(spec, dep.version, opts.saveExact)
         }
       }).filter(Boolean),
       saveType
@@ -307,7 +326,7 @@ async function installInContext (
 
     const getSpecFromPkg = (depName: string) => deps[depName] || devDeps[depName] || optionalDeps[depName]
 
-    pkgs.forEach(dep => {
+    pkgsToSave.forEach(dep => {
       const ref = pkgIdToRef(dep.id, dep.name, dep.resolution, ctx.shrinkwrap.registry)
       if (dep.dev) {
         ctx.shrinkwrap.devDependencies = ctx.shrinkwrap.devDependencies || {}
@@ -374,6 +393,17 @@ async function installInContext (
     )
   }
 
+  if (installCtx.linkedPkgs.length) {
+    const linkOpts = Object.assign({}, opts, {skipInstall: true})
+    await Promise.all(installCtx.linkedPkgs.map(async linkedPkg => {
+      await externalLink(linkedPkg.resolution.directory, opts.prefix, linkOpts)
+      logStatus({
+        status: 'installed',
+        pkgId: linkedPkg.id,
+      })
+    }))
+  }
+
   // waiting till the skipped packages are downloaded to the store
   await Promise.all(
     R.props<InstalledPackage>(Array.from(installCtx.skipped), installCtx.installs)
@@ -423,12 +453,12 @@ async function getTopParents (pkgNames: string[], modules: string) {
   }))
 }
 
-function getSaveSpec(spec: PackageSpec, pkg: InstalledPackage, saveExact: boolean) {
+function getSaveSpec(spec: PackageSpec, version: string, saveExact: boolean) {
   switch (spec.type) {
     case 'version':
     case 'range':
     case 'tag':
-      return `${saveExact ? '' : '^'}${pkg.version}`
+      return `${saveExact ? '' : '^'}${version}`
     default:
       return spec.saveSpec
   }
@@ -437,6 +467,7 @@ function getSaveSpec(spec: PackageSpec, pkg: InstalledPackage, saveExact: boolea
 async function createInstallCmd (opts: StrictPnpmOptions, shrinkwrap: Shrinkwrap, skipped: Set<string>): Promise<InstallContext> {
   return {
     installs: {},
+    linkedPkgs: [],
     childrenIdsByParentId: {},
     nodesToBuild: [],
     shrinkwrap,

src/api/link.ts

@@ -13,11 +13,13 @@ const linkLogger = logger('link')
 export default async function link (
   linkFrom: string,
   linkTo: string,
-  maybeOpts?: PnpmOptions
+  maybeOpts?: PnpmOptions & {skipInstall?: boolean}
 ) {
   const opts = extendOptions(maybeOpts)
 
-  await install(Object.assign({}, opts, { prefix: linkFrom, global: false }))
+  if (!maybeOpts || !maybeOpts.skipInstall) {
+    await install(Object.assign({}, opts, { prefix: linkFrom, global: false }))
+  }
 
   const destModules = path.join(linkTo, 'node_modules')
   await linkToModules(linkFrom, destModules)

src/install/fetch.ts

@@ -5,6 +5,7 @@ import path = require('path')
 import rimraf = require('rimraf-then')
 import resolve, {
   Resolution,
+  DirectoryResolution,
   PackageSpec,
   PackageMeta,
 } from '../resolve'
@@ -25,11 +26,16 @@ import symlinkDir = require('symlink-dir')
 import * as unpackStream from 'unpack-stream'
 
 export type FetchedPackage = {
+  isLink: true,
+  resolution: DirectoryResolution,
+  pkg: Package,
+  id: string,
+} | {
+  isLink: false,
   fetchingPkg: Promise<Package>,
   fetchingFiles: Promise<PackageContentInfo>,
   calculatingIntegrity: Promise<void>,
   path: string,
-  srcPath?: string,
   id: string,
   resolution: Resolution,
 }
@@ -78,10 +84,23 @@ export default async function fetch (
       pkgId = resolveResult.id
       pkg = resolveResult.package
     }
+
     const id = <string>pkgId
 
     logStatus({status: 'resolved', pkgId: id, pkg: options.loggedPkg})
 
+    if (resolution.type === 'directory') {
+      if (!pkg) {
+        throw new Error(`Couldn't read package.json of local dependency ${spec}`)
+      }
+      return {
+        isLink: true,
+        id,
+        pkg,
+        resolution,
+      }
+    }
+
     const target = path.join(options.storePath, pkgIdToFilename(id))
 
     if (!options.fetchingLocker[id]) {
@@ -97,15 +116,13 @@ export default async function fetch (
     }
 
     return {
+      isLink: false,
       fetchingPkg: options.fetchingLocker[id].fetchingPkg,
       fetchingFiles: options.fetchingLocker[id].fetchingFiles,
       calculatingIntegrity: options.fetchingLocker[id].calculatingIntegrity,
       id,
       resolution,
       path: target,
-      srcPath: resolution.type == 'directory'
-        ? path.join(options.prefix, resolution.directory)
-        : undefined,
     }
   } catch (err) {
     logStatus({status: 'error', pkg: options.loggedPkg})

src/install/fetchResolution.ts

@@ -1,7 +1,6 @@
 import logger from 'pnpm-logger'
 import fs = require('mz/fs')
 import path = require('path')
-import spawn = require('cross-spawn')
 import execa = require('execa')
 import {IncomingMessage} from 'http'
 import * as unpackStream from 'unpack-stream'
@@ -49,39 +48,12 @@ export default async function fetchResolution (
     case 'git':
       return await clone(resolution.repo, resolution.commit, target)
 
-    case 'directory': {
-      const tgzFilename = await npmPack(resolution.directory)
-      const tarball = path.resolve(resolution.directory, tgzFilename)
-      const dist = {tarball: tarball}
-      const index = await fetchFromLocalTarball(target, dist)
-      await fs.unlink(dist.tarball)
-      return index
+    default: {
+      throw new Error(`Fetching for dependency type "${resolution.type}" is not supported`)
     }
   }
 }
 
-function npmPack(dependencyPath: string): Promise<string> {
-  return new Promise((resolve, reject) => {
-    const proc = spawn('npm', ['pack'], {
-      cwd: dependencyPath
-    })
-
-    let stdout = ''
-
-    proc.stdout.on('data', (data: Object) => {
-      stdout += data.toString()
-    })
-
-    proc.on('error', reject)
-
-    proc.on('close', (code: number) => {
-      if (code > 0) return reject(new Error('Exit code ' + code))
-      const tgzFilename = stdout.trim()
-      return resolve(tgzFilename)
-    })
-  })
-}
-
 /**
  * clone a git repository.
  */

src/install/installMultiple.ts

@@ -32,7 +32,6 @@ export type PkgAddress = {
 export type InstalledPackage = {
   id: string,
   resolution: Resolution,
-  srcPath?: string,
   dev: boolean,
   optional: boolean,
   fetchingFiles: Promise<PackageContentInfo>,
@@ -53,7 +52,6 @@ export default async function installMultiple (
   options: {
     force: boolean,
     prefix: string,
-    referencedFrom: string,
     storePath: string,
     registry: string,
     metaCache: Map<string, PackageMeta>,
@@ -126,7 +124,6 @@ async function install (
   options: {
     force: boolean,
     prefix: string,
-    referencedFrom: string,
     storePath: string,
     registry: string,
     metaCache: Map<string, PackageMeta>,
@@ -182,6 +179,24 @@ async function install (
     offline: options.offline,
   })
 
+  if (fetchedPkg.isLink) {
+    if (options.currentDepth > 0) {
+      logger.warn(`Ignoring file dependency because it is not a root dependency ${spec}`)
+    } else {
+      ctx.linkedPkgs.push({
+        id: fetchedPkg.id,
+        specRaw: spec.raw,
+        name: fetchedPkg.pkg.name,
+        version: fetchedPkg.pkg.version,
+        dev: spec.dev,
+        optional: spec.optional,
+        resolution: fetchedPkg.resolution,
+      })
+    }
+    logStatus({status: 'downloaded_manifest', pkgId: fetchedPkg.id, pkgVersion: fetchedPkg.pkg.version})
+    return null
+  }
+
   if (options.parentNodeId.indexOf(`:${dependentId}:${fetchedPkg.id}:`) !== -1) {
     return null
   }
@@ -215,7 +230,6 @@ async function install (
     ctx.installs[fetchedPkg.id] = {
       id: fetchedPkg.id,
       resolution: fetchedPkg.resolution,
-      srcPath: fetchedPkg.srcPath,
       optional: spec.optional,
       name: pkg.name,
       version: pkg.version,
@@ -235,7 +249,6 @@ async function install (
       fetchedPkg.id,
       ctx,
       Object.assign({}, options, {
-        referencedFrom: fetchedPkg.srcPath,
         parentIsInstallable: installable,
         currentDepth: options.currentDepth + 1,
         parentNodeId: nodeId,
@@ -286,7 +299,6 @@ async function installDependencies (
   opts: {
     force: boolean,
     prefix: string,
-    referencedFrom: string,
     storePath: string,
     registry: string,
     metaCache: Map<string, PackageMeta>,
@@ -314,7 +326,7 @@ async function installDependencies (
   const deps = depsToSpecs(
     filterDeps(Object.assign({}, pkg.optionalDependencies, pkg.dependencies)),
     {
-      where: opts.referencedFrom,
+      where: opts.prefix,
       devDependencies: pkg.devDependencies || {},
       optionalDependencies: pkg.optionalDependencies || {},
     }

src/resolve/local.ts

@@ -35,5 +35,6 @@ export default async function resolveLocal (spec: PackageSpec, opts: ResolveOpti
   return {
     id,
     resolution,
+    package: localPkg,
   }
 }

test/install/local.ts

@@ -31,6 +31,10 @@ test('local file', async function (t: tape.Test) {
 
   await installPkgs(['file:../local-pkg'], testDefaults())
 
+  const pkgJson = await readPkg()
+  const expectedSpecs = {'local-pkg': `file:..${path.sep}local-pkg`}
+  t.deepEqual(pkgJson.dependencies, expectedSpecs, 'local-pkg has been added to dependencies')
+
   const m = project.requireModule('local-pkg')
 
   t.ok(m, 'localPkg() is available')
@@ -38,20 +42,11 @@ test('local file', async function (t: tape.Test) {
   const shr = await project.loadShrinkwrap()
 
   t.deepEqual(shr, {
-    specifiers: {
-      'local-pkg': `file:..${path.sep}local-pkg`,
-    },
+    specifiers: expectedSpecs,
     dependencies: {
       'local-pkg': 'file:../local-pkg',
     },
-    packages: {
-      'file:../local-pkg': {
-        resolution: {
-          directory: '../local-pkg',
-          type: 'directory',
-        },
-      },
-    },
+    packages: {},
     registry: 'http://localhost:4873/',
     version: 3,
   })
@@ -66,17 +61,6 @@ test('package with a broken symlink', async function (t) {
   t.ok(m, 'has-broken-symlink is available')
 })
 
-test('nested local dependency of a local dependency', async function (t: tape.Test) {
-  const project = prepare(t)
-  await installPkgs([local('pkg-with-local-dep')], testDefaults())
-
-  const m = project.requireModule('pkg-with-local-dep')
-
-  t.ok(m, 'pkgWithLocalDep() is available')
-
-  t.equal(m(), 'local-pkg', 'pkgWithLocalDep() returns data from local-pkg')
-})
-
 test('tarball local package', async function (t) {
   const project = prepare(t)
   await installPkgs([pathToLocalPkg('tar-pkg/tar-pkg-1.0.0.tgz')], testDefaults())

test/packages/pkg-with-local-dep/index.js

@@ -1,4 +0,0 @@
-'use strict'
-const localPkg = require('local-pkg')
-
-module.exports = () => localPkg()

test/packages/pkg-with-local-dep/package.json

@@ -1,7 +0,0 @@
-{
-  "name": "pkg-with-local-dep",
-  "version": "1.0.0",
-  "dependencies": {
-    "local-pkg": "file:../local-pkg"
-  }
-}