Skip to content

Commit a5290f7

Browse files
authored
[native_assets_builder] Lock the build directory (#1384)
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`.)
1 parent c3f93d2 commit a5290f7

File tree

11 files changed

+702
-51
lines changed

11 files changed

+702
-51
lines changed

pkgs/native_assets_builder/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
`LinkConfig.dependencies` no longer have to specify Dart sources.
55
- `DataAsset` test projects report all assets from `assets/` dir and default the
66
asset names to the path inside the package.
7+
- Automatically locks build directories to prevent concurrency issues with
8+
multiple concurrent `dart` and or `flutter` invocations.
79

810
## 0.8.1
911

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

Lines changed: 67 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:async';
66
import 'dart:io';
77

88
import 'package:logging/logging.dart';
9+
import 'package:native_assets_cli/locking.dart';
910
import 'package:native_assets_cli/native_assets_cli.dart' as api;
1011
import 'package:native_assets_cli/native_assets_cli_internal.dart';
1112
import 'package:package_config/package_config.dart';
@@ -34,11 +35,13 @@ typedef DependencyMetadata = Map<String, Metadata>;
3435
class NativeAssetsBuildRunner {
3536
final Logger logger;
3637
final Uri dartExecutable;
38+
final Duration singleHookTimeout;
3739

3840
NativeAssetsBuildRunner({
3941
required this.logger,
4042
required this.dartExecutable,
41-
});
43+
Duration? singleHookTimeout,
44+
}) : singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5);
4245

4346
/// [workingDirectory] is expected to contain `.dart_tool`.
4447
///
@@ -413,14 +416,19 @@ class NativeAssetsBuildRunner {
413416
continue;
414417
}
415418
// TODO(https://github.com/dart-lang/native/issues/1321): Should dry runs be cached?
416-
var (buildOutput, packageSuccess) = await _runHookForPackage(
417-
hook,
418-
config,
419-
packageConfigUri,
420-
workingDirectory,
421-
includeParentEnvironment,
422-
null,
423-
hookKernelFile,
419+
var (buildOutput, packageSuccess) = await runUnderDirectoryLock(
420+
Directory.fromUri(config.outputDirectory.parent),
421+
timeout: singleHookTimeout,
422+
logger: logger,
423+
() => _runHookForPackage(
424+
hook,
425+
config,
426+
packageConfigUri,
427+
workingDirectory,
428+
includeParentEnvironment,
429+
null,
430+
hookKernelFile,
431+
),
424432
);
425433
buildOutput = _expandArchsNativeCodeAssets(buildOutput);
426434
hookResult = hookResult.copyAdd(buildOutput, packageSuccess);
@@ -460,47 +468,54 @@ class NativeAssetsBuildRunner {
460468
Uri? resources,
461469
) async {
462470
final outDir = config.outputDirectory;
463-
final (
464-
compileSuccess,
465-
hookKernelFile,
466-
hookLastSourceChange,
467-
) = await _compileHookForPackageCached(
468-
config,
469-
packageConfigUri,
470-
workingDirectory,
471-
includeParentEnvironment,
472-
);
473-
if (!compileSuccess) {
474-
return (HookOutputImpl(), false);
475-
}
476-
477-
final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
478-
if (hookOutput != null) {
479-
final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
480-
final dependenciesLastChange =
481-
await hookOutput.dependenciesModel.lastModified();
482-
if (lastBuilt.isAfter(dependenciesLastChange) &&
483-
lastBuilt.isAfter(hookLastSourceChange)) {
484-
logger.info(
485-
'Skipping ${hook.name} for ${config.packageName} in $outDir. '
486-
'Last build on $lastBuilt. '
487-
'Last dependencies change on $dependenciesLastChange. '
488-
'Last hook change on $hookLastSourceChange.',
471+
return await runUnderDirectoryLock(
472+
Directory.fromUri(config.outputDirectory.parent),
473+
timeout: singleHookTimeout,
474+
logger: logger,
475+
() async {
476+
final (
477+
compileSuccess,
478+
hookKernelFile,
479+
hookLastSourceChange,
480+
) = await _compileHookForPackageCached(
481+
config,
482+
packageConfigUri,
483+
workingDirectory,
484+
includeParentEnvironment,
489485
);
490-
// All build flags go into [outDir]. Therefore we do not have to check
491-
// here whether the config is equal.
492-
return (hookOutput, true);
493-
}
494-
}
486+
if (!compileSuccess) {
487+
return (HookOutputImpl(), false);
488+
}
489+
490+
final hookOutput = HookOutputImpl.readFromFile(file: config.outputFile);
491+
if (hookOutput != null) {
492+
final lastBuilt = hookOutput.timestamp.roundDownToSeconds();
493+
final dependenciesLastChange =
494+
await hookOutput.dependenciesModel.lastModified();
495+
if (lastBuilt.isAfter(dependenciesLastChange) &&
496+
lastBuilt.isAfter(hookLastSourceChange)) {
497+
logger.info(
498+
'Skipping ${hook.name} for ${config.packageName} in $outDir. '
499+
'Last build on $lastBuilt. '
500+
'Last dependencies change on $dependenciesLastChange. '
501+
'Last hook change on $hookLastSourceChange.',
502+
);
503+
// All build flags go into [outDir]. Therefore we do not have to
504+
// check here whether the config is equal.
505+
return (hookOutput, true);
506+
}
507+
}
495508

496-
return await _runHookForPackage(
497-
hook,
498-
config,
499-
packageConfigUri,
500-
workingDirectory,
501-
includeParentEnvironment,
502-
resources,
503-
hookKernelFile,
509+
return await _runHookForPackage(
510+
hook,
511+
config,
512+
packageConfigUri,
513+
workingDirectory,
514+
includeParentEnvironment,
515+
resources,
516+
hookKernelFile,
517+
);
518+
},
504519
);
505520
}
506521

@@ -849,3 +864,7 @@ extension on DateTime {
849864
DateTime.fromMillisecondsSinceEpoch(millisecondsSinceEpoch -
850865
millisecondsSinceEpoch % const Duration(seconds: 1).inMilliseconds);
851866
}
867+
868+
extension on Uri {
869+
Uri get parent => File(toFilePath()).parent.uri;
870+
}

0 commit comments

Comments
 (0)