Skip to content
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

Remove package:cli_config & package:args dependencies in package:native_asset_cli #1598

Merged
merged 3 commits into from
Sep 25, 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
4 changes: 4 additions & 0 deletions pkgs/native_assets_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- Remove v1.0 / v1.1 related serialization
- Update SDK constraint to 3.5.0+
- Remove (deprecated) support for accepting yaml as config
- Remove usage of `package:cli_config` and `package:args`: it minimizes
dependencies and it simplifies logic any hook has to do (as it no longer has
to look into environment variables, arguments and json file, determine which
has presence over other, etc)

## 0.8.0

Expand Down
4 changes: 3 additions & 1 deletion pkgs/native_assets_cli/lib/src/api/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
// 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:cli_config/cli_config.dart';
import 'package:collection/collection.dart';
import 'package:pub_semver/pub_semver.dart';

import '../args_parser.dart';
import '../json_utils.dart';
import '../model/hook.dart';
import '../model/metadata.dart';
import '../utils/json.dart';
Expand Down
3 changes: 1 addition & 2 deletions pkgs/native_assets_cli/lib/src/api/hook_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
import 'dart:convert';
import 'dart:io';

import 'package:cli_config/cli_config.dart';
import 'package:collection/collection.dart';
import 'package:crypto/crypto.dart';
import 'package:pub_semver/pub_semver.dart';

import '../json_utils.dart';
import '../model/hook.dart';
import '../model/metadata.dart';
import '../model/target.dart';
Expand All @@ -22,7 +22,6 @@ import 'ios_sdk.dart';
import 'link_config.dart';
import 'link_mode_preference.dart';
import 'os.dart';

part '../model/hook_config.dart';

/// The shared properties of a [LinkConfig] and a [BuildConfig].
Expand Down
4 changes: 2 additions & 2 deletions pkgs/native_assets_cli/lib/src/api/link_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
import 'dart:convert';
import 'dart:io';

import 'package:args/args.dart';
import 'package:cli_config/cli_config.dart';
import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
import 'package:pub_semver/pub_semver.dart';

import '../args_parser.dart';
import '../json_utils.dart';
import '../model/hook.dart';
import '../utils/map.dart';
import 'architecture.dart';
Expand Down
18 changes: 18 additions & 0 deletions pkgs/native_assets_cli/lib/src/args_parser.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.

String getConfigArgument(List<String> arguments) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could actually decide that instead of making this based on arguments we set a DART_BUILD_HOOK_CONFIG environment variable containing the configuration.

for (var i = 0; i < arguments.length; ++i) {
final argument = arguments[i];
if (argument.startsWith('--config=')) {
return argument.substring('--config='.length);
}
if (argument == '--config') {
if ((i + 1) < arguments.length) {
return arguments[i + 1];
}
}
}
throw StateError('No --config argument given.');
}
106 changes: 106 additions & 0 deletions pkgs/native_assets_cli/lib/src/json_utils.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// 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:core';
import 'dart:core' as core;
import 'dart:io';

extension JsonUtils on Map<String, Object?> {
String string(String key, {Iterable<String>? validValues}) {
final value = get<String>(key);
if (validValues != null && !validValues.contains(value)) {
throw FormatException('Json "$key" had value $value but expected one of '
'${validValues.join(',')}');
}
return value;
}

String? optionalString(String key) => getOptional<String>(key);

bool? optionalBool(String key) => getOptional<bool>(key);
core.int int(String key) => get<core.int>(key);
core.int? optionalInt(String key) => getOptional<core.int>(key);

Uri path(String key,
{required Uri baseUri,
bool resolveUri = true,
bool mustExist = false}) =>
_pathToUri(get<String>(key), baseUri: baseUri, resolveUri: resolveUri);

Uri? optionalPath(String key,
{required Uri baseUri, bool resolveUri = true, bool mustExist = false}) {
final value = getOptional<String>(key);
if (value == null) return null;
final uri = _pathToUri(value, baseUri: baseUri, resolveUri: resolveUri);
if (mustExist) {
_throwIfNotExists(key, uri);
}
return uri;
}

List<String>? optionalStringList(String key) {
final value = getOptional<List<Object?>>(key);
if (value == null) return null;
return value.cast<String>();
}

List<Object?> list(String key) => get<List<Object?>>(key);
List<Object?>? optionalList(String key) => getOptional<List<Object?>>(key);
Map<String, Object?>? optionalMap(String key) =>
getOptional<Map<String, Object?>>(key);

T get<T extends Object>(String key) {
final value = this[key];
if (value == null) {
throw FormatException('No value was provided for required key: $key');
}
if (value is T) return value;
throw FormatException(
'Unexpected value \'$value\' for key \'.$key\' in config file. '
'Expected a $T.');
}

T? getOptional<T extends Object>(String key) {
final value = this[key];
if (value is T?) return value;
throw FormatException(
'Unexpected value \'$value\' for key \'.$key\' in config file. '
'Expected a $T?.');
}
}

