[native_toolchain_c] Include missing errors output when running CL (#2811)

CL outputs error-related messages to the standard output. This patch allows outputting these errors to the user while preserving the current level of logging for other toolchains.
Include `createCapturingRecordLogger` utility to check the level of a `LogRecord`, otherwise the test would always succeed.

Closes #2809

Author: poirierlouis

pkgs/native_toolchain_c/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.17.4
+
+- For Windows, include errors from the standard output of `cl` in the logger's
+  output of CBuilder.
+  ([#2809](https://github.com/dart-lang/native/issues/2809)) 
+
 ## 0.17.3
 
 - Bump `package:hooks` and `package:code_assets`to 1.0.0.

pkgs/native_toolchain_c/lib/src/cbuilder/run_cbuilder.dart

@@ -399,6 +399,7 @@ class RunCBuilder {
       environment: environment,
       logger: logger,
       captureOutput: false,
+      stdoutLogLevel: Level.INFO,
       throwOnUnexpectedExitCode: true,
     );
 
@@ -410,6 +411,7 @@ class RunCBuilder {
         environment: environment,
         logger: logger,
         captureOutput: false,
+        stdoutLogLevel: Level.INFO,
         throwOnUnexpectedExitCode: true,
       );
     }

pkgs/native_toolchain_c/lib/src/utils/run_process.dart

@@ -19,6 +19,7 @@ Future<RunProcessResult> runProcess({
   Map<String, String>? environment,
   required Logger? logger,
   bool captureOutput = true,
+  Level stdoutLogLevel = Level.FINE,
   int expectedExitCode = 0,
   bool throwOnUnexpectedExitCode = false,
 }) async {
@@ -46,7 +47,7 @@ Future<RunProcessResult> runProcess({
   final stdoutSub = process.stdout.listen((List<int> data) {
     try {
       final decoded = systemEncoding.decode(data);
-      logger?.fine(decoded);
+      logger?.log(stdoutLogLevel, decoded);
       if (captureOutput) {
         stdoutBuffer.write(decoded);
       }

pkgs/native_toolchain_c/pubspec.yaml

@@ -1,7 +1,7 @@
 name: native_toolchain_c
 description: >-
   A library to invoke the native C compiler installed on the host machine.
-version: 0.17.3
+version: 0.17.4
 repository: https://github.com/dart-lang/native/tree/main/pkgs/native_toolchain_c
 
 topics:

pkgs/native_toolchain_c/test/cbuilder/cbuilder_build_failure_test.dart

@@ -8,7 +8,9 @@ library;
 import 'dart:io';
 
 import 'package:code_assets/code_assets.dart';
+import 'package:collection/collection.dart';
 import 'package:hooks/hooks.dart';
+import 'package:logging/logging.dart';
 import 'package:native_toolchain_c/native_toolchain_c.dart';
 import 'package:test/test.dart';
 
@@ -68,4 +70,60 @@ void main() {
       throwsException,
     );
   });
+
+  test('CL build failure include error output', () async {
+    if (!Platform.isWindows) {
+      // Avoid needing status files on Dart SDK CI.
+      return;
+    }
+
+    final tempUri = await tempDirForTest();
+    final tempUri2 = await tempDirForTest();
+    final source = packageUri.resolve(
+      'test/cbuilder/testfiles/build_failure/cl.c',
+    );
+    const name = 'cl';
+
+    final buildInputBuilder = BuildInputBuilder()
+      ..setupShared(
+        packageName: name,
+        packageRoot: tempUri,
+        outputFile: tempUri.resolve('output.json'),
+        outputDirectoryShared: tempUri2,
+      )
+      ..config.setupBuild(linkingEnabled: false)
+      ..addExtension(
+        CodeAssetExtension(
+          targetOS: OS.windows,
+          targetArchitecture: Architecture.current,
+          linkModePreference: LinkModePreference.dynamic,
+          cCompiler: cCompiler,
+        ),
+      );
+
+    final buildInput = buildInputBuilder.build();
+    final buildOutput = BuildOutputBuilder();
+
+    final logs = <LogRecord>[];
+    final logger = createCapturingRecordLogger(logs);
+    final cbuilder = CBuilder.library(
+      sources: [source.toFilePath()],
+      name: name,
+      assetName: name,
+      includes: [],
+      buildMode: BuildMode.release,
+    );
+    await expectLater(
+      cbuilder.run(input: buildInput, output: buildOutput, logger: logger),
+      throwsException,
+    );
+
+    // Note: don't check the entire message as CL output is based on user
+    //       locale.
+    final line = logs.firstWhereOrNull(
+      (log) =>
+          log.level == Level.INFO && log.message.contains('fatal error C1070'),
+    );
+    expect(line != null, true);
+  });
 }

pkgs/native_toolchain_c/test/cbuilder/testfiles/build_failure/cl.c

@@ -0,0 +1,5 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#include "cl.h"

pkgs/native_toolchain_c/test/cbuilder/testfiles/build_failure/cl.h

@@ -0,0 +1,14 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+#ifndef BUILD_FAILURE_CL_H
+#define BUILD_FAILURE_CL_H
+
+#ifdef _WIN32
+#define INCLUDE_EXPORT __declspec(dllexport)
+#else
+#define INCLUDE_EXPORT
+//#endif
+
+#endif

pkgs/native_toolchain_c/test/helpers.dart

@@ -71,15 +71,19 @@ Logger? _logger;
 Logger createCapturingLogger(List<String> capturedMessages) =>
     _createTestLogger(capturedMessages: capturedMessages);
 
-Logger _createTestLogger({List<String>? capturedMessages}) =>
-    Logger.detached('')
-      ..level = Level.ALL
-      ..onRecord.listen((record) {
-        printOnFailure(
-          '${record.level.name}: ${record.time}: ${record.message}',
-        );
-        capturedMessages?.add(record.message);
-      });
+Logger createCapturingRecordLogger(List<LogRecord> capturedLogs) =>
+    _createTestLogger(capturedLogs: capturedLogs);
+
+Logger _createTestLogger({
+  List<String>? capturedMessages,
+  List<LogRecord>? capturedLogs,
+}) => Logger.detached('')
+  ..level = Level.ALL
+  ..onRecord.listen((record) {
+    printOnFailure('${record.level.name}: ${record.time}: ${record.message}');
+    capturedMessages?.add(record.message);
+    capturedLogs?.add(record);
+  });
 
 Uri packageUri = findPackageRoot('native_toolchain_c');