fix: retry copying from store on EBUSY error (#6207)

close #6201
This commit is contained in:
Zoltan Kochan
2023-03-11 17:09:34 +02:00
committed by GitHub
parent 272de3e227
commit 955874422c
8 changed files with 82 additions and 63 deletions

View File

@@ -0,0 +1,5 @@
---
"@pnpm/graceful-fs": minor
---
Add copyFile, link, stat.

View File

@@ -0,0 +1,6 @@
---
"@pnpm/fs.indexed-pkg-importer": patch
"pnpm": patch
---
Retry copying file on EBUSY error [#6201](https://github.com/pnpm/pnpm/issues/6201).

View File

@@ -2,7 +2,10 @@ import { promisify } from 'util'
import gfs from 'graceful-fs'
export default { // eslint-disable-line
copyFile: promisify(gfs.copyFile),
createReadStream: gfs.createReadStream,
link: promisify(gfs.link),
readFile: promisify(gfs.readFile),
stat: promisify(gfs.stat),
writeFile: promisify(gfs.writeFile),
}

View File

@@ -16,6 +16,7 @@
},
"dependencies": {
"@pnpm/core-loggers": "workspace:*",
"@pnpm/graceful-fs": "workspace:*",
"@pnpm/store-controller-types": "workspace:*",
"@zkochan/rimraf": "^2.1.2",
"fs-extra": "^11.1.0",

View File

@@ -1,4 +1,5 @@
import { constants, promises as fs, Stats } from 'fs'
import { constants, Stats } from 'fs'
import fs from '@pnpm/graceful-fs'
import path from 'path'
import { globalInfo, globalWarn } from '@pnpm/logger'
import { packageImportMethodLogger } from '@pnpm/core-loggers'

View File

@@ -1,31 +1,43 @@
import fs from 'fs'
import path from 'path'
import { createIndexedPkgImporter } from '@pnpm/fs.indexed-pkg-importer'
import gfs from '@pnpm/graceful-fs'
import { globalInfo } from '@pnpm/logger'
const fsMock = { promises: {} as any } as any // eslint-disable-line
jest.mock('fs', () => {
const { access, constants, promises } = jest.requireActual('fs')
fsMock.constants = constants
fsMock.promises.mkdir = promises.mkdir
fsMock.promises.readdir = promises.readdir
fsMock.access = access
return fsMock
jest.mock('@pnpm/graceful-fs', () => {
const { access, promises } = jest.requireActual('fs')
const fsMock = {
mkdir: promises.mkdir,
readdir: promises.readdir,
access,
copyFile: jest.fn(),
link: jest.fn(),
stat: jest.fn(),
}
return {
__esModule: true,
default: fsMock,
}
})
jest.mock('path-temp', () => (dir: string) => path.join(dir, '_tmp'))
jest.mock('rename-overwrite', () => jest.fn())
jest.mock('fs-extra', () => ({
copy: jest.fn(),
}))
const globalInfo = jest.fn()
const globalWarn = jest.fn()
const logger = jest.fn(() => ({ debug: jest.fn() }))
jest.mock('@pnpm/logger', () => ({ logger, globalWarn, globalInfo }))
jest.mock('@pnpm/logger', () => ({
logger: jest.fn(() => ({ debug: jest.fn() })),
globalWarn: jest.fn(),
globalInfo: jest.fn(),
}))
// eslint-disable-next-line
import { createIndexedPkgImporter } from '@pnpm/fs.indexed-pkg-importer'
beforeEach(() => {
;(gfs.copyFile as jest.Mock).mockClear()
;(gfs.link as jest.Mock).mockClear()
;(globalInfo as jest.Mock).mockReset()
})
test('packageImportMethod=auto: clone files by default', async () => {
const importPackage = createIndexedPkgImporter('auto')
fsMock.promises.copyFile = jest.fn()
fsMock.promises.rename = jest.fn()
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -34,25 +46,23 @@ test('packageImportMethod=auto: clone files by default', async () => {
force: false,
fromStore: false,
})).toBe('clone')
expect(fsMock.promises.copyFile).toBeCalledWith(
expect(gfs.copyFile).toBeCalledWith(
path.join('hash1'),
path.join('project', '_tmp', 'package.json'),
fsMock.constants.COPYFILE_FICLONE_FORCE
fs.constants.COPYFILE_FICLONE_FORCE
)
expect(fsMock.promises.copyFile).toBeCalledWith(
expect(gfs.copyFile).toBeCalledWith(
path.join('hash2'),
path.join('project', '_tmp', 'index.js'),
fsMock.constants.COPYFILE_FICLONE_FORCE
fs.constants.COPYFILE_FICLONE_FORCE
)
})
test('packageImportMethod=auto: link files if cloning fails', async () => {
const importPackage = createIndexedPkgImporter('auto')
fsMock.promises.copyFile = jest.fn(() => {
;(gfs.copyFile as jest.Mock).mockImplementation(async () => {
throw new Error('This file system does not support cloning')
})
fsMock.promises.link = jest.fn()
fsMock.promises.rename = jest.fn()
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -61,10 +71,10 @@ test('packageImportMethod=auto: link files if cloning fails', async () => {
force: false,
fromStore: false,
})).toBe('hardlink')
expect(fsMock.promises.link).toBeCalledWith(path.join('hash1'), path.join('project', '_tmp', 'package.json'))
expect(fsMock.promises.link).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(fsMock.promises.copyFile).toBeCalled()
fsMock.promises.copyFile.mockClear()
expect(gfs.link).toBeCalledWith(path.join('hash1'), path.join('project', '_tmp', 'package.json'))
expect(gfs.link).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(gfs.copyFile).toBeCalled()
;(gfs.copyFile as jest.Mock).mockClear()
// The copy function will not be called again
expect(await importPackage('project2/package', {
@@ -75,24 +85,23 @@ test('packageImportMethod=auto: link files if cloning fails', async () => {
force: false,
fromStore: false,
})).toBe('hardlink')
expect(fsMock.promises.copyFile).not.toBeCalled()
expect(fsMock.promises.link).toBeCalledWith(path.join('hash1'), path.join('project2', '_tmp', 'package.json'))
expect(fsMock.promises.link).toBeCalledWith(path.join('hash2'), path.join('project2', '_tmp', 'index.js'))
expect(gfs.copyFile).not.toBeCalled()
expect(gfs.link).toBeCalledWith(path.join('hash1'), path.join('project2', '_tmp', 'package.json'))
expect(gfs.link).toBeCalledWith(path.join('hash2'), path.join('project2', '_tmp', 'index.js'))
})
test('packageImportMethod=auto: link files if cloning fails and even hard linking fails but not with EXDEV error', async () => {
const importPackage = createIndexedPkgImporter('auto')
fsMock.promises.copyFile = jest.fn(() => {
;(gfs.copyFile as jest.Mock).mockImplementation(async () => {
throw new Error('This file system does not support cloning')
})
let linkFirstCall = true
fsMock.promises.link = jest.fn(() => {
;(gfs.link as jest.Mock).mockImplementation(async () => {
if (linkFirstCall) {
linkFirstCall = false
throw new Error()
}
})
fsMock.promises.rename = jest.fn()
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -100,22 +109,21 @@ test('packageImportMethod=auto: link files if cloning fails and even hard linkin
force: false,
fromStore: false,
})).toBe('hardlink')
expect(fsMock.promises.link).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(fsMock.promises.link).toBeCalledTimes(2)
expect(fsMock.promises.copyFile).toBeCalledTimes(1)
expect(gfs.link).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(gfs.link).toBeCalledTimes(2)
expect(gfs.copyFile).toBeCalledTimes(1)
})
test('packageImportMethod=auto: chooses copying if cloning and hard linking is not possible', async () => {
const importPackage = createIndexedPkgImporter('auto')
fsMock.promises.copyFile = jest.fn((src: string, dest: string, flags?: number) => {
if (flags === fsMock.constants.COPYFILE_FICLONE_FORCE) {
;(gfs.copyFile as jest.Mock).mockImplementation(async (src: string, dest: string, flags?: number) => {
if (flags === fs.constants.COPYFILE_FICLONE_FORCE) {
throw new Error('This file system does not support cloning')
}
})
fsMock.promises.link = jest.fn(() => {
;(gfs.link as jest.Mock).mockImplementation(() => {
throw new Error('EXDEV: cross-device link not permitted')
})
fsMock.promises.rename = jest.fn()
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -123,19 +131,18 @@ test('packageImportMethod=auto: chooses copying if cloning and hard linking is n
force: false,
fromStore: false,
})).toBe('copy')
expect(fsMock.promises.copyFile).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(fsMock.promises.copyFile).toBeCalledTimes(2)
expect(gfs.copyFile).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(gfs.copyFile).toBeCalledTimes(2)
})
test('packageImportMethod=hardlink: fall back to copying if hardlinking fails', async () => {
const importPackage = createIndexedPkgImporter('hardlink')
fsMock.promises.link = jest.fn((src: string, dest: string) => {
;(gfs.link as jest.Mock).mockImplementation(async (src: string, dest: string) => {
if (dest.endsWith('license')) {
throw Object.assign(new Error(''), { code: 'EEXIST' })
}
throw new Error('This file system does not support hard linking')
})
fsMock.promises.copyFile = jest.fn()
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -145,17 +152,15 @@ test('packageImportMethod=hardlink: fall back to copying if hardlinking fails',
force: false,
fromStore: false,
})).toBe('hardlink')
expect(fsMock.promises.link).toBeCalledTimes(3)
expect(fsMock.promises.copyFile).toBeCalledTimes(2) // One time the target already exists, so it won't be copied
expect(fsMock.promises.copyFile).toBeCalledWith(path.join('hash1'), path.join('project', '_tmp', 'package.json'))
expect(fsMock.promises.copyFile).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
expect(gfs.link).toBeCalledTimes(3)
expect(gfs.copyFile).toBeCalledTimes(2) // One time the target already exists, so it won't be copied
expect(gfs.copyFile).toBeCalledWith(path.join('hash1'), path.join('project', '_tmp', 'package.json'))
expect(gfs.copyFile).toBeCalledWith(path.join('hash2'), path.join('project', '_tmp', 'index.js'))
})
test('packageImportMethod=hardlink does not relink package from store if package.json is linked from the store', async () => {
const importPackage = createIndexedPkgImporter('hardlink')
fsMock.promises.copyFile = jest.fn()
fsMock.promises.rename = jest.fn()
fsMock.promises.stat = jest.fn(() => ({ ino: 1 }))
;(gfs.stat as jest.Mock).mockReturnValue(Promise.resolve({ ino: 1 }))
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -167,12 +172,9 @@ test('packageImportMethod=hardlink does not relink package from store if package
})
test('packageImportMethod=hardlink relinks package from store if package.json is not linked from the store', async () => {
globalInfo.mockReset()
const importPackage = createIndexedPkgImporter('hardlink')
fsMock.promises.copyFile = jest.fn()
fsMock.promises.rename = jest.fn()
let ino = 0
fsMock.promises.stat = jest.fn(() => ({ ino: ++ino }))
;(gfs.stat as jest.Mock).mockImplementation(async () => ({ ino: ++ino }))
expect(await importPackage('project/package', {
filesMap: {
'index.js': 'hash2',
@@ -186,9 +188,7 @@ test('packageImportMethod=hardlink relinks package from store if package.json is
test('packageImportMethod=hardlink does not relink package from store if package.json is not present in the store', async () => {
const importPackage = createIndexedPkgImporter('hardlink')
fsMock.promises.copyFile = jest.fn()
fsMock.promises.rename = jest.fn()
fsMock.promises.stat = jest.fn((file) => {
;(gfs.stat as jest.Mock).mockImplementation(async (file) => {
expect(typeof file).toBe('string')
return { ino: 1 }
})
@@ -202,11 +202,8 @@ test('packageImportMethod=hardlink does not relink package from store if package
})
test('packageImportMethod=hardlink links packages when they are not found', async () => {
globalInfo.mockReset()
const importPackage = createIndexedPkgImporter('hardlink')
fsMock.promises.copyFile = jest.fn()
fsMock.promises.rename = jest.fn()
fsMock.promises.stat = jest.fn((file) => {
;(gfs.stat as jest.Mock).mockImplementation(async (file) => {
if (file === path.join('project/package', 'package.json')) {
throw Object.assign(new Error(), { code: 'ENOENT' })
}

View File

@@ -17,6 +17,9 @@
},
{
"path": "../../store/store-controller-types"
},
{
"path": "../graceful-fs"
}
],
"composite": true

3
pnpm-lock.yaml generated
View File

@@ -1498,6 +1498,9 @@ importers:
'@pnpm/core-loggers':
specifier: workspace:*
version: link:../../packages/core-loggers
'@pnpm/graceful-fs':
specifier: workspace:*
version: link:../graceful-fs
'@pnpm/logger':
specifier: ^5.0.0
version: 5.0.0