Skip to content

Commit

Permalink
[native_assets_builder] Fix hook compile caching (#1933)
Browse files Browse the repository at this point in the history
The `package_config.json` can update with irrelevant information (such as the timestamp). Don't recompile the hooks if only the timestamp changed.

Testing: The existing tests started failing. Possibly after `dartdev` started running `pub get` more often automatically after https://dart-review.googlesource.com/c/sdk/+/404583.
  • Loading branch information
dcharkes authored Jan 23, 2025
1 parent ca5f3d2 commit 1d3e70c
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 12 deletions.
4 changes: 4 additions & 0 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.11.1

- Don't recompile hooks on `package_config.json` having an updated timestamp.

## 0.11.0

- **Breaking change** Complete overhaul of the use of `NativeAssetsBuildRunner`
Expand Down
30 changes: 19 additions & 11 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,6 @@ class NativeAssetsBuildRunner {
_HookValidator validator,
Uri? resources,
) async {
final packageConfigUri = packageLayout.packageConfigUri;
final outDir = input.outputDirectory;
return await runUnderDirectoriesLock(
_fileSystem,
Expand All @@ -325,7 +324,6 @@ class NativeAssetsBuildRunner {
input.packageName,
input.outputDirectory,
input.packageRoot.resolve('hook/${hook.scriptName}'),
packageConfigUri,
);
if (hookCompileResult == null) {
return null;
Expand Down Expand Up @@ -382,7 +380,6 @@ ${e.message}
hook,
input,
validator,
packageConfigUri,
resources,
hookKernelFile,
hookEnvironment,
Expand Down Expand Up @@ -432,7 +429,6 @@ ${e.message}
Hook hook,
HookInput input,
_HookValidator validator,
Uri packageConfigUri,
Uri? resources,
File hookKernelFile,
Map<String, String> environment,
Expand All @@ -458,7 +454,7 @@ ${e.message}
}

final arguments = [
'--packages=${packageConfigUri.toFilePath()}',
'--packages=${packageLayout.packageConfigUri.toFilePath()}',
hookKernelFile.path,
'--config=${inputFile.toFilePath()}',
if (resources != null) resources.toFilePath(),
Expand Down Expand Up @@ -584,10 +580,12 @@ ${e.message}
String packageName,
Uri outputDirectory,
Uri scriptUri,
Uri packageConfigUri,
) async {
// Don't invalidate cache with environment changes.
final environmentForCaching = <String, String>{};
final packageConfigHashable =
outputDirectory.resolve('../package_config_hashable.json');
await _makeHashablePackageConfig(packageConfigHashable);
final kernelFile = _fileSystem.file(
outputDirectory.resolve('../hook.dill'),
);
Expand Down Expand Up @@ -620,7 +618,6 @@ ${e.message}
final success = await _compileHookForPackage(
packageName,
scriptUri,
packageConfigUri,
kernelFile,
depFile,
);
Expand All @@ -629,9 +626,11 @@ ${e.message}
}

final dartSources = await _readDepFile(depFile);

final modifiedDuringBuild = await dependenciesHashes.hashDependencies(
[
...dartSources,
...dartSources.where((e) => e != packageLayout.packageConfigUri),
packageConfigHashable,
// If the Dart version changed, recompile.
dartExecutable.resolve('../version'),
],
Expand All @@ -645,22 +644,31 @@ ${e.message}
return (kernelFile, dependenciesHashes);
}

Future<void> _makeHashablePackageConfig(Uri uri) async {
final contents =
await _fileSystem.file(packageLayout.packageConfigUri).readAsString();
final jsonData = jsonDecode(contents) as Map<String, Object?>;
jsonData.remove('generated');
final contentsSanitized =
const JsonEncoder.withIndent(' ').convert(jsonData);
await _fileSystem.file(uri).writeAsString(contentsSanitized);
}

Future<bool> _compileHookForPackage(
String packageName,
Uri scriptUri,
Uri packageConfigUri,
File kernelFile,
File depFile,
) async {
final compileArguments = [
'compile',
'kernel',
'--packages=${packageConfigUri.toFilePath()}',
'--packages=${packageLayout.packageConfigUri.toFilePath()}',
'--output=${kernelFile.path}',
'--depfile=${depFile.path}',
scriptUri.toFilePath(),
];
final workingDirectory = packageConfigUri.resolve('../');
final workingDirectory = packageLayout.packageConfigUri.resolve('../');
final compileResult = await runProcess(
filesystem: _fileSystem,
workingDirectory: workingDirectory,
Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_assets_builder/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: native_assets_builder
description: >-
This package is the backend that invokes build hooks.
version: 0.11.0
version: 0.11.1
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder

# publish_to: none
Expand Down

0 comments on commit 1d3e70c

Please sign in to comment.