[native_assets_*] Validator (#1507)

Author: dcharkes

pkgs/native_assets_builder/CHANGELOG.md

@@ -1,6 +1,6 @@
 ## 0.8.3-wip
 
-- Nothing yet.
+- Added a validation step on the output of the build and link hooks.
 
 ## 0.8.2
 

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

@@ -209,6 +209,7 @@ class NativeAssetsBuildRunner {
         workingDirectory,
         includeParentEnvironment,
         resourceIdentifiers,
+        packageLayout,
       );
       hookResult = hookResult.copyAdd(hookOutput, packageSuccess);
       metadata[config.packageName] = hookOutput.metadata;
@@ -300,6 +301,7 @@ class NativeAssetsBuildRunner {
         dependencyMetadata: dependencyMetadata,
         linkingEnabled: linkingEnabled,
         targetAndroidNdkApi: targetAndroidNdkApi,
+        supportedAssetTypes: supportedAssetTypes,
       );
     }
   }
@@ -428,6 +430,7 @@ class NativeAssetsBuildRunner {
           includeParentEnvironment,
           null,
           hookKernelFile,
+          packageLayout!,
         ),
       );
       buildOutput = _expandArchsNativeCodeAssets(buildOutput);
@@ -466,6 +469,7 @@ class NativeAssetsBuildRunner {
     Uri workingDirectory,
     bool includeParentEnvironment,
     Uri? resources,
+    PackageLayout packageLayout,
   ) async {
     final outDir = config.outputDirectory;
     return await runUnderDirectoryLock(
@@ -514,6 +518,7 @@ class NativeAssetsBuildRunner {
           includeParentEnvironment,
           resources,
           hookKernelFile,
+          packageLayout,
         );
       },
     );
