fixes hardlink collitions (#3691)

* fixed #2734

* added another test

* fixes for Mael

* prettier

* added comment

__tests__/commands/install/integration-deduping.js

@@ -220,3 +220,86 @@ test.concurrent('install should not hardlink repeated dependencies if linkDuplic
     expect(b_a.ino).not.toEqual(c_a.ino);
   });
 });
+
+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
+  // B@2
+  // C@3
+  // D@1 -> B@1 (hardlink) -> C@1 (hardlink)
+  //     -> C@2
+  return runInstall({linkDuplicates: true}, 'hardlink-collision', 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);
+  });
+});
+
+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) -> *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'));
+    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/c/package.json'));
+    expect(a_1.ino).toEqual(d_1.ino);
+    // 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);
+  });
+});

__tests__/fixtures/install/hardlink-collision-2/deps/a-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision-2/deps/b-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "file:../c-1"
+  }
+}

__tests__/fixtures/install/hardlink-collision-2/deps/b-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "b",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-2/deps/c-1/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "1.0.0"
+}

__tests__/fixtures/install/hardlink-collision-2/deps/c-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-2/deps/c-3/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "3.0.0"
+}

__tests__/fixtures/install/hardlink-collision-2/deps/d-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "a",
+  "version": "2.0.0",
+  "dependencies": {
+    "b": "file:../b-1"
+  }
+}

__tests__/fixtures/install/hardlink-collision-2/package.json

@@ -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"
+  }
+}

__tests__/fixtures/install/hardlink-collision-3/deps/a-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "file:../b-1"
+  }
+}

__tests__/fixtures/install/hardlink-collision-3/deps/b-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "file:../c-1"
+  }
+}

__tests__/fixtures/install/hardlink-collision-3/deps/b-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "b",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-3/deps/c-1/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "1.0.0"
+}

__tests__/fixtures/install/hardlink-collision-3/deps/c-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-3/deps/c-3/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "3.0.0"
+}

__tests__/fixtures/install/hardlink-collision-3/deps/d-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "2.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision-3/package.json

@@ -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"
+  }
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/a-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/b-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "file:../c-1/c-1.0.0.tgz"
+  }
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/b-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "b",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/c-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/c-3/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "3.0.0"
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/deps/d-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "2.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision-with-bundled/package.json

@@ -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"
+  }
+}

__tests__/fixtures/install/hardlink-collision/deps/a-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "1.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision/deps/b-1/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "b",
+  "version": "1.0.0",
+  "dependencies": {
+    "c": "file:../c-1"
+  }
+}

__tests__/fixtures/install/hardlink-collision/deps/b-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "b",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision/deps/c-1/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "1.0.0"
+}

__tests__/fixtures/install/hardlink-collision/deps/c-2/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "2.0.0"
+}

__tests__/fixtures/install/hardlink-collision/deps/c-3/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "c",
+  "version": "3.0.0"
+}

__tests__/fixtures/install/hardlink-collision/deps/d-1/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "a",
+  "version": "2.0.0",
+  "dependencies": {
+    "b": "file:../b-1",
+    "c": "file:../c-2"
+  }
+}

__tests__/fixtures/install/hardlink-collision/package.json

@@ -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"
+  }
+}

src/util/fs.js

@@ -361,6 +361,15 @@ async function buildActionsForHardlink(
     const {src, dest} = data;
     const onFresh = data.onFresh || noop;
     const onDone = data.onDone || noop;
+    if (files.has(dest)) {
+      // Fixes issue https://github.com/yarnpkg/yarn/issues/2734
+      // When bulk hardlinking we have A -> B structure that we want to hardlink to A1 -> B1,
+      // package-linker passes that modules A1 and B1 need to be hardlinked,
+      // the recursive linking algorithm of A1 ends up scheduling files in B1 to be linked twice which will case
+      // an exception.
+      onDone();
+      return;
+    }
     files.add(dest);
 
     if (events.ignoreBasenames.indexOf(path.basename(src)) >= 0) {