Skip to content

Commit

Permalink
[native_assets_cli] Add outputDirectoryShared to config (#1557)
Browse files Browse the repository at this point in the history
  • Loading branch information
dcharkes authored Sep 13, 2024
1 parent 4af11a0 commit 989ef50
Show file tree
Hide file tree
Showing 61 changed files with 785 additions and 46 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/native.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ jobs:
- run: dart pub get -C test_data/add_asset_link/
if: ${{ matrix.package == 'native_assets_builder' }}

- run: dart pub get -C test_data/transformer/
if: ${{ matrix.package == 'native_assets_builder' }}

- run: dart pub get -C test_data/treeshaking_native_libs/
if: ${{ matrix.package == 'native_assets_builder' }}

Expand Down
3 changes: 2 additions & 1 deletion pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.8.4-wip

- Nothing yet.
- Also lock `BuildConfig` and `LinkConfig` `outputDirectoryShared` when invoking
hooks to prevent concurrency issues with shared output caching.

## 0.8.3

Expand Down
47 changes: 38 additions & 9 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,11 +6,11 @@ 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';

import '../locking/locking.dart';
import '../model/build_dry_run_result.dart';
import '../model/build_result.dart';
import '../model/hook_result.dart';
Expand Down Expand Up @@ -263,13 +263,21 @@ class NativeAssetsBuildRunner {
);
final buildDirUri =
packageLayout.dartToolNativeAssetsBuilder.resolve('$buildDirName/');
final outDirUri = buildDirUri.resolve('out/');
final outDir = Directory.fromUri(outDirUri);
final outputDirectory = buildDirUri.resolve('out/');
final outDir = Directory.fromUri(outputDirectory);
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);
}

