[native_assets_cli] Move targetOS to CodeConfig (#1806)

Next refactoring in: * #1738 We move `config.targetOS` to `config.codeConfig.targetOS`. Other asset types might not get access on the target OS. This will require a manual roll into dartdev and flutter tools due to the targetOS now having to be passed in via `setupCodeConfig` instead of the base config. The native assets builder will still emit the same JSON if code assets are supported. Since both Dart and Flutter support code assets currently, this will not break anything. Once data assets for web in Flutter are added, all existing build hooks that assume code assets are always supported will break.
dart-lang · Dec 11, 2024 · 2542108 · 2542108
1 parent 2cecc88
commit 2542108
Show file tree

Hide file tree

Showing 53 changed files with 251 additions and 205 deletions.
diff --git a/pkgs/native_assets_builder/CHANGELOG.md b/pkgs/native_assets_builder/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 - Removed support for dry run (Flutter no long requires it).
 - Various fixes to caching.
+- **Breaking change** `BuildConfig.targetOS` is now only provided if
+  `buildAssetTypes` contains the code asset.
 
 ## 0.9.0
 

diff --git a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
@@ -92,7 +92,6 @@ class NativeAssetsBuildRunner {
     required BuildConfigValidator configValidator,
     required BuildValidator buildValidator,
     required ApplicationAssetValidator applicationAssetValidator,
-    required OS targetOS,
     required Uri workingDirectory,
     PackageLayout? packageLayout,
     String? runPackageName,
@@ -121,7 +120,6 @@ class NativeAssetsBuildRunner {
 
       final configBuilder = configCreator()
         ..setupHookConfig(
-          targetOS: targetOS,
           buildAssetTypes: buildAssetTypes,
           packageName: package.name,
           packageRoot: packageLayout.packageRoot(package.name),
@@ -197,7 +195,6 @@ class NativeAssetsBuildRunner {
     required LinkConfigCreator configCreator,
     required LinkConfigValidator configValidator,
     required LinkValidator linkValidator,
-    required OS targetOS,
     required Uri workingDirectory,
     required ApplicationAssetValidator applicationAssetValidator,
     PackageLayout? packageLayout,
@@ -220,7 +217,6 @@ class NativeAssetsBuildRunner {
     for (final package in buildPlan) {
       final configBuilder = configCreator()
         ..setupHookConfig(
-          targetOS: targetOS,
           buildAssetTypes: buildAssetTypes,
           packageName: package.name,
           packageRoot: packageLayout.packageRoot(package.name),

diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart
@@ -30,12 +30,12 @@ void main() async {
       BuildConfigBuilder configCreator() => BuildConfigBuilder()
         ..setupCodeConfig(
           targetArchitecture: Architecture.current,
+          targetOS: OS.current,
           linkModePreference: LinkModePreference.dynamic,
         );
 
       await buildRunner.build(
         configCreator: configCreator,
-        targetOS: OS.current,
         workingDirectory: packageUri,
         linkingEnabled: false,
         buildAssetTypes: [],
@@ -45,7 +45,6 @@ void main() async {
       );
       await buildRunner.build(
         configCreator: configCreator,
-        targetOS: OS.current,
         workingDirectory: packageUri,
         linkingEnabled: false,
         buildAssetTypes: [],

diff --git a/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart b/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart
@@ -20,8 +20,14 @@ void main(List<String> args) async {
     logger: logger,
     dartExecutable: dartExecutable,
   ).build(
-    configCreator: BuildConfigBuilder.new,
-    targetOS: target.os,
+    // Set up the code config, so that the builds for different targets are
+    // in different directories.
+    configCreator: () => BuildConfigBuilder()
+      ..setupCodeConfig(
+        targetArchitecture: target.architecture,
+        targetOS: target.os,
+        linkModePreference: LinkModePreference.dynamic,
+      ),
     workingDirectory: packageUri,
     linkingEnabled: false,
     buildAssetTypes: [DataAsset.type],

diff --git a/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart b/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart
@@ -28,11 +28,11 @@ void main(List<String> args) async {
     configCreator: () => BuildConfigBuilder()
       ..setupCodeConfig(
         targetArchitecture: Architecture.current,
+        targetOS: OS.current,
         linkModePreference: LinkModePreference.dynamic,
         cCompilerConfig: dartCICompilerConfig,
         targetMacOSVersion: OS.current == OS.macOS ? defaultMacOSVersion : null,
       ),
-    targetOS: OS.current,
     workingDirectory: packageUri,
     linkingEnabled: false,
     buildAssetTypes: [CodeAsset.type, DataAsset.type],

diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart
@@ -59,6 +59,7 @@ Future<BuildResult?> build(
         if (buildAssetTypes.contains(CodeAsset.type)) {
           configBuilder.setupCodeConfig(
             targetArchitecture: target?.architecture ?? Architecture.current,
+            targetOS: targetOS,
             linkModePreference: linkModePreference,
             cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
             targetIOSSdk: targetIOSSdk,
@@ -71,7 +72,6 @@ Future<BuildResult?> build(
         return configBuilder;
       },
       configValidator: configValidator,
-      targetOS: targetOS,
       workingDirectory: packageUri,
       packageLayout: packageLayout,
       runPackageName: runPackageName,
@@ -124,6 +124,7 @@ Future<LinkResult?> link(
         if (buildAssetTypes.contains(CodeAsset.type)) {
           configBuilder.setupCodeConfig(
             targetArchitecture: target?.architecture ?? Architecture.current,
+            targetOS: target?.os ?? OS.current,
             linkModePreference: linkModePreference,
             cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
             targetIOSSdk: targetIOSSdk,
@@ -136,7 +137,6 @@ Future<LinkResult?> link(
         return configBuilder;
       },
       configValidator: configValidator,
-      targetOS: target?.os ?? OS.current,
       workingDirectory: packageUri,
       packageLayout: packageLayout,
       buildResult: buildResult,
@@ -185,6 +185,7 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
         configCreator: () => BuildConfigBuilder()
           ..setupCodeConfig(
             targetArchitecture: target?.architecture ?? Architecture.current,
+            targetOS: target?.os ?? OS.current,
             linkModePreference: linkModePreference,
             cCompilerConfig: cCompilerConfig ?? dartCICompilerConfig,
             targetIOSSdk: targetIOSSdk,
@@ -193,7 +194,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
             targetAndroidNdkApi: targetAndroidNdkApi,
           ),
         configValidator: buildConfigValidator,
-        targetOS: target?.os ?? OS.current,
         workingDirectory: packageUri,
         packageLayout: packageLayout,
         runPackageName: runPackageName,
@@ -217,6 +217,7 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
         configCreator: () => LinkConfigBuilder()
           ..setupCodeConfig(
             targetArchitecture: target?.architecture ?? Architecture.current,
+            targetOS: target?.os ?? OS.current,
             linkModePreference: linkModePreference,
             cCompilerConfig: cCompilerConfig,
             targetIOSSdk: targetIOSSdk,
@@ -225,7 +226,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
             targetAndroidNdkApi: targetAndroidNdkApi,
           ),
         configValidator: linkConfigValidator,
-        targetOS: target?.os ?? OS.current,
         workingDirectory: packageUri,
         packageLayout: packageLayout,
         buildResult: buildResult,

diff --git a/pkgs/native_assets_builder/test/test_data/native_dynamic_linking_test.dart b/pkgs/native_assets_builder/test/test_data/native_dynamic_linking_test.dart
@@ -34,7 +34,6 @@ void main() async {
         ..setupHookConfig(
           packageName: name,
           packageRoot: testPackageUri,
-          targetOS: OS.current,
           buildAssetTypes: [CodeAsset.type],
         )
         ..setupBuildConfig(dryRun: false, linkingEnabled: false)
@@ -44,6 +43,7 @@ void main() async {
         )
         ..setupCodeConfig(
           targetArchitecture: Architecture.current,
+          targetOS: OS.current,
           linkModePreference: LinkModePreference.dynamic,
           cCompilerConfig: cCompiler,
         );

diff --git a/pkgs/native_assets_builder/test/test_data/transformer_test.dart b/pkgs/native_assets_builder/test/test_data/transformer_test.dart
@@ -49,7 +49,6 @@ void main() async {
           ..setupHookConfig(
             packageName: packageName,
             packageRoot: packageUri,
-            targetOS: OS.current,
             buildAssetTypes: [DataAsset.type],
           )
           ..setupBuildConfig(dryRun: false, linkingEnabled: false)
@@ -59,6 +58,7 @@ void main() async {
           )
           ..setupCodeConfig(
             targetArchitecture: architecture,
+            targetOS: OS.current,
             linkModePreference: LinkModePreference.dynamic,
           );
 

diff --git a/pkgs/native_assets_builder/test_data/fail_on_os_sdk_version/hook/build.dart b/pkgs/native_assets_builder/test_data/fail_on_os_sdk_version/hook/build.dart
@@ -14,15 +14,15 @@ const minMacOSVersionForThisPackage = 13;
 
 void main(List<String> arguments) async {
   await build(arguments, (config, output) async {
-    if (config.targetOS == OS.android) {
+    if (config.codeConfig.targetOS == OS.android) {
       if (config.codeConfig.targetAndroidNdkApi! <
           minNdkApiVersionForThisPackage) {
         throw UnsupportedError(
           'The native assets for this package require at '
           'least Android NDK API level $minNdkApiVersionForThisPackage.',
         );
       }
-    } else if (config.targetOS == OS.iOS) {
+    } else if (config.codeConfig.targetOS == OS.iOS) {
       final iosVersion = config.codeConfig.targetIOSVersion;
       // iosVersion is nullable to deal with version skew.
       if (iosVersion != null && iosVersion < minIosVersionForThisPackage) {
@@ -31,7 +31,7 @@ void main(List<String> arguments) async {
           'least iOS version $minIosVersionForThisPackage.',
         );
       }
-    } else if (config.targetOS == OS.macOS) {
+    } else if (config.codeConfig.targetOS == OS.macOS) {
       final macosVersion = config.codeConfig.targetMacOSVersion;
       // macosVersion is nullable to deal with version skew.
       if (macosVersion != null &&

diff --git a/pkgs/native_assets_builder/test_data/fail_on_os_sdk_version_linker/hook/link.dart b/pkgs/native_assets_builder/test_data/fail_on_os_sdk_version_linker/hook/link.dart
@@ -14,15 +14,15 @@ const minMacOSVersionForThisPackage = 13;
 
 void main(List<String> arguments) async {
   await link(arguments, (config, output) async {
-    if (config.targetOS == OS.android) {
+    if (config.codeConfig.targetOS == OS.android) {
       if (config.codeConfig.targetAndroidNdkApi! <
           minNdkApiVersionForThisPackage) {
         throw UnsupportedError(
           'The native assets for this package require at '
           'least Android NDK API level $minNdkApiVersionForThisPackage.',
         );
       }
-    } else if (config.targetOS == OS.iOS) {
+    } else if (config.codeConfig.targetOS == OS.iOS) {
       final iosVersion = config.codeConfig.targetIOSVersion;
       // iosVersion is nullable to deal with version skew.
       if (iosVersion != null && iosVersion < minIosVersionForThisPackage) {
@@ -31,7 +31,7 @@ void main(List<String> arguments) async {
           'least iOS version $minIosVersionForThisPackage.',
         );
       }
-    } else if (config.targetOS == OS.macOS) {
+    } else if (config.codeConfig.targetOS == OS.macOS) {
       final macosVersion = config.codeConfig.targetMacOSVersion;
       // macosVersion is nullable to deal with version skew.
       if (macosVersion != null &&

diff --git a/pkgs/native_assets_cli/CHANGELOG.md b/pkgs/native_assets_cli/CHANGELOG.md
@@ -9,6 +9,8 @@
   it can be controlled in the build hook together with the `OptimizationLevel`.
   Most likely, every package should ship with `release`. `BuildMode.debug`
   should only be used while developing the package locally.
+- **Breaking change** Move `HookConfig.targetOS` to `CodeConfig`. `DataAsset`s
+  and other asset types might now support varying on the target OS.
 - **Breaking change**: Change the behavior of `testBuildHook` and
   `testCodeBuildHook`; instead of defining tests, these methods should now be
   called from within tests.

diff --git a/pkgs/native_assets_cli/example/build/local_asset/hook/build.dart b/pkgs/native_assets_cli/example/build/local_asset/hook/build.dart
@@ -38,7 +38,7 @@ Future<void> main(List<String> args) async {
         name: 'asset.txt',
         file: assetPath,
         linkMode: DynamicLoadingBundled(),
-        os: config.targetOS,
+        os: config.codeConfig.targetOS,
         architecture:
             // ignore: deprecated_member_use
             config.dryRun ? null : config.codeConfig.targetArchitecture,

diff --git a/pkgs/native_assets_cli/lib/code_assets.dart b/pkgs/native_assets_cli/lib/code_assets.dart
@@ -32,3 +32,4 @@ export 'src/code_assets/link_mode.dart'
         LookupInProcess,
         StaticLinking;
 export 'src/code_assets/link_mode_preference.dart' show LinkModePreference;
+export 'src/code_assets/os.dart' show OS;
diff --git a/pkgs/native_assets_cli/lib/native_assets_cli.dart b/pkgs/native_assets_cli/lib/native_assets_cli.dart
@@ -21,4 +21,3 @@ export 'src/config.dart'
         LinkOutputBuilder;
 export 'src/encoded_asset.dart' show EncodedAsset;
 export 'src/metadata.dart';
-export 'src/os.dart' show OS;
diff --git a/pkgs/native_assets_cli/lib/src/api/build.dart b/pkgs/native_assets_cli/lib/src/api/build.dart
@@ -82,7 +82,7 @@ import '../validation.dart';
 ///         name: 'asset.txt',
 ///         file: assetPath,
 ///         linkMode: DynamicLoadingBundled(),
-///         os: config.targetOS,
+///         os: config.codeConfig.targetOS,
 ///         architecture: config.codeConfig.targetArchitecture,
 ///       ),
 ///     );

diff --git a/pkgs/native_assets_cli/lib/src/code_assets/code_asset.dart b/pkgs/native_assets_cli/lib/src/code_assets/code_asset.dart
@@ -5,12 +5,11 @@
 import '../config.dart';
 import '../encoded_asset.dart';
 import '../json_utils.dart';
-import '../os.dart';
 import '../utils/json.dart';
 import '../utils/map.dart';
-
 import 'architecture.dart';
 import 'link_mode.dart';
+import 'os.dart';
 
 /// A code asset which respects the native application binary interface (ABI).
 ///

diff --git a/pkgs/native_assets_cli/lib/src/code_assets/config.dart b/pkgs/native_assets_cli/lib/src/code_assets/config.dart
@@ -4,12 +4,12 @@
 
 import '../config.dart';
 import '../json_utils.dart';
-
 import 'architecture.dart';
 import 'c_compiler_config.dart';
 import 'code_asset.dart';
 import 'ios_sdk.dart';
 import 'link_mode_preference.dart';
+import 'os.dart';
 
 /// Extension to the [BuildConfig] providing access to configuration specific to
 /// code assets (only available if code assets are supported).
@@ -48,6 +48,9 @@ class CodeConfig {
   final int? targetAndroidNdkApi;
   final IOSSdk? targetIOSSdk;
 
+  /// The operating system being compiled for.
+  final OS targetOS;
+
   CodeConfig(HookConfig config)
       : linkModePreference = LinkModePreference.fromString(
             config.json.string(_linkModePreferenceKey)),
@@ -56,6 +59,7 @@ class CodeConfig {
             ? null
             : Architecture.fromString(config.json.string(_targetArchitectureKey,
                 validValues: Architecture.values.map((a) => a.name))),
+        targetOS = OS.fromString(config.json.string(_targetOSConfigKey)),
         cCompiler = switch (config.json.optionalMap(_compilerKey)) {
           final Map<String, Object?> map => CCompilerConfig.fromJson(map),
           null => CCompilerConfig(),
@@ -121,6 +125,7 @@ extension type CodeAssetLinkOutputBuilderAdd._(LinkOutputBuilder _output) {
 extension CodeAssetBuildConfigBuilder on HookConfigBuilder {
   void setupCodeConfig({
     required Architecture? targetArchitecture,
+    required OS targetOS,
     required LinkModePreference linkModePreference,
     CCompilerConfig? cCompilerConfig,
     int? targetIOSVersion,
@@ -131,6 +136,7 @@ extension CodeAssetBuildConfigBuilder on HookConfigBuilder {
     if (targetArchitecture != null) {
       json[_targetArchitectureKey] = targetArchitecture.toString();
     }
+    json[_targetOSConfigKey] = targetOS.toString();
     json[_linkModePreferenceKey] = linkModePreference.toString();
     if (cCompilerConfig != null) {
       json[_compilerKey] = cCompilerConfig.toJson();
@@ -170,9 +176,10 @@ extension CodeAssetLinkOutput on LinkOutput {
 }
 
 const String _compilerKey = 'c_compiler';
+const String _linkModePreferenceKey = 'link_mode_preference';
+const String _targetAndroidNdkApiKey = 'target_android_ndk_api';
 const String _targetArchitectureKey = 'target_architecture';
 const String _targetIOSSdkKey = 'target_ios_sdk';
-const String _linkModePreferenceKey = 'link_mode_preference';
 const String _targetIOSVersionKey = 'target_ios_version';
 const String _targetMacOSVersionKey = 'target_macos_version';
-const String _targetAndroidNdkApiKey = 'target_android_ndk_api';
+const String _targetOSConfigKey = 'target_os';
diff --git a/pkgs/native_assets_cli/lib/src/os.dart → ...ve_assets_cli/lib/src/code_assets/os.dart b/pkgs/native_assets_cli/lib/src/os.dart → ...ve_assets_cli/lib/src/code_assets/os.dart
diff --git a/pkgs/native_assets_cli/lib/src/code_assets/testing.dart b/pkgs/native_assets_cli/lib/src/code_assets/testing.dart
@@ -42,6 +42,7 @@ Future<void> testCodeBuildHook({
         linkModePreference: linkModePreference ?? LinkModePreference.dynamic,
         cCompilerConfig: cCompiler,
         targetArchitecture: targetArchitecture ?? Architecture.current,
+        targetOS: targetOS ?? OS.current,
         targetIOSSdk: targetIOSSdk,
         targetIOSVersion: targetIOSVersion,
         targetMacOSVersion: targetMacOSVersion,
@@ -58,7 +59,6 @@ Future<void> testCodeBuildHook({
 
       await check(config, output);
     },
-    targetOS: targetOS,
     buildAssetTypes: buildAssetTypes,
     linkingEnabled: linkingEnabled,
   );