Skip to content

Commit

Permalink
Stop resolving Uris encoded in the build protocol (#1607)
Browse files Browse the repository at this point in the history
This makes a `hook/{build,link}.dart` no longer resolve uris encoded in the
`config.json`. It still validates that the uris that are encoded exist
when it decodes the json.

The creator of `config.json` has to ensure that the uris it encodes in
that config are existing file urls that the `hook/{build,link}.dart`
scripts can open as-is (i.e. without resolving against any base uri).

In many cases that means that an invoker should encode absolute uris in
there - or if it encodes relative uris - guarantees that they are
relative to the working directory the `hook/{build,link}.dart` is
invoked.

Issue #1605
  • Loading branch information
mkustermann authored Sep 26, 2024
1 parent 67a258c commit 46527a8
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 79 deletions.
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.

## 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

0 comments on commit 46527a8

Please sign in to comment.