From 01aecf0386399adc6cea229a582acc1038ffdb49 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Mon, 26 Oct 2020 03:09:48 +0200 Subject: [PATCH] fix: don't copy file during linking if the target exists --- .changeset/three-grapes-begin.md | 5 +++++ .../src/storeController/createImportPackage.ts | 3 +++ .../package-store/test/createImportPackage.spec.ts | 11 +++++++++-- 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 .changeset/three-grapes-begin.md diff --git a/.changeset/three-grapes-begin.md b/.changeset/three-grapes-begin.md new file mode 100644 index 0000000000..5890238e78 --- /dev/null +++ b/.changeset/three-grapes-begin.md @@ -0,0 +1,5 @@ +--- +"@pnpm/package-store": patch +--- + +Do not try to copy a file during linking, if the target already exists. diff --git a/packages/package-store/src/storeController/createImportPackage.ts b/packages/package-store/src/storeController/createImportPackage.ts index 068449a215..c0db0cc979 100644 --- a/packages/package-store/src/storeController/createImportPackage.ts +++ b/packages/package-store/src/storeController/createImportPackage.ts @@ -120,6 +120,9 @@ async function linkOrCopy (existingPath: string, newPath: string) { try { await fs.link(existingPath, newPath) } catch (err) { + // If a hard link to the same file already exists + // then trying to copy it will make an empty file from it. + if (err['code'] === 'EEXIST') return // In some VERY rare cases (1 in a thousand), hard-link creation fails on Windows. // In that case, we just fall back to copying. // This issue is reproducible with "pnpm add @material-ui/icons@4.9.1" diff --git a/packages/package-store/test/createImportPackage.spec.ts b/packages/package-store/test/createImportPackage.spec.ts index e2b3a011bb..74c7284d3e 100644 --- a/packages/package-store/test/createImportPackage.spec.ts +++ b/packages/package-store/test/createImportPackage.spec.ts @@ -120,7 +120,12 @@ test('packageImportMethod=auto: chooses copying if cloning and hard linking is n test('packageImportMethod=hardlink: fall back to copying if hardlinking fails', async () => { const importPackage = createImportPackage('hardlink') - fsMock.link = jest.fn(() => { + fsMock.link = jest.fn((src: string, dest: string) => { + if (dest.endsWith('license')) { + const err = new Error('') + err['code'] = 'EEXIST' + throw err + } throw new Error('This file system does not support hard linking') }) fsMock.copyFile = jest.fn() @@ -128,11 +133,13 @@ test('packageImportMethod=hardlink: fall back to copying if hardlinking fails', filesMap: { 'index.js': 'hash2', 'package.json': 'hash1', + license: 'hash3', }, force: false, fromStore: false, })).toBe('hardlink') - expect(fsMock.link).toBeCalled() + expect(fsMock.link).toBeCalledTimes(3) + expect(fsMock.copyFile).toBeCalledTimes(2) // One time the target already exists, so it won't be copied expect(fsMock.copyFile).toBeCalledWith(path.join('hash1'), path.join('project', '_tmp', 'package.json')) expect(fsMock.copyFile).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js')) })