Skip to content

Commit

Permalink
[native_assets_builder] Lock the build directory (#1384)
Browse files Browse the repository at this point in the history
This makes the native assets builder lock the build directories so that it can be invoked concurrently from multiple `dart` and `flutter` processes.

Closes: #1319

The implementation of the locking is in `package:native_assets_cli/locking.dart` contains `runUnderDirectoryLock`, so that we can reuse it for #1379. (#1379 Requires more changes though, it needs to also give users access to a shared directory via the `BuildConfig`.)
  • Loading branch information
dcharkes authored Aug 6, 2024
1 parent c3f93d2 commit a5290f7
Show file tree
Hide file tree
Showing 11 changed files with 702 additions and 51 deletions.
2 changes: 2 additions & 0 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
`LinkConfig.dependencies` no longer have to specify Dart sources.
- `DataAsset` test projects report all assets from `assets/` dir and default the
asset names to the path inside the package.
- Automatically locks build directories to prevent concurrency issues with
multiple concurrent `dart` and or `flutter` invocations.

## 0.8.1

Expand Down
115 changes: 67 additions & 48 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';
import 'dart:io';

import 'package:logging/logging.dart';
import 'package:native_assets_cli/locking.dart';
import 'package:native_assets_cli/native_assets_cli.dart' as api;
import 'package:native_assets_cli/native_assets_cli_internal.dart';
import 'package:package_config/package_config.dart';
Expand Down Expand Up @@ -34,11 +35,13 @@ typedef DependencyMetadata = Map<String, Metadata>;
class NativeAssetsBuildRunner {
final Logger logger;
final Uri dartExecutable;
final Duration singleHookTimeout;

NativeAssetsBuildRunner({
required this.logger,
required this.dartExecutable,
});
Duration? singleHookTimeout,
}) : singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5);

/// [workingDirectory] is expected to contain `.dart_tool`.
///
Expand Down Expand Up @@ -413,14 +416,19 @@ class NativeAssetsBuildRunner {
continue;
}
// TODO(https://github.com/dart-lang/native/issues/1321): Should dry runs be cached?
var (buildOutput, packageSuccess) = await _runHookForPackage(
hook,
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
null,
hookKernelFile,
var (buildOutput, packageSuccess) = await runUnderDirectoryLock(
Directory.fromUri(config.outputDirectory.parent),
timeout: singleHookTimeout,
logger: logger,
() => _runHookForPackage(
hook,
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
null,
hookKernelFile,
),
);
buildOutput = _expandArchsNativeCodeAssets(buildOutput);
hookResult = hookResult.copyAdd(buildOutput, packageSuccess);
Expand Down Expand Up @@ -460,47 +468,54 @@ class NativeAssetsBuildRunner {
Uri? resources,
) async {
final outDir = config.outputDirectory;
final (
compileSuccess,
hookKernelFile,
hookLastSourceChange,
) = await _compileHookForPackageCached(
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
);
if (!compileSuccess) {
return (HookOutputImpl(), false);
}

final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
if (hookOutput != null) {
final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
final dependenciesLastChange =
await hookOutput.dependenciesModel.lastModified();
if (lastBuilt.isAfter(dependenciesLastChange) &&
lastBuilt.isAfter(hookLastSourceChange)) {
logger.info(
'Skipping ${hook.name} for ${config.packageName} in $outDir. '
'Last build on $lastBuilt. '
'Last dependencies change on $dependenciesLastChange. '
'Last hook change on $hookLastSourceChange.',
return await runUnderDirectoryLock(
Directory.fromUri(config.outputDirectory.parent),
timeout: singleHookTimeout,
logger: logger,
() async {
final (
compileSuccess,
hookKernelFile,
hookLastSourceChange,
) = await _compileHookForPackageCached(
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
);
// All build flags go into [outDir]. Therefore we do not have to check
// here whether the config is equal.
return (hookOutput, true);
}
}
if (!compileSuccess) {
return (HookOutputImpl(), false);
}

final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
if (hookOutput != null) {
final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
final dependenciesLastChange =
await hookOutput.dependenciesModel.lastModified();
if (lastBuilt.isAfter(dependenciesLastChange) &&
lastBuilt.isAfter(hookLastSourceChange)) {
logger.info(
'Skipping ${hook.name} for ${config.packageName} in $outDir. '
'Last build on $lastBuilt. '
'Last dependencies change on $dependenciesLastChange. '
'Last hook change on $hookLastSourceChange.',
);
// All build flags go into [outDir]. Therefore we do not have to
// check here whether the config is equal.
return (hookOutput, true);
}
}

return await _runHookForPackage(
hook,
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
resources,
hookKernelFile,
return await _runHookForPackage(
hook,
config,
packageConfigUri,
workingDirectory,
includeParentEnvironment,
resources,
hookKernelFile,
);
},
);
}

Expand Down Expand Up @@ -849,3 +864,7 @@ extension on DateTime {
DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch -
millisecondsSinceEpoch % const Duration(seconds: 1).inMilliseconds);
}

extension on Uri {
Uri get parent => File(toFilePath()).parent.uri;
}
Loading

0 comments on commit a5290f7

Please sign in to comment.