Skip to content

Commit

Permalink
--link-duplicates flag. Creates hardlinks to duplicate files in node_…
Browse files Browse the repository at this point in the history
…modules (yarnpkg#2620)

* Feature: duplicated dependencies are hardlinked from the first copy

* cleanup and tests

* made linking non default

* added fallback if links are not supported

* lint fix

* nits

* nits
  • Loading branch information
bestander authored and mnsn committed Feb 15, 2017
1 parent d13d12f commit 758475d
Show file tree
Hide file tree
Showing 11 changed files with 400 additions and 48 deletions.
36 changes: 36 additions & 0 deletions __tests__/commands/install/integration-deduping.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
/* @flow */

import {getPackageVersion, runInstall} from '../_helpers.js';
import * as fs from '../../../src/util/fs.js';

const assert = require('assert');
const path = require('path');

jasmine.DEFAULT_TIMEOUT_INTERVAL = 120000;

Expand Down Expand Up @@ -201,3 +203,37 @@ test.concurrent('install should dedupe dependencies avoiding conflicts 9', (): P
assert.equal(await getPackageVersion(config, 'run-async'), '0.1.0');
});
});

test.concurrent('install should hardlink repeated dependencies', (): Promise<void> => {
// A@1
// B@1 -> A@2
// C@1 -> A@2 (this is hardlink to B@1->A@2)
return runInstall({linkDuplicates: true}, 'hardlink-repeated-dependencies', async (config) => {
const b_a = await fs.stat(path.join(
config.cwd,
'node_modules/b/node_modules/a/package.json',
));
const c_a = await fs.stat(path.join(
config.cwd,
'node_modules/c/node_modules/a/package.json',
));
assert.equal(b_a.ino, c_a.ino);
});
});