@@ -527,6 +532,7 @@ class NativeAssetsBuildRunner {
     bool includeParentEnvironment,
     Uri? resources,
     File hookKernelFile,
+    PackageLayout packageLayout,
   ) async {
     final configFile = config.outputDirectory.resolve('../config.json');
     final configFileContents = config.toJsonString();
@@ -577,20 +583,25 @@ ${result.stdout}
     }
 
     try {
-      final buildOutput = HookOutputImpl.readFromFile(
-              file: config.outputFile) ??
+      final output = HookOutputImpl.readFromFile(file: config.outputFile) ??
           (config.outputFileV1_1_0 == null
               ? null
               : HookOutputImpl.readFromFile(file: config.outputFileV1_1_0!)) ??
           HookOutputImpl();
-      // The link.dart can pipe through assets from other packages.
-      if (hook == Hook.build) {
-        success &= validateAssetsPackage(
-          buildOutput.assets,
-          config.packageName,
-        );
+
+      final validateResult = await validate(
+        config,
+        output,
+        packageLayout,
+      );
+      success &= validateResult.success;
+      if (!validateResult.success) {
+        logger.severe('package:${config.packageName}` has invalid output.');
+      }
+      for (final error in validateResult.errors) {
+        logger.severe('- $error');
       }
-      return (buildOutput, success);
+      return (output, success);
     } on FormatException catch (e) {
       logger.severe('''
 Building native assets for package:${config.packageName} failed.
@@ -793,22 +804,32 @@ ${compileResult.stdout}
     };
   }
 
-  bool validateAssetsPackage(Iterable<AssetImpl> assets, String packageName) {
-    final invalidAssetIds = assets
-        .map((a) => a.id)
-        .where((id) => !id.startsWith('package:$packageName/'))
-        .toSet()
-        .toList()
-      ..sort();
-    if (invalidAssetIds.isNotEmpty) {
-      logger.severe(
-        '`package:$packageName` declares the following assets which do not '
-        'start with `package:$packageName/`: ${invalidAssetIds.join(', ')}.',
-      );
-      return false;
-    } else {
-      return true;
+  Future<ValidateResult> validate(
+    HookConfigImpl config,
+    HookOutputImpl output,
+    PackageLayout packageLayout,
+  ) async {
+    var (errors: errors, success: success) = config is BuildConfigImpl
+        ? await validateBuild(config, output)
+        : await validateLink(config as LinkConfigImpl, output);
+
+    if (config is BuildConfigImpl) {
+      final packagesWithLink =
+          (await packageLayout.packagesWithAssets(Hook.link))
+              .map((p) => p.name);
+      for (final targetPackage in output.assetsForLinking.keys) {
+        if (!packagesWithLink.contains(targetPackage)) {
+          for (final asset in output.assetsForLinking[targetPackage]!) {
+            success &= false;
+            errors.add(
+              'Asset "${asset.id}" is sent to package "$targetPackage" for'
+              ' linking, but that package does not have a link hook.',
+            );
+          }
+        }
+      }
     }
+    return (errors: errors, success: success);
   }
 
   Future<(List<Package> plan, PackageGraph? dependencyGraph, bool success)>

pkgs/native_assets_builder/test/build_runner/build_runner_asset_id_test.dart

@@ -32,10 +32,7 @@ void main() async {
         expect(result.success, false);
         expect(
           fullLog,
-          contains(
-            '`package:wrong_namespace_asset` declares the following assets '
-            'which do not start with `package:wrong_namespace_asset/`:',
-          ),
+          contains('does not start with "package:wrong_namespace_asset/"'),
         );
       }
     });

pkgs/native_assets_builder/test/build_runner/helpers.dart

@@ -47,6 +47,7 @@ Future<BuildResult> build(
   int? targetAndroidNdkApi,
   Target? target,
   bool linkingEnabled = false,
+  Iterable<String>? supportedAssetTypes,
 }) async =>
     await runWithLog(capturedLogs, () async {
       final result = await NativeAssetsBuildRunner(
@@ -66,6 +67,7 @@ Future<BuildResult> build(
         targetMacOSVersion: targetMacOSVersion,
         targetAndroidNdkApi: targetAndroidNdkApi,
         linkingEnabled: linkingEnabled,
+        supportedAssetTypes: supportedAssetTypes,
       );
 
       if (result.success) {
@@ -94,6 +96,7 @@ Future<LinkResult> link(
   int? targetMacOSVersion,
   int? targetAndroidNdkApi,
   Target? target,
+  Iterable<String>? supportedAssetTypes,
 }) async =>
     await runWithLog(capturedLogs, () async {
       final result = await NativeAssetsBuildRunner(
@@ -113,6 +116,7 @@ Future<LinkResult> link(
         targetIOSVersion: targetIOSVersion,
         targetMacOSVersion: targetMacOSVersion,
         targetAndroidNdkApi: targetAndroidNdkApi,
+        supportedAssetTypes: supportedAssetTypes,
       );
 
       if (result.success) {
@@ -138,6 +142,7 @@ Future<(BuildResult, LinkResult)> buildAndLink(
   int? targetAndroidNdkApi,
   Target? target,
   Uri? resourceIdentifiers,
+  Iterable<String>? supportedAssetTypes,
 }) async =>
     await runWithLog(capturedLogs, () async {
       final buildRunner = NativeAssetsBuildRunner(
@@ -158,6 +163,7 @@ Future<(BuildResult, LinkResult)> buildAndLink(
         targetMacOSVersion: targetMacOSVersion,
         targetAndroidNdkApi: targetAndroidNdkApi,
         linkingEnabled: true,
+        supportedAssetTypes: supportedAssetTypes,
       );
 
       if (!buildResult.success) {
@@ -183,6 +189,7 @@ Future<(BuildResult, LinkResult)> buildAndLink(
         targetIOSVersion: targetIOSVersion,
         targetMacOSVersion: targetMacOSVersion,
         targetAndroidNdkApi: targetAndroidNdkApi,
+        supportedAssetTypes: supportedAssetTypes,
       );
 
       if (linkResult.success) {
@@ -221,6 +228,7 @@ Future<BuildDryRunResult> buildDryRun(
   List<String>? capturedLogs,
   PackageLayout? packageLayout,
   required bool linkingEnabled,
+  Iterable<String>? supportedAssetTypes,
 }) async =>
     runWithLog(capturedLogs, () async {
       final result = await NativeAssetsBuildRunner(
@@ -233,6 +241,7 @@ Future<BuildDryRunResult> buildDryRun(
         includeParentEnvironment: includeParentEnvironment,
         packageLayout: packageLayout,
         linkingEnabled: linkingEnabled,
+        supportedAssetTypes: supportedAssetTypes,
       );
       return result;
     });
@@ -247,6 +256,7 @@ Future<LinkDryRunResult> linkDryRun(
   List<String>? capturedLogs,
   PackageLayout? packageLayout,
   required BuildDryRunResult buildDryRunResult,
+  Iterable<String>? supportedAssetTypes,
 }) async =>
     runWithLog(capturedLogs, () async {
       final result = await NativeAssetsBuildRunner(
@@ -259,6 +269,7 @@ Future<LinkDryRunResult> linkDryRun(
         includeParentEnvironment: includeParentEnvironment,
         packageLayout: packageLayout,
         buildDryRunResult: buildDryRunResult,
+        supportedAssetTypes: supportedAssetTypes,
       );
       return result;
     });

pkgs/native_assets_builder/test/build_runner/link_dry_run_test.dart

@@ -12,6 +12,8 @@ import 'helpers.dart';
 const Timeout longTimeout = Timeout(Duration(minutes: 5));
 
 void main() async {
+  const supportedAssetTypes = [DataAsset.type];
+
   test(
     'simple_link linking',
     timeout: longTimeout,
@@ -31,6 +33,7 @@ void main() async {
           logger,
           dartExecutable,
           linkingEnabled: true,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(buildResult.assets.length, 0);
 
@@ -39,6 +42,7 @@ void main() async {
           logger,
           dartExecutable,
           buildDryRunResult: buildResult,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(linkResult.assets.length, 2);
       });
@@ -81,6 +85,7 @@ void main() async {
           logger,
           dartExecutable,
           linkingEnabled: true,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(buildResult.success, true);
         expect(
@@ -95,6 +100,7 @@ void main() async {
           logger,
           dartExecutable,
           buildDryRunResult: buildResult,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(linkResult.success, true);
 

pkgs/native_assets_builder/test/build_runner/link_test.dart

@@ -14,6 +14,8 @@ import 'helpers.dart';
 const Timeout longTimeout = Timeout(Duration(minutes: 5));
 
 void main() async {
+  const supportedAssetTypes = [DataAsset.type];
+
   test(
     'simple_link linking',
     timeout: longTimeout,
@@ -33,6 +35,7 @@ void main() async {
           logger,
           dartExecutable,
           linkingEnabled: true,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(buildResult.assets.length, 0);
 
@@ -41,6 +44,7 @@ void main() async {
           logger,
           dartExecutable,
           buildResult: buildResult,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(linkResult.assets.length, 2);
 
@@ -49,6 +53,7 @@ void main() async {
           logger,
           dartExecutable,
           linkingEnabled: false,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(buildNoLinkResult.assets.length, 4);
       });
@@ -91,6 +96,7 @@ void main() async {
           logger,
           dartExecutable,
           linkingEnabled: true,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(buildResult.success, true);
         expect(
@@ -105,6 +111,7 @@ void main() async {
           logger,
           dartExecutable,
           buildResult: buildResult,
+          supportedAssetTypes: supportedAssetTypes,
         );
         expect(linkResult.success, true);
 
@@ -129,6 +136,7 @@ void main() async {
         logger,
         dartExecutable,
         linkingEnabled: true,
+        supportedAssetTypes: supportedAssetTypes,
       );
       expect(buildResult.assets.length, 0);
       expect(buildResult.assetsForLinking.length, 0);
@@ -140,6 +148,7 @@ void main() async {
         dartExecutable,
         buildResult: buildResult,
         capturedLogs: logMessages,
+        supportedAssetTypes: supportedAssetTypes,
       );
       expect(linkResult.assets.length, 0);
       expect(

pkgs/native_assets_builder/test/build_runner/wrong_linker_test.dart

@@ -0,0 +1,40 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// 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:logging/logging.dart';
+import 'package:test/test.dart';
+
+import '../helpers.dart';
+import 'helpers.dart';
+
+const Timeout longTimeout = Timeout(Duration(minutes: 5));
+
+void main() async {
+  test('wrong linker', timeout: longTimeout, () async {
+    await inTempDir((tempUri) async {
+      await copyTestProjects(targetUri: tempUri);
+      final packageUri = tempUri.resolve('wrong_linker/');
+
+      await runPubGet(
+        workingDirectory: packageUri,
+        logger: logger,
+      );
+
+      {
+        final logMessages = <String>[];
+        final result = await build(
+          packageUri,
+          createCapturingLogger(logMessages, level: Level.SEVERE),
+          dartExecutable,
+        );
+        final fullLog = logMessages.join('\n');
+        expect(result.success, false);
+        expect(
+          fullLog,
+          contains('but that package does not have a link hook'),
+        );
+      }
+    });
+  });
+}

pkgs/native_assets_builder/test_data/manifest.yaml

@@ -111,5 +111,7 @@
 - wrong_build_output_3/pubspec.yaml
 - wrong_build_output/hook/build.dart
 - wrong_build_output/pubspec.yaml
+- wrong_linker/hook/build.dart
+- wrong_linker/pubspec.yaml
 - wrong_namespace_asset/hook/build.dart
 - wrong_namespace_asset/pubspec.yaml