ToolRunner.exec does not work on Windows when both toolPath and arguments contain spaces

[adriaanthomas]

Use case and traces: microsoft/azure-pipelines-tasks#2253

It looks like this is caused by nodejs/node#7367.

Manually patching the build agent fixes this (javascript resulting from https://github.com/Microsoft/vsts-task-lib/blob/master/node/toolrunner.ts#L232, slightly rewritten to remove typescript-specific syntax):

var isWindowsShellScript = this.toolPath.endsWith('.cmd') || this.toolPath.endsWith('.bat');
var toolPath = this.toolPath.indexOf(' ') === -1 ? this.toolPath : '"' + this.toolPath + '"';
var args = this.args.map((arg: string) => arg.indexOf(' ') === -1 ? arg : '"' + arg + '"');
var cp = child.spawn(toolPath, args, { cwd: ops.cwd, env: ops.env, shell: isWindowsShellScript });

(clearly wrapping args in double quotes is a hack, this is just to prove the point - without that Gulpfile.js still is not found).

[bryanmacfarlane]

Can you repro an issue with the task-lib with lib >= 0.8 (we're currently on 0.9.x)?

[bryanmacfarlane]

I confirmed the task-lib >= 0.8 does not have the issue. The issue is the gulp task is shipping 0.7 (old) version of the task lib.

> var tl = require('vsts-task-lib/task');
##vso[task.debug]agent.workFolder=undefined
##vso[task.debug]loading inputs and endpoints
##vso[task.debug]loaded 0

> var tr = tl.tool('gulp');

> tr.arg('/path/with spaces/in/it');
> tr.args
[ '/path/with spaces/in/it' ]
> 

The key is the path intact with spaces needs to get into process argv with spaces and without double quotes. We will update the gulp task to use 0.9

[adriaanthomas]

You're faster than I am. Does your Gulp path also have spaces? Because that is the only way to fully reproduce this. Your test code only covers the argument, not the path that is passed to child_process.spawn as the first argument.

[adriaanthomas]

To be specific, I don't see this error addressed yet:

2016-08-11T07:06:43.3600568Z 'C:\Program' is not recognized as an internal or external command,
2016-08-11T07:06:43.3600568Z operable program or batch file.

This would be explained by the nodejs bug I linked above...

[bryanmacfarlane]

Hmmm. With new lib. node 4.4.6
I'll dig more to see why new lib is not susceptible (or doesn't appear to be).

test.sh just does

echo arg 1: $1

~/Testing/space path$ npm install vsts-task-lib
~/Testing/space path$ node
> var tl = require('vsts-task-lib/task');
##vso[task.debug]agent.workFolder=undefined
##vso[task.debug]loading inputs and endpoints
##vso[task.debug]loaded 0

> var p = '/Users/bryan/Testing/space path/test.sh'

> var tr=tl.tool(p);

> tr.arg('/arg one/here');

> tr.exec(function() { console.log('done');});
##vso[task.debug]exec tool: /Users/bryan/Testing/space path/test.sh
##vso[task.debug]Arguments:
##vso[task.debug]   /arg one/here
[command]/Users/bryan/Testing/space path/test.sh /arg one/here
{ state: 'pending' }
> arg 1: /arg one/here
##vso[task.debug]rc:0
##vso[task.debug]success:true

[adriaanthomas]

It looks like you're running this on a Mac, where there is no issue. It only fails on Windows.

[bryanmacfarlane]

Yeah - that was my next step. I'm OOF on laptop/wifi. Will try when I'm back in office.

If that is the case then because toolPath is known to be a path, we can quote if not already quoted. Then gulp would have to pickup latest task-lib.

I'll try and produce private in the morning when @ desk.

[bryanmacfarlane]

quoting messes up on osx so if we do the workaround, might have to be win only. will validate in a.m.

[bryanmacfarlane]

I'll add unit tests as well. Good thing is we run them on all platforms.

[ericsciple]

todo: add middle-man process to tl.exec when on Windows

[ericsciple]

..then fix gulp task

[ericsciple]

fixed by PR #157