[native_assets_cli] Make CCompilerConfig fields less nullable (#1809)

dcharkes · web-flow · commit b86e34d51803 · 2024-12-12T16:10:36.000Z
Addressing: * #1738 (comment) Currently on Flutter and the Dart CI provide this information, and they always provide all 3. So this should not be a breaking change on the protocol level. It is a breaking change on the API level.
diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart
@@ -299,17 +299,13 @@ final CCompilerConfig? dartCICompilerConfig = (() {
           .toList();
   final hasEnvScriptArgs = envScriptArgs != null && envScriptArgs.isNotEmpty;
 
-  if (cc != null ||
-      ar != null ||
-      ld != null ||
-      envScript != null ||
-      hasEnvScriptArgs) {
+  if (cc != null && ar != null && ld != null) {
     return CCompilerConfig(
-      archiver: ar != null ? Uri.file(ar) : null,
-      compiler: cc != null ? Uri.file(cc) : null,
+      archiver: Uri.file(ar),
+      compiler: Uri.file(cc),
       envScript: envScript != null ? Uri.file(envScript) : null,
       envScriptArgs: hasEnvScriptArgs ? envScriptArgs : null,
-      linker: ld != null ? Uri.file(ld) : null,
+      linker: Uri.file(ld),
     );
   }
   return null;
diff --git a/pkgs/native_assets_builder/test/helpers.dart b/pkgs/native_assets_builder/test/helpers.dart
@@ -161,13 +161,15 @@ final List<String>? _envScriptArgs = Platform
 /// Configuration for the native toolchain.
 ///
 /// Provided on Dart CI.
-final cCompiler = CCompilerConfig(
-  compiler: _cc,
-  archiver: _ar,
-  linker: _ld,
-  envScript: _envScript,
-  envScriptArgs: _envScriptArgs,
-);
+final cCompiler = (_cc == null || _ar == null || _ld == null)
+    ? null
+    : CCompilerConfig(
+        compiler: _cc!,
+        archiver: _ar!,
+        linker: _ld!,
+        envScript: _envScript,
+        envScriptArgs: _envScriptArgs,
+      );
 
 extension on String {
   Uri asFileUri() => Uri.file(this);
diff --git a/pkgs/native_assets_cli/lib/src/code_assets/c_compiler_config.dart b/pkgs/native_assets_cli/lib/src/code_assets/c_compiler_config.dart
@@ -10,13 +10,13 @@ import '../utils/map.dart';
 /// The configuration for a C toolchain.
 final class CCompilerConfig {
   /// Path to a C compiler.
-  late final Uri? compiler;
+  late final Uri compiler;
 
   /// Path to a native linker.
-  late final Uri? linker;
+  late final Uri linker;
 
   /// Path to a native archiver.
-  late final Uri? archiver;
+  late final Uri archiver;
 
   /// Path to script that sets environment variables for [compiler], [linker],
   /// and [archiver].
@@ -27,9 +27,9 @@ final class CCompilerConfig {
 
   /// Constructs a new [CCompilerConfig] based on the given toolchain tools.
   CCompilerConfig({
-    this.archiver,
-    this.compiler,
-    this.linker,
+    required this.archiver,
+    required this.compiler,
+    required this.linker,
     this.envScript,
     this.envScriptArgs,
   });
@@ -40,21 +40,21 @@ final class CCompilerConfig {
   /// [CCompilerConfig.toJson].
   factory CCompilerConfig.fromJson(Map<String, Object?> json) =>
       CCompilerConfig(
-        archiver: json.optionalPath(_arConfigKey),
-        compiler: json.optionalPath(_ccConfigKey),
+        archiver: json.path(_arConfigKey),
+        compiler: json.path(_ccConfigKey),
         envScript: json.optionalPath(_envScriptConfigKey),
         envScriptArgs: json.optionalStringList(_envScriptArgsConfigKey),
-        linker: json.optionalPath(_ldConfigKey),
+        linker: json.path(_ldConfigKey),
       );
 
   /// The json representation of this [CCompilerConfig].
   ///
   /// The returned json can be used in [CCompilerConfig.fromJson] to
   /// obtain a [CCompilerConfig] again.
   Map<String, Object> toJson() => {
-        if (archiver != null) _arConfigKey: archiver!.toFilePath(),
-        if (compiler != null) _ccConfigKey: compiler!.toFilePath(),
-        if (linker != null) _ldConfigKey: linker!.toFilePath(),
+        _arConfigKey: archiver.toFilePath(),
+        _ccConfigKey: compiler.toFilePath(),
+        _ldConfigKey: linker.toFilePath(),
         if (envScript != null) _envScriptConfigKey: envScript!.toFilePath(),
         if (envScriptArgs != null) _envScriptArgsConfigKey: envScriptArgs!,
       }.sortOnKey();
diff --git a/pkgs/native_assets_cli/lib/src/code_assets/config.dart b/pkgs/native_assets_cli/lib/src/code_assets/config.dart
@@ -42,7 +42,7 @@ class CodeConfig {
   final Architecture? _targetArchitecture;
 
   final LinkModePreference linkModePreference;
-  final CCompilerConfig cCompiler;
+  final CCompilerConfig? cCompiler;
   final int? targetIOSVersion;
   final int? targetMacOSVersion;
   final int? targetAndroidNdkApi;
@@ -62,7 +62,7 @@ class CodeConfig {
         targetOS = OS.fromString(config.json.string(_targetOSConfigKey)),
         cCompiler = switch (config.json.optionalMap(_compilerKey)) {
           final Map<String, Object?> map => CCompilerConfig.fromJson(map),
-          null => CCompilerConfig(),
+          null => null,
         },
         targetIOSVersion = config.json.optionalInt(_targetIOSVersionKey),
         targetMacOSVersion = config.json.optionalInt(_targetMacOSVersionKey),
diff --git a/pkgs/native_assets_cli/lib/src/code_assets/validation.dart b/pkgs/native_assets_cli/lib/src/code_assets/validation.dart
@@ -53,21 +53,24 @@ ValidationErrors _validateCodeConfig(
       break;
   }
   final compilerConfig = codeConfig.cCompiler;
-  final compiler = compilerConfig.compiler?.toFilePath();
-  if (compiler != null && !File(compiler).existsSync()) {
-    errors.add('$configName.codeConfig.compiler ($compiler) does not exist.');
-  }
-  final linker = compilerConfig.linker?.toFilePath();
-  if (linker != null && !File(linker).existsSync()) {
-    errors.add('$configName.codeConfig.linker ($linker) does not exist.');
-  }
-  final archiver = compilerConfig.archiver?.toFilePath();
-  if (archiver != null && !File(archiver).existsSync()) {
-    errors.add('$configName.codeConfig.archiver ($archiver) does not exist.');
-  }
-  final envScript = compilerConfig.envScript?.toFilePath();
-  if (envScript != null && !File(envScript).existsSync()) {
-    errors.add('$configName.codeConfig.envScript ($envScript) does not exist.');
+  if (compilerConfig != null) {
+    final compiler = compilerConfig.compiler.toFilePath();
+    if (!File(compiler).existsSync()) {
+      errors.add('$configName.codeConfig.compiler ($compiler) does not exist.');
+    }
+    final linker = compilerConfig.linker.toFilePath();
+    if (!File(linker).existsSync()) {
+      errors.add('$configName.codeConfig.linker ($linker) does not exist.');
+    }
+    final archiver = compilerConfig.archiver.toFilePath();
+    if (!File(archiver).existsSync()) {
+      errors.add('$configName.codeConfig.archiver ($archiver) does not exist.');
+    }
+    final envScript = compilerConfig.envScript?.toFilePath();
+    if (envScript != null && !File(envScript).existsSync()) {
+      errors
+          .add('$configName.codeConfig.envScript ($envScript) does not exist.');
+    }
   }
   return errors;
 }
diff --git a/pkgs/native_assets_cli/test/code_assets/config_test.dart b/pkgs/native_assets_cli/test/code_assets/config_test.dart
@@ -55,9 +55,7 @@ void main() async {
     expect(() => codeConfig.targetArchitecture, throwsStateError);
     expect(codeConfig.targetAndroidNdkApi, null);
     expect(codeConfig.linkModePreference, LinkModePreference.preferStatic);
-    expect(codeConfig.cCompiler.compiler, null);
-    expect(codeConfig.cCompiler.linker, null);
-    expect(codeConfig.cCompiler.archiver, null);
+    expect(codeConfig.cCompiler, null);
   }
 
   void expectCorrectCodeConfig(
@@ -81,9 +79,9 @@ void main() async {
     expect(codeConfig.targetArchitecture, Architecture.arm64);
     expect(codeConfig.targetAndroidNdkApi, 30);
     expect(codeConfig.linkModePreference, LinkModePreference.preferStatic);
-    expect(codeConfig.cCompiler.compiler, fakeClang);
-    expect(codeConfig.cCompiler.linker, fakeLd);
-    expect(codeConfig.cCompiler.archiver, fakeAr);
+    expect(codeConfig.cCompiler?.compiler, fakeClang);
+    expect(codeConfig.cCompiler?.linker, fakeLd);
+    expect(codeConfig.cCompiler?.archiver, fakeAr);
   }
 
   test('BuildConfig.codeConfig (dry-run)', () {
diff --git a/pkgs/native_assets_cli/test/helpers.dart b/pkgs/native_assets_cli/test/helpers.dart
@@ -115,13 +115,15 @@ final List<String>? _envScriptArgs = Platform
 /// Configuration for the native toolchain.
 ///
 /// Provided on Dart CI.
-final cCompiler = CCompilerConfig(
-  compiler: _cc,
-  archiver: _ar,
-  linker: _ld,
-  envScript: _envScript,
-  envScriptArgs: _envScriptArgs,
-);
+final cCompiler = (_cc == null || _ar == null || _ld == null)
+    ? null
+    : CCompilerConfig(
+        compiler: _cc!,
+        archiver: _ar!,
+        linker: _ld!,
+        envScript: _envScript,
+        envScriptArgs: _envScriptArgs,
+      );
 
 extension on String {
   Uri asFileUri() => Uri.file(this);
diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart
@@ -99,7 +99,7 @@ class CompilerResolver {
   }
 
   Future<ToolInstance?> _tryLoadCompilerFromConfig() async {
-    final configCcUri = codeConfig.cCompiler.compiler;
+    final configCcUri = codeConfig.cCompiler?.compiler;
     if (configCcUri != null) {
       assert(await File.fromUri(configCcUri).exists());
       logger?.finer('Using compiler ${configCcUri.toFilePath()} '
@@ -184,7 +184,7 @@ class CompilerResolver {
   }
 
   Future<ToolInstance?> _tryLoadArchiverFromConfig() async {
-    final configArUri = codeConfig.cCompiler.archiver;
+    final configArUri = codeConfig.cCompiler?.archiver;
     if (configArUri != null) {
       assert(await File.fromUri(configArUri).exists());
       logger?.finer('Using archiver ${configArUri.toFilePath()} '
@@ -197,7 +197,7 @@ class CompilerResolver {
   }
 
   Future<Uri?> toolchainEnvironmentScript(ToolInstance compiler) async {
-    final fromConfig = codeConfig.cCompiler.envScript;
+    final fromConfig = codeConfig.cCompiler?.envScript;
     if (fromConfig != null) {
       logger?.fine('Using envScript from config: $fromConfig');
       return fromConfig;
@@ -211,7 +211,7 @@ class CompilerResolver {
   }
 
   List<String>? toolchainEnvironmentScriptArguments() {
-    final fromConfig = codeConfig.cCompiler.envScriptArgs;
+    final fromConfig = codeConfig.cCompiler?.envScriptArgs;
     if (fromConfig != null) {
       logger?.fine('Using envScriptArgs from config: $fromConfig');
       return fromConfig;
@@ -245,7 +245,7 @@ class CompilerResolver {
   }
 
   Future<ToolInstance?> _tryLoadLinkerFromConfig() async {
-    final configLdUri = codeConfig.cCompiler.linker;
+    final configLdUri = codeConfig.cCompiler?.linker;
     if (configLdUri != null) {
       assert(await File.fromUri(configLdUri).exists());
       logger?.finer('Using linker ${configLdUri.toFilePath()} '
diff --git a/pkgs/native_toolchain_c/test/cbuilder/compiler_resolver_test.dart b/pkgs/native_toolchain_c/test/cbuilder/compiler_resolver_test.dart
@@ -71,8 +71,8 @@ void main() {
         CompilerResolver(codeConfig: buildConfig.codeConfig, logger: logger);
     final compiler = await resolver.resolveCompiler();
     final archiver = await resolver.resolveArchiver();
-    expect(compiler.uri, buildConfig.codeConfig.cCompiler.compiler);
-    expect(archiver.uri, buildConfig.codeConfig.cCompiler.archiver);
+    expect(compiler.uri, buildConfig.codeConfig.cCompiler?.compiler);
+    expect(archiver.uri, buildConfig.codeConfig.cCompiler?.archiver);
   });
 
   test('No compiler found', () async {
diff --git a/pkgs/native_toolchain_c/test/helpers.dart b/pkgs/native_toolchain_c/test/helpers.dart
@@ -155,13 +155,15 @@ final List<String>? _envScriptArgs = Platform
 /// Configuration for the native toolchain.
 ///
 /// Provided on Dart CI.
-final cCompiler = CCompilerConfig(
-  compiler: _cc,
-  archiver: _ar,
-  linker: _ld,
-  envScript: _envScript,
-  envScriptArgs: _envScriptArgs,
-);
+final cCompiler = (_cc == null || _ar == null || _ld == null)
+    ? null
+    : CCompilerConfig(
+        compiler: _cc!,
+        archiver: _ar!,
+        linker: _ld!,
+        envScript: _envScript,
+        envScriptArgs: _envScriptArgs,
+      );
 
 extension on String {
   Uri asFileUri() => Uri.file(this);
