Skip to content

[native_assets_cli] Use JSON instead of YAML #1019

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 3 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,14 @@ class NativeAssetsBuildRunner {
required bool dryRun,
}) async {
final outDir = config.outputDirectory;
final configFile = outDir.resolve('../config.yaml');
final configFile = outDir.resolve('../config.json');
final buildHook = config.packageRoot.resolve('hook/').resolve('build.dart');
final buildHookLegacy = config.packageRoot.resolve('build.dart');
final configFileContents = config.toYamlString();
logger.info('config.yaml contents: $configFileContents');
final configFileContents = config.toJsonString();
logger.info('config.json contents: $configFileContents');
await File.fromUri(configFile).writeAsString(configFileContents);
final buildOutputFile =
File.fromUri(outDir.resolve(BuildOutputImpl.fileName));
File.fromUri(outDir.resolve(BuildOutputImpl.fileNameV1_1_0));
if (await buildOutputFile.exists()) {
// Ensure we'll never read outdated build results.
await buildOutputFile.delete();
Expand Down Expand Up @@ -320,7 +320,7 @@ ${result.stdout}
} on FormatException catch (e) {
logger.severe('''
Building native assets for package:${config.packageName} failed.
build_output.yaml contained a format error.
build_output.json contained a format error.
${e.message}
''');
success = false;
Expand All @@ -332,14 +332,14 @@ ${e.message}
} on TypeError {
logger.severe('''
Building native assets for package:${config.packageName} failed.
build_output.yaml contained a format error.
build_output.json contained a format error.
''');
success = false;
return (<NativeCodeAssetImpl>[], <Uri>[], const Metadata({}), false);
} finally {
if (!success) {
final buildOutputFile =
File.fromUri(outDir.resolve(BuildOutputImpl.fileName));
File.fromUri(outDir.resolve(BuildOutputImpl.fileNameV1_1_0));
if (await buildOutputFile.exists()) {
await buildOutputFile.delete();
}
Expand Down
16 changes: 8 additions & 8 deletions pkgs/native_assets_builder/lib/src/model/kernel_assets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/// kernel file.
///
/// The `native_assets.yaml` embedded in a kernel file has a different format
Copy link
Member

Choose a reason for hiding this comment

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

native_assets.yaml?

Copy link
Collaborator Author

@dcharkes dcharkes Mar 19, 2024

Choose a reason for hiding this comment

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

yep, that's not part of the protocol it's internal to the native_assets_builder and the CFE.

Maybe we should consider moving that to JSON as well. If so, in a separate PR. (It will require different changes in dartdev/flutter_tools.)

/// from the assets passed in the `build_output.yaml` from individual native
/// from the assets passed in the `build_output.json` from individual native
/// assets builds. This library defines the format of the former so that it
/// can be reused in `package:dartdev` and `package:flutter_tools`.
///
Expand Down Expand Up @@ -36,7 +36,7 @@ class KernelAssets {
'native-assets': {
for (final entry in assetsPerTarget.entries)
entry.key.toString(): {
for (final e in entry.value) e.id: e.path.toYaml(),
for (final e in entry.value) e.id: e.path.toJson(),
}
},
};
Expand All @@ -58,7 +58,7 @@ class KernelAsset {
}

abstract class KernelAssetPath {
List<String> toYaml();
List<String> toJson();
}

/// Asset at absolute path [uri] on the target device where Dart is run.
Expand All @@ -78,7 +78,7 @@ class KernelAssetAbsolutePath implements KernelAssetPath {
int get hashCode => uri.hashCode;

@override
List<String> toYaml() => [_pathTypeValue, uri.toFilePath()];
List<String> toJson() => [_pathTypeValue, uri.toFilePath()];
}

/// Asset at relative path [uri], relative to the 'dart file' executed.
Expand Down Expand Up @@ -111,7 +111,7 @@ class KernelAssetRelativePath implements KernelAssetPath {
int get hashCode => uri.hashCode;

@override
List<String> toYaml() => [_pathTypeValue, uri.toFilePath()];
List<String> toJson() => [_pathTypeValue, uri.toFilePath()];
}

/// Asset is available on the system `PATH`.
Expand All @@ -136,7 +136,7 @@ class KernelAssetSystemPath implements KernelAssetPath {
String toString() => 'KernelAssetAbsolutePath($uri)';

@override
List<String> toYaml() => [_pathTypeValue, uri.toFilePath()];
List<String> toJson() => [_pathTypeValue, uri.toFilePath()];
}

/// Asset is loaded in the process and symbols are available through
Expand All @@ -151,7 +151,7 @@ class KernelAssetInProcess implements KernelAssetPath {
static const _pathTypeValue = 'process';

@override
List<String> toYaml() => [_pathTypeValue];
List<String> toJson() => [_pathTypeValue];
}

/// Asset is embedded in executable and symbols are available through
Expand All @@ -166,5 +166,5 @@ class KernelAssetInExecutable implements KernelAssetPath {
static const _pathTypeValue = 'executable';

@override
List<String> toYaml() => [_pathTypeValue];
List<String> toJson() => [_pathTypeValue];
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void main() async {
} else {
expect(
fullLog,
contains('build_output.yaml contained a format error.'),
contains('build_output.json contained a format error.'),
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ void main() async {

final dryRunDir = packageUri.resolve(
'.dart_tool/native_assets_builder/dry_run_${Target.current.os}_dynamic/');
expect(File.fromUri(dryRunDir.resolve('config.yaml')), exists);
expect(File.fromUri(dryRunDir.resolve('out/build_output.yaml')), exists);
expect(File.fromUri(dryRunDir.resolve('config.json')), exists);
expect(File.fromUri(dryRunDir.resolve('out/build_output.json')), exists);
//
});
});
Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_assets_builder/test/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ Future<void> copyTestProjects({
final manifestString = await manifestFile.readAsString();
final manifestYaml = loadYamlDocument(manifestString);
final manifest = [
for (final path in manifestYaml.contents as YamlList)
for (final path in manifestYaml.contents as List<Object?>)
Uri(path: path as String)
];
final filesToCopy = manifest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:native_assets_cli/native_assets_cli_internal.dart';
void main(List<String> args) async {
final buildConfig = BuildConfigImpl.fromArguments(args);
await File.fromUri(
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileName))
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileNameV1_1_0))
.writeAsString(_wrongContents);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:native_assets_cli/native_assets_cli_internal.dart';
void main(List<String> args) async {
final buildConfig = BuildConfigImpl.fromArguments(args);
await File.fromUri(
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileName))
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileNameV1_1_0))
.writeAsString(_wrongContents);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:native_assets_cli/native_assets_cli_internal.dart';
void main(List<String> args) async {
final buildConfig = BuildConfigImpl.fromArguments(args);
await File.fromUri(
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileName))
buildConfig.outputDirectory.resolve(BuildOutputImpl.fileNameV1_1_0))
.writeAsString(_rightContents);
exit(1);
}
Expand Down
2 changes: 2 additions & 0 deletions pkgs/native_assets_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
https://github.com/dart-lang/native/pull/946
- **Breaking change** Move `build.dart` to `hook/build.dart`.
https://github.com/dart-lang/native/issues/823
- **Breaking change** Use JSON instead of YAML in the protocol.
https://github.com/dart-lang/native/issues/991
- Bump examples dependencies to path dependencies.

