Skip to content

[native_assets_builder] If dill file is equal, don't invalidate cache #1789

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 3 commits into from
Dec 10, 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
9 changes: 3 additions & 6 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
## 0.9.1-wip
## 0.10.0-wip

- **Breaking change**: Rename `supportedAssetTypes` to `buildAssetTypes`. Hooks
should no longer fail. Instead, the code should fail at runtime if an asset is
missing. This enables (1) code to run if an asset is missing but that code is
not invoked at runtime, and (2) doing fallback implementations in Dart if an
asset is missing.
- Removed support for dry run (Flutter no long requires it).
- Various fixes to caching.

## 0.9.0

Expand Down
1 change: 0 additions & 1 deletion pkgs/native_assets_builder/lib/native_assets_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// BSD-style license that can be found in the LICENSE file.

export 'package:native_assets_builder/src/build_runner/build_runner.dart';
export 'package:native_assets_builder/src/model/build_dry_run_result.dart';
export 'package:native_assets_builder/src/model/build_result.dart';
export 'package:native_assets_builder/src/model/kernel_assets.dart';
export 'package:native_assets_builder/src/model/link_result.dart';
Expand Down
132 changes: 3 additions & 129 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import 'package:package_config/package_config.dart';

import '../dependencies_hash_file/dependencies_hash_file.dart';
import '../locking/locking.dart';
import '../model/build_dry_run_result.dart';
import '../model/build_result.dart';
import '../model/hook_result.dart';
import '../model/link_result.dart';
Expand Down Expand Up @@ -315,132 +314,6 @@ class NativeAssetsBuildRunner {
return (buildDirUri, outDirUri, outDirSharedUri);
}

/// [workingDirectory] is expected to contain `.dart_tool`.
///
/// This method is invoked by launchers such as dartdev (for `dart run`) and
/// flutter_tools (for `flutter run` and `flutter build`).
///
/// If provided, only native assets of all transitive dependencies of
/// [runPackageName] are built.
Future<BuildDryRunResult?> buildDryRun({
required BuildConfigCreator configCreator,
required BuildValidator buildValidator,
required OS targetOS,
required Uri workingDirectory,
required bool linkingEnabled,
PackageLayout? packageLayout,
String? runPackageName,
required List<String> buildAssetTypes,
}) =>
_runDryRun(
targetOS: targetOS,
configCreator: configCreator,
validator: (HookConfig config, HookOutput output) =>
buildValidator(config as BuildConfig, output as BuildOutput),
workingDirectory: workingDirectory,
packageLayout: packageLayout,
runPackageName: runPackageName,
buildAssetTypes: buildAssetTypes,
linkingEnabled: linkingEnabled,
);

Future<HookResult?> _runDryRun({
required BuildConfigCreator configCreator,
required _HookValidator validator,
required OS targetOS,
required Uri workingDirectory,
PackageLayout? packageLayout,
String? runPackageName,
required bool linkingEnabled,
required List<String> buildAssetTypes,
}) async {
const hook = Hook.build;

packageLayout ??= await PackageLayout.fromRootPackageRoot(workingDirectory);
final (buildPlan, _) = await _makePlan(
hook: hook,
packageLayout: packageLayout,
runPackageName: runPackageName,
);
if (buildPlan == null) {
return null;
}

var hookResult = HookResult();
for (final package in buildPlan) {
final configBuilder = configCreator();
configBuilder.setupHookConfig(
targetOS: targetOS,
buildAssetTypes: buildAssetTypes,
buildMode: null, // not set in dry-run mode
packageName: package.name,
packageRoot: packageLayout.packageRoot(package.name),
);
configBuilder.setupBuildConfig(
dryRun: true,
linkingEnabled: linkingEnabled,
metadata: const {},
);

final buildDirName = configBuilder.computeChecksum();
final buildDirUri =
packageLayout.dartToolNativeAssetsBuilder.resolve('$buildDirName/');
final outDirUri = buildDirUri.resolve('out/');
final outDir = Directory.fromUri(outDirUri);
if (!await outDir.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDir.create(recursive: true);
}
final outputDirectoryShared = packageLayout.dartToolNativeAssetsBuilder
.resolve('shared/${package.name}/$hook/');
final outDirShared = Directory.fromUri(outputDirectoryShared);
if (!await outDirShared.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDirShared.create(recursive: true);
}
configBuilder.setupBuildRunConfig(
outputDirectory: outDirUri,
outputDirectoryShared: outputDirectoryShared,
);

final config = BuildConfig(configBuilder.json);
final packageConfigUri = packageLayout.packageConfigUri;
final hookCompileResult = await _compileHookForPackageCached(
config.packageName,
config.outputDirectory,
config.packageRoot.resolve('hook/${hook.scriptName}'),
packageConfigUri,
workingDirectory,
);
if (hookCompileResult == null) return null;
final (hookKernelFile, _) = hookCompileResult;

// TODO(https://github.com/dart-lang/native/issues/1321): Should dry runs be cached?
final buildOutput = await runUnderDirectoriesLock(
[
Directory.fromUri(config.outputDirectoryShared.parent),
Directory.fromUri(config.outputDirectory.parent),
],
timeout: singleHookTimeout,
logger: logger,
() => _runHookForPackage(
hook,
config,
validator,
packageConfigUri,
workingDirectory,
null,
hookKernelFile,
packageLayout!,
_filteredEnvironment(_environmentVariablesFilter),
),
);
if (buildOutput == null) return null;
hookResult = hookResult.copyAdd(buildOutput, [/*dry run is not cached*/]);
}
return hookResult;
}

