Skip to content

[native_assets_cli] Add base path for user-defines #2209

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 2 commits into from
Apr 16, 2025
Merged
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
22 changes: 21 additions & 1 deletion pkgs/hooks/doc/schema/shared/shared_definitions.schema.json
Original file line number Diff line number Diff line change
@@ -176,7 +176,11 @@
},
"user_defines": {
"type": "object",
"additionalProperties": true
"properties": {
"workspace_pubspec": {
"$ref": "#/definitions/UserDefinesSource"
}
}
},
"version": {
"type": "string"
@@ -256,6 +260,22 @@
"key"
]
},
"UserDefinesSource": {
"type": "object",
"properties": {
"base_path": {
"$ref": "#/definitions/absolutePath"
},
"defines": {
"type": "object",
"additionalProperties": true
}
},
"required": [
"base_path",
"defines"
]
},
"absolutePath": {
"type": "string",
"pattern": "^(\\/|[A-Za-z]:)"
5 changes: 5 additions & 0 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 0.16.0-wip

- Pass in path to pubspec (to read user-defines) in this package rather than in
the SDKs using this package.

## 0.15.0

- Bump `package:native_assets_cli` to 0.15.0.
6 changes: 5 additions & 1 deletion pkgs/native_assets_builder/lib/native_assets_builder.dart
Original file line number Diff line number Diff line change
@@ -3,7 +3,11 @@
// BSD-style license that can be found in the LICENSE file.

export 'package:native_assets_builder/src/build_runner/build_runner.dart'
show BuildInputCreator, LinkInputCreator, NativeAssetsBuildRunner;
show
BuildInputCreator,
LinkInputCreator,
NativeAssetsBuildRunner,
UserDefines;
export 'package:native_assets_builder/src/model/build_result.dart'
show BuildResult;
export 'package:native_assets_builder/src/model/kernel_assets.dart';
119 changes: 102 additions & 17 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@ import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import 'package:native_assets_cli/native_assets_cli_internal.dart';
import 'package:package_config/package_config.dart';
import 'package:yaml/yaml.dart';

import '../dependencies_hash_file/dependencies_hash_file.dart';
import '../locking/locking.dart';
@@ -46,7 +47,7 @@ class NativeAssetsBuildRunner {
final Uri dartExecutable;
final Duration singleHookTimeout;
final Map<String, String> hookEnvironment;
final Map<String, Map<String, Object?>?> userDefines;
final UserDefines? userDefines;
final PackageLayout packageLayout;

NativeAssetsBuildRunner({
@@ -56,7 +57,7 @@ class NativeAssetsBuildRunner {
required this.packageLayout,
Duration? singleHookTimeout,
Map<String, String>? hookEnvironment,
this.userDefines = const {},
this.userDefines,
}) : _fileSystem = fileSystem,
singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5),
hookEnvironment =
@@ -73,6 +74,24 @@ class NativeAssetsBuildRunner {
return packagesWithHook.map((e) => e.name).toList();
}

Future<HookResult?> _checkUserDefines(
LoadedUserDefines? loadedUserDefines,
) async {
if (loadedUserDefines?.pubspecErrors.isNotEmpty ?? false) {
logger.severe('pubspec.yaml contains errors');
for (final error in loadedUserDefines!.pubspecErrors) {
logger.severe(error);
}
return null;
}
return HookResult(
dependencies: switch (userDefines?.workspacePubspec) {
null => [],
final pubspec => [pubspec],
},
);
}

/// This method is invoked by launchers such as dartdev (for `dart run`) and
/// flutter_tools (for `flutter run` and `flutter build`).
///
@@ -86,14 +105,19 @@ class NativeAssetsBuildRunner {
required List<ProtocolExtension> extensions,
required bool linkingEnabled,
}) async {
final loadedUserDefines = await _loadedUserDefines;
final hookResultUserDefines = await _checkUserDefines(loadedUserDefines);
if (hookResultUserDefines == null) {
return null;
}
var hookResult = hookResultUserDefines;

final (buildPlan, packageGraph) = await _makePlan(
hook: Hook.build,
buildResult: null,
);
if (buildPlan == null) return null;

var hookResult = HookResult();

/// Key is packageName.
final globalMetadata = <String, Metadata>{};

@@ -133,7 +157,7 @@ class NativeAssetsBuildRunner {
outputFile: buildDirUri.resolve('output.json'),
outputDirectory: outDirUri,
outputDirectoryShared: outDirSharedUri,
userDefines: userDefines[package.name],
userDefines: loadedUserDefines?[package.name],
);

final input = BuildInput(inputBuilder.json);
@@ -199,13 +223,19 @@ class NativeAssetsBuildRunner {
Uri? resourceIdentifiers,
required BuildResult buildResult,
}) async {
final loadedUserDefines = await _loadedUserDefines;
final hookResultUserDefines = await _checkUserDefines(loadedUserDefines);
if (hookResultUserDefines == null) {
return null;
}
var linkResult = hookResultUserDefines;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if its worthwhile to factor this logic out since it seems to be 1:1 repeated from above?

final (buildPlan, packageGraph) = await _makePlan(
hook: Hook.link,
buildResult: buildResult,
);
if (buildPlan == null) return null;

var linkResult = HookResult();
for (final package in buildPlan) {
final inputBuilder = LinkInputBuilder();
for (final e in extensions) {
@@ -231,7 +261,7 @@ class NativeAssetsBuildRunner {
outputFile: buildDirUri.resolve('output.json'),
outputDirectory: outDirUri,
outputDirectoryShared: outDirSharedUri,
userDefines: userDefines[package.name],
userDefines: loadedUserDefines?[package.name],
);
inputBuilder.setupLink(
assets: buildResult.encodedAssetsForLinking[package.name] ?? [],
@@ -461,9 +491,9 @@ ${e.message}
) async {
final inputFile = buildDirUri.resolve('input.json');
final inputFileContents = const JsonEncoder.withIndent(
' ',
' ',
).convert(input.json);
logger.info('input.json contents: $inputFileContents');
logger.info('input.json contents:\n$inputFileContents');
await _fileSystem.file(inputFile).writeAsString(inputFileContents);
final hookOutputUri = input.outputFile;
final hookOutputFile = _fileSystem.file(hookOutputUri);
@@ -856,18 +886,18 @@ ${compileResult.stdout}
// TODO(dcharkes): Remove when hooks with 1.7.0 are no longer supported.
File hookOutputFileDeprecated,
) {
final decode = const Utf8Decoder().fuse(const JsonDecoder()).convert;
final file =
hookOutputFile.existsSync() ? hookOutputFile : hookOutputFileDeprecated;
final hookOutputJson =
decode(file.readAsBytesSync()) as Map<String, Object?>;
final fileContents = file.readAsStringSync();
logger.info('output.json contents:\n$fileContents');
final hookOutputJson = jsonDecode(fileContents) as Map<String, Object?>;
return hook == Hook.build
? BuildOutput(hookOutputJson)
: LinkOutput(hookOutputJson);
}

/// Returns a list of errors for [readHooksUserDefinesFromPubspec].
static List<String> validateHooksUserDefinesFromPubspec(
/// Returns a list of errors for [_readHooksUserDefinesFromPubspec].
static List<String> _validateHooksUserDefinesFromPubspec(
Map<Object?, Object?> pubspec,
) {
final hooks = pubspec['hooks'];
@@ -916,11 +946,11 @@ ${compileResult.stdout}
///
/// The [pubspec] is expected to be the decoded yaml, a Map.
///
/// Before invoking, check errors with [validateHooksUserDefinesFromPubspec].
static Map<String, Map<String, Object?>> readHooksUserDefinesFromPubspec(
/// Before invoking, check errors with [_validateHooksUserDefinesFromPubspec].
static Map<String, Map<String, Object?>> _readHooksUserDefinesFromPubspec(
Map<Object?, Object?> pubspec,
) {
assert(validateHooksUserDefinesFromPubspec(pubspec).isEmpty);
assert(_validateHooksUserDefinesFromPubspec(pubspec).isEmpty);
final hooks = pubspec['hooks'];
if (hooks is! Map) {
return {};
@@ -939,6 +969,61 @@ ${compileResult.stdout}
},
};
}

late final Future<LoadedUserDefines?> _loadedUserDefines = () async {
final pubspec = userDefines?.workspacePubspec;
if (pubspec == null) {
return null;
}
final contents = await _fileSystem.file(pubspec).readAsString();
final decoded = loadYaml(contents) as Map<Object?, Object?>;
final errors = _validateHooksUserDefinesFromPubspec(decoded);
final defines = _readHooksUserDefinesFromPubspec(decoded);
return LoadedUserDefines(
pubspecErrors: errors,
pubspecDefines: defines,
pubspecBasePath: pubspec,
);
}();
}

/// The user-defines information passed from the SDK to the
/// [NativeAssetsBuildRunner].
///
/// Currently only holds [workspacePubspec]. (In the future this class will also
/// take command-line arguments and a working directory for the command-line
/// argument paths to be resolved against.)
class UserDefines {
/// The pubspec.yaml of the pub workspace.
///
/// User-defines are read from this file.
final Uri? workspacePubspec;

UserDefines({required this.workspacePubspec});
}

class LoadedUserDefines {
final List<String> pubspecErrors;

final Map<String, Map<String, Object?>> pubspecDefines;

final Uri pubspecBasePath;

LoadedUserDefines({
required this.pubspecErrors,
required this.pubspecDefines,
required this.pubspecBasePath,
});

PackageUserDefines operator [](String packageName) => PackageUserDefines(
workspacePubspec: switch (pubspecDefines[packageName]) {
null => null,
final defines => PackageUserDefinesSource(
defines: defines,
basePath: pubspecBasePath,
),
},
);
}

/// Parses depfile contents.
Original file line number Diff line number Diff line change
@@ -22,7 +22,7 @@ import '../../native_assets_builder.dart';
/// convention for caching:
/// https://dart.dev/tools/pub/package-layout#project-specific-caching-for-tools
class PackageLayout {
/// Package config containing the information of where to foot the root [Uri]s
/// Package config containing the information of where to find the root [Uri]s
/// of other packages.
///
/// Can be `null` to enable quick construction of a
4 changes: 2 additions & 2 deletions 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.15.0
version: 0.16.0-wip
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder

resolution: workspace
@@ -16,7 +16,7 @@ dependencies:
graphs: ^2.3.2
logging: ^1.3.0
meta: ^1.16.0
native_assets_cli: ^0.15.0
native_assets_cli: ^0.16.0-wip
package_config: ^2.1.0
pub_semver: ^2.2.0
yaml: ^3.1.3
4 changes: 2 additions & 2 deletions pkgs/native_assets_builder/test/build_runner/helpers.dart
Original file line number Diff line number Diff line change
@@ -75,7 +75,7 @@ Future<BuildResult?> build(
bool linkingEnabled = false,
required List<BuildAssetType> buildAssetTypes,
Map<String, String>? hookEnvironment,
Map<String, Map<String, Object?>?>? userDefines,
UserDefines? userDefines,
}) async {
final targetOS = target?.os ?? OS.current;
final runPackageName_ =
@@ -92,7 +92,7 @@ Future<BuildResult?> build(
fileSystem: const LocalFileSystem(),
hookEnvironment: hookEnvironment,
packageLayout: packageLayout,
userDefines: userDefines ?? {},
userDefines: userDefines,
).build(
extensions: [
if (buildAssetTypes.contains(BuildAssetType.code))
31 changes: 14 additions & 17 deletions pkgs/native_assets_builder/test/test_data/user_defines_test.dart
Original file line number Diff line number Diff line change
@@ -7,9 +7,10 @@ library;

import 'dart:io';

import 'package:file_testing/file_testing.dart';
import 'package:native_assets_builder/native_assets_builder.dart';
import 'package:native_assets_builder/src/build_runner/build_runner.dart';
import 'package:test/test.dart';
import 'package:yaml/yaml.dart';

import '../build_runner/helpers.dart';
import '../helpers.dart';
@@ -25,32 +26,28 @@ void main() async {

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

final pubspec =
loadYamlDocument(
File.fromUri(
packageUri.resolve('pubspec.yaml'),
).readAsStringSync(),
).contents
as YamlMap;
expect(
NativeAssetsBuildRunner.validateHooksUserDefinesFromPubspec(pubspec),
isEmpty,
);
final userDefines =
NativeAssetsBuildRunner.readHooksUserDefinesFromPubspec(pubspec);

final logMessages = <String>[];
final pubspecUri = packageUri.resolve('pubspec.yaml');
final result =
(await build(
packageUri,
logger,
dartExecutable,
capturedLogs: logMessages,
buildAssetTypes: [BuildAssetType.data],
userDefines: userDefines,
userDefines: UserDefines(workspacePubspec: pubspecUri),
))!;

expect(result.encodedAssets.length, 1);
final dataAssets =
result.encodedAssets.map((e) => e.asDataAsset).toList();
expect(dataAssets.length, 2);
for (final dataAsset in dataAssets) {
expect(File.fromUri(dataAsset.file), exists);
}

// The native assets build runner must be reinvoked if the pubspec
// changes, as the pubspec could contain user-defines.
expect(result.dependencies, contains(pubspecUri));
}),
);
}
Original file line number Diff line number Diff line change
@@ -12,8 +12,8 @@ environment:
dependencies:
logging: ^1.3.0
meta: ^1.16.0
native_assets_cli: ^0.15.0
native_toolchain_c: ^0.12.0
native_assets_cli: ^0.16.0-wip
native_toolchain_c: ^0.13.0-wip

dev_dependencies:
lints: ^5.1.1
Loading