Skip to content

[native_assets_builder] Recompile hook kernel if Dart changes #1763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 65 additions & 43 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -510,20 +510,23 @@ ${e.message}
''');
return null;
}

final outdated =
(await dependenciesHashes.findOutdatedFileSystemEntity()) != null;
if (!outdated) {
final outdatedFile =
await dependenciesHashes.findOutdatedFileSystemEntity();
if (outdatedFile == null) {
logger.info(
[
'Skipping ${hook.name} for ${config.packageName} in $outDir.',
'Last build on ${output.timestamp}.',
].join(' '),
'Skipping ${hook.name} for ${config.packageName}'
' in ${outDir.toFilePath()}.'
' Last build on ${output.timestamp}.',
);
// All build flags go into [outDir]. Therefore we do not have to
// check here whether the config is equal.
return output;
}
logger.info(
'Rerunning ${hook.name} for ${config.packageName}'
' in ${outDir.toFilePath()}.'
' ${outdatedFile.toFilePath()} changed.',
);
}

final result = await _runHookForPackage(
Expand All @@ -542,7 +545,8 @@ ${e.message}
await dependenciesHashFile.delete();
}
} else {
final modifiedDuringBuild = await dependenciesHashes.hashFiles(
final modifiedDuringBuild =
await dependenciesHashes.hashFilesAndDirectories(
[
...result.dependencies,
// Also depend on the hook source code.
Expand Down Expand Up @@ -689,50 +693,68 @@ ${e.message}
);
final dependenciesHashes = DependenciesHashFile(file: dependenciesHashFile);
final lastModifiedCutoffTime = DateTime.now();
final bool mustCompile;
var mustCompile = false;
if (!await dependenciesHashFile.exists()) {
mustCompile = true;
} else {
mustCompile =
(await dependenciesHashes.findOutdatedFileSystemEntity()) != null;
final outdatedFile =
await dependenciesHashes.findOutdatedFileSystemEntity();
if (outdatedFile != null) {
mustCompile = true;
logger.info(
'Recompiling ${scriptUri.toFilePath()}, '
'${outdatedFile.toFilePath()} changed.',
);
}
}
final bool success;

if (!mustCompile) {
success = true;
} else {
success = await _compileHookForPackage(
packageName,
scriptUri,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
kernelFile,
depFile,
);
return (true, kernelFile, dependenciesHashFile);
}

if (success) {
// Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart`
final depFileContents = await depFile.readAsString();
final dartSources = depFileContents
.trim()
.split(' ')
.skip(1) // '<kernel file>:'
.map(Uri.file)
.toList();
final modifiedDuringBuild = await dependenciesHashes.hashFiles(
dartSources,
validBeforeLastModified: lastModifiedCutoffTime,
);
if (modifiedDuringBuild != null) {
logger.severe('File modified during build. Build must be rerun.');
}
} else {
await dependenciesHashFile.delete();
}
final success = await _compileHookForPackage(
packageName,
scriptUri,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
kernelFile,
depFile,
);
if (!success) {
await dependenciesHashFile.delete();
return (success, kernelFile, dependenciesHashFile);
}

final dartSources = await _readDepFile(depFile);
final modifiedDuringBuild =
await dependenciesHashes.hashFilesAndDirectories(
[
...dartSources,
// If the Dart version changed, recompile.
dartExecutable.resolve('../version'),
],
validBeforeLastModified: lastModifiedCutoffTime,
);
if (modifiedDuringBuild != null) {
logger.severe('File modified during build. Build must be rerun.');
}

return (success, kernelFile, dependenciesHashFile);
}

Future<List<Uri>> _readDepFile(File depFile) async {
// Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart`
final depFileContents = await depFile.readAsString();
final dartSources = depFileContents
.trim()
.split(' ')
.skip(1) // '<kernel file>:'
.map(Uri.file)
.toList();
return dartSources;
}

Future<bool> _compileHookForPackage(
String packageName,
Uri scriptUri,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class DependenciesHashFile {
}
final jsonObject =
(json.decode(utf8.decode(await _file.readAsBytes())) as Map)
.cast<String, dynamic>();
.cast<String, Object>();
_hashes = FileSystemHashes.fromJson(jsonObject);
}

Expand All @@ -38,7 +38,7 @@ class DependenciesHashFile {
/// If [validBeforeLastModified] is provided, any entities that were modified
/// after [validBeforeLastModified] will get a dummy hash so that they will
/// show up as outdated. If any such entity exists, its uri will be returned.
Future<Uri?> hashFiles(
Future<Uri?> hashFilesAndDirectories(
List<Uri> fileSystemEntities, {
DateTime? validBeforeLastModified,
}) async {
Expand Down Expand Up @@ -134,32 +134,25 @@ class DependenciesHashFile {
/// [Directory] hashes are a hash of the names of the direct children.
class FileSystemHashes {
FileSystemHashes({
this.version = 1,
List<FilesystemEntityHash>? files,
}) : files = files ?? [];

factory FileSystemHashes.fromJson(Map<String, dynamic> json) {
final version = json[_versionKey] as int;
final rawEntries =
(json[_entitiesKey] as List<dynamic>).cast<Map<String, dynamic>>();
factory FileSystemHashes.fromJson(Map<String, Object> json) {
final rawEntries = (json[_entitiesKey] as List).cast<Object>();
final files = <FilesystemEntityHash>[
for (final Map<String, dynamic> rawEntry in rawEntries)
FilesystemEntityHash._fromJson(rawEntry),
for (final rawEntry in rawEntries)
FilesystemEntityHash._fromJson((rawEntry as Map).cast()),
];
return FileSystemHashes(
version: version,
files: files,
);
}

final int version;
final List<FilesystemEntityHash> files;

static const _versionKey = 'version';
static const _entitiesKey = 'entities';

Map<String, Object> toJson() => <String, Object>{
_versionKey: version,
_entitiesKey: <Object>[
for (final FilesystemEntityHash file in files) file.toJson(),
],
Expand All @@ -177,7 +170,7 @@ class FilesystemEntityHash {
this.hash,
);

factory FilesystemEntityHash._fromJson(Map<String, dynamic> json) =>
factory FilesystemEntityHash._fromJson(Map<String, Object> json) =>
FilesystemEntityHash(
_fileSystemPathToUri(json[_pathKey] as String),
json[_hashKey] as int,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ void main() async {
buildValidator: validateCodeAssetBuildOutput,
applicationAssetValidator: validateCodeAssetInApplication,
))!;
final hookUri = packageUri.resolve('hook/build.dart');
print(logMessages.join('\n'));
expect(
logMessages.join('\n'),
isNot(contains('Recompiling ${hookUri.toFilePath()}')),
);
expect(
logMessages.join('\n'),
contains('Skipping build for native_add'),
Expand All @@ -87,10 +93,14 @@ void main() 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();

{
final result = (await build(
Expand All @@ -105,6 +115,7 @@ void main() async {
await expectSymbols(
asset: CodeAsset.fromEncoded(result.encodedAssets.single),
symbols: ['add']);
logMessages.clear();
}

await copyTestProjects(
Expand All @@ -122,6 +133,18 @@ void main() async {
buildValidator: validateCodeAssetBuildOutput,
applicationAssetValidator: validateCodeAssetInApplication,
))!;

final cUri = packageUri.resolve('src/').resolve('native_add.c');
expect(
logMessages.join('\n'),
stringContainsInOrder(
[
'Rerunning build for native_add in',
'${cUri.toFilePath()} changed.'
],
),
);

await expectSymbols(
asset: CodeAsset.fromEncoded(result.encodedAssets.single),
symbols: ['add', 'subtract'],
Expand Down Expand Up @@ -181,14 +204,13 @@ void main() async {
buildValidator: validateCodeAssetBuildOutput,
applicationAssetValidator: validateCodeAssetInApplication,
))!;
{
final compiledHook = logMessages
.where((m) =>
m.contains('dart compile kernel') ||
m.contains('dart.exe compile kernel'))
.isNotEmpty;
expect(compiledHook, isTrue);
}

final hookUri = packageUri.resolve('hook/build.dart');
expect(
logMessages.join('\n'),
contains('Recompiling ${hookUri.toFilePath()}'),
);

logMessages.clear();
await expectSymbols(
asset: CodeAsset.fromEncoded(result.encodedAssets.single),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void main() async {
await tempFile.writeAsString('hello');
await subFile.writeAsString('world');

await hashes.hashFiles([
await hashes.hashFilesAndDirectories([
tempFile.uri,
tempSubDir.uri,
]);
Expand Down Expand Up @@ -95,7 +95,7 @@ void main() async {

// If a file is modified after the valid timestamp, it should be marked
// as changed.
await hashes.hashFiles(
await hashes.hashFilesAndDirectories(
[
tempFile.uri,
],
Expand Down