[native_assets_builder] Rerun link hook if input assets change

pkgs/native_assets_builder/CHANGELOG.md

@@ -2,6 +2,8 @@
 
 - Added a validation step on the output of the build and link hooks (both as a
   per package, and as in all the packages together).
+- Fixed caching bug for link hooks
+  [#1515](https://github.com/dart-lang/native/pull/1515).
 
 ## 0.8.2
 

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

@@ -506,13 +506,27 @@ class NativeAssetsBuildRunner {
           final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
           final dependenciesLastChange =
               await hookOutput.dependenciesModel.lastModified();
+          late final DateTime assetsLastChange;
+          if (hook == Hook.link) {
+            assetsLastChange = await (config as LinkConfigImpl)
+                .assets
+                .map((a) => a.file)
+                .whereType<Uri>()
+                .map((u) => u.fileSystemEntity)
+                .lastModified();
+          }
           if (lastBuilt.isAfter(dependenciesLastChange) &&
-              lastBuilt.isAfter(hookLastSourceChange)) {
+              lastBuilt.isAfter(hookLastSourceChange) &&
+              (hook == Hook.build || lastBuilt.isAfter(assetsLastChange))) {
             logger.info(
-              'Skipping ${hook.name} for ${config.packageName} in $outDir. '
-              'Last build on $lastBuilt. '
-              'Last dependencies change on $dependenciesLastChange. '
-              'Last hook change on $hookLastSourceChange.',
+              [
+                'Skipping ${hook.name} for ${config.packageName} in $outDir.',
+                'Last build on $lastBuilt.',
+                'Last dependencies change on $dependenciesLastChange.',
+                if (hook == Hook.link)
+                  'Last assets for linking change on $assetsLastChange.',
+                'Last hook change on $hookLastSourceChange.',
+              ].join(' '),
             );
             // All build flags go into [outDir]. Therefore we do not have to
             // check here whether the config is equal.

pkgs/native_assets_builder/test/build_runner/link_caching_test.dart

@@ -0,0 +1,139 @@
+// Copyright (c) 2024, 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 'dart:io';
+
+import 'package:native_assets_builder/native_assets_builder.dart';
+import 'package:native_assets_cli/src/api/asset.dart';
+import 'package:test/test.dart';
+
+import '../helpers.dart';
+import 'helpers.dart';
+
+void main() async {
+  const supportedAssetTypes = [DataAsset.type];
+  const packageName = 'simple_link';
+
+  test('link hook caching', () async {
+    await inTempDir((tempUri) async {
+      await copyTestProjects(targetUri: tempUri);
+      final packageUri = tempUri.resolve('$packageName/');
+
+      // First, run `pub get`, we need pub to resolve our dependencies.
+      await runPubGet(
+        workingDirectory: packageUri,
+        logger: logger,
+      );
+      // Make sure the first compile is at least one second after the
+      // package_config.json is written, otherwise dill compilation isn't
+      // cached.
+      await Future<void>.delayed(const Duration(seconds: 1));
+
+      final logMessages = <String>[];
+      late BuildResult buildResult;
+      late LinkResult linkResult;
+      Future<void> runBuild() async {
+        logMessages.clear();
+        buildResult = await build(
+          packageUri,
+          logger,
+          dartExecutable,
+          linkingEnabled: true,
+          supportedAssetTypes: supportedAssetTypes,
+          capturedLogs: logMessages,
+        );
+      }
+
+      Future<void> runLink() async {
+        logMessages.clear();
+        linkResult = await link(
+          packageUri,
+          logger,
+          dartExecutable,
+          buildResult: buildResult,
+          supportedAssetTypes: supportedAssetTypes,
+          capturedLogs: logMessages,
+        );
+      }
+
+      await runBuild();
+      expect(buildResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        stringContainsInOrder([
+          'Running',
+          'compile kernel',
+          '$packageName${Platform.pathSeparator}hook'
+              '${Platform.pathSeparator}build.dart',
+          'Running',
+          'hook.dill',
+        ]),
+      );
+
+      await runLink();
+      expect(linkResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        stringContainsInOrder([
+          'Running',
+          'compile kernel',
+          '$packageName${Platform.pathSeparator}hook'
+              '${Platform.pathSeparator}link.dart',
+          'Running',
+          'hook.dill',
+        ]),
+      );
+
+      await runBuild();
+      expect(buildResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        contains('Skipping build for $packageName'),
+      );
+
+      await runLink();
+      expect(linkResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        contains('Skipping link for $packageName'),
+      );
+
+      await copyTestProjects(
+        sourceUri: testDataUri.resolve('simple_link_change_asset/'),
+        targetUri: packageUri,
+      );
+      // Make sure the first hook is at least one second after the last
+      // change, or caching will not work.
+      await Future<void>.delayed(const Duration(seconds: 1));
+
+      await runBuild();
+      expect(buildResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        stringContainsInOrder(['Running', 'hook.dill']),
+      );
+
+      await runLink();
+      expect(linkResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        stringContainsInOrder(['Running', 'hook.dill']),
+      );
+
+      await runBuild();
+      expect(buildResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        contains('Skipping build for $packageName'),
+      );
+
+      await runLink();
+      expect(linkResult.success, isTrue);
+      expect(
+        logMessages.join('\n'),
+        contains('Skipping link for $packageName'),
+      );
+    });
+  });
+}

pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_0.json

@@ -0,0 +1,3 @@
+{
+    "foo": "bar"
+}

pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_1.json

@@ -0,0 +1,3 @@
+{
+    "foo": "bar"
+}

pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_2.json

@@ -0,0 +1,3 @@
+{
+    "foo": "bar"
+}

pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_3.json

@@ -0,0 +1,3 @@
+{
+    "foo": "bar"
+}

pkgs/native_assets_builder/test_data/simple_link_change_asset/manifest.yaml

@@ -0,0 +1,4 @@
+- assets/data_0.json
+- assets/data_1.json
+- assets/data_2.json
+- assets/data_3.json