[native_assets_builder] return build dependencies (#107)

Closes: #105

Added expectations to the existing tests for: (1) normal build, (2) cached build, and (3) build after a failing build.

This change breaks the programmatic API, so (1) I've bumped the version to 0.2.0, and (2) will roll this into the Dart SDK manually. (To minimize rolling, we might as well address #106 as well right after.)

Author: dcharkes

pkgs/native_assets_builder/CHANGELOG.md

@@ -1,10 +1,12 @@
-## 0.1.1-wip
+## 0.2.0-wip
 
+- **Breaking change** `NativeAssetsBuildRunner`s method now return an object
+  ([#105](https://github.com/dart-lang/native/issues/105)).
 - Use an `out/` sub directory for building native assets
   ([#98](https://github.com/dart-lang/native/issues/98)).
 - Check asset ids on having having a package uri with the owning package
   ([#96](https://github.com/dart-lang/native/issues/96)).
-- `NativeAssetsBuildRunner` can now support multiple calls
+- `NativeAssetsBuildRunner` now supports multiple calls
   ([#102](https://github.com/dart-lang/native/issues/102)).
 
 ## 0.1.0

pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

@@ -27,15 +27,13 @@ class NativeAssetsBuildRunner {
     required this.dartExecutable,
   });
 
-  final _metadata = <Target, DependencyMetadata>{};
-
   /// [workingDirectory] is expected to contain `.dart_tool`.
   ///
   /// This method is invoked by launchers such as dartdev (for `dart run`) and
   /// flutter_tools (for `flutter run` and `flutter build`).
   ///
   /// Completes the future with an error if the build fails.
-  Future<List<Asset>> build({
+  Future<BuildResult> build({
     required LinkModePreference linkModePreference,
     required Target target,
     required Uri workingDirectory,
@@ -45,7 +43,6 @@ class NativeAssetsBuildRunner {
     int? targetAndroidNdkApi,
     required bool includeParentEnvironment,
   }) async {
-    _metadata.clear();
     final packageLayout =
         await PackageLayout.fromRootPackageRoot(workingDirectory);
     final packagesWithNativeAssets =
@@ -56,12 +53,14 @@ class NativeAssetsBuildRunner {
       dartExecutable: Uri.file(Platform.resolvedExecutable),
     );
     final plan = planner.plan();
-    final assetList = <Asset>[];
+    final assets = <Asset>[];
+    final dependencies = <Uri>[];
+    final metadata = <String, Metadata>{};
     for (final package in plan) {
       final dependencyMetadata = _metadataForPackage(
         packageGraph: planner.packageGraph,
         packageName: package.name,
-        targetMetadata: _metadata[target],
+        targetMetadata: metadata,
       );
       final config = await _cliConfig(
         packageRoot: packageLayout.packageRoot(package.name),
@@ -74,16 +73,23 @@ class NativeAssetsBuildRunner {
         targetIOSSdk: targetIOSSdk,
         targetAndroidNdkApi: targetAndroidNdkApi,
       );
-      final assets = await _buildPackageCached(
+      final (packageAssets, packageDependencies, packageMetadata) =
+          await _buildPackageCached(
         config,
         packageLayout.packageConfigUri,
         workingDirectory,
         includeParentEnvironment,
       );
-      validateAssetsPackage(assets, package.name);
-      assetList.addAll(assets);
+      assets.addAll(packageAssets);
+      dependencies.addAll(packageDependencies);
+      if (packageMetadata != null) {
+        metadata[config.packageName] = packageMetadata;
+      }
     }
-    return assetList;
+    return _BuildResultImpl(
+      assets: assets,
+      dependencies: dependencies..sort(_uriCompare),
+    );
   }
 
   /// [workingDirectory] is expected to contain `.dart_tool`.
@@ -92,7 +98,7 @@ class NativeAssetsBuildRunner {
   /// flutter_tools (for `flutter run` and `flutter build`).
   ///
   /// Completes the future with an error if the build fails.
-  Future<List<Asset>> dryRun({
+  Future<DryRunResult> dryRun({
     required LinkModePreference linkModePreference,
     required OS targetOs,
     required Uri workingDirectory,
@@ -108,7 +114,7 @@ class NativeAssetsBuildRunner {
       dartExecutable: Uri.file(Platform.resolvedExecutable),
     );
     final plan = planner.plan();
-    final assetList = <Asset>[];
+    final assets = <Asset>[];
     for (final package in plan) {
       final config = await _cliConfigDryRun(
         packageName: package.name,
@@ -117,19 +123,21 @@ class NativeAssetsBuildRunner {
         linkMode: linkModePreference,
         buildParentDir: packageLayout.dartToolNativeAssetsBuilder,
       );
-      final assets = await _buildPackage(
+      final (packageAssets, _, _) = await _buildPackage(
         config,
         packageLayout.packageConfigUri,
         workingDirectory,
         includeParentEnvironment,
         dryRun: true,
       );
-      assetList.addAll(assets);
+      assets.addAll(packageAssets);
     }
-    return assetList;
+    return _DryRunResultImpl(
+      assets: assets,
+    );
   }
 
-  Future<List<Asset>> _buildPackageCached(
+  Future<(List<Asset>, List<Uri>, Metadata?)> _buildPackageCached(
     BuildConfig config,
     Uri packageConfigUri,
     Uri workingDirectory,
@@ -152,12 +160,13 @@ class NativeAssetsBuildRunner {
           'Last build on $lastBuilt, last input change on $lastChange.');
       // All build flags go into [outDir]. Therefore we do not have to check
       // here whether the config is equal.
-
-      setMetadata(config.target, packageName, buildOutput?.metadata);
-      return buildOutput!.assets;
+      final assets = buildOutput!.assets;
+      final dependencies = buildOutput.dependencies.dependencies;
+      final metadata = buildOutput.metadata;
+      return (assets, dependencies, metadata);
     }
 
-    return _buildPackage(
+    return await _buildPackage(
       config,
       packageConfigUri,
       workingDirectory,
@@ -166,7 +175,7 @@ class NativeAssetsBuildRunner {
     );
   }
 
-  Future<List<Asset>> _buildPackage(
+  Future<(List<Asset>, List<Uri>, Metadata?)> _buildPackage(
     BuildConfig config,
     Uri packageConfigUri,
     Uri workingDirectory,
@@ -198,18 +207,11 @@ class NativeAssetsBuildRunner {
       throwOnUnexpectedExitCode: true,
     );
     final buildOutput = await BuildOutput.readFromFile(outDir: outDir);
-    if (!dryRun) {
-      setMetadata(config.target, config.packageName, buildOutput?.metadata);
-    }
-    return buildOutput?.assets ?? [];
-  }
-
-  void setMetadata(Target target, String packageName, Metadata? metadata) {
-    if (metadata == null) {
-      return;
-    }
-    _metadata[target] ??= {};
-    _metadata[target]![packageName] = metadata;
+    final assets = buildOutput?.assets ?? [];
+    validateAssetsPackage(assets, config.packageName);
+    final dependencies = buildOutput?.dependencies.dependencies ?? [];
+    final metadata = dryRun ? null : buildOutput?.metadata;
+    return (assets, dependencies, metadata);
   }
 
   static Future<BuildConfig> _cliConfig({
@@ -306,6 +308,43 @@ class NativeAssetsBuildRunner {
   }
 }
 
+/// The result from a [NativeAssetsBuildRunner.dryRun].
+abstract interface class DryRunResult {
+  /// The native assets for all [Target]s for the build or dry run.
+  List<Asset> get assets;
+}
+
+final class _DryRunResultImpl implements DryRunResult {
+  @override
+  final List<Asset> assets;
+
+  _DryRunResultImpl({required this.assets});
+}
+
+/// The result from a [NativeAssetsBuildRunner.build].
+abstract class BuildResult implements DryRunResult {
+  /// All the files used for building the native assets of all packages.
+  ///
+  /// This aggregated list can be used to determine whether the
+  /// [NativeAssetsBuildRunner] needs to be invoked again. The
+  /// [NativeAssetsBuildRunner] determines per package with native assets
+  /// if it needs to run the build again.
+  List<Uri> get dependencies;
+}
+
+final class _BuildResultImpl implements BuildResult {
+  @override
+  final List<Asset> assets;
+
+  @override
+  final List<Uri> dependencies;
+
+  _BuildResultImpl({
+    required this.assets,
+    required this.dependencies,
+  });
+}
+
 extension on DateTime {
   DateTime roundDownToSeconds() =>
       DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch -
@@ -316,3 +355,5 @@ extension on BuildConfig {
   String get packageName =>
       packageRoot.pathSegments.lastWhere((e) => e.isNotEmpty);
 }
+
+int _uriCompare(Uri u1, Uri u2) => u1.toString().compareTo(u2.toString());

pkgs/native_assets_builder/pubspec.yaml

@@ -1,7 +1,7 @@
 name: native_assets_builder
 description: >-
   This package is the backend that invokes top-level `build.dart` scripts.
-version: 0.1.1-wip
+version: 0.2.0-wip
 repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder
 
 environment:

pkgs/native_assets_builder/test/build_runner/build_dependencies_test.dart

@@ -26,15 +26,29 @@ void main() async {
       // Trigger a build, should invoke build for libraries with native assets.
       {
         final logMessages = <String>[];
-        final assets = await build(packageUri, logger, dartExecutable,
+        final result = await build(packageUri, logger, dartExecutable,
             capturedLogs: logMessages);
         expect(
-            logMessages.join('\n'),
-            stringContainsInOrder([
+          logMessages.join('\n'),
+          stringContainsInOrder(
+            [
               'native_add${Platform.pathSeparator}build.dart',
               'native_subtract${Platform.pathSeparator}build.dart'
-            ]));
-        expect(assets.length, 2);
+            ],
+          ),
+        );
+        expect(result.assets.length, 2);
+        expect(
+          result.dependencies,
+          [
+            tempUri.resolve('native_add/').resolve('build.dart'),
+            tempUri.resolve('native_add/').resolve('src/native_add.c'),
+            tempUri.resolve('native_subtract/').resolve('build.dart'),
+            tempUri
+                .resolve('native_subtract/')
+                .resolve('src/native_subtract.c'),
+          ],
+        );
       }
     });
   });

pkgs/native_assets_builder/test/build_runner/build_runner_asset_id_test.dart

@@ -2,7 +2,6 @@
 // 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.
 
-import 'package:native_assets_cli/native_assets_cli.dart';
 import 'package:test/test.dart';
 
 import '../helpers.dart';
@@ -23,13 +22,12 @@ void main() async {
 
       {
         var buildFailed = false;
-        final assets = await build(packageUri, logger, dartExecutable)
-            .onError((error, stackTrace) {
+        try {
+          await build(packageUri, logger, dartExecutable);
+        } catch (error) {
           buildFailed = true;
-          return [];
-        });
+        }
         expect(buildFailed, true);
-        expect(assets, <Asset>[]);
       }
     });
   });

pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart

@@ -24,25 +24,40 @@ void main() async {
 
       {
         final logMessages = <String>[];
-        await build(packageUri, logger, dartExecutable,
+        final result = await build(packageUri, logger, dartExecutable,
             capturedLogs: logMessages);
         expect(
-            logMessages.join('\n'),
-            stringContainsInOrder(
-                ['native_add${Platform.pathSeparator}build.dart']));
+          logMessages.join('\n'),
+          contains('native_add${Platform.pathSeparator}build.dart'),
+        );
+        expect(
+          result.dependencies,
+          [
+            packageUri.resolve('build.dart'),
+            packageUri.resolve('src/native_add.c'),
+          ],
+        );
       }
 
       {
         final logMessages = <String>[];
-        await build(packageUri, logger, dartExecutable,
+        final result = await build(packageUri, logger, dartExecutable,
             capturedLogs: logMessages);
-        expect(logMessages.join('\n'),
-            stringContainsInOrder(['Skipping build for native_add']));
         expect(
-            false,
-            logMessages
-                .join('\n')
-                .contains('native_add${Platform.pathSeparator}build.dart'));
+          logMessages.join('\n'),
+          contains('Skipping build for native_add'),
+        );
+        expect(
+          logMessages.join('\n'),
+          isNot(contains('native_add${Platform.pathSeparator}build.dart')),
+        );
+        expect(
+          result.dependencies,
+          [
+            packageUri.resolve('build.dart'),
+            packageUri.resolve('src/native_add.c'),
+          ],
+        );
       }
     });
   });
@@ -58,8 +73,8 @@ void main() async {
       );
 
       {
-        final assets = await build(packageUri, logger, dartExecutable);
-        await expectSymbols(asset: assets.single, symbols: ['add']);
+        final result = await build(packageUri, logger, dartExecutable);
+        await expectSymbols(asset: result.assets.single, symbols: ['add']);
       }
 
       await copyTestProjects(
@@ -68,8 +83,11 @@ void main() async {
       );
 
       {
-        final assets = await build(packageUri, logger, dartExecutable);
-        await expectSymbols(asset: assets.single, symbols: ['add', 'subtract']);
+        final result = await build(packageUri, logger, dartExecutable);
+        await expectSymbols(
+          asset: result.assets.single,
+          symbols: ['add', 'subtract'],
+        );
       }
     });
   });
@@ -82,17 +100,18 @@ void main() async {
       await runPubGet(workingDirectory: packageUri, logger: logger);
 
       {
-        final assets = await build(packageUri, logger, dartExecutable);
-        await expectSymbols(asset: assets.single, symbols: ['add']);
+        final result = await build(packageUri, logger, dartExecutable);
+        await expectSymbols(asset: result.assets.single, symbols: ['add']);
       }
 
       await copyTestProjects(
           sourceUri: testDataUri.resolve('native_add_add_source/'),
           targetUri: packageUri);
 
       {
-        final assets = await build(packageUri, logger, dartExecutable);
-        await expectSymbols(asset: assets.single, symbols: ['add', 'multiply']);
+        final result = await build(packageUri, logger, dartExecutable);
+        await expectSymbols(
+            asset: result.assets.single, symbols: ['add', 'multiply']);
       }
     });
   });