Skip to content

Commit

Permalink
[native_assets_builder] Use package:files FileSystem abstraction (
Browse files Browse the repository at this point in the history
…#1825)

The main motivation for this change is to make `package:native_assets_builder` cleanly usable in flutter tools. Right now flutter tools operates on a `FileSystem` and checks whether `.dart_tool/package_config.json` exists using that `FileSystem` but then reads the file with this packagag's code which uses `dart:io`.

This inconsistency causes issues in flutter tools. Using `package:file` should solve this and would also
allow injecting mocks. To be fully mockable the implementation would also need to make process invocations
mockable via `package:process`. That's a future task.

See also #90
  • Loading branch information
mkustermann authored Dec 19, 2024
1 parent 8c16b6c commit 9596c1f
Show file tree
Hide file tree
Showing 20 changed files with 158 additions and 102 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 @@
- Various fixes to caching.
- **Breaking change** `BuildConfig.targetOS` is now only provided if
`buildAssetTypes` contains the code asset.
- **Breaking change** `NativeAssetsBuildRunner` and `PackageLayout` now take a
`FileSystem` from `package:file/file.dart`s.

## 0.9.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:io' show Process;

import 'package:graphs/graphs.dart' as graphs;
import 'package:logging/logging.dart';
Expand Down
73 changes: 39 additions & 34 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

import 'dart:async';
import 'dart:convert';
import 'dart:io';
import 'dart:io' show Platform;

import 'package:file/file.dart';
import 'package:logging/logging.dart';
import 'package:native_assets_cli/native_assets_cli_internal.dart';
import 'package:package_config/package_config.dart';
Expand Down Expand Up @@ -63,15 +64,18 @@ typedef ApplicationAssetValidator = Future<ValidationErrors> Function(
/// [BuildConfig] and [LinkConfig]! For more info see:
/// https://github.com/dart-lang/native/issues/1319
class NativeAssetsBuildRunner {
final FileSystem _fileSystem;
final Logger logger;
final Uri dartExecutable;
final Duration singleHookTimeout;

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

/// [workingDirectory] is expected to contain `.dart_tool`.
///
Expand All @@ -98,7 +102,8 @@ class NativeAssetsBuildRunner {
required List<String> buildAssetTypes,
required bool linkingEnabled,
}) async {
packageLayout ??= await PackageLayout.fromRootPackageRoot(workingDirectory);
packageLayout ??=
await PackageLayout.fromRootPackageRoot(_fileSystem, workingDirectory);

final (buildPlan, packageGraph) = await _makePlan(
hook: Hook.build,
Expand Down Expand Up @@ -203,7 +208,8 @@ class NativeAssetsBuildRunner {
required List<String> buildAssetTypes,
required BuildResult buildResult,
}) async {
packageLayout ??= await PackageLayout.fromRootPackageRoot(workingDirectory);
packageLayout ??=
await PackageLayout.fromRootPackageRoot(_fileSystem, workingDirectory);

final (buildPlan, packageGraph) = await _makePlan(
hook: Hook.link,
Expand Down Expand Up @@ -231,9 +237,9 @@ class NativeAssetsBuildRunner {

File? resourcesFile;
if (resourceIdentifiers != null) {
resourcesFile = File.fromUri(buildDirUri.resolve('resources.json'));
resourcesFile = _fileSystem.file(buildDirUri.resolve('resources.json'));
await resourcesFile.create();
await File.fromUri(resourceIdentifiers).copy(resourcesFile.path);
await _fileSystem.file(resourceIdentifiers).copy(resourcesFile.path);
}
configBuilder.setupLinkRunConfig(
outputDirectory: outDirUri,
Expand Down Expand Up @@ -291,14 +297,14 @@ class NativeAssetsBuildRunner {
final buildDirUri =
packageLayout.dartToolNativeAssetsBuilder.resolve('$buildDirName/');
final outDirUri = buildDirUri.resolve('out/');
final outDir = Directory.fromUri(outDirUri);
final outDir = _fileSystem.directory(outDirUri);
if (!await outDir.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDir.create(recursive: true);
}
final outDirSharedUri = packageLayout.dartToolNativeAssetsBuilder
.resolve('shared/${package.name}/$hook/');
final outDirShared = Directory.fromUri(outDirSharedUri);
final outDirShared = _fileSystem.directory(outDirSharedUri);
if (!await outDirShared.exists()) {
// TODO(https://dartbug.com/50565): Purge old or unused folders.
await outDirShared.create(recursive: true);
Expand All @@ -318,9 +324,10 @@ class NativeAssetsBuildRunner {
final environment = _filteredEnvironment(_environmentVariablesFilter);
final outDir = config.outputDirectory;
return await runUnderDirectoriesLock(
_fileSystem,
[
Directory.fromUri(config.outputDirectoryShared.parent),
Directory.fromUri(config.outputDirectory.parent),
_fileSystem.directory(config.outputDirectoryShared).parent.uri,
_fileSystem.directory(config.outputDirectory).parent.uri,
],
timeout: singleHookTimeout,
logger: logger,
Expand All @@ -338,15 +345,13 @@ class NativeAssetsBuildRunner {
final (hookKernelFile, hookHashes) = hookCompileResult;

final buildOutputFile =
File.fromUri(config.outputDirectory.resolve(hook.outputName));
final dependenciesHashFile = File.fromUri(
config.outputDirectory
.resolve('../dependencies.dependencies_hash_file.json'),
);
_fileSystem.file(config.outputDirectory.resolve(hook.outputName));
final dependenciesHashFile = config.outputDirectory
.resolve('../dependencies.dependencies_hash_file.json');
final dependenciesHashes =
DependenciesHashFile(file: dependenciesHashFile);
DependenciesHashFile(_fileSystem, fileUri: dependenciesHashFile);
final lastModifiedCutoffTime = DateTime.now();
if (buildOutputFile.existsSync() && dependenciesHashFile.existsSync()) {
if (buildOutputFile.existsSync() && await dependenciesHashes.exists()) {
late final HookOutput output;
try {
output = _readHookOutputFromUri(hook, buildOutputFile);
Expand Down Expand Up @@ -391,8 +396,8 @@ ${e.message}
environment,
);
if (result == null) {
if (await dependenciesHashFile.exists()) {
await dependenciesHashFile.delete();
if (await dependenciesHashes.exists()) {
await dependenciesHashes.delete();
}
return null;
} else {
Expand Down Expand Up @@ -446,9 +451,9 @@ ${e.message}
final configFileContents =
const JsonEncoder.withIndent(' ').convert(config.json);
logger.info('config.json contents: $configFileContents');
await File.fromUri(configFile).writeAsString(configFileContents);
await _fileSystem.file(configFile).writeAsString(configFileContents);
final hookOutputUri = config.outputDirectory.resolve(hook.outputName);
final hookOutputFile = File.fromUri(hookOutputUri);
final hookOutputFile = _fileSystem.file(hookOutputUri);
if (await hookOutputFile.exists()) {
// Ensure we'll never read outdated build results.
await hookOutputFile.delete();
Expand All @@ -461,6 +466,7 @@ ${e.message}
if (resources != null) resources.toFilePath(),
];
final result = await runProcess(
filesystem: _fileSystem,
workingDirectory: workingDirectory,
executable: dartExecutable,
arguments: arguments,
Expand All @@ -472,7 +478,8 @@ ${e.message}
var deleteOutputIfExists = false;
try {
if (result.exitCode != 0) {
final printWorkingDir = workingDirectory != Directory.current.uri;
final printWorkingDir =
workingDirectory != _fileSystem.currentDirectory.uri;
final commandString = [
if (printWorkingDir) '(cd ${workingDirectory.toFilePath()};',
dartExecutable.toFilePath(),
Expand Down Expand Up @@ -558,19 +565,19 @@ ${e.message}
) async {
// Don't invalidate cache with environment changes.
final environmentForCaching = <String, String>{};
final kernelFile = File.fromUri(
final kernelFile = _fileSystem.file(
outputDirectory.resolve('../hook.dill'),
);
final depFile = File.fromUri(
final depFile = _fileSystem.file(
outputDirectory.resolve('../hook.dill.d'),
);
final dependenciesHashFile = File.fromUri(
outputDirectory.resolve('../hook.dependencies_hash_file.json'),
);
final dependenciesHashes = DependenciesHashFile(file: dependenciesHashFile);
final dependenciesHashFile =
outputDirectory.resolve('../hook.dependencies_hash_file.json');
final dependenciesHashes =
DependenciesHashFile(_fileSystem, fileUri: dependenciesHashFile);
final lastModifiedCutoffTime = DateTime.now();
var mustCompile = false;
if (!await dependenciesHashFile.exists()) {
if (!await dependenciesHashes.exists()) {
mustCompile = true;
} else {
final outdatedDependency = await dependenciesHashes
Expand Down Expand Up @@ -633,6 +640,7 @@ ${e.message}
scriptUri.toFilePath(),
];
final compileResult = await runProcess(
filesystem: _fileSystem,
workingDirectory: workingDirectory,
executable: dartExecutable,
arguments: compileArguments,
Expand All @@ -641,7 +649,8 @@ ${e.message}
);
var success = true;
if (compileResult.exitCode != 0) {
final printWorkingDir = workingDirectory != Directory.current.uri;
final printWorkingDir =
workingDirectory != _fileSystem.currentDirectory.uri;
final commandString = [
if (printWorkingDir) '(cd ${workingDirectory.toFilePath()};',
dartExecutable.toFilePath(),
Expand Down Expand Up @@ -780,10 +789,6 @@ ${compileResult.stdout}
}
}

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

/// Parses depfile contents.
///
/// Format: `path/to/my.dill: path/to/my.dart, path/to/more.dart`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,32 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:convert';
import 'dart:io';
import 'dart:io' show Platform;
import 'dart:typed_data';

import 'package:crypto/crypto.dart';
import 'package:file/file.dart';

import '../utils/file.dart';
import '../utils/uri.dart';

class DependenciesHashFile {
DependenciesHashFile({
required this.file,
DependenciesHashFile(
this._fileSystem, {
required this.fileUri,
});

final File file;
final FileSystem _fileSystem;
final Uri fileUri;
FileSystemHashes _hashes = FileSystemHashes();

List<Uri> get fileSystemEntities => _hashes.files.map((e) => e.path).toList();

Future<bool> exists() async => await _fileSystem.file(fileUri).exists();
Future<void> delete() async => await _fileSystem.file(fileUri).delete();

Future<void> _readFile() async {
final file = _fileSystem.file(fileUri);
if (!await file.exists()) {
_hashes = FileSystemHashes();
return;
Expand Down Expand Up @@ -51,7 +58,7 @@ class DependenciesHashFile {
Uri? modifiedAfterTimeStamp;
for (final uri in fileSystemEntities) {
int hash;
if ((await uri.fileSystemEntity.lastModified())
if ((await _fileSystem.fileSystemEntity(uri).lastModified(_fileSystem))
.isAfter(fileSystemValidBeforeLastModified)) {
hash = _hashLastModifiedAfterCutoff;
modifiedAfterTimeStamp = uri;
Expand All @@ -72,7 +79,8 @@ class DependenciesHashFile {
return modifiedAfterTimeStamp;
}

Future<void> _persist() => file.writeAsString(json.encode(_hashes.toJson()));
Future<void> _persist() =>
_fileSystem.file(fileUri).writeAsString(json.encode(_hashes.toJson()));

/// Reads the file with hashes and reports if there is an outdated file,
/// directory or environment variable.
Expand Down Expand Up @@ -124,15 +132,15 @@ class DependenciesHashFile {
}

Future<int> _hashFile(Uri uri) async {
final file = File.fromUri(uri);
final file = _fileSystem.file(uri);
if (!await file.exists()) {
return _hashNotExists;
}
return _md5int64(await file.readAsBytes());
}

Future<int> _hashDirectory(Uri uri) async {
final directory = Directory.fromUri(uri);
final directory = _fileSystem.directory(uri);
if (!await directory.exists()) {
return _hashNotExists;
}
Expand Down
15 changes: 10 additions & 5 deletions pkgs/native_assets_builder/lib/src/locking/locking.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
// BSD-style license that can be found in the LICENSE file.

import 'dart:async';
import 'dart:io';
import 'dart:io' show Platform, pid;
import 'package:file/file.dart';

import 'package:logging/logging.dart';

Future<T> runUnderDirectoriesLock<T>(
List<Directory> directories,
FileSystem fileSystem,
List<Uri> directories,
Future<T> Function() callback, {
Duration? timeout,
Logger? logger,
Expand All @@ -17,8 +19,10 @@ Future<T> runUnderDirectoriesLock<T>(
return await callback();
}
return await runUnderDirectoryLock(
fileSystem,
directories.first,
() => runUnderDirectoriesLock<T>(
fileSystem,
directories.skip(1).toList(),
callback,
timeout: timeout,
Expand All @@ -41,13 +45,14 @@ Future<T> runUnderDirectoriesLock<T>(
/// If provided, the [logger] streams information on the the locking status, and
/// also streams error messages.
Future<T> runUnderDirectoryLock<T>(
Directory directory,
FileSystem fileSystem,
Uri directory,
Future<T> Function() callback, {
Duration? timeout,
Logger? logger,
}) async {
const lockFileName = '.lock';
final lockFile = _fileInDir(directory, lockFileName);
final lockFile = _fileInDir(fileSystem.directory(directory), lockFileName);
return _runUnderFileLock(
lockFile,
callback,
Expand All @@ -60,7 +65,7 @@ File _fileInDir(Directory path, String filename) {
final dirPath = path.path;
var separator = Platform.pathSeparator;
if (dirPath.endsWith(separator)) separator = '';
return File('$dirPath$separator$filename');
return path.fileSystem.file('$dirPath$separator$filename');
}

/// Run [callback] with this Dart process having exclusive access to [file].
Expand Down
Loading

0 comments on commit 9596c1f

Please sign in to comment.