[native_assets_builder] Fix hook kernel compilation caching (#1406)
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 70a37a8..e86b36d 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
@@ -610,9 +610,20 @@
}
}
- /// Compiles the hook to dill and caches the dill.
+ /// Compiles the hook to kernel and caches the kernel.
///
- /// It does not reuse the cached dill for different [config]s, due to
+ /// If any of the Dart source files, or the package config changed after
+ /// the last time the kernel file is compiled, the kernel file is
+ /// recompiled. Otherwise a cached version is used.
+ ///
+ /// Due to some OSes only providing last-modified timestamps with second
+ /// precision. The kernel compilation cache might be considered stale if
+ /// the last modification and the kernel compilation happened within one
+ /// second of each other. We error on the side of caution, rather recompile
+ /// one time too many, then not recompiling when recompilation should have
+ /// happened.
+ ///
+ /// It does not reuse the cached kernel for different [config]s, due to
/// reentrancy requirements. For more info see:
/// https://github.com/dart-lang/native/issues/1319
Future<(bool success, File kernelFile, DateTime lastSourceChange)>
@@ -639,7 +650,7 @@
final dartSourceFiles = depFileContents
.trim()
.split(' ')
- .skip(1) // '<dill>:'
+ .skip(1) // '<kernel file>:'
.map((u) => Uri.file(u).fileSystemEntity)
.toList();
final dartFilesLastChange = await dartSourceFiles.lastModified();
@@ -648,8 +659,9 @@
sourceLastChange = packageConfigLastChange.isAfter(dartFilesLastChange)
? packageConfigLastChange
: dartFilesLastChange;
- final dillLastChange = await kernelFile.lastModified();
- mustCompile = sourceLastChange.isAfter(dillLastChange);
+ final kernelLastChange = await kernelFile.lastModified();
+ mustCompile = sourceLastChange == kernelLastChange ||
+ sourceLastChange.isAfter(kernelLastChange);
}
final bool success;
if (!mustCompile) {
diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart
index 10f3231..88501a2 100644
--- a/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart
+++ b/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart
@@ -110,26 +110,35 @@
test(
'add C file, modify hook',
timeout: longTimeout,
- onPlatform: {
- 'linux': const Skip('https://github.com/dart-lang/native/issues/1391.'),
- },
() async {
await inTempDir((tempUri) async {
await copyTestProjects(targetUri: tempUri);
final packageUri = tempUri.resolve('native_add/');
+ final logMessages = <String>[];
+ final logger = createCapturingLogger(logMessages);
+
await runPubGet(workingDirectory: packageUri, logger: logger);
+ logMessages.clear();
// 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 result = await build(packageUri, logger, dartExecutable);
{
- final result = await build(packageUri, logger, dartExecutable);
- await expectSymbols(
- asset: result.assets.single as NativeCodeAssetImpl,
- symbols: ['add']);
+ final compiledHook = logMessages
+ .where((m) =>
+ m.contains('dart compile kernel') ||
+ m.contains('dart.exe compile kernel'))
+ .isNotEmpty;
+ expect(compiledHook, isTrue);
}
+ logMessages.clear();
+ await expectSymbols(
+ asset: result.assets.single as NativeCodeAssetImpl,
+ symbols: ['add'],
+ );
await copyTestProjects(
sourceUri: testDataUri.resolve('native_add_add_source/'),
@@ -137,9 +146,19 @@
{
final result = await build(packageUri, logger, dartExecutable);
+ {
+ final compiledHook = logMessages
+ .where((m) =>
+ m.contains('dart compile kernel') ||
+ m.contains('dart.exe compile kernel'))
+ .isNotEmpty;
+ expect(compiledHook, isTrue);
+ }
+ logMessages.clear();
await expectSymbols(
- asset: result.assets.single as NativeCodeAssetImpl,
- symbols: ['add', 'multiply']);
+ asset: result.assets.single as NativeCodeAssetImpl,
+ symbols: ['add', 'multiply'],
+ );
}
});
},
diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart
index 2c2d33f..cdd19fa 100644
--- a/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart
+++ b/pkgs/native_assets_builder/test/build_runner/build_runner_test.dart
@@ -24,6 +24,11 @@
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));
+
// Trigger a build, should invoke build for libraries with native assets.
{
final logMessages = <String>[];
