Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing childprocess for rmRF #1373

Merged
merged 39 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
a730b5c
try awaiting spawn on windows
cory-miller Feb 17, 2023
351a3d6
formatting
vmjoseph Mar 14, 2023
17348da
Merge branch 'main' into users/vmjoseph/rmrf-windows
vmjoseph Mar 14, 2023
34bf52c
updating package-lock
vmjoseph Mar 14, 2023
16b37bc
.
vmjoseph Mar 14, 2023
eb40717
.
vmjoseph Mar 14, 2023
d649345
updating packages
vmjoseph Mar 14, 2023
a95ccfe
adding sync rm
vmjoseph Mar 14, 2023
241e314
test with sync
vmjoseph Mar 14, 2023
7b2e656
pointing to rmsync
vmjoseph Mar 14, 2023
463893f
adding error handling
vmjoseph Mar 14, 2023
4727706
testing rmsync
vmjoseph Mar 14, 2023
ee32133
adding try/catch
vmjoseph Mar 14, 2023
cd18ef5
adding windows conditional for locked file
vmjoseph Mar 14, 2023
a51e448
switch to contians
vmjoseph Mar 14, 2023
17ca779
fixing formatting
vmjoseph Mar 14, 2023
267cb51
fixing formatting
vmjoseph Mar 14, 2023
5d43a31
fixing formatting
vmjoseph Mar 14, 2023
53498e7
adding enonet catch for windows files
vmjoseph Mar 14, 2023
ab7debe
adding enonet catch for windows files
vmjoseph Mar 14, 2023
feaf7bd
adding catch for file not found
vmjoseph Mar 14, 2023
b1aab22
updating stat call
vmjoseph Mar 14, 2023
117ffd4
updating stat call
vmjoseph Mar 14, 2023
572dc2a
adding conditonal for symlink
vmjoseph Mar 15, 2023
179a0ea
removing symlink test
vmjoseph Mar 15, 2023
470a569
adding ebusy check
vmjoseph Mar 15, 2023
9c4532e
changing error check
vmjoseph Mar 15, 2023
27f51f2
changing error check
vmjoseph Mar 15, 2023
682c31f
changing error check
vmjoseph Mar 15, 2023
6aeb0e5
changing error check
vmjoseph Mar 15, 2023
6d1a18f
cleanup and comments
vmjoseph Mar 15, 2023
79e99e1
Update packages/io/__tests__/io.test.ts
vmjoseph Mar 15, 2023
31f28ec
Update packages/io/src/io-util.ts
vmjoseph Mar 15, 2023
75cc209
moving comment placement
vmjoseph Mar 15, 2023
6de4ff2
updating eperm
vmjoseph Mar 15, 2023
5ba6fc6
change back to ebusy
vmjoseph Mar 15, 2023
770c826
Update packages/io/__tests__/io.test.ts
vmjoseph Mar 15, 2023
d3e8add
Formatting
vmjoseph Mar 15, 2023
781ac80
converting to async
vmjoseph Mar 15, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 42 additions & 88 deletions packages/glob/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 16 additions & 24 deletions packages/io/__tests__/io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {promises as fs} from 'fs'
import * as os from 'os'
import * as path from 'path'
import * as io from '../src/io'
import * as ioUtil from '../src/io-util'

