Skip to content

Commit 37decc2

Browse files
chadlwilsonchadlwilson
committed
fix: Correct spawning of Windows binaries by batch/cmd with spaces
Signed-off-by: Chad Wilson <chadw@thoughtworks.com>
1 parent ae8904a commit 37decc2

File tree

11 files changed

+108
-75
lines changed

11 files changed

+108
-75
lines changed

‎package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"description": "Gauge support for VScode.",
99
"author": "ThoughtWorks",
1010
"license": "MIT",
11-
"version": "0.2.1",
11+
"version": "0.2.2",
1212
"publisher": "getgauge",
1313
"engines": {
1414
"vscode": "^1.82.0"

‎src/cli.ts

Lines changed: 43 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ export class CLI {
1010
private readonly _gaugeVersion: string;
1111
private readonly _gaugeCommitHash: string;
1212
private readonly _gaugePlugins: Array<any>;
13-
private readonly _gaugeCommand: string;
14-
private readonly _mvnCommand: string;
15-
private readonly _gradleCommand: string;
13+
private readonly _gaugeCommand: Command;
14+
private readonly _mvnCommand: Command;
15+
private readonly _gradleCommand: Command;
1616

17-
public constructor(cmd: string, manifest: any, mvnCommand: string, gradleCommand: string) {
17+
public constructor(cmd: Command, manifest: any, mvnCommand: Command, gradleCommand: Command) {
1818
this._gaugeCommand = cmd;
1919
this._mvnCommand = mvnCommand;
2020
this._gradleCommand = gradleCommand;
@@ -23,18 +23,12 @@ export class CLI {
2323
this._gaugePlugins = manifest.plugins;
2424
}
2525

26-
public static getDefaultSpawnOptions(): CommonSpawnOptions {
27-
// should only deal with platform specific options
28-
return platform() === "win32" ? { shell: true } : {};
29-
}
30-
3126
public static instance(): CLI {
3227
const gaugeCommand = this.getCommand(GaugeCommands.Gauge);
3328
const mvnCommand = this.getCommand(MAVEN_COMMAND);
34-
let gradleCommand = this.getGradleCommand();
35-
if (!gaugeCommand || gaugeCommand === '') return new CLI(gaugeCommand, {}, mvnCommand, gradleCommand);
36-
let options = this.getDefaultSpawnOptions();
37-
let gv = spawnSync(gaugeCommand, [GaugeCommands.Version, GaugeCommands.MachineReadable], options);
29+
const gradleCommand = this.getGradleCommand();
30+
if (!gaugeCommand) return new CLI(undefined, {}, mvnCommand, gradleCommand);
31+
let gv = spawnSync(gaugeCommand.command, [GaugeCommands.Version, GaugeCommands.MachineReadable], gaugeCommand.defaultSpawnOptions);
3832
let gaugeVersionInfo;
3933
try {
4034
gaugeVersionInfo = JSON.parse(gv.stdout.toString());
@@ -49,12 +43,12 @@ export class CLI {
4943
return this._gaugePlugins.some((p: any) => p.name === pluginName);
5044
}
5145

52-
public gaugeCommand(): string {
46+
public gaugeCommand(): Command {
5347
return this._gaugeCommand;
5448
}
5549

5650
public isGaugeInstalled(): boolean {
57-
return !!this._gaugeCommand && this._gaugeCommand !== '';
51+
return !!this._gaugeCommand;
5852
}
5953

6054
public isGaugeVersionGreaterOrEqual(version: string): boolean {
@@ -65,31 +59,21 @@ export class CLI {
6559
return this._gaugePlugins.find((p) => p.name === language).version;
6660
}
6761

68-
public getGaugeVersion(): string {
69-
return this._gaugeVersion;
70-
}
71-
7262
public async installGaugeRunner(language: string): Promise<any> {
7363
let oc = window.createOutputChannel("Gauge Install");
7464
let chan = new OutputChannel(oc, `Installing gauge ${language} plugin ...\n`, "");
7565
return new Promise((resolve, reject) => {
76-
let options = CLI.getDefaultSpawnOptions();
77-
let childProcess = spawn(this._gaugeCommand, [GaugeCommands.Install, language], options);
66+
let childProcess = spawn(this._gaugeCommand.command, [GaugeCommands.Install, language], this._gaugeCommand.defaultSpawnOptions);
7867
childProcess.stdout.on('data', (chunk) => chan.appendOutBuf(chunk.toString()));
7968
childProcess.stderr.on('data', (chunk) => chan.appendErrBuf(chunk.toString()));
8069
childProcess.on('exit', (code) => {
81-
let postFailureMessage = '\nRefer https://docs.gauge.org/plugin.html' +
82-
' to install manually';
70+
let postFailureMessage = '\nRefer to https://docs.gauge.org/plugin.html to install manually';
8371
chan.onFinish(resolve, code, "", postFailureMessage, false);
8472
});
8573
});
8674
}
8775

88-
public isMavenInstalled(): boolean {
89-
return !!this._mvnCommand && this._mvnCommand !== '';
90-
}
91-
92-
public mavenCommand(): string {
76+
public mavenCommand(): Command {
9377
return this._mvnCommand;
9478
}
9579

@@ -107,23 +91,46 @@ export class CLI {
10791
return `${v}\n${cm}\n\n${plugins}`;
10892
}
10993

110-
public static getCommandCandidates(command: string): string[] {
111-
return (platform() === 'win32' ? [".exe", ".bat", ".cmd"] : [""])
112-
.map((ext) => `${command}${ext}`);
94+
public static getCommandCandidates(command: string): Command[] {
95+
return platform() === 'win32' ? [
96+
new Command(command, ".exe"),
97+
new Command(command, ".bat", true),
98+
new Command(command, ".cmd", true),
99+
] : [
100+
new Command(command)
101+
]
113102
}
114103

115-
public static checkSpawnable(command: string): boolean {
116-
const result = spawnSync(command, [], CLI.getDefaultSpawnOptions());
104+
public static isSpawnable(command: Command): boolean {
105+
const result = spawnSync(command.command, [], command.defaultSpawnOptions);
117106
return result.status === 0 && !result.error;
118107
}
119108

120-
private static getCommand(command: string): string {
109+
private static getCommand(command: string): Command | undefined {
121110
for (const candidate of this.getCommandCandidates(command)) {
122-
if (this.checkSpawnable(candidate)) return candidate;
111+
if (this.isSpawnable(candidate)) return candidate;
123112
}
124113
}
125114

126115
private static getGradleCommand() {
127-
return platform() === 'win32' ? `${GRADLE_COMMAND}.bat` : `./${GRADLE_COMMAND}`;
116+
return platform() === 'win32' ? new Command(GRADLE_COMMAND, ".bat", true) : new Command(`./${GRADLE_COMMAND}`);
117+
}
118+
}
119+
120+
export type PlatformDependentSpawnOptions = {
121+
shell?: boolean
122+
}
123+
124+
export class Command {
125+
public readonly command: string
126+
public readonly defaultSpawnOptions: PlatformDependentSpawnOptions
127+
128+
constructor(public readonly cmdPrefix: string, public readonly cmdSuffix: string = "", public readonly shellMode: boolean = false) {
129+
this.command = this.cmdPrefix + this.cmdSuffix;
130+
this.defaultSpawnOptions = this.shellMode ? { shell: true } : {};
131+
}
132+
133+
argsForSpawnType(args: string[]): string[] {
134+
return this.shellMode ? args.map(arg => `"${arg}"`) : args;
128135
}
129136
}

‎src/execution/gaugeExecutor.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export class GaugeExecutor extends Disposable {
3737
private postExecute: Function[] = [];
3838
private _disposables: Disposable[] = [];
3939
private gaugeDebugger: GaugeDebugger;
40-
private processors: Array<LineTextProcessor> = new Array();
40+
private processors: Array<LineTextProcessor> = [];
4141

4242
constructor(private gaugeWorkspace: GaugeWorkspace, private cli: CLI) {
4343
super(() => this.dispose());
@@ -67,13 +67,13 @@ export class GaugeExecutor extends Disposable {
6767
const relPath = relative(config.getProject().root(), config.getStatus());
6868
this.preExecute.forEach((f) => { f.call(null, env, relPath); });
6969
this.aborted = false;
70-
let options: SpawnOptions = { cwd: config.getProject().root(), env: env , detached: false };
71-
if (platform() !== 'win32') {
72-
options.detached = true;
73-
} else {
74-
options.shell = true;
75-
}
76-
this.childProcess = spawn(cmd, args, options);
70+
let options: SpawnOptions = {
71+
cwd: config.getProject().root(),
72+
env: env,
73+
detached: platform() !== 'win32',
74+
...cmd.defaultSpawnOptions,
75+
};
76+
this.childProcess = spawn(cmd.command, cmd.argsForSpawnType(args), options);
7777
this.childProcess.stdout.on('data', this.filterStdoutDataDumpsToTextLines((lineText: string) => {
7878
chan.appendOutBuf(lineText);
7979
lineText.split("\n").forEach((lineText) => {

‎src/gaugeWorkspace.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -146,16 +146,16 @@ export class GaugeWorkspace extends Disposable {
146146
let project = ProjectFactory.get(folder);
147147
if (this._clientsMap.has(project.root())) return;
148148
process.env.GAUGE_IGNORE_RUNNER_BUILD_FAILURES = "true";
149+
let cmd = this.cli.gaugeCommand();
149150
let serverOptions: ServerOptions = {
150-
command: this.cli.gaugeCommand(),
151-
args: ["daemon", "--lsp", "--dir=" + project.root()],
152-
options: { env: { ...process.env, ...project.envs(this.cli) } },
151+
command: cmd.command,
152+
args: cmd.argsForSpawnType(["daemon", "--lsp", "--dir", project.root()]),
153+
options: {
154+
env: { ...process.env, ...project.envs(this.cli) },
155+
...cmd.defaultSpawnOptions,
156+
},
153157
};
154158

155-
if (platform() === "win32") {
156-
serverOptions.options.shell = true;
157-
}
158-
159159
this._launchConfig = workspace.getConfiguration(GAUGE_LAUNCH_CONFIG);
160160
if (this._launchConfig.get(DEBUG_LOG_LEVEL_CONFIG)) {
161161
serverOptions.args.push("-l");

‎src/init/projectInit.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ export class ProjectInitializer extends Disposable {
6868

6969
private async createFromCommandLine(template: FileListItem, projectFolder: Uri, p: ProgressHandler) {
7070
let args = [GaugeCommands.Init, template.label];
71-
let options: CommonSpawnOptions = { cwd: projectFolder.fsPath, env: process.env, ...CLI.getDefaultSpawnOptions() };
71+
const cmd = this.cli.gaugeCommand();
72+
let options: CommonSpawnOptions = { cwd: projectFolder.fsPath, env: process.env, ...cmd.defaultSpawnOptions };
7273
p.report("Initializing project...");
73-
let proc = spawn(this.cli.gaugeCommand(), args, options);
74+
let proc = spawn(cmd.command, cmd.argsForSpawnType(args), options);
7475
proc.addListener('error', async (err) => {
7576
this.handleError(p, "Failed to create template. " + err.message, projectFolder.fsPath);
7677
});
@@ -83,8 +84,9 @@ export class ProjectInitializer extends Disposable {
8384

8485
private async getTemplatesList(): Promise<Array<FileListItem>> {
8586
let args = ["template", "--list", "--machine-readable"];
86-
let options: CommonSpawnOptions = { env: process.env, ...CLI.getDefaultSpawnOptions() };
87-
let cp = spawnSync(this.cli.gaugeCommand(), args, options);
87+
const cmd = this.cli.gaugeCommand();
88+
let options: CommonSpawnOptions = { env: process.env, ...cmd.defaultSpawnOptions };
89+
let cp = spawnSync(cmd.command, cmd.argsForSpawnType(args), options);
8890
try {
8991
let _templates = JSON.parse(cp.stdout.toString());
9092
return _templates.map((tmpl) => new FileListItem(tmpl.key, tmpl.Description, tmpl.value));

‎src/project/gaugeProject.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
'use strict';
22

33
import { isAbsolute, relative } from 'path';
4-
import { CLI } from '../cli';
4+
import { CLI, Command } from '../cli';
55

66
export class GaugeProject {
77
private readonly _projectRoot: string;
@@ -18,7 +18,7 @@ export class GaugeProject {
1818
}
1919
}
2020

21-
public getExecutionCommand(cli: CLI): string {
21+
public getExecutionCommand(cli: CLI): Command {
2222
return cli.gaugeCommand();
2323
}
2424

‎test/cli.test.ts

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as assert from 'assert';
2-
import { CLI } from '../src/cli';
2+
import { CLI, Command } from '../src/cli';
33
import path = require('path');
4+
import { spawnSync } from "child_process";
45

56
let testCommandsPath = path.join(__dirname, '..', '..', 'test', 'commands');
67

@@ -15,7 +16,7 @@ suite('CLI', () => {
1516
]
1617
};
1718

18-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
19+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
1920
assert.ok(cli.isPluginInstalled('java'));
2021
assert.notEqual(true, cli.isPluginInstalled('foo'));
2122
});
@@ -30,7 +31,7 @@ suite('CLI', () => {
3031
]
3132
};
3233

33-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
34+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
3435
assert.equal('1.0.0', cli.getGaugePluginVersion('java'));
3536
});
3637

@@ -45,7 +46,7 @@ suite('CLI', () => {
4546
]
4647
};
4748

48-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
49+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
4950
assert.ok(cli.isPluginInstalled('java'));
5051
assert.notEqual(true, cli.isPluginInstalled('foo'));
5152
});
@@ -61,7 +62,7 @@ suite('CLI', () => {
6162
]
6263
};
6364

64-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
65+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
6566

6667
let expected = `Gauge version: 1.2.3
6768
Commit Hash: 3db28e6
@@ -86,7 +87,7 @@ ruby (1.2.0)`;
8687
]
8788
};
8889

89-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
90+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
9091

9192
let expected = `Gauge version: 1.2.3
9293
@@ -113,7 +114,7 @@ ruby (1.2.0)`;
113114
]
114115
};
115116

116-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
117+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
117118

118119
assert.ok(cli.isGaugeVersionGreaterOrEqual('1.2.3'));
119120
assert.ok(cli.isGaugeVersionGreaterOrEqual('1.2.0'));
@@ -133,22 +134,28 @@ ruby (1.2.0)`;
133134
{ name: "ruby", version: "1.2.0" },
134135
]
135136
};
136-
let cli = new CLI("gauge", info, 'mvn', 'gradle');
137+
let cli = new CLI(new Command("gauge"), info, new Command('mvn'), new Command('gradle'));
137138

138139
assert.ok(!cli.isGaugeVersionGreaterOrEqual('2.0.0'));
139140
assert.ok(!cli.isGaugeVersionGreaterOrEqual('2.1.3'));
140141
assert.ok(!cli.isGaugeVersionGreaterOrEqual('1.3.0'));
141142
done();
142143
});
143144

144-
test('.getCommandCandidates choices all valid', (done) => {
145+
test('.isGaugeInstalled should tell if gauge is installed or not', (done) => {
146+
assert.ok(new CLI(new Command("gauge"), {}, undefined, undefined).isGaugeInstalled());
147+
assert.ok(!new CLI(null, {}, undefined, undefined).isGaugeInstalled());
148+
done();
149+
});
150+
151+
test('.getCommandCandidates choices all valid by .isSpawnable', (done) => {
145152
let candidates = CLI.getCommandCandidates('test_command');
146153
const originalPath = process.env.PATH;
147154
process.env.PATH = testCommandsPath;
148155
let invalid_candidates = [];
149156
try {
150157
for (const candidate of candidates) {
151-
if (!CLI.checkSpawnable(candidate)) {
158+
if (!CLI.isSpawnable(candidate)) {
152159
invalid_candidates.push(candidate);
153160
}
154161
}
@@ -159,14 +166,14 @@ ruby (1.2.0)`;
159166
done();
160167
});
161168

162-
test('.getCommandCandidates choices are all not valid', (done) => {
169+
test('.getCommandCandidates choices can be found as in valid via .isSpawnable', (done) => {
163170
let candidates = CLI.getCommandCandidates('test_command_not_found');
164171
const originalPath = process.env.PATH;
165172
process.env.PATH = testCommandsPath;
166173
let valid_candidates = [];
167174
try {
168175
for (const candidate of candidates) {
169-
if (CLI.checkSpawnable(candidate)) {
176+
if (CLI.isSpawnable(candidate)) {
170177
valid_candidates.push(candidate);
171178
}
172179
}
@@ -176,4 +183,20 @@ ruby (1.2.0)`;
176183
}
177184
done();
178185
});
186+
187+
test('.getCommandCandidates can be spawned with an arg', (done) => {
188+
let candidates = CLI.getCommandCandidates('test_command');
189+
const originalPath = process.env.PATH;
190+
process.env.PATH = testCommandsPath;
191+
try {
192+
for (const candidate of candidates.filter(c => (c.cmdSuffix !== ".exe"))) {
193+
const result = spawnSync(candidate.command, candidate.argsForSpawnType(["Hello World"]), candidate.defaultSpawnOptions);
194+
assert.ok(result.status === 0 && !result.error, `Command candidate ${candidate.command} failed to spawn`);
195+
assert.equal(result.stdout.toString().trim(), "Success: Hello World", `Command candidate ${candidate.command} has wrong output`)
196+
}
197+
} finally {
198+
process.env.PATH = originalPath;
199+
}
200+
done();
201+
});
179202
});

0 commit comments

Comments
 (0)