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

Stop resolving Uris encoded in the build protocol #1607

Merged
merged 4 commits into from
Sep 26, 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
3 changes: 3 additions & 0 deletions pkgs/native_assets_cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
has presence over other, etc)
- Use `DART_HOOK_TESTING` prefix for environment variables used for testing on
Dart CI
- No longer try to resolve uris encoded in `config.json` against any base uri.
The `hook/{build,link}.dart` invoker has to ensure the uris it encodes can be
opened as-is (i.e. without resolving against any base uri)

## 0.8.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ extension on LinkConfig {
RecordedUsages get usages {
final usagesFile = recordedUsagesFile;
final usagesContent = File.fromUri(usagesFile!).readAsStringSync();
final usagesJson = jsonDecode(usagesContent) as Map<String, dynamic>;
final usagesJson = jsonDecode(usagesContent) as Map<String, Object?>;
final usages = RecordedUsages.fromJson(usagesJson);
return usages;
}
Expand Down
24 changes: 4 additions & 20 deletions pkgs/native_assets_cli/lib/src/json_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,13 @@ extension JsonUtils on Map<String, Object?> {
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 path(String key, {bool mustExist = false}) =>
_fileSystemPathToUri(get<String>(key));

Uri? optionalPath(String key,
{required Uri baseUri, bool resolveUri = true, bool mustExist = false}) {
Uri? optionalPath(String key, {bool mustExist = false}) {
final value = getOptional<String>(key);
if (value == null) return null;
final uri = _pathToUri(value, baseUri: baseUri, resolveUri: resolveUri);
final uri = _fileSystemPathToUri(value);
if (mustExist) {
_throwIfNotExists(key, uri);
}
Expand Down Expand Up @@ -70,18 +66,6 @@ extension JsonUtils on Map<String, Object?> {
}
}

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()) {
Expand Down
13 changes: 6 additions & 7 deletions pkgs/native_assets_cli/lib/src/model/build_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,21 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
final linkConfigJson = const Utf8Decoder()
.fuse(const JsonDecoder())
.convert(bytes) as Map<String, Object?>;
return fromJson(linkConfigJson, baseUri: Uri.parse(configPath));
return fromJson(linkConfigJson);
}

static const dependencyMetadataConfigKey = 'dependency_metadata';

static const linkingEnabledKey = 'linking_enabled';

static BuildConfigImpl fromJson(Map<String, dynamic> config, {Uri? baseUri}) {
baseUri ??= Uri.base;
static BuildConfigImpl fromJson(Map<String, Object?> config) {
final dryRun = HookConfigImpl.parseDryRun(config) ?? false;
final targetOS = HookConfigImpl.parseTargetOS(config);
return BuildConfigImpl(
outputDirectory: HookConfigImpl.parseOutDir(baseUri, config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(baseUri, config),
outputDirectory: HookConfigImpl.parseOutDir(config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(config),
packageName: HookConfigImpl.parsePackageName(config),
packageRoot: HookConfigImpl.parsePackageRoot(baseUri, config),
packageRoot: HookConfigImpl.parsePackageRoot(config),
buildMode: HookConfigImpl.parseBuildMode(config, dryRun),
targetOS: targetOS,
targetArchitecture:
Expand All @@ -131,7 +130,7 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig {
dependencyMetadata: parseDependencyMetadata(config),
linkingEnabled: parseHasLinkPhase(config),
version: HookConfigImpl.parseVersion(config),
cCompiler: HookConfigImpl.parseCCompiler(baseUri, config, dryRun),
cCompiler: HookConfigImpl.parseCCompiler(config, dryRun),
supportedAssetTypes: HookConfigImpl.parseSupportedAssetTypes(config),
targetAndroidNdkApi:
HookConfigImpl.parseTargetAndroidNdkApi(config, dryRun, targetOS),
Expand Down
42 changes: 18 additions & 24 deletions pkgs/native_assets_cli/lib/src/model/hook_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -226,19 +226,19 @@ abstract class HookConfigImpl implements HookConfig {
static bool? parseDryRun(Map<String, Object?> config) =>
config.optionalBool(dryRunConfigKey);

static Uri parseOutDir(Uri baseUri, Map<String, Object?> config) =>
config.path(outDirConfigKey, baseUri: baseUri, mustExist: true);
static Uri parseOutDir(Map<String, Object?> config) =>
config.path(outDirConfigKey, mustExist: true);

static Uri parseOutDirShared(Uri baseUri, Map<String, Object?> config) {
final configResult = config.optionalPath(outDirSharedConfigKey,
baseUri: baseUri, mustExist: true);
static Uri parseOutDirShared(Map<String, Object?> config) {
final configResult =
config.optionalPath(outDirSharedConfigKey, mustExist: true);
if (configResult != null) {
return configResult;
}
// Backwards compatibility, create a directory next to the output dir.
// This is will not be shared so caching doesn't work, but it will make
// the newer hooks not crash.
final outDir = config.path(outDirConfigKey, baseUri: baseUri);
final outDir = config.path(outDirConfigKey);
final outDirShared = outDir.resolve('../out_shared/');
Directory.fromUri(outDirShared).createSync();
return outDirShared;
Expand All @@ -247,8 +247,8 @@ abstract class HookConfigImpl implements HookConfig {
static String parsePackageName(Map<String, Object?> config) =>
config.string(packageNameConfigKey);

static Uri parsePackageRoot(Uri baseUri, Map<String, Object?> config) =>
config.path(packageRootConfigKey, baseUri: baseUri, mustExist: true);
static Uri parsePackageRoot(Map<String, Object?> config) =>
config.path(packageRootConfigKey, mustExist: true);

static BuildModeImpl? parseBuildMode(
Map<String, Object?> config, bool dryRun) {
Expand Down Expand Up @@ -368,32 +368,26 @@ abstract class HookConfigImpl implements HookConfig {
}
}

static Uri? _parseArchiver(Uri baseUri, Map<String, Object?> config) =>
static Uri? _parseArchiver(Map<String, Object?> config) =>
config.optionalPath(
CCompilerConfigImpl.arConfigKey,
baseUri: baseUri,
mustExist: true,
);

static Uri? _parseCompiler(Uri baseUri, Map<String, Object?> config) =>
static Uri? _parseCompiler(Map<String, Object?> config) =>
config.optionalPath(
CCompilerConfigImpl.ccConfigKey,
baseUri: baseUri,
mustExist: true,
);

static Uri? _parseLinker(Uri baseUri, Map<String, Object?> config) =>
config.optionalPath(
static Uri? _parseLinker(Map<String, Object?> config) => config.optionalPath(
CCompilerConfigImpl.ldConfigKey,
baseUri: baseUri,
mustExist: true,
);

static Uri? _parseEnvScript(
Uri baseUri, Map<String, Object?> config, Uri? compiler) =>
static Uri? _parseEnvScript(Map<String, Object?> config, Uri? compiler) =>
(compiler != null && compiler.toFilePath().endsWith('cl.exe'))
? config.path(CCompilerConfigImpl.envScriptConfigKey,
baseUri: baseUri, mustExist: true)
? config.path(CCompilerConfigImpl.envScriptConfigKey, mustExist: true)
: null;

static List<String>? _parseEnvScriptArgs(Map<String, Object?> config) =>
Expand All @@ -404,7 +398,7 @@ abstract class HookConfigImpl implements HookConfig {
[NativeCodeAsset.type];

static CCompilerConfigImpl parseCCompiler(
Uri baseUri, Map<String, Object?> config, bool dryRun) {
Map<String, Object?> config, bool dryRun) {
if (dryRun) {
_throwIfNotNullInDryRun<int>(config, CCompilerConfigImpl.configKey);
}
Expand All @@ -413,13 +407,13 @@ abstract class HookConfigImpl implements HookConfig {
config.getOptional<Map<String, Object?>>(CCompilerConfigImpl.configKey);
if (cCompilerJson == null) return CCompilerConfigImpl();

final compiler = _parseCompiler(baseUri, cCompilerJson);
final compiler = _parseCompiler(cCompilerJson);
return CCompilerConfigImpl(
archiver: _parseArchiver(baseUri, cCompilerJson),
archiver: _parseArchiver(cCompilerJson),
compiler: compiler,
envScript: _parseEnvScript(baseUri, cCompilerJson, compiler),
envScript: _parseEnvScript(cCompilerJson, compiler),
envScriptArgs: _parseEnvScriptArgs(cCompilerJson),
linker: _parseLinker(baseUri, cCompilerJson),
linker: _parseLinker(cCompilerJson),
);
}

Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_assets_cli/lib/src/model/hook_output.dart
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ final class HookOutputImpl implements BuildOutput, LinkOutput {
return HookOutputImpl(
timestamp: DateTime.parse(get<String>(jsonMap, _timestampKey)),
assets: AssetImpl.listFromJson(get<List<Object?>?>(jsonMap, _assetsKey)),
assetsForLinking: get<Map<String, dynamic>?>(
assetsForLinking: get<Map<String, Object?>?>(
jsonMap, _assetsForLinkingKey)
?.map((packageName, assets) => MapEntry(
packageName, AssetImpl.listFromJson(as<List<Object?>>(assets)))),
Expand Down
20 changes: 9 additions & 11 deletions pkgs/native_assets_cli/lib/src/model/link_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,24 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
final linkConfigJson = const Utf8Decoder()
.fuse(const JsonDecoder())
.convert(bytes) as Map<String, Object?>;
return fromJson(linkConfigJson, baseUri: Uri.parse(configPath));
return fromJson(linkConfigJson);
}

static LinkConfigImpl fromJson(Map<String, Object?> config, {Uri? baseUri}) {
baseUri = Uri.base;
static LinkConfigImpl fromJson(Map<String, Object?> config) {
final dryRun = HookConfigImpl.parseDryRun(config) ?? false;
final targetOS = HookConfigImpl.parseTargetOS(config);
return LinkConfigImpl(
outputDirectory: HookConfigImpl.parseOutDir(baseUri, config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(baseUri, config),
outputDirectory: HookConfigImpl.parseOutDir(config),
outputDirectoryShared: HookConfigImpl.parseOutDirShared(config),
packageName: HookConfigImpl.parsePackageName(config),
packageRoot: HookConfigImpl.parsePackageRoot(baseUri, config),
packageRoot: HookConfigImpl.parsePackageRoot(config),
buildMode: HookConfigImpl.parseBuildMode(config, dryRun),
targetOS: targetOS,
targetArchitecture:
HookConfigImpl.parseTargetArchitecture(config, dryRun, targetOS),
linkModePreference: HookConfigImpl.parseLinkModePreference(config),
version: HookConfigImpl.parseVersion(config),
cCompiler: HookConfigImpl.parseCCompiler(baseUri, config, dryRun),
cCompiler: HookConfigImpl.parseCCompiler(config, dryRun),
supportedAssetTypes: HookConfigImpl.parseSupportedAssetTypes(config),
targetAndroidNdkApi:
HookConfigImpl.parseTargetAndroidNdkApi(config, dryRun, targetOS),
Expand All @@ -111,14 +110,13 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig {
targetMacOSVersion:
HookConfigImpl.parseTargetMacOSVersion(config, dryRun, targetOS),
assets: parseAssets(config),
recordedUsagesFile: parseRecordedUsagesUri(baseUri, config),
recordedUsagesFile: parseRecordedUsagesUri(config),
dryRun: dryRun,
);
}

static Uri? parseRecordedUsagesUri(
Uri baseUri, Map<String, Object?> config) =>
config.optionalPath(resourceIdentifierKey, baseUri: baseUri);
static Uri? parseRecordedUsagesUri(Map<String, Object?> config) =>
config.optionalPath(resourceIdentifierKey);

static List<AssetImpl> parseAssets(Map<String, Object?> config) =>
AssetImpl.listFromJson(config.optionalList(assetsKey));
Expand Down
26 changes: 13 additions & 13 deletions pkgs/native_assets_cli/lib/src/model/resource_identifiers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,20 @@ class ResourceIdentifiers {
final fileJson = (jsonDecode(fileContents) as Map)['identifiers'] as List;
return ResourceIdentifiers(
identifiers: fileJson
.map((e) => e as Map<String, dynamic>)
.map((e) => e as Map<String, Object?>)
.map(Identifier.fromJson)
.toList());
}

factory ResourceIdentifiers.fromJson(Map<String, dynamic> map) =>
factory ResourceIdentifiers.fromJson(Map<String, Object?> map) =>
ResourceIdentifiers(
identifiers: List<Identifier>.from((map['identifiers'] as List?)
?.whereType<Map<String, dynamic>>()
?.whereType<Map<String, Object?>>()
.map(Identifier.fromJson) ??
[]),
);

Map<String, dynamic> toJson() => {
Map<String, Object?> toJson() => {
'identifiers': identifiers.map((x) => x.toJson()).toList(),
};

Expand Down Expand Up @@ -69,7 +69,7 @@ class Identifier {
required this.files,
});

Map<String, dynamic> toJson() => {
Map<String, Object?> toJson() => {
'name': name,
'id': id,
'uri': uri.toFilePath(),
Expand All @@ -81,13 +81,13 @@ class Identifier {
String toString() =>
'''Identifier(name: $name, id: $id, uri: $uri, nonConstant: $nonConstant, files: $files)''';

factory Identifier.fromJson(Map<String, dynamic> map) => Identifier(
factory Identifier.fromJson(Map<String, Object?> map) => Identifier(
name: map['name'] as String,
id: map['id'] as String,
uri: Uri.file(map['uri'] as String),
nonConstant: map['nonConstant'] as bool,
files: List<ResourceFile>.from((map['files'] as List)
.map((e) => e as Map<String, dynamic>)
.map((e) => e as Map<String, Object?>)
.map(ResourceFile.fromJson)),
);

Expand Down Expand Up @@ -119,15 +119,15 @@ class ResourceFile {

ResourceFile({required this.part, required this.references});

Map<String, dynamic> toJson() => {
Map<String, Object?> toJson() => {
'part': part,
'references': references.map((x) => x.toJson()).toList(),
};

factory ResourceFile.fromJson(Map<String, dynamic> map) => ResourceFile(
factory ResourceFile.fromJson(Map<String, Object?> map) => ResourceFile(
part: map['part'] as int,
references: List<ResourceReference>.from((map['references'] as List)
.map((e) => e as Map<String, dynamic>)
.map((e) => e as Map<String, Object?>)
.map(ResourceReference.fromJson)),
);

Expand Down Expand Up @@ -161,7 +161,7 @@ class ResourceReference {
required this.arguments,
});

Map<String, dynamic> toJson() => {
Map<String, Object?> toJson() => {
'@': {
'uri': uri.toFilePath(),
'line': line,
Expand All @@ -174,8 +174,8 @@ class ResourceReference {
String toString() =>
'''ResourceReference(uri: $uri, line: $line, column: $column, arguments: $arguments)''';

factory ResourceReference.fromJson(Map<String, dynamic> map) {
final submap = map['@'] as Map<String, dynamic>;
factory ResourceReference.fromJson(Map<String, Object?> map) {
final submap = map['@'] as Map<String, Object?>;
return ResourceReference(
uri: Uri.file(submap['uri'] as String),
line: submap['line'] as int,
Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_toolchain_c/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## 0.5.5-wip

- Nothing yet.
- Address analyzer info diagnostic about multi-line if requiring a block body.
mkustermann marked this conversation as resolved.
Show resolved Hide resolved

## 0.5.4

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ class CompilerResolver {
// TODO(dacoharkes): Support falling back on other tools.
if (targetArch == hostArchitecture &&
targetOS == hostOS &&
hostOS == OS.linux) return clang;
hostOS == OS.linux) {
return clang;
}
if (targetOS == OS.macOS || targetOS == OS.iOS) return appleClang;
if (targetOS == OS.android) return androidNdkClang;
if (hostOS == OS.linux) {
Expand Down