From 175f50d01edb595f7d41eafb8950b59564e4c706 Mon Sep 17 00:00:00 2001 From: zkochan Date: Sat, 17 Jun 2017 17:54:17 +0300 Subject: [PATCH] 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 | 45 ++++++++++++++++--- src/api/link.ts | 6 ++- src/install/fetch.ts | 25 +++++++++-- src/install/fetchResolution.ts | 32 +------------ src/install/installMultiple.ts | 26 ++++++++--- src/resolve/local.ts | 1 + test/install/local.ts | 28 +++--------- test/packages/pkg-with-local-dep/index.js | 4 -- test/packages/pkg-with-local-dep/package.json | 7 --- 9 files changed, 91 insertions(+), 83 deletions(-) delete mode 100644 test/packages/pkg-with-local-dep/index.js delete mode 100644 test/packages/pkg-with-local-dep/package.json diff --git a/src/api/install.ts b/src/api/install.ts index e0e4025d95..095abe105e 100644 --- a/src/api/install.ts +++ b/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(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, - pkgs.map(dep => { // tslint:disable-line + 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(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): Promise { return { installs: {}, + linkedPkgs: [], childrenIdsByParentId: {}, nodesToBuild: [], shrinkwrap, diff --git a/src/api/link.ts b/src/api/link.ts index f4b3d0b69f..a0070a82a0 100644 --- a/src/api/link.ts +++ b/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) diff --git a/src/install/fetch.ts b/src/install/fetch.ts index ec877876cc..059c23d324 100644 --- a/src/install/fetch.ts +++ b/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, fetchingFiles: Promise, calculatingIntegrity: Promise, 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 = 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}) diff --git a/src/install/fetchResolution.ts b/src/install/fetchResolution.ts index 4072bdbce9..dc6c6009b2 100644 --- a/src/install/fetchResolution.ts +++ b/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 { - 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. */ diff --git a/src/install/installMultiple.ts b/src/install/installMultiple.ts index 5dab6de521..2ec4a33a0c 100644 --- a/src/install/installMultiple.ts +++ b/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, @@ -53,7 +52,6 @@ export default async function installMultiple ( options: { force: boolean, prefix: string, - referencedFrom: string, storePath: string, registry: string, metaCache: Map, @@ -126,7 +124,6 @@ async function install ( options: { force: boolean, prefix: string, - referencedFrom: string, storePath: string, registry: string, metaCache: Map, @@ -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, @@ -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 || {}, } diff --git a/src/resolve/local.ts b/src/resolve/local.ts index 8963a2cac4..f04db615a4 100644 --- a/src/resolve/local.ts +++ b/src/resolve/local.ts @@ -35,5 +35,6 @@ export default async function resolveLocal (spec: PackageSpec, opts: ResolveOpti return { id, resolution, + package: localPkg, } } diff --git a/test/install/local.ts b/test/install/local.ts index 02b099284d..4ed8eba3bc 100644 --- a/test/install/local.ts +++ b/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()) diff --git a/test/packages/pkg-with-local-dep/index.js b/test/packages/pkg-with-local-dep/index.js deleted file mode 100644 index 3159416ba8..0000000000 --- a/test/packages/pkg-with-local-dep/index.js +++ /dev/null @@ -1,4 +0,0 @@ -'use strict' -const localPkg = require('local-pkg') - -module.exports = () => localPkg() diff --git a/test/packages/pkg-with-local-dep/package.json b/test/packages/pkg-with-local-dep/package.json deleted file mode 100644 index 9997e7c0cc..0000000000 --- a/test/packages/pkg-with-local-dep/package.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "name": "pkg-with-local-dep", - "version": "1.0.0", - "dependencies": { - "local-pkg": "file:../local-pkg" - } -}