toolrunner should which tool before invoking (#220)

.github/workflows/unit-tests.yml

@@ -6,18 +6,27 @@ on:
   pull_request:
     paths-ignore:
       - '**.md'     
+
 jobs:
-  Ubuntu:
+
-    name: Run Ubuntu
+  build:
-    runs-on: ubuntu-latest
+    name: Build
+
+    strategy:
+      matrix:
+        runs-on: [ubuntu-latest, macOS-latest, windows-latest]
+      fail-fast: false
+
+    runs-on: ${{ matrix.runs-on }}
+
     steps:
     - name: Checkout
-      uses: actions/checkout@master
+      uses: actions/checkout@v1
 
-    - name: Set Node.js 10.x
+    - name: Set Node.js 12.x
       uses: actions/setup-node@master
       with:
-        node-version: 10.x
+        node-version: 12.x
 
     - name: npm install
       run: npm install
@@ -36,52 +45,3 @@ jobs:
 
     - name: Format
       run: npm run format-check
-  macOS:
-    name: Run macOS
-    runs-on: macos-latest
-    steps:
-      - name: Checkout
-        uses: actions/checkout@master
-
-      - name: Set Node.js 10.x
-        uses: actions/setup-node@master
-        with:
-          node-version: 10.x
-
-      - name: npm install
-        run: npm install
-
-      - name: Bootstrap
-        run: npm run bootstrap
-
-      - name: Compile
-        run: npm run build
-
-      - name: npm test
-        run: npm test
-  Windows:
-    name: Run Windows
-    runs-on: windows-latest
-    steps:
-    - name: Checkout
-      uses: actions/checkout@master
-
-    - name: Set Node.js 10.x
-      uses: actions/setup-node@master
-      with:
-        node-version: 10.x
-
-    - name: npm install
-      run: npm install
-
-    - name: Bootstrap
-      run: npm run bootstrap
-
-    - name: Compile
-      run: npm run build
-
-    # TODO: This currently ignores exec due to issues with Node and spawning on Windows, which I think is exacerbated by Jest.
-    # It doesn't seem to affect behavior in actions themselves, just when testing with Jest.
-    # See other similar issues here: https://github.com/nodejs/node/issues/25484
-    - name: npm test
-      run: npm run test-ci

package.json

@@ -9,8 +9,7 @@
     "format-check": "prettier --check packages/**/*.ts",
     "lint": "eslint packages/**/*.ts",
     "new-package": "scripts/create-package",
-    "test": "jest",
+    "test": "jest"
-    "test-ci": "jest --testPathIgnorePatterns=\"<rootDir>/packages/exec/__tests__/exec.test.ts\""
   },
   "devDependencies": {
     "@types/jest": "^24.0.11",

packages/core/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/core",
-  "version": "1.1.1",
+  "version": "1.2.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

packages/exec/README.md

@@ -48,13 +48,10 @@ await exec.exec('node', ['index.js', 'foo=bar'], options);
 
 #### Exec tools not in the PATH
 
-You can use it in conjunction with the `which` function from `@actions/io` to execute tools that are not in the PATH:
+You can specify the full path for tools not in the PATH:
 
 ```js
 const exec = require('@actions/exec');
-const io = require('@actions/io');
 
-const pythonPath: string = await io.which('python', true)
+await exec.exec('"/path/to/my-tool"', ['arg1']);
-
-await exec.exec(`"${pythonPath}"`, ['main.py']);
 ```

packages/exec/__tests__/exec.test.ts

@@ -121,6 +121,38 @@ describe('@actions/exec', () => {
     }
   })
 
