Skip to content

Commit f776e58

Browse files
hanniavaleraHannia Valera
andauthored
Fixing Command Line Truncation (#4844)
* refactor: remove unused terminal management and streamline compilation command execution * test: add unit tests for compile command argument handling to address issue #4836 * ensure active compiler process is terminated when closing the terminal --------- Co-authored-by: Hannia Valera <[email protected]>
1 parent 1f68884 commit f776e58

3 files changed

Lines changed: 271 additions & 58 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ Bug Fixes:
5050
- Fix bookmarked targets running a full build instead of building only the specific target when triggered from the Bookmarks view. [#4771](https://github.com/microsoft/vscode-cmake-tools/issues/4771)
5151
- Fix build errors not being added to the Problems tab when using `cmake.buildTask: true`. The build task's pseudoterminal now feeds output to the diagnostic parser so compiler and linker errors are properly surfaced. [#4489](https://github.com/microsoft/vscode-cmake-tools/issues/4489)
5252
- Fix `cmake.compileFile` command truncating long compile commands at ~1024 characters on macOS. The command is now sent to the terminal in chunks to avoid VS Code terminal buffer limitations. [#4470](https://github.com/microsoft/vscode-cmake-tools/issues/4470)
53+
- Fix `cmake.compileFile` truncating compile commands longer than 4096 bytes due to the PTY input buffer limit. The compiler is now spawned directly via `proc.execute()` behind a pseudoterminal, bypassing the shell entirely so argument lists of any length are passed intact. [#4836](https://github.com/microsoft/vscode-cmake-tools/issues/4836)
5354
- Fix configure/build sometimes using stale preset values when unsaved changes to included preset files are auto-saved before configure. The extension now explicitly refreshes presets from disk after saving, instead of relying solely on the asynchronous file watcher. [#4502](https://github.com/microsoft/vscode-cmake-tools/issues/4502)
5455
- Fix overly aggressive preset reloading on every build/debug/launch. Preset reapplication now only runs when `cmake.configureOnEdit` is enabled and preset files had unsaved changes, instead of unconditionally re-reading all preset files. [#4792](https://github.com/microsoft/vscode-cmake-tools/issues/4792)
5556
- Reduce overly verbose logging when CMake configure or build fails. The Output panel no longer floods with duplicated output, and the channel is only revealed on error rather than unconditionally. [#4749](https://github.com/microsoft/vscode-cmake-tools/issues/4749)

src/drivers/cmakeDriver.ts

Lines changed: 64 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import * as lodash from "lodash";
88

99
import { CMakeExecutable } from '@cmt/cmakeExecutable';
1010
import * as codepages from '@cmt/codePageTable';
11-
import * as shlex from '@cmt/shlex';
1211
import { ConfigureCancelInformation, ConfigureTrigger, DiagnosticsConfiguration } from "@cmt/cmakeProject";
1312
import { CompileCommand } from '@cmt/compilationDatabase';
1413
import { ConfigurationReader, checkBuildOverridesPresent, checkConfigureOverridesPresent, checkTestOverridesPresent, checkPackageOverridesPresent, defaultNumJobs } from '@cmt/config';
@@ -240,17 +239,6 @@ export abstract class CMakeDriver implements vscode.Disposable {
240239
private readonly usingFileApi: boolean = false
241240
) {
242241
this.sourceDir = this.sourceDirUnexpanded;
243-
// We have a cache of file-compilation terminals. Wipe them out when the
244-
// user closes those terminals.
245-
vscode.window.onDidCloseTerminal(closed => {
246-
for (const [key, term] of this._compileTerms) {
247-
if (term === closed) {
248-
log.debug(localize('user.closed.file.compilation.terminal', 'User closed a file compilation terminal'));
249-
this._compileTerms.delete(key);
250-
break;
251-
}
252-
}
253-
});
254242
}
255243

256244
/**
@@ -275,9 +263,6 @@ export abstract class CMakeDriver implements vscode.Disposable {
275263
*/
276264
dispose() {
277265
log.debug(localize('disposing.base.cmakedriver', 'Disposing base CMakeDriver'));
278-
for (const term of this._compileTerms.values()) {
279-
term.dispose();
280-
}
281266
for (const sub of [this._settingsSub, this._argsSub, this._envSub, this._buildArgsSub, this._buildEnvSub, this._testArgsSub, this._testEnvSub, this._packEnvSub, this._packArgsSub, this._generalEnvSub]) {
282267
sub.dispose();
283268
}
@@ -535,18 +520,6 @@ export abstract class CMakeDriver implements vscode.Disposable {
535520
return proc.execute(command, args, consumer, exec_options);
536521
}
537522

538-
/**
539-
* File compilation terminals. This is a map, rather than a single terminal
540-
* instance for two reasons:
541-
*
542-
* 1. Different compile commands may require different environment variables.
543-
* 2. Different compile commands may require different working directories.
544-
*
545-
* The key of each terminal is generated deterministically in `runCompileCommand()`
546-
* based on the CWD and environment of the compile command.
547-
*/
548-
private readonly _compileTerms = new Map<string, vscode.Terminal>();
549-
550523
/**
551524
* Launch the given compilation command in an embedded terminal.
552525
* @param cmd The compilation command from a compilation database to run
@@ -555,44 +528,77 @@ export abstract class CMakeDriver implements vscode.Disposable {
555528
const env = await this.getCMakeBuildCommandEnvironment();
556529

557530
if (this.useCMakePresets && this._buildPreset && checkBuildOverridesPresent(this.config)) {
558-
log.info(localize('compile.with.overrides', 'NOTE: You are compiling with preset {0}, but there are some overrides being applied from your VS Code settings.', this._buildPreset.displayName ?? this._buildPreset.name));
531+
log.info(localize('compile.with.overrides',
532+
'NOTE: You are compiling with preset {0}, but there are some overrides being applied from your VS Code settings.',
533+
this._buildPreset.displayName ?? this._buildPreset.name));
559534
}
560535

561-
const key = `${cmd.directory}${JSON.stringify(env)}`;
562-
let existing = this._compileTerms.get(key);
563-
if (existing && this.config.clearOutputBeforeBuild) {
564-
this._compileTerms.delete(key);
565-
existing.dispose();
566-
existing = undefined;
567-
}
568-
if (!existing) {
569-
const shellPath = this.config.shell ?? (process.platform === 'win32' ? 'cmd.exe' : undefined);
536+
const args = cmd.arguments;
537+
if (!args || args.length === 0) {
538+
// Fallback: no structured args available — send the raw command string via terminal.
539+
// This path should never be reached because CompilationDatabase always populates arguments.
570540
const term = vscode.window.createTerminal({
571541
name: localize('file.compilation', 'File Compilation'),
572542
cwd: cmd.directory,
573-
env,
574-
shellPath
543+
env
575544
});
576-
this._compileTerms.set(key, term);
577-
existing = term;
578-
}
579-
existing.show();
580-
// Send the command and arguments individually to avoid terminal buffer truncation
581-
// issues with long command lines (see https://github.com/microsoft/vscode/issues/233420).
582-
// The arguments array is always populated by CompilationDatabase, either from the
583-
// compile_commands.json or by parsing the command string.
584-
const args = cmd.arguments;
585-
if (args && args.length > 0) {
586-
existing.sendText(shlex.quote(args[0]), false);
587-
for (let i = 1; i < args.length; i++) {
588-
existing.sendText(` ${shlex.quote(args[i])}`, false);
545+
term.show();
546+
term.sendText(cmd.command, true);
547+
return term;
548+
}
549+
550+
// Spawn the compiler directly via proc.execute() to avoid the PTY 4096-byte
551+
// input-buffer truncation bug (https://github.com/microsoft/vscode-cmake-tools/issues/4836).
552+
// We open a Pseudoterminal so output appears in the Terminal panel without any
553+
// shell intermediary — args are passed as an array straight to child_process.spawn.
554+
const writeEmitter = new vscode.EventEmitter<string>();
555+
const closeEmitter = new vscode.EventEmitter<number>();
556+
let activeProcess: proc.Subprocess | undefined;
557+
558+
const pty: vscode.Pseudoterminal = {
559+
onDidWrite: writeEmitter.event,
560+
onDidClose: closeEmitter.event,
561+
open: () => {
562+
const executable = args[0];
563+
const execArgs = args.slice(1);
564+
writeEmitter.fire(proc.buildCmdStr(executable, execArgs) + '\r\n');
565+
activeProcess = proc.execute(executable, execArgs, {
566+
output: (line: string) => writeEmitter.fire(line + '\r\n'),
567+
error: (line: string) => writeEmitter.fire(line + '\r\n')
568+
}, { cwd: cmd.directory, environment: env });
569+
activeProcess.result.then(result => {
570+
activeProcess = undefined;
571+
const retc = result.retc ?? 0;
572+
if (retc !== 0) {
573+
writeEmitter.fire(localize('compile.finished.with.error',
574+
'Compilation finished with error(s).') + '\r\n');
575+
} else {
576+
writeEmitter.fire(localize('compile.finished.successfully',
577+
'Compilation finished successfully.') + '\r\n');
578+
}
579+
closeEmitter.fire(retc);
580+
}, (e: any) => {
581+
activeProcess = undefined;
582+
writeEmitter.fire((e?.message ?? String(e)) + '\r\n');
583+
closeEmitter.fire(-1);
584+
});
585+
},
586+
close: () => {
587+
// Terminate the compiler process if the user closes the terminal
588+
// while compilation is still running, to avoid orphaned processes.
589+
if (activeProcess?.child) {
590+
void util.termProc(activeProcess.child);
591+
activeProcess = undefined;
592+
}
589593
}
590-
existing.sendText('', true); // Send newline to execute the command
591-
} else {
592-
// Fallback to original behavior if arguments not available
593-
existing.sendText(cmd.command + '\r\n');
594-
}
595-
return existing;
594+
};
595+
596+
const term = vscode.window.createTerminal({
597+
name: localize('file.compilation', 'File Compilation'),
598+
pty
599+
});
600+
term.show();
601+
return term;
596602
}
597603

598604
/**
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
import { expect } from 'chai';
2+
import * as shlex from '@cmt/shlex';
3+
4+
/**
5+
* Tests for the compile command argument handling used by CMakeDriver.runCompileCommand().
6+
*
7+
* The fix (https://github.com/microsoft/vscode-cmake-tools/issues/4836) replaces
8+
* terminal.sendText() — which is subject to the PTY 4096-byte input buffer
9+
* truncation — with proc.execute(), which passes args as an array directly to
10+
* child_process.spawn() and has no such limit.
11+
*
12+
* These tests validate the data flow:
13+
* 1. CompilationDatabase splits a command string into an arguments array via shlex
14+
* 2. runCompileCommand() extracts args[0] as the executable and args.slice(1) as
15+
* the spawn arguments
16+
* 3. The full argument list is preserved regardless of total command length
17+
*/
18+
19+
// Mirror of proc.buildCmdStr (cannot import proc.ts directly
20+
// because it transitively depends on 'vscode').
21+
function buildCmdStr(command: string, args?: string[]): string {
22+
let cmdarr = [command];
23+
if (args) {
24+
cmdarr = cmdarr.concat(args);
25+
}
26+
return cmdarr.map(a => /[ \n\r\f;\t]/.test(a) ? `"${a}"` : a).join(' ');
27+
}
28+
29+
/**
30+
* Mirrors the argument-population logic from CompilationDatabase's constructor:
31+
* arguments: cur.arguments ? cur.arguments : [...shlex.split(cur.command)]
32+
*/
33+
function populateArguments(entry: { command: string; arguments?: string[] }): string[] {
34+
return entry.arguments ? entry.arguments : [...shlex.split(entry.command)];
35+
}
36+
37+
/**
38+
* Mirrors the executable/args extraction from runCompileCommand():
39+
* const executable = args[0];
40+
* const execArgs = args.slice(1);
41+
*/
42+
function extractExecAndArgs(args: string[]): { executable: string; execArgs: string[] } {
43+
return { executable: args[0], execArgs: args.slice(1) };
44+
}
45+
46+
suite('Compile command argument handling (issue #4836)', () => {
47+
48+
suite('CompilationDatabase argument population', () => {
49+
test('Pre-split arguments are preserved as-is', () => {
50+
const entry = {
51+
command: '/usr/bin/g++ -o main.o -c main.cpp',
52+
arguments: ['/usr/bin/g++', '-o', 'main.o', '-c', 'main.cpp']
53+
};
54+
const args = populateArguments(entry);
55+
expect(args).to.deep.equal(['/usr/bin/g++', '-o', 'main.o', '-c', 'main.cpp']);
56+
});
57+
58+
test('Command string is split via shlex when arguments not provided', () => {
59+
const entry = {
60+
command: '/usr/bin/g++ -DBOOST_THREAD_VERSION=3 -isystem ../extern -g -std=gnu++11 -o out.o -c main.cpp'
61+
};
62+
const args = populateArguments(entry);
63+
expect(args[0]).to.equal('/usr/bin/g++');
64+
expect(args).to.include('-DBOOST_THREAD_VERSION=3');
65+
expect(args).to.include('-std=gnu++11');
66+
expect(args[args.length - 1]).to.equal('main.cpp');
67+
});
68+
69+
test('Command with quoted paths is correctly split', () => {
70+
const entry = {
71+
command: '"C:\\Program Files\\MSVC\\cl.exe" /nologo /TP "-IC:\\My Project\\include" /Fo"build\\main.obj" /c "C:\\My Project\\main.cpp"'
72+
};
73+
const args = populateArguments(entry);
74+
expect(args[0]).to.equal('"C:\\Program Files\\MSVC\\cl.exe"');
75+
expect(args.length).to.be.greaterThan(1);
76+
});
77+
});
78+
79+
suite('Executable and arguments extraction', () => {
80+
test('First element is the executable, rest are arguments', () => {
81+
const args = ['/usr/bin/g++', '-o', 'main.o', '-c', 'main.cpp'];
82+
const { executable, execArgs } = extractExecAndArgs(args);
83+
expect(executable).to.equal('/usr/bin/g++');
84+
expect(execArgs).to.deep.equal(['-o', 'main.o', '-c', 'main.cpp']);
85+
});
86+
87+
test('Single-element array yields executable with no arguments', () => {
88+
const args = ['/usr/bin/g++'];
89+
const { executable, execArgs } = extractExecAndArgs(args);
90+
expect(executable).to.equal('/usr/bin/g++');
91+
expect(execArgs).to.deep.equal([]);
92+
});
93+
94+
test('MSVC-style executable with many flags', () => {
95+
const args = ['cl.exe', '/nologo', '/TP', '/DWIN32', '/D_WINDOWS', '/W3', '/GR', '/EHsc',
96+
'/MDd', '/Zi', '/Ob0', '/Od', '/RTC1', '/Fo"build\\main.obj"', '/c', 'main.cpp'];
97+
const { executable, execArgs } = extractExecAndArgs(args);
98+
expect(executable).to.equal('cl.exe');
99+
expect(execArgs.length).to.equal(15);
100+
expect(execArgs[execArgs.length - 1]).to.equal('main.cpp');
101+
});
102+
});
103+
104+
suite('Long command lines exceeding 4096 bytes', () => {
105+
/**
106+
* Generate a realistic compile command that exceeds 4096 bytes.
107+
* This simulates the real-world scenario from issue #4836 where
108+
* many -I include paths and -D defines push the command past the
109+
* PTY input buffer limit.
110+
*/
111+
function generateLongCommand(minLength: number): { command: string; expectedArgCount: number } {
112+
const compiler = '/usr/bin/g++';
113+
const baseFlags = ['-std=gnu++17', '-g', '-O2', '-Wall', '-Wextra', '-fPIC'];
114+
// Generate enough -I and -D flags to exceed the target length
115+
const extraFlags: string[] = [];
116+
for (let i = 0; extraFlags.join(' ').length < minLength; i++) {
117+
extraFlags.push(`-I/very/long/path/to/include/directory/number_${i}/nested/deeply`);
118+
extraFlags.push(`-DSOME_VERY_LONG_DEFINE_NAME_${i}=some_long_value_${i}`);
119+
}
120+
const tail = ['-o', 'CMakeFiles/myTarget.dir/src/main.cpp.o', '-c', '/home/user/project/src/main.cpp'];
121+
const allArgs = [compiler, ...baseFlags, ...extraFlags, ...tail];
122+
return {
123+
command: allArgs.join(' '),
124+
expectedArgCount: allArgs.length
125+
};
126+
}
127+
128+
test('Command exceeding 4096 chars is fully preserved when split via shlex', () => {
129+
const { command, expectedArgCount } = generateLongCommand(5000);
130+
// Verify the command actually exceeds 4096 bytes
131+
expect(command.length).to.be.greaterThan(4096);
132+
133+
const args = populateArguments({ command });
134+
expect(args.length).to.equal(expectedArgCount);
135+
expect(args[0]).to.equal('/usr/bin/g++');
136+
expect(args[args.length - 1]).to.equal('/home/user/project/src/main.cpp');
137+
});
138+
139+
test('Command exceeding 8192 chars is fully preserved', () => {
140+
const { command, expectedArgCount } = generateLongCommand(10000);
141+
expect(command.length).to.be.greaterThan(8192);
142+
143+
const args = populateArguments({ command });
144+
expect(args.length).to.equal(expectedArgCount);
145+
});
146+
147+
test('Long command roundtrips through extraction and buildCmdStr', () => {
148+
const { command } = generateLongCommand(5000);
149+
150+
const args = populateArguments({ command });
151+
const { executable, execArgs } = extractExecAndArgs(args);
152+
const displayed = buildCmdStr(executable, execArgs);
153+
154+
// The displayed string should contain the executable
155+
expect(displayed).to.include('/usr/bin/g++');
156+
// ... the last source file
157+
expect(displayed).to.include('/home/user/project/src/main.cpp');
158+
// ... and all include paths (spot-check a few)
159+
expect(displayed).to.include('-I/very/long/path/to/include/directory/number_0/nested/deeply');
160+
expect(displayed).to.include('-I/very/long/path/to/include/directory/number_10/nested/deeply');
161+
// The displayed string should also exceed 4096 chars
162+
expect(displayed.length).to.be.greaterThan(4096);
163+
});
164+
165+
test('Pre-split arguments array for a long command bypasses shlex entirely', () => {
166+
// When compile_commands.json provides "arguments" directly (CMake >= 3.something),
167+
// shlex is never invoked. Verify the array passes through untouched.
168+
const compiler = '/usr/bin/clang++';
169+
const flags: string[] = [];
170+
for (let i = 0; i < 200; i++) {
171+
flags.push(`-I/workspace/third_party/library_${i}/include`);
172+
}
173+
flags.push('-c', '/workspace/src/main.cpp');
174+
const allArgs = [compiler, ...flags];
175+
const totalLength = allArgs.join(' ').length;
176+
expect(totalLength).to.be.greaterThan(4096);
177+
178+
const args = populateArguments({ command: 'ignored', arguments: allArgs });
179+
expect(args).to.deep.equal(allArgs);
180+
expect(args.length).to.equal(allArgs.length);
181+
182+
const { executable, execArgs } = extractExecAndArgs(args);
183+
expect(executable).to.equal(compiler);
184+
expect(execArgs.length).to.equal(allArgs.length - 1);
185+
});
186+
});
187+
188+
suite('buildCmdStr display formatting', () => {
189+
test('Simple command without spaces', () => {
190+
expect(buildCmdStr('gcc', ['-c', 'main.cpp'])).to.equal('gcc -c main.cpp');
191+
});
192+
193+
test('Arguments with spaces are quoted', () => {
194+
expect(buildCmdStr('gcc', ['-I/path with spaces/include', '-c', 'main.cpp']))
195+
.to.equal('gcc "-I/path with spaces/include" -c main.cpp');
196+
});
197+
198+
test('Empty args array shows only command', () => {
199+
expect(buildCmdStr('gcc', [])).to.equal('gcc');
200+
});
201+
202+
test('Undefined args shows only command', () => {
203+
expect(buildCmdStr('gcc')).to.equal('gcc');
204+
});
205+
});
206+
});

0 commit comments

Comments
 (0)