From 46527a82170403acc3e676d4cc4043c32e006e33 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Thu, 26 Sep 2024 20:26:46 +0200 Subject: [PATCH] Stop resolving Uris encoded in the build protocol (#1607) 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 https://github.com/dart-lang/native/issues/1605 --- pkgs/native_assets_cli/CHANGELOG.md | 3 ++ .../link/package_with_assets/hook/link.dart | 2 +- .../native_assets_cli/lib/src/json_utils.dart | 24 ++--------- .../lib/src/model/build_config.dart | 13 +++--- .../lib/src/model/hook_config.dart | 42 ++++++++----------- .../lib/src/model/hook_output.dart | 2 +- .../lib/src/model/link_config.dart | 20 ++++----- .../lib/src/model/resource_identifiers.dart | 26 ++++++------ pkgs/native_toolchain_c/CHANGELOG.md | 2 +- .../lib/src/cbuilder/compiler_resolver.dart | 4 +- 10 files changed, 59 insertions(+), 79 deletions(-) diff --git a/pkgs/native_assets_cli/CHANGELOG.md b/pkgs/native_assets_cli/CHANGELOG.md index 913579781..f04086cd1 100644 --- a/pkgs/native_assets_cli/CHANGELOG.md +++ b/pkgs/native_assets_cli/CHANGELOG.md @@ -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 diff --git a/pkgs/native_assets_cli/example/link/package_with_assets/hook/link.dart b/pkgs/native_assets_cli/example/link/package_with_assets/hook/link.dart index a464244ad..a5cd73bfc 100644 --- a/pkgs/native_assets_cli/example/link/package_with_assets/hook/link.dart +++ b/pkgs/native_assets_cli/example/link/package_with_assets/hook/link.dart @@ -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; + final usagesJson = jsonDecode(usagesContent) as Map; final usages = RecordedUsages.fromJson(usagesJson); return usages; } diff --git a/pkgs/native_assets_cli/lib/src/json_utils.dart b/pkgs/native_assets_cli/lib/src/json_utils.dart index 88ec59ca3..e64be65f7 100644 --- a/pkgs/native_assets_cli/lib/src/json_utils.dart +++ b/pkgs/native_assets_cli/lib/src/json_utils.dart @@ -22,17 +22,13 @@ extension JsonUtils on Map { core.int int(String key) => get(key); core.int? optionalInt(String key) => getOptional(key); - Uri path(String key, - {required Uri baseUri, - bool resolveUri = true, - bool mustExist = false}) => - _pathToUri(get(key), baseUri: baseUri, resolveUri: resolveUri); + Uri path(String key, {bool mustExist = false}) => + _fileSystemPathToUri(get(key)); - Uri? optionalPath(String key, - {required Uri baseUri, bool resolveUri = true, bool mustExist = false}) { + Uri? optionalPath(String key, {bool mustExist = false}) { final value = getOptional(key); if (value == null) return null; - final uri = _pathToUri(value, baseUri: baseUri, resolveUri: resolveUri); + final uri = _fileSystemPathToUri(value); if (mustExist) { _throwIfNotExists(key, uri); } @@ -70,18 +66,6 @@ extension JsonUtils on Map { } } -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()) { diff --git a/pkgs/native_assets_cli/lib/src/model/build_config.dart b/pkgs/native_assets_cli/lib/src/model/build_config.dart index b7d0d5e45..211613b6b 100644 --- a/pkgs/native_assets_cli/lib/src/model/build_config.dart +++ b/pkgs/native_assets_cli/lib/src/model/build_config.dart @@ -107,22 +107,21 @@ final class BuildConfigImpl extends HookConfigImpl implements BuildConfig { final linkConfigJson = const Utf8Decoder() .fuse(const JsonDecoder()) .convert(bytes) as Map; - return fromJson(linkConfigJson, baseUri: Uri.parse(configPath)); + return fromJson(linkConfigJson); } static const dependencyMetadataConfigKey = 'dependency_metadata'; static const linkingEnabledKey = 'linking_enabled'; - static BuildConfigImpl fromJson(Map config, {Uri? baseUri}) { - baseUri ??= Uri.base; + static BuildConfigImpl fromJson(Map 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: @@ -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), diff --git a/pkgs/native_assets_cli/lib/src/model/hook_config.dart b/pkgs/native_assets_cli/lib/src/model/hook_config.dart index 59de1e5bd..f67ce2513 100644 --- a/pkgs/native_assets_cli/lib/src/model/hook_config.dart +++ b/pkgs/native_assets_cli/lib/src/model/hook_config.dart @@ -226,19 +226,19 @@ abstract class HookConfigImpl implements HookConfig { static bool? parseDryRun(Map config) => config.optionalBool(dryRunConfigKey); - static Uri parseOutDir(Uri baseUri, Map config) => - config.path(outDirConfigKey, baseUri: baseUri, mustExist: true); + static Uri parseOutDir(Map config) => + config.path(outDirConfigKey, mustExist: true); - static Uri parseOutDirShared(Uri baseUri, Map config) { - final configResult = config.optionalPath(outDirSharedConfigKey, - baseUri: baseUri, mustExist: true); + static Uri parseOutDirShared(Map 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; @@ -247,8 +247,8 @@ abstract class HookConfigImpl implements HookConfig { static String parsePackageName(Map config) => config.string(packageNameConfigKey); - static Uri parsePackageRoot(Uri baseUri, Map config) => - config.path(packageRootConfigKey, baseUri: baseUri, mustExist: true); + static Uri parsePackageRoot(Map config) => + config.path(packageRootConfigKey, mustExist: true); static BuildModeImpl? parseBuildMode( Map config, bool dryRun) { @@ -368,32 +368,26 @@ abstract class HookConfigImpl implements HookConfig { } } - static Uri? _parseArchiver(Uri baseUri, Map config) => + static Uri? _parseArchiver(Map config) => config.optionalPath( CCompilerConfigImpl.arConfigKey, - baseUri: baseUri, mustExist: true, ); - static Uri? _parseCompiler(Uri baseUri, Map config) => + static Uri? _parseCompiler(Map config) => config.optionalPath( CCompilerConfigImpl.ccConfigKey, - baseUri: baseUri, mustExist: true, ); - static Uri? _parseLinker(Uri baseUri, Map config) => - config.optionalPath( + static Uri? _parseLinker(Map config) => config.optionalPath( CCompilerConfigImpl.ldConfigKey, - baseUri: baseUri, mustExist: true, ); - static Uri? _parseEnvScript( - Uri baseUri, Map config, Uri? compiler) => + static Uri? _parseEnvScript(Map 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? _parseEnvScriptArgs(Map config) => @@ -404,7 +398,7 @@ abstract class HookConfigImpl implements HookConfig { [NativeCodeAsset.type]; static CCompilerConfigImpl parseCCompiler( - Uri baseUri, Map config, bool dryRun) { + Map config, bool dryRun) { if (dryRun) { _throwIfNotNullInDryRun(config, CCompilerConfigImpl.configKey); } @@ -413,13 +407,13 @@ abstract class HookConfigImpl implements HookConfig { config.getOptional>(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), ); } diff --git a/pkgs/native_assets_cli/lib/src/model/hook_output.dart b/pkgs/native_assets_cli/lib/src/model/hook_output.dart index 225d07fe2..a87a85be2 100644 --- a/pkgs/native_assets_cli/lib/src/model/hook_output.dart +++ b/pkgs/native_assets_cli/lib/src/model/hook_output.dart @@ -82,7 +82,7 @@ final class HookOutputImpl implements BuildOutput, LinkOutput { return HookOutputImpl( timestamp: DateTime.parse(get(jsonMap, _timestampKey)), assets: AssetImpl.listFromJson(get?>(jsonMap, _assetsKey)), - assetsForLinking: get?>( + assetsForLinking: get?>( jsonMap, _assetsForLinkingKey) ?.map((packageName, assets) => MapEntry( packageName, AssetImpl.listFromJson(as>(assets)))), diff --git a/pkgs/native_assets_cli/lib/src/model/link_config.dart b/pkgs/native_assets_cli/lib/src/model/link_config.dart index 211dbefe9..bf10c27c6 100644 --- a/pkgs/native_assets_cli/lib/src/model/link_config.dart +++ b/pkgs/native_assets_cli/lib/src/model/link_config.dart @@ -83,25 +83,24 @@ class LinkConfigImpl extends HookConfigImpl implements LinkConfig { final linkConfigJson = const Utf8Decoder() .fuse(const JsonDecoder()) .convert(bytes) as Map; - return fromJson(linkConfigJson, baseUri: Uri.parse(configPath)); + return fromJson(linkConfigJson); } - static LinkConfigImpl fromJson(Map config, {Uri? baseUri}) { - baseUri = Uri.base; + static LinkConfigImpl fromJson(Map 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), @@ -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 config) => - config.optionalPath(resourceIdentifierKey, baseUri: baseUri); + static Uri? parseRecordedUsagesUri(Map config) => + config.optionalPath(resourceIdentifierKey); static List parseAssets(Map config) => AssetImpl.listFromJson(config.optionalList(assetsKey)); diff --git a/pkgs/native_assets_cli/lib/src/model/resource_identifiers.dart b/pkgs/native_assets_cli/lib/src/model/resource_identifiers.dart index 0ed06d619..b1b1211cf 100644 --- a/pkgs/native_assets_cli/lib/src/model/resource_identifiers.dart +++ b/pkgs/native_assets_cli/lib/src/model/resource_identifiers.dart @@ -21,20 +21,20 @@ class ResourceIdentifiers { final fileJson = (jsonDecode(fileContents) as Map)['identifiers'] as List; return ResourceIdentifiers( identifiers: fileJson - .map((e) => e as Map) + .map((e) => e as Map) .map(Identifier.fromJson) .toList()); } - factory ResourceIdentifiers.fromJson(Map map) => + factory ResourceIdentifiers.fromJson(Map map) => ResourceIdentifiers( identifiers: List.from((map['identifiers'] as List?) - ?.whereType>() + ?.whereType>() .map(Identifier.fromJson) ?? []), ); - Map toJson() => { + Map toJson() => { 'identifiers': identifiers.map((x) => x.toJson()).toList(), }; @@ -69,7 +69,7 @@ class Identifier { required this.files, }); - Map toJson() => { + Map toJson() => { 'name': name, 'id': id, 'uri': uri.toFilePath(), @@ -81,13 +81,13 @@ class Identifier { String toString() => '''Identifier(name: $name, id: $id, uri: $uri, nonConstant: $nonConstant, files: $files)'''; - factory Identifier.fromJson(Map map) => Identifier( + factory Identifier.fromJson(Map 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.from((map['files'] as List) - .map((e) => e as Map) + .map((e) => e as Map) .map(ResourceFile.fromJson)), ); @@ -119,15 +119,15 @@ class ResourceFile { ResourceFile({required this.part, required this.references}); - Map toJson() => { + Map toJson() => { 'part': part, 'references': references.map((x) => x.toJson()).toList(), }; - factory ResourceFile.fromJson(Map map) => ResourceFile( + factory ResourceFile.fromJson(Map map) => ResourceFile( part: map['part'] as int, references: List.from((map['references'] as List) - .map((e) => e as Map) + .map((e) => e as Map) .map(ResourceReference.fromJson)), ); @@ -161,7 +161,7 @@ class ResourceReference { required this.arguments, }); - Map toJson() => { + Map toJson() => { '@': { 'uri': uri.toFilePath(), 'line': line, @@ -174,8 +174,8 @@ class ResourceReference { String toString() => '''ResourceReference(uri: $uri, line: $line, column: $column, arguments: $arguments)'''; - factory ResourceReference.fromJson(Map map) { - final submap = map['@'] as Map; + factory ResourceReference.fromJson(Map map) { + final submap = map['@'] as Map; return ResourceReference( uri: Uri.file(submap['uri'] as String), line: submap['line'] as int, diff --git a/pkgs/native_toolchain_c/CHANGELOG.md b/pkgs/native_toolchain_c/CHANGELOG.md index a3404936b..7543f7510 100644 --- a/pkgs/native_toolchain_c/CHANGELOG.md +++ b/pkgs/native_toolchain_c/CHANGELOG.md @@ -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 diff --git a/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart b/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart index f1241563f..b33a12199 100644 --- a/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart +++ b/pkgs/native_toolchain_c/lib/src/cbuilder/compiler_resolver.dart @@ -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) {