## 0.4.2
Expand Down
9 changes: 4 additions & 5 deletions pkgs/native_assets_cli/lib/src/api/asset.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,20 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:pub_semver/pub_semver.dart';
import 'package:yaml/yaml.dart';

import '../model/target.dart';
import '../utils/json.dart';
import '../utils/map.dart';
import '../utils/yaml.dart';
import 'architecture.dart';
import 'build_config.dart';
import 'build_output.dart';
import 'os.dart';

part 'native_code_asset.dart';
part 'data_asset.dart';
part '../model/asset.dart';
part '../model/native_code_asset.dart';
part '../model/data_asset.dart';
part '../model/native_code_asset.dart';
part 'data_asset.dart';
part 'native_code_asset.dart';

/// Data or code bundled with a Dart or Flutter application.
///
Expand Down
4 changes: 2 additions & 2 deletions pkgs/native_assets_cli/lib/src/api/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import 'package:pub_semver/pub_semver.dart';

import '../model/metadata.dart';
import '../model/target.dart';
import '../utils/json.dart';
import '../utils/map.dart';
import '../utils/yaml.dart';
import 'architecture.dart';
import 'asset.dart';
import 'build.dart';
Expand All @@ -22,9 +22,9 @@ import 'ios_sdk.dart';
import 'link_mode_preference.dart';
import 'os.dart';

