Removing childprocess for rmRF (actions#1373)

* try awaiting spawn on windows * formatting * updating package-lock * . * . * updating packages * adding sync rm * test with sync * pointing to rmsync * adding error handling * testing rmsync * adding try/catch * adding windows conditional for locked file * switch to contians * fixing formatting * fixing formatting * fixing formatting * adding enonet catch for windows files * adding enonet catch for windows files * adding catch for file not found * updating stat call * updating stat call * adding conditonal for symlink * removing symlink test * adding ebusy check * changing error check * changing error check * changing error check * changing error check * cleanup and comments * Update packages/io/__tests__/io.test.ts Co-authored-by: Cory Miller <[email protected]> * Update packages/io/src/io-util.ts Co-authored-by: Cory Miller <[email protected]> * moving comment placement * updating eperm * change back to ebusy * Update packages/io/__tests__/io.test.ts Co-authored-by: Cory Miller <[email protected]> * Formatting * converting to async --------- Co-authored-by: Cory Miller <[email protected]>
inossidabile · Mar 15, 2023 · 463b49d · 463b49d
1 parent a91ee0b
commit 463b49d
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 162 deletions.
diff --git a/packages/glob/package-lock.json b/packages/glob/package-lock.json
diff --git a/packages/io/__tests__/io.test.ts b/packages/io/__tests__/io.test.ts
@@ -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 () => {
@@ -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)
@@ -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 () => {

diff --git a/packages/io/src/io-util.ts b/packages/io/src/io-util.ts
@@ -6,16 +6,21 @@ export const {
   copyFile,
   lstat,
   mkdir,
+  open,
   readdir,
   readlink,
   rename,
+  rm,
   rmdir,
   stat,
   symlink,
   unlink
 } = fs.promises
-
+// export const {open} = 'fs'
 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
+export const READONLY = fs.constants.O_RDONLY
 
 export async function exists(fsPath: string): Promise<boolean> {
   try {

diff --git a/packages/io/src/io.ts b/packages/io/src/io.ts
@@ -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
  */
@@ -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}`)
   }
 }