From 5c894298f2f24441718eb049215aa6decd507193 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Mon, 18 Nov 2019 16:20:01 -0500 Subject: [PATCH] toolrunner should which tool before invoking (#220) --- .github/workflows/unit-tests.yml | 70 ++----- package.json | 3 +- packages/core/package-lock.json | 2 +- packages/exec/README.md | 7 +- packages/exec/__tests__/exec.test.ts | 196 ++++++++++++++++++ .../exec/__tests__/scripts/print-args-cmd.cmd | 12 ++ .../exec/__tests__/scripts/print-args-sh.sh | 11 + packages/exec/package.json | 2 +- packages/exec/src/toolrunner.ts | 21 ++ packages/tool-cache/package-lock.json | 2 +- 10 files changed, 261 insertions(+), 65 deletions(-) create mode 100644 packages/exec/__tests__/scripts/print-args-cmd.cmd create mode 100755 packages/exec/__tests__/scripts/print-args-sh.sh diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index ac66f9ac..feaa75fb 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -6,18 +6,27 @@ on: pull_request: paths-ignore: - '**.md' + jobs: - Ubuntu: - name: Run Ubuntu - runs-on: ubuntu-latest + + build: + 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 diff --git a/package.json b/package.json index 16617490..563eab1e 100644 --- a/package.json +++ b/package.json @@ -9,8 +9,7 @@ "format-check": "prettier --check packages/**/*.ts", "lint": "eslint packages/**/*.ts", "new-package": "scripts/create-package", - "test": "jest", - "test-ci": "jest --testPathIgnorePatterns=\"/packages/exec/__tests__/exec.test.ts\"" + "test": "jest" }, "devDependencies": { "@types/jest": "^24.0.11", diff --git a/packages/core/package-lock.json b/packages/core/package-lock.json index 3449e3e3..fc47b0cb 100644 --- a/packages/core/package-lock.json +++ b/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": { diff --git a/packages/exec/README.md b/packages/exec/README.md index e3eff742..7897ba5a 100644 --- a/packages/exec/README.md +++ b/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(`"${pythonPath}"`, ['main.py']); +await exec.exec('"/path/to/my-tool"', ['arg1']); ``` diff --git a/packages/exec/__tests__/exec.test.ts b/packages/exec/__tests__/exec.test.ts index e0b69641..c0a5dda0 100644 --- a/packages/exec/__tests__/exec.test.ts +++ b/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: 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]: "my arg 1"\r\n' + + 'args[1]: "my arg 2"' + ) + } 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( diff --git a/packages/exec/__tests__/scripts/print-args-cmd.cmd b/packages/exec/__tests__/scripts/print-args-cmd.cmd new file mode 100644 index 00000000..7f3e4e66 --- /dev/null +++ b/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:"=%" +echo args[%index%]: "%arg%" +set /a index=%index%+1 +shift +goto check_arg \ No newline at end of file diff --git a/packages/exec/__tests__/scripts/print-args-sh.sh b/packages/exec/__tests__/scripts/print-args-sh.sh new file mode 100755 index 00000000..40f18cb2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/packages/exec/package.json b/packages/exec/package.json index a57920ea..ee3a773d 100644 --- a/packages/exec/package.json +++ b/packages/exec/package.json @@ -32,7 +32,7 @@ "bugs": { "url": "https://github.com/actions/toolkit/issues" }, - "devDependencies": { + "dependencies": { "@actions/io": "^1.0.1" } } diff --git a/packages/exec/src/toolrunner.ts b/packages/exec/src/toolrunner.ts index 32dc5f94..2182fedd 100644 --- a/packages/exec/src/toolrunner.ts +++ b/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 { + // 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((resolve, reject) => { this._debug(`exec tool: ${this.toolPath}`) this._debug('arguments:') diff --git a/packages/tool-cache/package-lock.json b/packages/tool-cache/package-lock.json index ffea2aa0..3c3e3862 100644 --- a/packages/tool-cache/package-lock.json +++ b/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": {