if (hook == Hook.link) {
File? resourcesFile;
if (resourceIdentifiers != null) {
Expand All @@ -279,7 +287,8 @@ class NativeAssetsBuildRunner {
}

return LinkConfigImpl(
outputDirectory: outDirUri,
outputDirectory: outputDirectory,
outputDirectoryShared: outputDirectoryShared,
packageName: package.name,
packageRoot: package.root,
targetOS: target.os,
Expand All @@ -297,7 +306,8 @@ class NativeAssetsBuildRunner {
);
} else {
return BuildConfigImpl(
outputDirectory: outDirUri,
outputDirectory: outputDirectory,
outputDirectoryShared: outputDirectoryShared,
packageName: package.name,
packageRoot: package.root,
targetOS: target.os,
Expand Down Expand Up @@ -428,8 +438,11 @@ class NativeAssetsBuildRunner {
continue;
}
// TODO(https://github.com/dart-lang/native/issues/1321): Should dry runs be cached?
var (buildOutput, packageSuccess) = await runUnderDirectoryLock(
Directory.fromUri(config.outputDirectory.parent),
var (buildOutput, packageSuccess) = await runUnderDirectoriesLock(
[
Directory.fromUri(config.outputDirectoryShared.parent),
Directory.fromUri(config.outputDirectory.parent),
],
timeout: singleHookTimeout,
logger: logger,
() => _runHookForPackage(
Expand Down Expand Up @@ -482,8 +495,11 @@ class NativeAssetsBuildRunner {
PackageLayout packageLayout,
) async {
final outDir = config.outputDirectory;
return await runUnderDirectoryLock(
Directory.fromUri(config.outputDirectory.parent),
return await runUnderDirectoriesLock(
[
Directory.fromUri(config.outputDirectoryShared.parent),
Directory.fromUri(config.outputDirectory.parent),
],
timeout: singleHookTimeout,
logger: logger,
() async {
Expand Down Expand Up @@ -784,15 +800,27 @@ ${compileResult.stdout}
hook: hook,
linkingEnabled: linkingEnabled,
);

final outDirUri = buildParentDir.resolve('$buildDirName/out/');
final outDir = Directory.fromUri(outDirUri);
if (!await outDir.exists()) {
await outDir.create(recursive: true);
}

// Shared between dry run and wet run.
final outputDirectoryShared =
buildParentDir.resolve('shared/${package.name}/$hook/out/');
final outDirShared = Directory.fromUri(outputDirectoryShared);
if (!await outDirShared.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDirShared.create(recursive: true);
}

switch (hook) {
case Hook.build:
return BuildConfigImpl.dryRun(
outputDirectory: outDirUri,
outputDirectoryShared: outputDirectoryShared,
packageName: packageName,
packageRoot: packageRoot,
targetOS: targetOS,
Expand All @@ -803,6 +831,7 @@ ${compileResult.stdout}
case Hook.link:
return LinkConfigImpl.dryRun(
outputDirectory: outDirUri,
outputDirectoryShared: outputDirectoryShared,
packageName: packageName,
packageRoot: packageRoot,
targetOS: targetOS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,28 @@ import 'dart:io';

import 'package:logging/logging.dart';

Future<T> runUnderDirectoriesLock<T>(
List<Directory> directories,
Future<T> Function() callback, {
Duration? timeout,
Logger? logger,
}) async {
if (directories.isEmpty) {
return await callback();
}
return await runUnderDirectoryLock(
directories.first,
() => runUnderDirectoriesLock<T>(
directories.skip(1).toList(),
callback,
timeout: timeout,
logger: logger,
),
timeout: timeout,
logger: logger,
);
}

/// Run [callback] with this Dart process having exclusive access to
/// [directory].
///
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';

import 'package:native_assets_builder/src/utils/run_process.dart'
show RunProcessResult;
import 'package:native_assets_cli/native_assets_cli_internal.dart';
import 'package:test/test.dart';

import '../helpers.dart';
import 'helpers.dart';

const Timeout longTimeout = Timeout(Duration(minutes: 5));

void main() async {
const packageName = 'transformer';

test('Concurrent invocations', timeout: longTimeout, () async {
await inTempDir((tempUri) async {
final packageUri = tempUri.resolve('$packageName/');
await copyTestProjects(
sourceUri: testDataUri.resolve('$packageName/'),
targetUri: packageUri,
);

await runPubGet(
workingDirectory: packageUri,
logger: logger,
);

Future<RunProcessResult> runBuildInProcess(
Target target,
) async {
final result = await runProcess(
executable: dartExecutable,
arguments: [
pkgNativeAssetsBuilderUri
.resolve(
'test/build_runner/concurrency_shared_test_helper.dart',
)
.toFilePath(),
packageUri.toFilePath(),
target.toString(),
],
workingDirectory: packageUri,
logger: logger,
);
expect(result.exitCode, 0);
return result;
}

// Simulate running `dart run` concurrently in 3 different terminals.
// Twice for the same target and also for a different target.
final results = await Future.wait([
runBuildInProcess(Target.androidArm64),
runBuildInProcess(Target.current),
runBuildInProcess(Target.current),
]);
final stdouts = results.map((e) => e.stdout).join('\n');
// One run for the two targets wins in running first.
expect(stdouts, stringContainsInOrder(['Reused 0 cached files.']));
// The other run reuses the cached results.
expect(stdouts, stringContainsInOrder(['Reused 10 cached files.']));
// One of the two runs for the same target will not be invoked at all.
expect(
stdouts,
stringContainsInOrder([
'Waiting to be able to obtain lock of directory',
'Skipping build for',
]),
);
});
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:logging/logging.dart';
import 'package:native_assets_builder/native_assets_builder.dart';
import 'package:native_assets_cli/native_assets_cli.dart';
import 'package:native_assets_cli/native_assets_cli_internal.dart';

import '../helpers.dart';

// Is invoked concurrently multiple times in separate processes.
void main(List<String> args) async {
final packageUri = Uri.directory(args[0]);
final target = Target.fromString(args[1]);

final logger = Logger('')
..level = Level.ALL
..onRecord.listen((event) => print(event.message));

final result = await NativeAssetsBuildRunner(
logger: logger,
dartExecutable: dartExecutable,
).build(
buildMode: BuildModeImpl.release,
linkModePreference: LinkModePreferenceImpl.dynamic,
target: target,
workingDirectory: packageUri,
includeParentEnvironment: true,
linkingEnabled: false,
supportedAssetTypes: [DataAsset.type],
targetAndroidNdkApi: target.os == OS.android ? 30 : null,
);
if (!result.success) {
throw Error();
}
print('done');
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import '../helpers.dart';
const Timeout longTimeout = Timeout(Duration(minutes: 5));

void main() async {
final packageUri = findPackageRoot('native_assets_builder');

test('Concurrent invocations', timeout: longTimeout, () async {
await inTempDir((tempUri) async {
Future<ProcessResult> runInProcess() async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import 'dart:io';

import 'package:native_assets_cli/locking.dart';
import 'package:native_assets_builder/src/locking/locking.dart';

void main(List<String> args) async {
final directory = Directory.fromUri(Uri.directory(args[0]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,18 @@ void main() async {
test(
'native_dynamic_linking build$testSuffix',
() => inTempDir((tempUri) async {
final outputDirectory = tempUri.resolve('out/');
await Directory.fromUri(outputDirectory).create();
final outputDirectoryShared = tempUri.resolve('out_shared/');
await Directory.fromUri(outputDirectoryShared).create();
final testTempUri = tempUri.resolve('test1/');
await Directory.fromUri(testTempUri).create();
final testPackageUri = testDataUri.resolve('$name/');
final dartUri = Uri.file(Platform.resolvedExecutable);

final config = BuildConfigImpl(
outputDirectory: tempUri,
outputDirectory: outputDirectory,
outputDirectoryShared: outputDirectoryShared,
packageName: name,
packageRoot: testPackageUri,
targetOS: OSImpl.current,
Expand Down Expand Up @@ -62,7 +67,7 @@ void main() async {
}
expect(processResult.exitCode, 0);

final buildOutputUri = tempUri.resolve('build_output.json');
final buildOutputUri = outputDirectory.resolve('build_output.json');
final buildOutput = HookOutputImpl.fromJsonString(
await File.fromUri(buildOutputUri).readAsString());
final assets = buildOutput.assets;
Expand Down Expand Up @@ -100,9 +105,12 @@ void main() async {
environment: {
// Add the directory containing the linked dynamic libraries to
// the PATH so that the dynamic linker can find them.
// TODO(https://github.com/dart-lang/sdk/issues/56551): We could
// skip this if Dart would implicitly add dylib containing
// directories to the PATH.
if (Platform.isWindows)
'PATH':
'${tempUri.toFilePath()};${Platform.environment['PATH']}',
'PATH': '${outputDirectory.toFilePath()};'
'${Platform.environment['PATH']}',
},
throwOnUnexpectedExitCode: true,
logger: logger,
Expand Down
Loading

0 comments on commit 989ef50

Please sign in to comment.