Removing childprocess for rmRF (#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 <13227161+cory-miller@users.noreply.github.com>

* Update packages/io/src/io-util.ts

Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>

* moving comment placement

* updating eperm

* change back to ebusy

* Update packages/io/__tests__/io.test.ts

Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>

* Formatting

* converting to async

---------

Co-authored-by: Cory Miller <13227161+cory-miller@users.noreply.github.com>

packages/glob/package-lock.json

@@ -1,12 +1,27 @@
 {
   "name": "@actions/glob",
   "version": "0.4.0",
-  "lockfileVersion": 2,
+  "lockfileVersion": 3,
   "requires": true,
+  "description": "Actions glob lib",
+  "files": [
+    "lib",
+    "!.DS_Store"
+  ],
+  "homepage": "https://github.com/actions/toolkit/tree/main/packages/glob",
+  "keywords": [
+    "github",
+    "actions",
+    "glob"
+  ],
+  "license": "MIT",
+  "main": "lib/glob.js",
+  "preview": true,
+  "types": "lib/glob.d.ts",
   "packages": {
     "": {
       "name": "@actions/glob",
-      "version": "0.3.0",
+      "version": "0.4.0",
       "license": "MIT",
       "dependencies": {
         "@actions/core": "^1.9.1",
@@ -14,26 +29,17 @@
       }
     },
     "node_modules/@actions/core": {
-      "version": "1.9.1",
+      "version": "1.10.0",
-      "resolved": "https://registry.npmjs.org/@actions/core/-/core-1.9.1.tgz",
+      "integrity": "sha512-2aZDDa3zrrZbP5ZYg159sNoLRb61nQ7awl5pSvIq5Qpj81vwDzdMRKzkWJGJuwVvWpvZKx7vspJALyvaaIQyug==",
-      "integrity": "sha512-5ad+U2YGrmmiw6du20AQW5XuWo7UKN2052FjSV7MX+Wfjf8sCqcsZe62NfgHys4QI4/Y+vQvLKYL8jWtA1ZBTA==",
       "dependencies": {
         "@actions/http-client": "^2.0.1",
         "uuid": "^8.3.2"
       }
     },
-    "node_modules/@actions/http-client": {
-      "version": "2.0.1",
-      "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.0.1.tgz",
-      "integrity": "sha512-PIXiMVtz6VvyaRsGY268qvj57hXQEpsYogYOu2nrQhlf+XCGmZstmuZBbAybUl1nQGnvS1k1eEsQ69ZoD7xlSw==",
-      "dependencies": {
-        "tunnel": "^0.0.6"
-      }
-    },
     "node_modules/balanced-match": {
-      "version": "1.0.0",
+      "version": "1.0.2",
-      "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz",
+      "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz",
-      "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c="
+      "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw=="
     },
     "node_modules/brace-expansion": {
       "version": "1.1.11",
@@ -47,7 +53,7 @@
     "node_modules/concat-map": {
       "version": "0.0.1",
       "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz",
-      "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s="
+      "integrity": "sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg=="
     },
     "node_modules/minimatch": {
       "version": "3.1.2",
@@ -59,78 +65,26 @@
       "engines": {
         "node": "*"
       }
-    },
-    "node_modules/tunnel": {
-      "version": "0.0.6",
-      "resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz",
-      "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg==",
-      "engines": {
-        "node": ">=0.6.11 <=0.7.0 || >=0.7.3"
     }
   },
-    "node_modules/uuid": {
+  "bugs": {
-      "version": "8.3.2",
+    "url": "https://github.com/actions/toolkit/issues"
-      "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
-      "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg==",
-      "bin": {
-        "uuid": "dist/bin/uuid"
-      }
-    }
   },
-  "dependencies": {
+  "directories": {
-    "@actions/core": {
+    "lib": "lib",
-      "version": "1.9.1",
+    "test": "__tests__"
-      "resolved": "https://registry.npmjs.org/@actions/core/-/core-1.9.1.tgz",
-      "integrity": "sha512-5ad+U2YGrmmiw6du20AQW5XuWo7UKN2052FjSV7MX+Wfjf8sCqcsZe62NfgHys4QI4/Y+vQvLKYL8jWtA1ZBTA==",
-      "requires": {
-        "@actions/http-client": "^2.0.1",
-        "uuid": "^8.3.2"
-      }
   },
-    "@actions/http-client": {
+  "publishConfig": {
-      "version": "2.0.1",
+    "access": "public"
-      "resolved": "https://registry.npmjs.org/@actions/http-client/-/http-client-2.0.1.tgz",
-      "integrity": "sha512-PIXiMVtz6VvyaRsGY268qvj57hXQEpsYogYOu2nrQhlf+XCGmZstmuZBbAybUl1nQGnvS1k1eEsQ69ZoD7xlSw==",
-      "requires": {
-        "tunnel": "^0.0.6"
-      }
   },
-    "balanced-match": {
+  "repository": {
-      "version": "1.0.0",
+    "directory": "packages/glob",
-      "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.0.tgz",
+    "type": "git",
-      "integrity": "sha1-ibTRmasr7kneFk6gK4nORi1xt2c="
+    "url": "git+https://github.com/actions/toolkit.git"
   },
-    "brace-expansion": {
+  "scripts": {
-      "version": "1.1.11",
+    "audit-moderate": "npm install && npm audit --json --audit-level=moderate > audit.json",
-      "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.11.tgz",
+    "test": "echo \"Error: run tests from root\" && exit 1",
-      "integrity": "sha512-iCuPHDFgrHX7H2vEI/5xpz07zSHB00TpugqhmYtVmMO6518mCuRMoOYFldEBl0g187ufozdaHgWKcYFb61qGiA==",
+    "tsc": "tsc"
-      "requires": {
-        "balanced-match": "^1.0.0",
-        "concat-map": "0.0.1"
-      }
-    },
-    "concat-map": {
-      "version": "0.0.1",
-      "resolved": "https://registry.npmjs.org/concat-map/-/concat-map-0.0.1.tgz",
-      "integrity": "sha1-2Klr13/Wjfd5OnMDajug1UBdR3s="
-    },
-    "minimatch": {
-      "version": "3.1.2",
-      "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-3.1.2.tgz",
-      "integrity": "sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==",
-      "requires": {
-        "brace-expansion": "^1.1.7"
-      }
-    },
-    "tunnel": {
-      "version": "0.0.6",
-      "resolved": "https://registry.npmjs.org/tunnel/-/tunnel-0.0.6.tgz",
-      "integrity": "sha512-1h/Lnq9yajKY2PEbBadPXj3VxsDDu844OnaAo52UVmIzIvwwtBPIuNvkjuzBlTWpfJyUbG3ez0KSBibQkj4ojg=="
-    },
-    "uuid": {
-      "version": "8.3.2",
-      "resolved": "https://registry.npmjs.org/uuid/-/uuid-8.3.2.tgz",
-      "integrity": "sha512-+NYs2QeMWy+GWFOEm9xnn6HCDp0l7QBD7ml8zLUmJ+93Q5NF0NocErnwkTkXVFNiX3/fpC6afS8Dhb/gz7R7eg=="
-    }
   }
 }

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')
+    // 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 () => {

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 {

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,9 +111,6 @@ 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)) {
@@ -126,47 +118,17 @@ export async function rmRF(inputPath: string): Promise<void> {
         'File path must not contain `*`, `"`, `<`, `>` or `|` on Windows'
       )
     }
+  }
   try {
-      const cmdPath = ioUtil.getCmdPath()
+    // note if path does not exist, error is silent
-      if (await ioUtil.isDirectory(inputPath, true)) {
+    await ioUtil.rm(inputPath, {
-        await exec(`${cmdPath} /s /c "rd /s /q "%inputPath%""`, {
+      force: true,
-          env: {inputPath}
+      maxRetries: 3,
+      recursive: true,
+      retryDelay: 300
     })
-      } 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
+    throw new Error(`File was unable to be removed ${err}`)
-      // 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)
-    }
   }
 }