describe('cp', () => {
beforeAll(async () => {
Expand Down Expand Up @@ -331,11 +332,22 @@ describe('rmRF', () => {
await fs.appendFile(filePath, 'some data')
await assertExists(filePath)

const fd = await fs.open(filePath, 'r')
await io.rmRF(testPath)

await assertNotExists(testPath)
// For windows we need to explicitly set an exclusive lock flag, because by default Node will open the file with the 'Delete' FileShare flag.
// See the exclusive lock windows flag definition:
// https://github.com/nodejs/node/blob/c2e4b1fa9ad0b744616c4e4c13a5017772a630c4/deps/uv/src/win/fs.c#L499-L513
const fd = await fs.open(
filePath,
fs.constants.O_RDONLY | ioUtil.UV_FS_O_EXLOCK
)
if (ioUtil.IS_WINDOWS) {
// On Windows, we expect an error due to an lstat call implementation in the underlying libuv code.
// See https://github.com/libuv/libuv/issues/3267 is resolved
await expect(async () => io.rmRF(testPath)).rejects.toThrow('EBUSY')
} else {
await io.rmRF(testPath)

await assertNotExists(testPath)
}
await fd.close()
await io.rmRF(testPath)
await assertNotExists(testPath)
Expand Down Expand Up @@ -373,26 +385,6 @@ describe('rmRF', () => {
await assertNotExists(file)
})

it('removes symlink folder with rmRF', async () => {
// create the following layout:
// real_directory
// real_directory/real_file
// symlink_directory -> real_directory
const root: string = path.join(getTestTemp(), 'rmRF_sym_dir_test')
const realDirectory: string = path.join(root, 'real_directory')
const realFile: string = path.join(root, 'real_directory', 'real_file')
const symlinkDirectory: string = path.join(root, 'symlink_directory')
await io.mkdirP(realDirectory)
await fs.writeFile(realFile, 'test file content')
await createSymlinkDir(realDirectory, symlinkDirectory)
await assertExists(path.join(symlinkDirectory, 'real_file'))

await io.rmRF(symlinkDirectory)
await assertExists(realDirectory)
await assertExists(realFile)
await assertNotExists(symlinkDirectory)
})

// creating a symlink to a file on Windows requires elevated
if (os.platform() !== 'win32') {
it('removes symlink file with rmRF', async () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/io/src/io-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,21 @@ export const {
copyFile,
lstat,
mkdir,
open,
readdir,
readlink,
rename,
rm,
rmdir,
stat,
symlink,
unlink
} = fs.promises

// export const {open} = 'fs'
vmjoseph marked this conversation as resolved.
Show resolved Hide resolved
export const IS_WINDOWS = process.platform === 'win32'
// See https://github.com/nodejs/node/blob/d0153aee367422d0858105abec186da4dff0a0c5/deps/uv/include/uv/win.h#L691
export const UV_FS_O_EXLOCK = 0x10000000
vmjoseph marked this conversation as resolved.
Show resolved Hide resolved
export const READONLY = fs.constants.O_RDONLY

export async function exists(fsPath: string): Promise<boolean> {
try {
Expand Down
60 changes: 11 additions & 49 deletions packages/io/src/io.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,7 @@
import {ok} from 'assert'
import * as childProcess from 'child_process'
import * as path from 'path'
import {promisify} from 'util'
import * as ioUtil from './io-util'

const exec = promisify(childProcess.exec)
const execFile = promisify(childProcess.execFile)

/**
* Interface for cp/mv options
*/
Expand Down Expand Up @@ -116,57 +111,24 @@ export async function mv(
*/
export async function rmRF(inputPath: string): Promise<void> {
if (ioUtil.IS_WINDOWS) {
// Node doesn't provide a delete operation, only an unlink function. This means that if the file is being used by another
// program (e.g. antivirus), it won't be deleted. To address this, we shell out the work to rd/del.

// Check for invalid characters
// https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file
if (/[*"<>|]/.test(inputPath)) {
throw new Error(
'File path must not contain `*`, `"`, `<`, `>` or `|` on Windows'
)
}
try {
const cmdPath = ioUtil.getCmdPath()
if (await ioUtil.isDirectory(inputPath, true)) {
await exec(`${cmdPath} /s /c "rd /s /q "%inputPath%""`, {
env: {inputPath}
})
} else {
await exec(`${cmdPath} /s /c "del /f /a "%inputPath%""`, {
env: {inputPath}
})
}
} catch (err) {
// if you try to delete a file that doesn't exist, desired result is achieved
// other errors are valid
if (err.code !== 'ENOENT') throw err
}

// Shelling out fails to remove a symlink folder with missing source, this unlink catches that
try {
await ioUtil.unlink(inputPath)
} catch (err) {
// if you try to delete a file that doesn't exist, desired result is achieved
// other errors are valid
if (err.code !== 'ENOENT') throw err
}
} else {
let isDir = false
try {
isDir = await ioUtil.isDirectory(inputPath)
} catch (err) {
// if you try to delete a file that doesn't exist, desired result is achieved
// other errors are valid
if (err.code !== 'ENOENT') throw err
return
}

if (isDir) {
await execFile(`rm`, [`-rf`, `${inputPath}`])
} else {
await ioUtil.unlink(inputPath)
}
}
try {
// note if path does not exist, error is silent
await ioUtil.rm(inputPath, {
force: true,
maxRetries: 3,
recursive: true,
retryDelay: 300
})
} catch (err) {
throw new Error(`File was unable to be removed ${err}`)
}
}

Expand Down