Skip to content

Commit 3b8fe87

Browse files
hanniavaleraHannia Valera
andauthored
[Fix] Reduce logging verbosity during CMake configure and build failures (#4759)
* Reduce logging verbosity during CMake configure and build failures; update logging levels for output and error messages in CMakeOutputConsumer. * fixing bad merge --------- Co-authored-by: Hannia Valera <[email protected]>
1 parent 009f352 commit 3b8fe87

6 files changed

Lines changed: 71 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Improvements:
1818
- Allow preset modification commands to target CMakeUserPresets.json. The target file is determined by the focused editor, or by prompting the user when both files exist. [#4564](https://github.com/microsoft/vscode-cmake-tools/issues/4564)
1919

2020
Bug Fixes:
21+
- 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)
2122
- Fix Test Results panel not hyperlinking file paths for GoogleTest failures. The default `cmake.ctest.failurePatterns` now includes a pattern matching GoogleTest's `file:line: Failure` output format. [#4589](https://github.com/microsoft/vscode-cmake-tools/issues/4589)
2223
- Fix wrong path created for artifact when parsing code model using CMake File API. [#3015](https://github.com/microsoft/vscode-cmake-tools/issues/3015)
2324
- Fix garbled characters (Mojibake) in the Output panel when MSVC outputs UTF-8 (e.g., with `/utf-8`) on non-UTF-8 Windows systems. When `cmake.outputLogEncoding` is `auto`, the build output is now validated as UTF-8 before falling back to the system code page. [#4520](https://github.com/microsoft/vscode-cmake-tools/issues/4520)

src/cmakeProject.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1905,7 +1905,6 @@ export class CMakeProject {
19051905
log.debug(localize('no.preset.abort', 'No preset selected. Abort configure'));
19061906
return { exitCode: -1, resultType: ConfigureResultType.Other };
19071907
}
1908-
log.showChannel();
19091908
const consumer = new CMakeOutputConsumer(this.sourceDir, cmakeLogger);
19101909
const result = await cb(consumer);
19111910
result.stdout = consumer.stdout;
@@ -2135,7 +2134,6 @@ export class CMakeProject {
21352134
});
21362135
const combinedToken = util.createCombinedCancellationToken(cancel, cancellationToken);
21372136
combinedToken.onCancellationRequested(() => rollbar.invokeAsync(localize('stop.on.cancellation', 'Stop on cancellation'), () => this.stop()));
2138-
log.showChannel();
21392137
buildLogger.info(localize('starting.build', 'Starting build'));
21402138
await setContextAndStore(isBuildingKey, true);
21412139
const rc = await drv!.build(newTargets, consumer, isBuildCommand);

src/diagnostics/cmake.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class CMakeOutputConsumer extends CommandConsumer {
5151
*/
5252
output(line: string) {
5353
if (this.logger) {
54-
this.logger.info(line);
54+
this.logger.trace(line);
5555
}
5656
super.output(line);
5757
this._parseDiags(line);
@@ -88,7 +88,7 @@ export class CMakeOutputConsumer extends CommandConsumer {
8888
error(line: string) {
8989
// First, just log the line
9090
if (this.logger) {
91-
this.logger.error(line);
91+
this.logger.warning(line);
9292
}
9393
super.error(line);
9494
this._parseDiags(line);

src/drivers/cmakeFileApiDriver.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,8 +306,6 @@ export class CMakeFileApiDriver extends CMakeDriver {
306306

307307
const result = await child.result;
308308
this.configureProcess = null;
309-
log.trace(result.stderr);
310-
log.trace(result.stdout);
311309
if (result.retc === 0) {
312310
if (!configurePreset || (configurePreset && defaultConfigurePresetName && configurePreset.name === defaultConfigurePresetName)) {
313311
this._needsReconfigure = false;

src/drivers/cmakeLegacyDriver.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,6 @@ export class CMakeLegacyDriver extends CMakeDriver {
126126
this.configureProcess = child;
127127
const result = await child.result;
128128
this.configureProcess = null;
129-
log.trace(result.stderr);
130-
log.trace(result.stdout);
131129
if (result.retc === 0 && (!configurePreset || (configurePreset && defaultConfigurePresetName && configurePreset.name === defaultConfigurePresetName))) {
132130
this._needsReconfigure = false;
133131
}

test/unit-tests/diagnostics.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@ import { CMakeOutputConsumer } from '@cmt/diagnostics/cmake';
1717
import { populateCollection } from '@cmt/diagnostics/util';
1818
import collections from '@cmt/diagnostics/collections';
1919
import { getTestResourceFilePath } from '@test/util';
20+
import { Logger } from '@cmt/logging';
21+
22+
/** A spy logger that records which log methods were called and at what level. */
23+
class SpyLogger extends Logger {
24+
readonly calls: { level: string; args: string[] }[] = [];
25+
constructor() {
26+
super('spy');
27+
}
28+
override trace(...args: any[]) {
29+
this.calls.push({ level: 'trace', args: args.map(String) });
30+
}
31+
override debug(...args: any[]) {
32+
this.calls.push({ level: 'debug', args: args.map(String) });
33+
}
34+
override info(...args: any[]) {
35+
this.calls.push({ level: 'info', args: args.map(String) });
36+
}
37+
override warning(...args: any[]) {
38+
this.calls.push({ level: 'warning', args: args.map(String) });
39+
}
40+
override error(...args: any[]) {
41+
this.calls.push({ level: 'error', args: args.map(String) });
42+
}
43+
}
2044

2145
function feedLines(consumer: OutputConsumer, output: string[], error: string[]) {
2246
for (const line of output) {
@@ -1254,6 +1278,50 @@ suite('Diagnostics', () => {
12541278
expect(collections.presets.has(testUri)).to.be.false;
12551279
});
12561280

1281+
test('CMakeOutputConsumer logs stdout lines at trace level, not info', () => {
1282+
const spy = new SpyLogger();
1283+
const consumerWithLogger = new CMakeOutputConsumer('dummyPath', spy);
1284+
consumerWithLogger.output('-- Configuring done');
1285+
consumerWithLogger.output('-- Generating done');
1286+
1287+
const traceCalls = spy.calls.filter(c => c.level === 'trace');
1288+
const infoCalls = spy.calls.filter(c => c.level === 'info');
1289+
expect(traceCalls.length).to.eq(2);
1290+
expect(infoCalls.length).to.eq(0);
1291+
});
1292+
1293+
test('CMakeOutputConsumer logs stderr lines at warning level, not error', () => {
1294+
const spy = new SpyLogger();
1295+
const consumerWithLogger = new CMakeOutputConsumer('dummyPath', spy);
1296+
consumerWithLogger.error('CMake Error at CMakeLists.txt:1 (message):');
1297+
consumerWithLogger.error(' Something went wrong');
1298+
1299+
const warningCalls = spy.calls.filter(c => c.level === 'warning');
1300+
const errorCalls = spy.calls.filter(c => c.level === 'error');
1301+
expect(warningCalls.length).to.eq(2);
1302+
expect(errorCalls.length).to.eq(0);
1303+
});
1304+
1305+
test('CMakeOutputConsumer still parses diagnostics after log level change', () => {
1306+
const spy = new SpyLogger();
1307+
const consumerWithLogger = new CMakeOutputConsumer('dummyPath', spy);
1308+
const error_output = [
1309+
'CMake Warning at CMakeLists.txt:14 (message):',
1310+
' I am a warning!',
1311+
'',
1312+
''
1313+
];
1314+
for (const line of error_output) {
1315+
consumerWithLogger.error(line);
1316+
}
1317+
// Diagnostics should still be parsed correctly regardless of log level
1318+
expect(consumerWithLogger.diagnostics.length).to.eq(1);
1319+
const diag = consumerWithLogger.diagnostics[0];
1320+
expect(diag.filepath).to.eq('dummyPath/CMakeLists.txt');
1321+
expect(diag.diag.severity).to.eq(vscode.DiagnosticSeverity.Warning);
1322+
expect(diag.diag.message).to.match(/I am a warning!$/);
1323+
});
1324+
12571325
// ===== Custom Problem Matcher (cmake.additionalBuildProblemMatchers) tests =====
12581326

12591327
test('CustomParser matches clang-tidy-style output', () => {

0 commit comments

Comments
 (0)