Skip to content

Commit 79b29f2

Browse files
dcharkesdcharkes
authored
[native_assets_builder] Rerun link hook if input assets change (#1515)
1 parent be73de7 commit 79b29f2

File tree

8 files changed

+176
-5
lines changed

8 files changed

+176
-5
lines changed

‎pkgs/native_assets_builder/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

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

68
## 0.8.2
79

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -506,13 +506,27 @@ class NativeAssetsBuildRunner {
506506
final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
507507
final dependenciesLastChange =
508508
await hookOutput.dependenciesModel.lastModified();
509+
late final DateTime assetsLastChange;
510+
if (hook == Hook.link) {
511+
assetsLastChange = await (config as LinkConfigImpl)
512+
.assets
513+
.map((a) => a.file)
514+
.whereType<Uri>()
515+
.map((u) => u.fileSystemEntity)
516+
.lastModified();
517+
}
509518
if (lastBuilt.isAfter(dependenciesLastChange) &&
510-
lastBuilt.isAfter(hookLastSourceChange)) {
519+
lastBuilt.isAfter(hookLastSourceChange) &&
520+
(hook == Hook.build || lastBuilt.isAfter(assetsLastChange))) {
511521
logger.info(
512-
'Skipping ${hook.name} for ${config.packageName} in $outDir. '
513-
'Last build on $lastBuilt. '
514-
'Last dependencies change on $dependenciesLastChange. '
515-
'Last hook change on $hookLastSourceChange.',
522+
[
523+
'Skipping ${hook.name} for ${config.packageName} in $outDir.',
524+
'Last build on $lastBuilt.',
525+
'Last dependencies change on $dependenciesLastChange.',
526+
if (hook == Hook.link)
527+
'Last assets for linking change on $assetsLastChange.',
528+
'Last hook change on $hookLastSourceChange.',
529+
].join(' '),
516530
);
517531
// All build flags go into [outDir]. Therefore we do not have to
518532
// check here whether the config is equal.

‎pkgs/native_assets_builder/test/build_runner/link_caching_test.dart

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'dart:io';
6+
7+
import 'package:native_assets_builder/native_assets_builder.dart';
8+
import 'package:native_assets_cli/src/api/asset.dart';
9+
import 'package:test/test.dart';
10+
11+
import '../helpers.dart';
12+
import 'helpers.dart';
13+
14+
void main() async {
15+
const supportedAssetTypes = [DataAsset.type];
16+
const packageName = 'simple_link';
17+
18+
test('link hook caching', () async {
19+
await inTempDir((tempUri) async {
20+
await copyTestProjects(targetUri: tempUri);
21+
final packageUri = tempUri.resolve('$packageName/');
22+
23+
// First, run `pub get`, we need pub to resolve our dependencies.
24+
await runPubGet(
25+
workingDirectory: packageUri,
26+
logger: logger,
27+
);
28+
// Make sure the first compile is at least one second after the
29+
// package_config.json is written, otherwise dill compilation isn't
30+
// cached.
31+
await Future<void>.delayed(const Duration(seconds: 1));
32+
33+
final logMessages = <String>[];
34+
late BuildResult buildResult;
35+
late LinkResult linkResult;
36+
Future<void> runBuild() async {
37+
logMessages.clear();
38+
buildResult = await build(
39+
packageUri,
40+
logger,
41+
dartExecutable,
42+
linkingEnabled: true,
43+
supportedAssetTypes: supportedAssetTypes,
44+
capturedLogs: logMessages,
45+
);
46+
}
47+
48+
Future<void> runLink() async {
49+
logMessages.clear();
50+
linkResult = await link(
51+
packageUri,
52+
logger,
53+
dartExecutable,
54+
buildResult: buildResult,
55+
supportedAssetTypes: supportedAssetTypes,
56+
capturedLogs: logMessages,
57+
);
58+
}
59+
60+
await runBuild();
61+
expect(buildResult.success, isTrue);
62+
expect(
63+
logMessages.join('\n'),
64+
stringContainsInOrder([
65+
'Running',
66+
'compile kernel',
67+
'$packageName${Platform.pathSeparator}hook'
68+
'${Platform.pathSeparator}build.dart',
69+
'Running',
70+
'hook.dill',
71+
]),
72+
);
73+
74+
await runLink();
75+
expect(linkResult.success, isTrue);
76+
expect(
77+
logMessages.join('\n'),
78+
stringContainsInOrder([
79+
'Running',
80+
'compile kernel',
81+
'$packageName${Platform.pathSeparator}hook'
82+
'${Platform.pathSeparator}link.dart',
83+
'Running',
84+
'hook.dill',
85+
]),
86+
);
87+
88+
await runBuild();
89+
expect(buildResult.success, isTrue);
90+
expect(
91+
logMessages.join('\n'),
92+
contains('Skipping build for $packageName'),
93+
);
94+
95+
await runLink();
96+
expect(linkResult.success, isTrue);
97+
expect(
98+
logMessages.join('\n'),
99+
contains('Skipping link for $packageName'),
100+
);
101+
102+
await copyTestProjects(
103+
sourceUri: testDataUri.resolve('simple_link_change_asset/'),
104+
targetUri: packageUri,
105+
);
106+
// Make sure the first hook is at least one second after the last
107+
// change, or caching will not work.
108+
await Future<void>.delayed(const Duration(seconds: 1));
109+
110+
await runBuild();
111+
expect(buildResult.success, isTrue);
112+
expect(
113+
logMessages.join('\n'),
114+
stringContainsInOrder(['Running', 'hook.dill']),
115+
);
116+
117+
await runLink();
118+
expect(linkResult.success, isTrue);
119+
expect(
120+
logMessages.join('\n'),
121+
stringContainsInOrder(['Running', 'hook.dill']),
122+
);
123+
124+
await runBuild();
125+
expect(buildResult.success, isTrue);
126+
expect(
127+
logMessages.join('\n'),
128+
contains('Skipping build for $packageName'),
129+
);
130+
131+
await runLink();
132+
expect(linkResult.success, isTrue);
133+
expect(
134+
logMessages.join('\n'),
135+
contains('Skipping link for $packageName'),
136+
);
137+
});
138+
});
139+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"foo": "bar"
3+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"foo": "bar"
3+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"foo": "bar"
3+
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"foo": "bar"
3+
}

‎pkgs/native_assets_builder/test_data/simple_link_change_asset/manifest.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
- assets/data_0.json
2+
- assets/data_1.json
3+
- assets/data_2.json
4+
- assets/data_3.json

0 commit comments

Comments
 (0)