Uri _pathToUri(
String path, {
required core.bool resolveUri,
required Uri? baseUri,
}) {
final uri = _fileSystemPathToUri(path);
if (resolveUri && baseUri != null) {
return baseUri.resolveUri(uri);
}
return uri;
}

void _throwIfNotExists(String key, Uri value) {
final fileSystemEntity = value.fileSystemEntity;
if (!fileSystemEntity.existsSync()) {
throw FormatException("Path '$value' for key '$key' doesn't exist.");
}
}

extension on Uri {
FileSystemEntity get fileSystemEntity {
if (path.endsWith(Platform.pathSeparator) || path.endsWith('/')) {
return Directory.fromUri(this);
}
return File.fromUri(this);
}
}

Uri _fileSystemPathToUri(String path) {
if (path.endsWith(Platform.pathSeparator)) {
return Uri.directory(path);
}
return Uri.file(path);
}
42 changes: 17 additions & 25 deletions pkgs/native_assets_cli/lib/src/model/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,37 +97,32 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
_supportedAssetTypesBackwardsCompatibility(supportedAssetTypes),
);

factory BuildConfigImpl._fromConfig(Config config) =>
_readFieldsFromConfig(config);

static BuildConfigImpl fromArguments(
List<String> args, {
List<String> arguments, {
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,
workingDirectory: workingDirectory,
);
return BuildConfigImpl._fromConfig(config);
final configPath = getConfigArgument(arguments);
final bytes = File(configPath).readAsBytesSync();
final linkConfigJson = const Utf8Decoder()
.fuse(const JsonDecoder())
.convert(bytes) as Map<String, Object?>;
return fromJson(linkConfigJson, baseUri: Uri.parse(configPath));
}

static const dependencyMetadataConfigKey = 'dependency_metadata';

static const linkingEnabledKey = 'linking_enabled';

static BuildConfigImpl _readFieldsFromConfig(Config config) {
static BuildConfigImpl fromJson(Map<String, dynamic> config, {Uri? baseUri}) {
baseUri ??= Uri.base;
final dryRun = HookConfigImpl.parseDryRun(config) ?? false;
final targetOS = HookConfigImpl.parseTargetOS(config);
return BuildConfigImpl(
outputDirectory: HookConfigImpl.parseOutDir(config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(config),
outputDirectory: HookConfigImpl.parseOutDir(baseUri, config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(baseUri, config),
packageName: HookConfigImpl.parsePackageName(config),
packageRoot: HookConfigImpl.parsePackageRoot(config),
packageRoot: HookConfigImpl.parsePackageRoot(baseUri, config),
buildMode: HookConfigImpl.parseBuildMode(config, dryRun),
targetOS: targetOS,
targetArchitecture:
Expand All @@ -136,7 +131,7 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
dependencyMetadata: parseDependencyMetadata(config),
linkingEnabled: parseHasLinkPhase(config),
version: HookConfigImpl.parseVersion(config),
cCompiler: HookConfigImpl.parseCCompiler(config, dryRun),
cCompiler: HookConfigImpl.parseCCompiler(baseUri, config, dryRun),
supportedAssetTypes: HookConfigImpl.parseSupportedAssetTypes(config),
targetAndroidNdkApi:
HookConfigImpl.parseTargetAndroidNdkApi(config, dryRun, targetOS),
Expand All @@ -149,9 +144,9 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
);
}

static Map<String, Metadata>? parseDependencyMetadata(Config config) {
final fileValue =
config.valueOf<Map<Object?, Object?>?>(dependencyMetadataConfigKey);
static Map<String, Metadata>? parseDependencyMetadata(
Map<String, Object?> config) {
final fileValue = config.optionalMap(dependencyMetadataConfigKey);
if (fileValue == null) {
return null;
}
Expand All @@ -174,12 +169,9 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
).sortOnKey();
}

static bool? parseHasLinkPhase(Config config) =>
static bool? parseHasLinkPhase(Map<String, Object?> config) =>
config.optionalBool(linkingEnabledKey);

static BuildConfigImpl fromJson(Map<String, dynamic> buildConfigJson) =>
BuildConfigImpl._fromConfig(Config(fileParsed: buildConfigJson));

@override
Map<String, Object> toJson() => {
...hookToJson(),
Expand Down
Loading