Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
dcharkes committed Jan 22, 2024
1 parent 30964ac commit 499feae
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 56 deletions.
31 changes: 19 additions & 12 deletions pkgs/native_assets_cli/lib/src/api/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import 'target.dart';

/// The configuration for a `build.dart` invocation.
///
/// The Flutter and Dart SDK invoke `build.dart` with commandline arguments
/// that can be parsed by this class.
/// A package can choose to have a toplevel `build.dart` script. If such a
/// script exists, it will be automatically run, by the Flutter and Dart SDK
/// tools. The script will then be run with specific commandline arguments,
/// which [BuildConfig] can parse and provide more convenient access to.
abstract class BuildConfig {
/// The folder in which all output and intermediate artifacts should be
/// placed.
Expand All @@ -39,12 +41,12 @@ abstract class BuildConfig {

/// The target being compiled for.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
Target get target;

/// The architecture being compiled for.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
Architecture get targetArchitecture;

/// The operating system being compiled for.
Expand All @@ -54,15 +56,15 @@ abstract class BuildConfig {
///
/// Required when [targetOs] equals [OS.iOS].
///
/// Not available in [dryRun].s
/// Not available during a [dryRun].
IOSSdk? get targetIOSSdk;

/// When compiling for Android, the minimum Android SDK API version to that
/// the compiled code will be compatible with.
///
/// Required when [targetOs] equals [OS.android].
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
///
/// For more information about the Android API version, refer to
/// [`minSdkVersion`](https://developer.android.com/ndk/guides/sdk-versions#minsdkversion)
Expand All @@ -78,28 +80,33 @@ abstract class BuildConfig {
///
/// The key in the nested map is the key for the metadata from the dependency.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@Deprecated('Use getMetadata.')
Map<String, Metadata>? get dependencyMetadata;

/// Get the metadata from a direct dependency.
///
/// The [packageName] of is the package name of the direct dependency.
///
/// Not available in [dryRun].
T? getMetadata<T>(String packageName, String key);
/// Returns `null` if metadata was not provided.
///
/// Not available during a [dryRun].
Object? metadata(String packageName, String key);

/// The configuration for invoking the C compiler.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
CCompilerConfig get cCompiler;

/// Don't run the build, only report the native assets produced.
/// Whether the current run is a "dry run".
///
/// If so, the build won't actually be run, but will report the native assets
/// which would have been produced.
bool get dryRun;

/// The build mode that the code should be compiled in.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
BuildMode get buildMode;

/// The underlying config.
Expand Down
43 changes: 23 additions & 20 deletions pkgs/native_assets_cli/lib/src/api/build_output.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@ import 'target.dart';

/// The output of a `build.dart` invocation.
///
/// The Dart and Flutter SDK consume this data.
/// A package can choose to have a toplevel `build.dart` script. If such a
/// script exists, it will be automatically run, by the Flutter and Dart SDK
/// tools. The script is expect to produce a specific output which [BuildOutput]
/// can produce.
abstract class BuildOutput {
/// The [timestamp] indicates the time the build this output belongs to
/// started.
// Start time for the build of this output.
///
/// The [timestamp] is rounded down to whole seconds, because
/// [File.lastModified] is rounded to whole seconds and caching logic compares
Expand All @@ -42,7 +44,7 @@ abstract class BuildOutput {

/// The files used by this build.
///
/// If any of the files in [dependencies2] is modified after [timestamp], the
/// If any of the files in [dependencies2] are modified after [timestamp], the
/// build will be re-run.
// TODO: Rename to `dependencies` after removing old one.
Iterable<Uri> get dependencies2;
Expand All @@ -52,24 +54,25 @@ abstract class BuildOutput {

/// Metadata can to be passed to `build.dart` invocations of dependent
/// packages.
// TODO(dacoharkes): Then we also need to make the accessors and setters
// in BuildConfig methods.
T getMetadata<T>(String key);
// TODO(dacoharkes): Rename to metadata.
Object? metadata2(String key);

/// Create a build output.
///
/// The [timestamp] indicates the time the build this output belongs to
/// started. If omitted, [timestamp] defaults to the time the build started.
/// The [timestamp] is rounded down to whole seconds, because
/// [File.lastModified] is rounded to whole seconds and caching logic compares
/// these timestamps.
/// The [timestamp] must be before any any [dependencies2] are read by the
/// build this output belongs to. If the [BuildOutput] object is created at
/// the beginning of the `build.dart` script, it can be omitted and will
/// default to [DateTime.now]. The [timestamp] is rounded down to whole
/// seconds, because [File.lastModified] is rounded to whole seconds and
/// caching logic compares these timestamps.
///
/// The [Asset]s produced by this build or dry run can be passed in [assets]
/// or [addAssets]. In dry runs, the assets for all [Architecture]s for the
/// [OS] specified in the dry run must be provided.
/// The [Asset]s produced by this build or dry-run can be provided to the
/// constructor as [assets], or can be added later using [addAssets]. In dry
/// runs, the assets for all [Architecture]s for the [OS] specified in the dry
/// run must be provided.
///
/// The files used by this build must be passed in [dependencies2] or
/// [addDependencies]. If any of these files is modified after [timestamp],
/// [addDependencies]. If any of these files are modified after [timestamp],
/// the build will be re-run.
///
/// Metadata can be passed to `build.dart` invocations of dependent packages
Expand All @@ -93,19 +96,19 @@ abstract class BuildOutput {
: metadata as model.Metadata?,
);

/// Add [Asset]s produced by this build or dry run.
/// Adds [Asset]s produced by this build or dry run.
///
/// In dry runs, the assets for all [Architecture]s for the [OS] specified in
/// the dry run must be provided.
void addAssets(Iterable<Asset> assets);

/// Add files used by this build.
/// Adds files used by this build.
///
/// If any of the files is modified after [timestamp], the build will be
/// If any of the files are modified after [timestamp], the build will be
/// re-run.
void addDependencies(Iterable<Uri> dependencies);

/// Add metadata to be passed to `build.dart` invocations of dependent
/// Adds metadata to be passed to `build.dart` invocations of dependent
/// packages.
void addMetadata(String key, Object value);

Expand Down
30 changes: 16 additions & 14 deletions pkgs/native_assets_cli/lib/src/api/build_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ final class BuildState {
/// Example using `package:native_toolchain_c`:
///
/// ```dart
/// void main(List<String> args) async =>
/// await BuildState.build(args, (buildState) async {
/// void main(List<String> args) =>
/// BuildState.build(args, (buildState) async {
/// final cbuilder = CBuilder.library(
/// name: packageName,
/// assetId: 'package:$packageName/${packageName}.dart',
/// assetId: 'package:$packageName/$packageName.dart',
/// sources: [
/// 'src/$packageName.c',
/// ],
Expand All @@ -33,12 +33,14 @@ final class BuildState {
/// Example outputting assets manually:
///
/// ```dart
/// void main(List<String> args) async =>
/// await BuildState.build(args, (buildState) async {
/// void main(List<String> args) =>
/// BuildState.build(args, (buildState) async {
/// final config = buildState.config;
/// if (config.linkModePreference == LinkModePreference.static) {
/// // Simulat that this script only supports dynamic libraries.
/// throw Exception('LinkModePreference.static is not supported.');
/// // Simulate that this script only supports dynamic libraries.
/// throw UnsupportedError(
/// 'LinkModePreference.static is not supported.',
/// );
/// }
///
/// final Iterable<Target> targets;
Expand Down Expand Up @@ -70,13 +72,13 @@ final class BuildState {
/// });
/// ```
static Future<void> build(
List<String> args,
Future<void> Function(BuildState) callback,
List<String> commandlineArguments,
Future<void> Function(BuildState) builder,
) async {
final config = await BuildConfig.fromArgs(args);
final config = await BuildConfig.fromArgs(commandlineArguments);
final output = BuildOutput();
final state = BuildState._(config: config, output: output);
await callback(state);
await builder(state);
await output.writeToFile(outDir: config.outDir);
}

Expand All @@ -88,18 +90,18 @@ final class BuildState {
required this.output,
});

/// Add assets to build output.
/// Adds assets to build output.
///
/// See [BuildOutput.addAssets] for more info.
void addAssets(Iterable<Asset> assets) => output.addAssets(assets);

/// Add dependencies to build output.
/// Adds dependencies to build output.
///
/// See [BuildOutput.addDependencies] for more info.
void addDependencies(Iterable<Uri> dependencies) =>
output.addDependencies(dependencies);

/// Add metadata to build output.
/// Adds metadata to build output.
///
/// See [BuildOutput.addMetadata] for more info.
void addMetadata(String key, Object value) => output.addMetadata(key, value);
Expand Down
18 changes: 9 additions & 9 deletions pkgs/native_assets_cli/lib/src/model/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ class BuildConfig implements api.BuildConfig {

/// The target being compiled for.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@override
late final Target target =
Target.fromArchitectureAndOs(targetArchitecture, targetOs);

/// The architecture being compiled for.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@override
Architecture get targetArchitecture {
_ensureNotDryRun();
Expand All @@ -66,7 +66,7 @@ class BuildConfig implements api.BuildConfig {
///
/// Required when [targetOs] equals [OS.iOS].
///
/// Not available in [dryRun].s
/// Not available during a [dryRun].
@override
IOSSdk? get targetIOSSdk {
_ensureNotDryRun();
Expand All @@ -80,7 +80,7 @@ class BuildConfig implements api.BuildConfig {
///
/// Required when [targetOs] equals [OS.android].
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
///
/// For more information about the Android API version, refer to
/// [`minSdkVersion`](https://developer.android.com/ndk/guides/sdk-versions#minsdkversion)
Expand All @@ -104,24 +104,24 @@ class BuildConfig implements api.BuildConfig {
///
/// The key in the nested map is the key for the metadata from the dependency.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@override
Map<String, Metadata>? get dependencyMetadata {
_ensureNotDryRun();
return _dependencyMetadata;
}

@override
T? getMetadata<T>(String packageName, String key) {
Object? metadata(String packageName, String key) {
_ensureNotDryRun();
return _dependencyMetadata?[packageName]?.metadata[key] as T?;
return _dependencyMetadata?[packageName]?.metadata[key];
}

late final Map<String, Metadata>? _dependencyMetadata;

/// The configuration for invoking the C compiler.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@override
CCompilerConfig get cCompiler {
_ensureNotDryRun();
Expand All @@ -137,7 +137,7 @@ class BuildConfig implements api.BuildConfig {

/// The build mode that the code should be compiled in.
///
/// Not available in [dryRun].
/// Not available during a [dryRun].
@override
BuildMode get buildMode {
_ensureNotDryRun();
Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_assets_cli/lib/src/model/build_output.dart
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class BuildOutput implements api.BuildOutput {
}

@override
T getMetadata<T>(String key) => metadata.metadata[key] as T;
Object? metadata2(String key) => metadata.metadata[key];

@override
void addAssets(Iterable<api.Asset> assets) {
Expand Down

0 comments on commit 499feae

Please sign in to comment.