test.concurrent('install should not hardlink repeated dependencies if linkDuplicates=false', (): Promise<void> => {
// A@1
// B@1 -> A@2
// C@1 -> A@2
return runInstall({linkDuplicates: false}, 'hardlink-repeated-dependencies', async (config) => {
const b_a = await fs.stat(path.join(
config.cwd,
'node_modules/b/node_modules/a/package.json',
));
const c_a = await fs.stat(path.join(
config.cwd,
'node_modules/c/node_modules/a/package.json',
));
assert(b_a.ino != c_a.ino);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "a",
"version": "1.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "a",
"version": "2.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "b",
"version": "1.0.0",
"dependencies": {
"a": "file:../a-2"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "c",
"version": "1.0.0",
"dependencies": {
"a": "file:../a-2"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"dependencies": {
"a": "file:deps/a-1",
"b": "file:deps/b-1",
"c": "file:deps/c-1"
}
}
6 changes: 4 additions & 2 deletions src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ export type IntegrityMatch = {

type Flags = {
// install
har: boolean,
ignorePlatform: boolean,
ignoreEngines: boolean,
ignoreScripts: boolean,
ignoreOptional: boolean,
har: boolean,
linkDuplicates: boolean,
force: boolean,
flat: boolean,
lockfile: boolean,
Expand Down Expand Up @@ -128,6 +129,7 @@ function normalizeFlags(config: Config, rawFlags: Object): Flags {
pureLockfile: !!rawFlags.pureLockfile,
skipIntegrity: !!rawFlags.skipIntegrity,
frozenLockfile: !!rawFlags.frozenLockfile,
linkDuplicates: !!rawFlags.linkDuplicates,

// add
peer: !!rawFlags.peer,
Expand Down Expand Up @@ -396,7 +398,7 @@ export class Install {
const loc = await this.getIntegrityHashLocation();
await fs.unlink(loc);
this.reporter.step(curr, total, this.reporter.lang('linkingDependencies'), emoji.get('link'));
await this.linker.init(patterns);
await this.linker.init(patterns, this.flags.linkDuplicates);
});

steps.push(async (curr: number, total: number) => {
Expand Down
1 change: 1 addition & 0 deletions src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ commander.option('--prod, --production [prod]', '');
commander.option('--no-lockfile', "don't read or generate a lockfile");
commander.option('--pure-lockfile', "don't generate a lockfile");
commander.option('--frozen-lockfile', "don't generate a lockfile and fail if an update is needed");
commander.option('--link-duplicates', 'create hardlinks to the repeated modules in node_modules');
commander.option('--global-folder <path>', '');
commander.option(
'--modules-folder <path>',
Expand Down
79 changes: 60 additions & 19 deletions src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export default class PackageLinker {
return Promise.resolve(hoister.init());
}

async copyModules(patterns: Array<string>): Promise<void> {
async copyModules(patterns: Array<string>, linkDuplicates: boolean): Promise<void> {
let flatTree = await this.getFlatHoistedTree(patterns);

// sorted tree makes file creation and copying not to interfere with each other
Expand All @@ -121,10 +121,13 @@ export default class PackageLinker {
});

// list of artifacts in modules to remove from extraneous removal
const phantomFiles = [];
const artifactFiles = [];

//
const queue: Map<string, CopyQueueItem> = new Map();
const copyQueue: Map<string, CopyQueueItem> = new Map();
const hardlinkQueue: Map<string, CopyQueueItem> = new Map();
const hardlinksEnabled = linkDuplicates && await fs.hardlinksWork(this.config.cwd);

const copiedSrcs: Map<string, string> = new Map();
for (const [dest, {pkg, loc: src}] of flatTree) {
const ref = pkg._reference;
invariant(ref, 'expected package reference');
Expand All @@ -134,18 +137,34 @@ export default class PackageLinker {
// extraneous
const metadata = await this.config.readPackageMetadata(src);
for (const file of metadata.artifacts) {
phantomFiles.push(path.join(dest, file));
artifactFiles.push(path.join(dest, file));
}

queue.set(dest, {
src,
dest,
onFresh() {
if (ref) {
ref.setFresh(true);
}
},
});
const copiedDest = copiedSrcs.get(src);
if (!copiedDest) {
if (hardlinksEnabled) {
copiedSrcs.set(src, dest);
}
copyQueue.set(dest, {
src,
dest,
onFresh() {
if (ref) {
ref.setFresh(true);
}
},
});
} else {
hardlinkQueue.set(dest, {
src: copiedDest,
dest,
onFresh() {
if (ref) {
ref.setFresh(true);
}
},
});
}
}

// keep track of all scoped paths to remove empty scopes after copy
Expand Down Expand Up @@ -179,15 +198,15 @@ export default class PackageLinker {
const stat = await fs.lstat(loc);
if (stat.isSymbolicLink()) {
possibleExtraneous.delete(loc);
queue.delete(loc);
copyQueue.delete(loc);
}
}

//
let tick;
await fs.copyBulk(Array.from(queue.values()), this.reporter, {
await fs.copyBulk(Array.from(copyQueue.values()), this.reporter, {
possibleExtraneous,
phantomFiles,
artifactFiles,

ignoreBasenames: [
constants.METADATA_FILENAME,
Expand All @@ -204,6 +223,28 @@ export default class PackageLinker {
}
},
});
await fs.hardlinkBulk(Array.from(hardlinkQueue.values()), this.reporter, {
possibleExtraneous,
artifactFiles,

onStart: (num: number) => {
tick = this.reporter.progress(num);
},

onProgress(src: string) {
if (tick) {
tick(src);
}
},
});

// remove all extraneous files that weren't in the tree
for (const loc of possibleExtraneous) {
this.reporter.verbose(
this.reporter.lang('verboseFileRemoveExtraneous', loc),
);
await fs.unlink(loc);
}

// remove any empty scoped directories
for (const scopedPath of scopedPaths) {
Expand Down Expand Up @@ -287,9 +328,9 @@ export default class PackageLinker {
return range === '*' || semver.satisfies(version, range, this.config.looseSemver);
}

async init(patterns: Array<string>): Promise<void> {
async init(patterns: Array<string>, linkDuplicates: boolean): Promise<void> {
this.resolvePeerModules();
await this.copyModules(patterns);
await this.copyModules(patterns, linkDuplicates);
await this.saveAll(patterns);
}

Expand Down
2 changes: 2 additions & 0 deletions src/reporters/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ const messages = {
manifestDirectoryNotFound: 'Unable to read $0 directory of module $1',

verboseFileCopy: 'Copying $0 to $1.',
verboseFileLink: 'Creating hardlink at $0 to $1.',
verboseFileSymlink: 'Creating symlink at $0 to $1.',
verboseFileSkip: 'Skipping copying of file $0 as the file at $1 is the same size ($2) and mtime ($3).',
verboseFileSkipSymlink: 'Skipping copying of $0 as the file at $1 is the same symlink ($2).',
verboseFileSkipHardlink: 'Skipping copying of $0 as the file at $1 is the same hardlink ($2).',
verboseFileRemoveExtraneous: 'Removing extraneous file $0.',
verboseFilePhantomExtraneous: "File $0 would be marked as extraneous but has been removed as it's listed as a phantom file.",
verboseFileFolder: 'Creating directory $0.',
Expand Down
Loading

0 comments on commit 758475d

Please sign in to comment.