Future<(HookOutput, List<Uri>)?> _runHookForPackageCached(
Hook hook,
HookConfig config,
Expand Down Expand Up @@ -534,8 +407,9 @@ ${e.message}
final modifiedDuringBuild = await dependenciesHashes.hashDependencies(
[
...result.dependencies,
// Also depend on the hook source code.
hookHashes.file.uri,
// Also depend on the compiled hook. Don't depend on the sources,
// if only whitespace changes, we don't need to rerun the hook.
hookKernelFile.uri,
],
lastModifiedCutoffTime,
environment,
Expand Down
15 changes: 0 additions & 15 deletions pkgs/native_assets_builder/lib/src/model/build_dry_run_result.dart

This file was deleted.

2 changes: 1 addition & 1 deletion pkgs/native_assets_builder/lib/src/model/hook_result.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import '../../native_assets_builder.dart';

/// The result from a [NativeAssetsBuildRunner.build] or
/// [NativeAssetsBuildRunner.link].
final class HookResult implements BuildResult, BuildDryRunResult, LinkResult {
final class HookResult implements BuildResult, LinkResult {
/// The native encodedAssets produced by the hooks, which should be bundled.
@override
final List<EncodedAsset> encodedAssets;
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.9.1-wip
version: 0.10.0-wip
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder

publish_to: none
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,6 @@ void main() async {
workingDirectory: packageUri,
logger: logger,
);
{
final logMessages = <String>[];
final result = await buildDryRun(
packageUri,
createCapturingLogger(logMessages, level: Level.SEVERE),
dartExecutable,
linkingEnabled: false,
buildAssetTypes: [],
buildValidator: (config, output) async => [],
);
final fullLog = logMessages.join('\n');
expect(result, isNull);
expect(
fullLog,
contains(
'Cyclic dependency for native asset builds in the following '
'packages: [cyclic_package_1, cyclic_package_2]',
),
);
}

{
final logMessages = <String>[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'helpers.dart';
const Timeout longTimeout = Timeout(Duration(minutes: 5));

void main() async {
test('multiple dryRun and build invocations', timeout: longTimeout, () async {
test('multiple build invocations', timeout: longTimeout, () async {
await inTempDir((tempUri) async {
await copyTestProjects(targetUri: tempUri);
final packageUri = tempUri.resolve('package_reading_metadata/');
Expand All @@ -33,22 +33,6 @@ void main() async {
linkModePreference: LinkModePreference.dynamic,
);

await buildRunner.buildDryRun(
configCreator: configCreator,
targetOS: Target.current.os,
workingDirectory: packageUri,
linkingEnabled: false,
buildAssetTypes: [],
buildValidator: (config, output) async => [],
);
await buildRunner.buildDryRun(
configCreator: configCreator,
targetOS: Target.current.os,
workingDirectory: packageUri,
linkingEnabled: false,
buildAssetTypes: [],
buildValidator: (config, output) async => [],
);
await buildRunner.build(
configCreator: configCreator,
targetOS: OS.current,
Expand Down
32 changes: 0 additions & 32 deletions pkgs/native_assets_builder/test/build_runner/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -265,38 +265,6 @@ Future<T> runWithLog<T>(
return result;
}

Future<BuildDryRunResult?> buildDryRun(
Uri packageUri,
Logger logger,
Uri dartExecutable, {
required BuildValidator buildValidator,
LinkModePreference linkModePreference = LinkModePreference.dynamic,
CCompilerConfig? cCompilerConfig,
List<String>? capturedLogs,
PackageLayout? packageLayout,
required bool linkingEnabled,
required List<String> buildAssetTypes,
}) async =>
runWithLog(capturedLogs, () async {
final result = await NativeAssetsBuildRunner(
logger: logger,
dartExecutable: dartExecutable,
).buildDryRun(
configCreator: () => BuildConfigBuilder()
..setupCodeConfig(
targetArchitecture: null,
linkModePreference: linkModePreference,
),
targetOS: Target.current.os,
workingDirectory: packageUri,
packageLayout: packageLayout,
linkingEnabled: linkingEnabled,
buildAssetTypes: buildAssetTypes,
buildValidator: buildValidator,
);
return result;
});

Future<void> expectSymbols({
required CodeAsset asset,
required List<String> symbols,
Expand Down
Loading
Loading