From b86e34d518033c55e141ffd47e489aa6c45352a7 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Thu, 12 Dec 2024 17:10:36 +0100 Subject: [PATCH] [native_assets_cli] Make `CCompilerConfig` fields less nullable (#1809) Addressing: * https://github.com/dart-lang/native/issues/1738#issuecomment-2535959430 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. --- .../test/build_runner/helpers.dart | 12 +++---- pkgs/native_assets_builder/test/helpers.dart | 16 +++++---- .../src/code_assets/c_compiler_config.dart | 24 +++++++------- .../lib/src/code_assets/config.dart | 4 +-- .../lib/src/code_assets/validation.dart | 33 ++++++++++--------- .../test/code_assets/config_test.dart | 10 +++--- pkgs/native_assets_cli/test/helpers.dart | 16 +++++---- .../lib/src/cbuilder/compiler_resolver.dart | 10 +++--- .../test/cbuilder/compiler_resolver_test.dart | 4 +-- pkgs/native_toolchain_c/test/helpers.dart | 16 +++++---- 10 files changed, 74 insertions(+), 71 deletions(-) diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart index 6be66afea..432b5dade 100644 --- 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 index ba864da71..0242bcd06 100644 --- a/pkgs/native_assets_builder/test/helpers.dart +++ b/pkgs/native_assets_builder/test/helpers.dart @@ -161,13 +161,15 @@ final List? _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 index 9c23236a2..32384f282 100644 --- 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,11 +40,11 @@ final class CCompilerConfig { /// [CCompilerConfig.toJson]. factory CCompilerConfig.fromJson(Map 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]. @@ -52,9 +52,9 @@ final class CCompilerConfig { /// The returned json can be used in [CCompilerConfig.fromJson] to /// obtain a [CCompilerConfig] again. Map 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 index 6f749a106..9ae1fe9f4 100644 --- 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 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 index b01dcc559..40b17f8ec 100644 --- 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 index fe7943afa..74ce351d3 100644 --- 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 index 6fbc01635..cdd9c5d2b 100644 --- a/pkgs/native_assets_cli/test/helpers.dart +++ b/pkgs/native_assets_cli/test/helpers.dart @@ -115,13 +115,15 @@ final List? _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 index 3e4854764..e329435eb 100644 --- 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 _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 _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 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? 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 _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 index 929312368..8578f2893 100644 --- 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 index ac5a51034..b2c647259 100644 --- a/pkgs/native_toolchain_c/test/helpers.dart +++ b/pkgs/native_toolchain_c/test/helpers.dart @@ -155,13 +155,15 @@ final List? _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);