+  it('Runs exec successfully with command from PATH', async () => {
+    const execOptions = getExecOptions()
+    const outStream = new StringStream()
+    execOptions.outStream = outStream
+    let output = ''
+    execOptions.listeners = {
+      stdout: (data: Buffer) => {
+        output += data.toString()
+      }
+    }
+
+    let exitCode = 1
+    let tool: string
+    let args: string[]
+    if (IS_WINDOWS) {
+      tool = 'cmd'
+      args = ['/c', 'echo', 'hello']
+    } else {
+      tool = 'sh'
+      args = ['-c', 'echo hello']
+    }
+
+    exitCode = await exec.exec(tool, args, execOptions)
+
+    expect(exitCode).toBe(0)
+    const rootedTool = await io.which(tool, true)
+    expect(outStream.getContents().split(os.EOL)[0]).toBe(
+      `[command]${rootedTool} ${args.join(' ')}`
+    )
+    expect(output.trim()).toBe(`hello`)
+  })
+
   it('Exec fails with error on bad call', async () => {
     const _testExecOptions = getExecOptions()
 
@@ -418,6 +450,134 @@ describe('@actions/exec', () => {
     fs.unlinkSync(semaphorePath)
   })
 
+  it('Exec roots relative tool path using unrooted options.cwd', async () => {
+    let exitCode: number
+    let command: string
+    if (IS_WINDOWS) {
+      command = './print-args-cmd' // let ToolRunner resolve the extension
+    } else {
+      command = './print-args-sh.sh'
+    }
+    const execOptions = getExecOptions()
+    execOptions.cwd = 'scripts'
+    const outStream = new StringStream()
+    execOptions.outStream = outStream
+    let output = ''
+    execOptions.listeners = {
+      stdout: (data: Buffer) => {
+        output += data.toString()
+      }
+    }
+
+    const originalCwd = process.cwd()
+    try {
+      process.chdir(__dirname)
+      exitCode = await exec.exec(`${command} hello world`, [], execOptions)
+    } catch (err) {
+      process.chdir(originalCwd)
+      throw err
+    }
+
+    expect(exitCode).toBe(0)
+    const toolPath = path.resolve(
+      __dirname,
+      execOptions.cwd,
+      `${command}${IS_WINDOWS ? '.cmd' : ''}`
+    )
+    if (IS_WINDOWS) {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${process.env.ComSpec} /D /S /C "${toolPath} hello world"`
+      )
+    } else {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${toolPath} hello world`
+      )
+    }
+    expect(output.trim()).toBe(`args[0]: "hello"${os.EOL}args[1]: "world"`)
+  })
+
+  it('Exec roots relative tool path using rooted options.cwd', async () => {
+    let command: string
+    if (IS_WINDOWS) {
+      command = './print-args-cmd' // let ToolRunner resolve the extension
+    } else {
+      command = './print-args-sh.sh'
+    }
+    const execOptions = getExecOptions()
+    execOptions.cwd = path.join(__dirname, 'scripts')
+    const outStream = new StringStream()
+    execOptions.outStream = outStream
+    let output = ''
+    execOptions.listeners = {
+      stdout: (data: Buffer) => {
+        output += data.toString()
+      }
+    }
+
+    const exitCode = await exec.exec(`${command} hello world`, [], execOptions)
+
+    expect(exitCode).toBe(0)
+    const toolPath = path.resolve(
+      __dirname,
+      execOptions.cwd,
+      `${command}${IS_WINDOWS ? '.cmd' : ''}`
+    )
+    if (IS_WINDOWS) {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${process.env.ComSpec} /D /S /C "${toolPath} hello world"`
+      )
+    } else {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${toolPath} hello world`
+      )
+    }
+    expect(output.trim()).toBe(`args[0]: "hello"${os.EOL}args[1]: "world"`)
+  })
+
+  it('Exec roots relative tool path using process.cwd', async () => {
+    let exitCode: number
+    let command: string
+    if (IS_WINDOWS) {
+      command = 'scripts/print-args-cmd' // let ToolRunner resolve the extension
+    } else {
+      command = 'scripts/print-args-sh.sh'
+    }
+    const execOptions = getExecOptions()
+    const outStream = new StringStream()
+    execOptions.outStream = outStream
+    let output = ''
+    execOptions.listeners = {
+      stdout: (data: Buffer) => {
+        output += data.toString()
+      }
+    }
+
+    const originalCwd = process.cwd()
+    try {
+      process.chdir(__dirname)
+      exitCode = await exec.exec(`${command} hello world`, [], execOptions)
+    } catch (err) {
+      process.chdir(originalCwd)
+      throw err
+    }
+
+    expect(exitCode).toBe(0)
+    const toolPath = path.resolve(
+      __dirname,
+      `${command}${IS_WINDOWS ? '.cmd' : ''}`
+    )
+    if (IS_WINDOWS) {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${process.env.ComSpec} /D /S /C "${toolPath} hello world"`
+      )
+    } else {
+      expect(outStream.getContents().split(os.EOL)[0]).toBe(
+        `[command]${toolPath} hello world`
+      )
+    }
+    expect(output.trim()).toBe(`args[0]: "hello"${os.EOL}args[1]: "world"`)
+  })
+
   if (IS_WINDOWS) {
     // Win specific quoting tests
     it('execs .exe with verbatim args (Windows)', async () => {
@@ -572,6 +732,42 @@ describe('@actions/exec', () => {
       )
     })
 
+    it('execs .cmd from path (Windows)', async () => {
+      // this test validates whether a .cmd is resolved from the PATH when the extension is not specified
+      const cmd = 'print-args-cmd' // note, not print-args-cmd.cmd
+      const cmdPath = path.join(__dirname, 'scripts', `${cmd}.cmd`)
+      const args: string[] = ['my arg 1', 'my arg 2']
+      const outStream = new StringStream()
+      let output = ''
+      const options = {
+        outStream: <stream.Writable>outStream,
+        listeners: {
+          stdout: (data: Buffer) => {
+            output += data.toString()
+          }
+        }
+      }
+
+      const originalPath = process.env['Path']
+      try {
+        process.env['Path'] = `${originalPath};${path.dirname(cmdPath)}`
+        const exitCode = await exec.exec(`${cmd}`, args, options)
+        expect(exitCode).toBe(0)
+        expect(outStream.getContents().split(os.EOL)[0]).toBe(
+          `[command]${
+            process.env.ComSpec
+          } /D /S /C "${cmdPath} "my arg 1" "my arg 2""`
+        )
+        expect(output.trim()).toBe(
+          'args[0]: "<quote>my arg 1<quote>"\r\n' +
+            'args[1]: "<quote>my arg 2<quote>"'
+        )
+      } catch (err) {
+        process.env['Path'] = originalPath
+        throw err
+      }
+    })
+
     it('execs .cmd with arg quoting (Windows)', async () => {
       // this test validates .cmd quoting rules are applied, not the default libuv rules
       const cmdPath = path.join(

packages/exec/__tests__/scripts/print-args-cmd.cmd

@@ -0,0 +1,12 @@
+@echo off
+setlocal
+set index=0
+
+:check_arg
+set arg=%1
+if not defined arg goto :eof
+set "arg=%arg:"=<quote>%"
+echo args[%index%]: "%arg%"
+set /a index=%index%+1
+shift
+goto check_arg

packages/exec/__tests__/scripts/print-args-sh.sh

@@ -0,0 +1,11 @@
+#!/usr/bin/env bash
+
+# store arguments in a special array
+args=("$@")
+# get number of elements
+ELEMENTS=${#args[@]}
+
+# echo each element
+for (( i=0;i<$ELEMENTS;i++)); do
+    echo "args[$i]: \"${args[${i}]}\""
+done

packages/exec/package.json

@@ -32,7 +32,7 @@
   "bugs": {
     "url": "https://github.com/actions/toolkit/issues"
   },
-  "devDependencies": {
+  "dependencies": {
     "@actions/io": "^1.0.1"
   }
 }

packages/exec/src/toolrunner.ts

@@ -1,8 +1,11 @@
 import * as os from 'os'
 import * as events from 'events'
 import * as child from 'child_process'
+import * as path from 'path'
 import * as stream from 'stream'
 import * as im from './interfaces'
+import * as io from '@actions/io'
+import * as ioUtil from '@actions/io/lib/io-util'
 
 /* eslint-disable @typescript-eslint/unbound-method */
 
@@ -392,6 +395,24 @@ export class ToolRunner extends events.EventEmitter {
    * @returns   number
    */
   async exec(): Promise<number> {
+    // root the tool path if it is unrooted and contains relative pathing
+    if (
+      !ioUtil.isRooted(this.toolPath) &&
+      (this.toolPath.includes('/') ||
+        (IS_WINDOWS && this.toolPath.includes('\\')))
+    ) {
+      // prefer options.cwd if it is specified, however options.cwd may also need to be rooted
+      this.toolPath = path.resolve(
+        process.cwd(),
+        this.options.cwd || process.cwd(),
+        this.toolPath
+      )
+    }
+
+    // if the tool is only a file name, then resolve it from the PATH
+    // otherwise verify it exists (add extension on Windows if necessary)
+    this.toolPath = await io.which(this.toolPath, true)
+
     return new Promise<number>((resolve, reject) => {
       this._debug(`exec tool: ${this.toolPath}`)
       this._debug('arguments:')

packages/tool-cache/package-lock.json

@@ -1,6 +1,6 @@
 {
 	"name": "@actions/tool-cache",
-	"version": "1.1.1",
+	"version": "1.1.2",
 	"lockfileVersion": 1,
 	"requires": true,
 	"dependencies": {