part 'c_compiler_config.dart';
part '../model/build_config.dart';
part '../model/c_compiler_config.dart';
part 'c_compiler_config.dart';

/// The configuration for a `build.dart` invocation.
///
Expand Down
5 changes: 3 additions & 2 deletions pkgs/native_assets_cli/lib/src/api/build_output.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@
// 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:convert';
import 'dart:io';

import 'package:collection/collection.dart';
import 'package:pub_semver/pub_semver.dart';
import 'package:yaml/yaml.dart';
import 'package:yaml/yaml.dart' show loadYaml;

import '../model/dependencies.dart';
import '../model/metadata.dart';
import '../utils/datetime.dart';
import '../utils/file.dart';
import '../utils/json.dart';
import '../utils/map.dart';
import '../utils/yaml.dart';
import 'architecture.dart';
import 'asset.dart';
import 'build_config.dart';
Expand Down
14 changes: 7 additions & 7 deletions pkgs/native_assets_cli/lib/src/model/asset.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@
part of '../api/asset.dart';

abstract final class AssetImpl implements Asset {
Map<String, Object> toYaml(Version version);
Map<String, Object> toJson(Version version);

static List<AssetImpl> listFromYamlList(YamlList list) {
static List<AssetImpl> listFromJsonList(List<Object?> list) {
final assets = <AssetImpl>[];
for (final yamlElement in list) {
final yamlMap = as<YamlMap>(yamlElement);
final type = yamlMap[NativeCodeAssetImpl.typeKey];
for (final jsonElement in list) {
final jsonMap = as<Map<Object?, Object?>>(jsonElement);
final type = jsonMap[NativeCodeAssetImpl.typeKey];
switch (type) {
case NativeCodeAsset.type:
case null: // Backwards compatibility with v1.0.0.
assets.add(NativeCodeAssetImpl.fromYaml(yamlMap));
assets.add(NativeCodeAssetImpl.fromJson(jsonMap));
case DataAsset.type:
assets.add(DataAssetImpl.fromYaml(yamlMap));
assets.add(DataAssetImpl.fromJson(jsonMap));
default:
// Do nothing, some other launcher might define it's own asset types.
}
Expand Down
34 changes: 19 additions & 15 deletions pkgs/native_assets_cli/lib/src/model/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,10 @@ final class BuildConfigImpl implements BuildConfig {
required LinkModePreferenceImpl linkModePreference,
Map<String, Metadata>? dependencyMetadata,
Iterable<String>? supportedAssetTypes,
Version? version,
}) {
final nonValidated = BuildConfigImpl._()
.._version = latestVersion
.._version = version ?? latestVersion
.._outDir = outDir
.._packageName = packageName
.._packageRoot = packageRoot
Expand All @@ -120,7 +121,7 @@ final class BuildConfigImpl implements BuildConfig {
.._dryRun = false
.._supportedAssetTypes =
_supportedAssetTypesBackwardsCompatibility(supportedAssetTypes);
final parsedConfigFile = nonValidated.toYaml();
final parsedConfigFile = nonValidated.toJson();
final config = Config(fileParsed: parsedConfigFile);
return BuildConfigImpl.fromConfig(config);
}
Expand All @@ -145,7 +146,7 @@ final class BuildConfigImpl implements BuildConfig {
.._dryRun = true
.._supportedAssetTypes =
_supportedAssetTypesBackwardsCompatibility(supportedAssetTypes);
final parsedConfigFile = nonValidated.toYaml();
final parsedConfigFile = nonValidated.toJson();
final config = Config(fileParsed: parsedConfigFile);
return BuildConfigImpl.fromConfig(config);
}
Expand Down Expand Up @@ -189,7 +190,7 @@ final class BuildConfigImpl implements BuildConfig {
if (dependencyMetadata != null)
for (final entry in dependencyMetadata.entries) ...[
entry.key,
json.encode(entry.value.toYaml()),
json.encode(entry.value.toJson()),
],
..._supportedAssetTypesBackwardsCompatibility(supportedAssetTypes),
].join('###');
Expand Down Expand Up @@ -217,9 +218,9 @@ final class BuildConfigImpl implements BuildConfig {
/// and packages through `build.dart` invocations.
///
/// If we ever were to make breaking changes, it would be useful to give
/// proper error messages rather than just fail to parse the YAML
/// proper error messages rather than just fail to parse the JSON
/// representation in the protocol.
static Version latestVersion = Version(1, 1, 0);
static Version latestVersion = Version(1, 2, 0);

factory BuildConfigImpl.fromConfig(Config config) {
final result = BuildConfigImpl._().._cCompiler = CCompilerConfigImpl._();
Expand All @@ -246,6 +247,9 @@ final class BuildConfigImpl implements BuildConfig {
Map<String, String>? environment,
Uri? workingDirectory,
}) {
// TODO(https://github.com/dart-lang/native/issues/1000): At some point,
// migrate away from package:cli_config, to get rid of package:yaml
// dependency.
final config = Config.fromArgumentsSync(
arguments: args,
environment: environment,
Expand Down Expand Up @@ -451,10 +455,10 @@ final class BuildConfigImpl implements BuildConfig {
return result.sortOnKey();
}

Map<String, Object> toYaml() {
late Map<String, Object> cCompilerYaml;
Map<String, Object> toJson() {
late Map<String, Object> cCompilerJson;
if (!dryRun) {
cCompilerYaml = _cCompiler.toYaml();
cCompilerJson = _cCompiler.toJson();
}

return {
Expand All @@ -464,7 +468,7 @@ final class BuildConfigImpl implements BuildConfig {
OSImpl.configKey: _targetOS.toString(),
LinkModePreferenceImpl.configKey: _linkModePreference.toString(),
supportedAssetTypesKey: _supportedAssetTypes,
_versionKey: latestVersion.toString(),
_versionKey: version.toString(),
if (dryRun) dryRunConfigKey: dryRun,
if (!dryRun) ...{
BuildModeImpl.configKey: _buildMode.toString(),
Expand All @@ -473,18 +477,18 @@ final class BuildConfigImpl implements BuildConfig {
IOSSdkImpl.configKey: _targetIOSSdk.toString(),
if (_targetAndroidNdkApi != null)
targetAndroidNdkApiConfigKey: _targetAndroidNdkApi,
if (cCompilerYaml.isNotEmpty)
CCompilerConfigImpl.configKey: cCompilerYaml,
if (cCompilerJson.isNotEmpty)
CCompilerConfigImpl.configKey: cCompilerJson,
if (_dependencyMetadata != null && _dependencyMetadata.isNotEmpty)
dependencyMetadataConfigKey: {
for (final entry in _dependencyMetadata.entries)
entry.key: entry.value.toYaml(),
entry.key: entry.value.toJson(),
},
},
}.sortOnKey();
}

String toYamlString() => yamlEncode(toYaml());
String toJsonString() => const JsonEncoder.withIndent(' ').convert(toJson());

@override
bool operator ==(Object other) {
Expand Down Expand Up @@ -533,7 +537,7 @@ final class BuildConfigImpl implements BuildConfig {
]);

@override
String toString() => 'BuildConfig.build(${toYaml()})';
String toString() => 'BuildConfig.build(${toJson()})';

void _ensureNotDryRun() {
if (dryRun) {
Expand Down
Loading