Skip to content

Commit

Permalink
fixes for Mael
Browse files Browse the repository at this point in the history
  • Loading branch information
bestander committed Jun 24, 2017
1 parent 4d71984 commit af333aa
Show file tree
Hide file tree
Showing 20 changed files with 145 additions and 9 deletions.
48 changes: 44 additions & 4 deletions __tests__/commands/install/integration-deduping.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ test.concurrent('install should not hardlink repeated dependencies if linkDuplic
});
});

test.concurrent('install should not copy node_modules when hardlinking', (): Promise<void> => {
test.concurrent('install should not crash when hardlinking deep structures', (): Promise<void> => {
// https://github.com/yarnpkg/yarn/issues/2734
// A@1 -> B@1 -> C@1
// -> C@2
Expand All @@ -239,13 +239,13 @@ test.concurrent('install should not copy node_modules when hardlinking', (): Pro
});
});

test.concurrent('install should not hardlink full package structure', (): Promise<void> => {
test.concurrent('install should consider different hoisting with --link-duplicate', (): Promise<void> => {
// https://github.com/yarnpkg/yarn/issues/2734
// A@1 -> B@1 -> C@1
// -> C@2
// B@2
// C@3
// D@1 -> B@1 (hardlink)
// D@1 -> B@1 (hardlink) -> *C@1* (redundant)
// -> C@1 (hardlink)
return runInstall({linkDuplicates: true}, 'hardlink-collision-2', async config => {
let a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/package.json'));
Expand All @@ -254,6 +254,46 @@ test.concurrent('install should not hardlink full package structure', (): Promis
a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/node_modules/c/package.json'));
d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/c/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
expect(await fs.exists(path.join(config.cwd, 'node_modules/d/node_modules/b/node_modules/c/package.json'))).toBe(false);
// this is redundant but we are ok with it
expect(await fs.exists(path.join(config.cwd, 'node_modules/d/node_modules/b/node_modules/c/package.json'))).toBe(true);
});
});

test.concurrent('install should consider different hoisting with --link-duplicate 2', (): Promise<void> => {
// https://github.com/yarnpkg/yarn/issues/2734
// A@1 -> B@1
// -> C@1
// B@2
// C@3
// D@1 -> B@1 (hardlink) -> C@1 (hardlink)
// -> C@2
return runInstall({linkDuplicates: true}, 'hardlink-collision-3', async config => {
let a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/package.json'));
let d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/b/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/c/package.json'));
d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/b/node_modules/c/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
});
});

test.concurrent('install should not hardlink full package structure', (): Promise<void> => {
// https://github.com/yarnpkg/yarn/issues/2734
// A@1 -> B@1 -> C@1 -> (bundle leftpad)
// -> C@2
// B@2
// C@3
// D@1 -> B@1 (hardlink) -> C@1 (hardlink) -> (bundle leftpad) (hardlink)
// -> C@2
return runInstall({linkDuplicates: true}, 'hardlink-collision-with-bundled', async config => {
let a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/package.json'));
let d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/b/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/node_modules/c/package.json'));
d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/b/node_modules/c/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
a_1 = await fs.stat(path.join(config.cwd, 'node_modules/a/node_modules/b/node_modules/c/node_modules/left-pad/package.json'));
d_1 = await fs.stat(path.join(config.cwd, 'node_modules/d/node_modules/b/node_modules/c/node_modules/left-pad/package.json'));
expect(a_1.ino).toEqual(d_1.ino);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "1.0.0",
"dependencies": {
"b": "file:../b-1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "b",
"version": "1.0.0",
"dependencies": {
"c": "file:../c-1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "b",
"version": "2.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "c",
"version": "1.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "c",
"version": "2.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "c",
"version": "3.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "a",
"version": "2.0.0",
"dependencies": {
"b": "file:../b-1",
"c": "file:../c-2"
}
}
8 changes: 8 additions & 0 deletions __tests__/fixtures/install/hardlink-collision-3/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"dependencies": {
"a": "file:deps/a-1",
"b": "file:deps/b-2",
"c": "file:deps/c-3",
"d": "file:deps/d-1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "a",
"version": "1.0.0",
"dependencies": {
"b": "file:../b-1",
"c": "file:../c-2"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "b",
"version": "1.0.0",
"dependencies": {
"c": "file:../c-1/c-1.0.0.tgz"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "b",
"version": "2.0.0"
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "c",
"version": "2.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "c",
"version": "3.0.0"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "a",
"version": "2.0.0",
"dependencies": {
"b": "file:../b-1",
"c": "file:../c-2"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"dependencies": {
"a": "file:deps/a-1",
"b": "file:deps/b-2",
"c": "file:deps/c-3",
"d": "file:deps/d-1"
}
}
8 changes: 6 additions & 2 deletions src/fetchers/tarball-fetcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const url = require('url');
const fs = require('fs');
const stream = require('stream');
const gunzip = require('gunzip-maybe');
import {removePrefix} from '../util/misc.js';

export default class TarballFetcher extends BaseFetcher {
async setupMirrorFromCache(): Promise<?string> {
Expand Down Expand Up @@ -174,13 +175,16 @@ export default class TarballFetcher extends BaseFetcher {
}

async _fetch(): Promise<FetchedOverride> {
const isFilePath = this.reference.startsWith('file:');
this.reference = removePrefix(this.reference, 'file:');
const urlParse = url.parse(this.reference);

const isFilePath = urlParse.protocol
// legacy support for local paths in yarn.lock entries
const isRelativePath = urlParse.protocol
? urlParse.protocol.match(/^[a-z]:$/i)
: urlParse.pathname ? urlParse.pathname.match(/^(?:\.{1,2})?[\\\/]/) : false;

if (isFilePath) {
if (isFilePath || isRelativePath) {
return this.fetchFromLocal(this.reference);
}

Expand Down
3 changes: 1 addition & 2 deletions src/resolvers/exotics/tarball-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type PackageRequest from '../../package-request.js';
import TarballFetcher from '../../fetchers/tarball-fetcher.js';
import ExoticResolver from './exotic-resolver.js';
import Git from './git-resolver.js';
import {removePrefix} from '../../util/misc.js';
import guessName from '../../util/guess-name.js';
import * as versionUtil from '../../util/version.js';
import * as crypto from '../../util/crypto.js';
Expand Down Expand Up @@ -52,7 +51,7 @@ export default class TarballResolver extends ExoticResolver {
return shrunk;
}

const url = removePrefix(this.url, 'file:');
const url = this.url;
let {hash, registry} = this;
let pkgJson;

Expand Down
6 changes: 5 additions & 1 deletion src/util/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,11 @@ async function buildActionsForHardlink(
const {src, dest} = data;
const onFresh = data.onFresh || noop;
const onDone = data.onDone || noop;
if (files.has(dest)) {
// don't hardlink a file twice
onDone();
return;
}
files.add(dest);

if (events.ignoreBasenames.indexOf(path.basename(src)) >= 0) {
Expand Down Expand Up @@ -450,7 +455,6 @@ async function buildActionsForHardlink(

// push all files to queue
invariant(srcFiles, 'src files not initialised');
srcFiles = srcFiles.filter(f => f !== 'node_modules');
let remaining = srcFiles.length;
if (!remaining) {
onDone();
Expand Down

0 comments on commit af333aa

Please sign in to comment.