diff --git a/pkgs/native_assets_builder/CHANGELOG.md b/pkgs/native_assets_builder/CHANGELOG.md index c364f0a81..6e6bc9a6c 100644 --- a/pkgs/native_assets_builder/CHANGELOG.md +++ b/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 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 index 511492e70..2a1f05a54 100644 --- a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart +++ b/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() + .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. diff --git a/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart b/pkgs/native_assets_builder/test/build_runner/link_caching_test.dart new file mode 100644 index 000000000..8089e0912 --- /dev/null +++ b/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.delayed(const Duration(seconds: 1)); + + final logMessages = []; + late BuildResult buildResult; + late LinkResult linkResult; + Future runBuild() async { + logMessages.clear(); + buildResult = await build( + packageUri, + logger, + dartExecutable, + linkingEnabled: true, + supportedAssetTypes: supportedAssetTypes, + capturedLogs: logMessages, + ); + } + + Future 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.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'), + ); + }); + }); +} diff --git a/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_0.json b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_0.json new file mode 100644 index 000000000..e63d37b65 --- /dev/null +++ b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_0.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_1.json b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_1.json new file mode 100644 index 000000000..e63d37b65 --- /dev/null +++ b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_1.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_2.json b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_2.json new file mode 100644 index 000000000..e63d37b65 --- /dev/null +++ b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_2.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_3.json b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_3.json new file mode 100644 index 000000000..e63d37b65 --- /dev/null +++ b/pkgs/native_assets_builder/test_data/simple_link_change_asset/assets/data_3.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/pkgs/native_assets_builder/test_data/simple_link_change_asset/manifest.yaml b/pkgs/native_assets_builder/test_data/simple_link_change_asset/manifest.yaml new file mode 100644 index 000000000..784440787 --- /dev/null +++ b/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