[native_assets_builder] Move packageLayout to builder constructor (#…

…1919) All Dart and Flutter commands have a single package config and runPackageName. Part of a larger refactoring that will make the `NativeAssetsBuildRunner` take more arguments in the constructor such as the package config, and reduces the number of arguments to the `build` and `link` methods. This will enable: * Caching `BuildPlanner` logic (only one `runPackageName`) * Changing `packagesWithAssets` to take `runPackageName` into account (and possibly moving it to the native assets builder instead). #1905
dart-lang · Jan 21, 2025 · 3ed5d3d · 3ed5d3d
1 parent b2ad4a8
commit 3ed5d3d
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 39 deletions.
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
@@ -68,11 +68,13 @@ class NativeAssetsBuildRunner {
   final Uri dartExecutable;
   final Duration singleHookTimeout;
   final Map<String, String> hookEnvironment;
+  final PackageLayout packageLayout;
 
   NativeAssetsBuildRunner({
     required this.logger,
     required this.dartExecutable,
     required FileSystem fileSystem,
+    required this.packageLayout,
     Duration? singleHookTimeout,
     Map<String, String>? hookEnvironment,
   })  : _fileSystem = fileSystem,
@@ -94,13 +96,11 @@ class NativeAssetsBuildRunner {
     required BuildInputValidator inputValidator,
     required BuildValidator buildValidator,
     required ApplicationAssetValidator applicationAssetValidator,
-    required PackageLayout packageLayout,
     required List<String> buildAssetTypes,
     required bool linkingEnabled,
   }) async {
     final (buildPlan, packageGraph) = await _makePlan(
       hook: Hook.build,
-      packageLayout: packageLayout,
       buildResult: null,
     );
     if (buildPlan == null) return null;
@@ -127,7 +127,6 @@ class NativeAssetsBuildRunner {
 
       final (buildDirUri, outDirUri, outDirSharedUri) = await _setupDirectories(
         Hook.build,
-        packageLayout,
         inputBuilder,
         package,
       );
@@ -155,9 +154,7 @@ class NativeAssetsBuildRunner {
         input,
         (input, output) =>
             buildValidator(input as BuildInput, output as BuildOutput),
-        packageLayout.packageConfigUri,
         null,
-        packageLayout,
       );
       if (result == null) return null;
       final (hookOutput, hookDeps) = result;
@@ -188,14 +185,12 @@ class NativeAssetsBuildRunner {
     required LinkInputValidator inputValidator,
     required LinkValidator linkValidator,
     required ApplicationAssetValidator applicationAssetValidator,
-    required PackageLayout packageLayout,
     Uri? resourceIdentifiers,
     required List<String> buildAssetTypes,
     required BuildResult buildResult,
   }) async {
     final (buildPlan, packageGraph) = await _makePlan(
       hook: Hook.link,
-      packageLayout: packageLayout,
       buildResult: buildResult,
     );
     if (buildPlan == null) return null;
@@ -207,7 +202,6 @@ class NativeAssetsBuildRunner {
 
       final (buildDirUri, outDirUri, outDirSharedUri) = await _setupDirectories(
         Hook.link,
-        packageLayout,
         inputBuilder,
         package,
       );
@@ -246,9 +240,7 @@ class NativeAssetsBuildRunner {
         input,
         (input, output) =>
             linkValidator(input as LinkInput, output as LinkOutput),
-        packageLayout.packageConfigUri,
         resourceIdentifiers,
-        packageLayout,
       );
       if (result == null) return null;
       final (hookOutput, hookDeps) = result;
@@ -273,7 +265,6 @@ class NativeAssetsBuildRunner {
 
   Future<(Uri, Uri, Uri)> _setupDirectories(
     Hook hook,
-    PackageLayout packageLayout,
     HookInputBuilder inputBuilder,
     Package package,
   ) async {
@@ -301,10 +292,9 @@ class NativeAssetsBuildRunner {
     Hook hook,
     HookInput input,
     _HookValidator validator,
-    Uri packageConfigUri,
     Uri? resources,
-    PackageLayout packageLayout,
   ) async {
+    final packageConfigUri = packageLayout.packageConfigUri;
     final outDir = input.outputDirectory;
     return await runUnderDirectoriesLock(
       _fileSystem,
@@ -379,7 +369,6 @@ ${e.message}
           packageConfigUri,
           resources,
           hookKernelFile,
-          packageLayout,
           hookEnvironment,
         );
         if (result == null) {
@@ -430,7 +419,6 @@ ${e.message}
     Uri packageConfigUri,
     Uri? resources,
     File hookKernelFile,
-    PackageLayout packageLayout,
     Map<String, String> environment,
   ) async {
     final inputFile = input.outputDirectory.resolve('../input.json');
@@ -503,7 +491,7 @@ ${e.message}
         hookOutputFile,
         hookOutputFileDeprecated,
       );
-      final errors = await _validate(input, output, packageLayout, validator);
+      final errors = await _validate(input, output, validator);
       if (errors.isNotEmpty) {
         _printErrors(
             '$hook hook of package:${input.packageName} has invalid output',
@@ -716,7 +704,6 @@ ${compileResult.stdout}
   Future<ValidationErrors> _validate(
     HookInput input,
     HookOutput output,
-    PackageLayout packageLayout,
     _HookValidator validator,
   ) async {
     final errors = input is BuildInput
@@ -745,7 +732,6 @@ ${compileResult.stdout}
   }
 
   Future<(List<Package>? plan, PackageGraph? dependencyGraph)> _makePlan({
-    required PackageLayout packageLayout,
     required Hook hook,
     // TODO(dacoharkes): How to share these two? Make them extend each other?
     BuildResult? buildResult,

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
@@ -25,10 +25,16 @@ void main() async {
         logger: logger,
       );
 
+      final packageLayout = await PackageLayout.fromWorkingDirectory(
+        const LocalFileSystem(),
+        packageUri,
+        packageName,
+      );
       final buildRunner = NativeAssetsBuildRunner(
         logger: logger,
         dartExecutable: dartExecutable,
         fileSystem: const LocalFileSystem(),
+        packageLayout: packageLayout,
       );
 
       final targetOS = OS.current;
@@ -43,14 +49,8 @@ void main() async {
           linkModePreference: LinkModePreference.dynamic,
         );
 
-      final packageLayout = await PackageLayout.fromWorkingDirectory(
-        const LocalFileSystem(),
-        packageUri,
-        packageName,
-      );
       await buildRunner.build(
         inputCreator: inputCreator,
-        packageLayout: packageLayout,
         linkingEnabled: false,
         buildAssetTypes: [],
         inputValidator: (input) async => [],
@@ -59,7 +59,6 @@ void main() async {
       );
       await buildRunner.build(
         inputCreator: inputCreator,
-        packageLayout: packageLayout,
         linkingEnabled: false,
         buildAssetTypes: [],
         inputValidator: (input) async => [],

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
@@ -19,10 +19,16 @@ void main(List<String> args) async {
     ..onRecord.listen((event) => print(event.message));
 
   final targetOS = target.os;
+  final packageLayout = await PackageLayout.fromWorkingDirectory(
+    const LocalFileSystem(),
+    packageUri,
+    packageName,
+  );
   final result = await NativeAssetsBuildRunner(
     logger: logger,
     dartExecutable: dartExecutable,
     fileSystem: const LocalFileSystem(),
+    packageLayout: packageLayout,
   ).build(
     // Set up the code input, so that the builds for different targets are
     // in different directories.
@@ -37,11 +43,6 @@ void main(List<String> args) async {
             targetOS == OS.android ? AndroidCodeConfig(targetNdkApi: 30) : null,
         linkModePreference: LinkModePreference.dynamic,
       ),
-    packageLayout: await PackageLayout.fromWorkingDirectory(
-      const LocalFileSystem(),
-      packageUri,
-      packageName,
-    ),
     linkingEnabled: false,
     buildAssetTypes: [DataAsset.type, CodeAsset.type],
     inputValidator: (input) async => [

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
@@ -23,11 +23,17 @@ void main(List<String> args) async {
     ..onRecord.listen((event) => print(event.message));
 
   final targetOS = OS.current;
+  final packageLayout = await PackageLayout.fromWorkingDirectory(
+    const LocalFileSystem(),
+    packageUri,
+    packageName,
+  );
   final result = await NativeAssetsBuildRunner(
     logger: logger,
     dartExecutable: dartExecutable,
     singleHookTimeout: timeout,
     fileSystem: const LocalFileSystem(),
+    packageLayout: packageLayout,
   ).build(
     inputCreator: () => BuildInputBuilder()
       ..config.setupCode(
@@ -39,11 +45,6 @@ void main(List<String> args) async {
             ? MacOSCodeConfig(targetVersion: defaultMacOSVersion)
             : null,
       ),
-    packageLayout: await PackageLayout.fromWorkingDirectory(
-      const LocalFileSystem(),
-      packageUri,
-      packageName,
-    ),
     linkingEnabled: false,
     buildAssetTypes: [CodeAsset.type, DataAsset.type],
     inputValidator: (input) async => [

diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart
@@ -80,6 +80,7 @@ Future<BuildResult?> build(
       dartExecutable: dartExecutable,
       fileSystem: const LocalFileSystem(),
       hookEnvironment: hookEnvironment,
+      packageLayout: packageLayout,
     ).build(
       inputCreator: () {
         final inputBuilder = BuildInputBuilder();
@@ -107,7 +108,6 @@ Future<BuildResult?> build(
         return inputBuilder;
       },
       inputValidator: inputValidator,
-      packageLayout: packageLayout,
       linkingEnabled: linkingEnabled,
       buildAssetTypes: buildAssetTypes,
       buildValidator: buildValidator,
@@ -159,6 +159,7 @@ Future<LinkResult?> link(
       logger: logger,
       dartExecutable: dartExecutable,
       fileSystem: const LocalFileSystem(),
+      packageLayout: packageLayout,
     ).link(
       inputCreator: () {
         final inputBuilder = LinkInputBuilder();
@@ -186,7 +187,6 @@ Future<LinkResult?> link(
         return inputBuilder;
       },
       inputValidator: inputValidator,
-      packageLayout: packageLayout,
       buildResult: buildResult,
       resourceIdentifiers: resourceIdentifiers,
       buildAssetTypes: buildAssetTypes,
@@ -236,6 +236,7 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
         logger: logger,
         dartExecutable: dartExecutable,
         fileSystem: const LocalFileSystem(),
+        packageLayout: packageLayout,
       );
       final targetOS = target?.os ?? OS.current;
       final buildResult = await buildRunner.build(
@@ -265,7 +266,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
           return inputBuilder;
         },
         inputValidator: buildInputValidator,
-        packageLayout: packageLayout,
         linkingEnabled: true,
         buildAssetTypes: buildAssetTypes,
         buildValidator: buildValidator,
@@ -309,7 +309,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink(
           return inputBuilder;
         },
         inputValidator: linkInputValidator,
-        packageLayout: packageLayout,
         buildResult: buildResult,
         resourceIdentifiers: resourceIdentifiers,
         buildAssetTypes: buildAssetTypes,