From 67a258ce5a365f13180978637be0f10996cced43 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Wed, 25 Sep 2024 21:33:07 +0200 Subject: [PATCH] Make the `package:native_assets_builder` have special logic to create C Compiler configuration for testing on Dart CI (#1600) This removes the somewhat ugly piece of recognition of environment variables in `HookConfig.fromJson()` to the `package:native_assets_builder`. We also rename those variables to prefix them with `DART_HOOK_TESTING`. This way we make it clear that this isn't intended to be used by end-users. => Rolling this to Dart SDK will require updating the test runner to also do this prefixing. --- pkgs/native_assets_builder/CHANGELOG.md | 4 ++ .../lib/src/build_runner/build_runner.dart | 37 +++++++++++++ .../build_runner_run_in_isolation_test.dart | 4 +- pkgs/native_assets_builder/test/helpers.dart | 3 +- pkgs/native_assets_cli/CHANGELOG.md | 2 + .../lib/src/model/hook_config.dart | 54 +++---------------- pkgs/native_assets_cli/test/helpers.dart | 3 +- pkgs/native_toolchain_c/test/helpers.dart | 3 +- 8 files changed, 58 insertions(+), 52 deletions(-) diff --git a/pkgs/native_assets_builder/CHANGELOG.md b/pkgs/native_assets_builder/CHANGELOG.md index 6378db8e7..ed1a89875 100644 --- a/pkgs/native_assets_builder/CHANGELOG.md +++ b/pkgs/native_assets_builder/CHANGELOG.md @@ -5,6 +5,10 @@ - Fix test packages with RecordUse annotations [#1586](https://github.com/dart-lang/native/issues/1586). - Update SDK constraint to 3.5.0+ +- Rename the environment variables we use to communicate CCompilerConfig from + Dart CI test runner to the `package:native_assets_builder` for testing the + dart-lang/native repository to make it clear those are not intended to be used + by end-users. ## 0.8.3 diff --git a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart index e11e986bd..5c0a2a0cb 100644 --- a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart +++ b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart @@ -158,6 +158,43 @@ class NativeAssetsBuildRunner { assert(hook == Hook.link || buildResult == null); assert(hook == Hook.build || linkingEnabled == null); + // Specifically for running our tests on Dart CI with the test runner, we + // recognize specific variables to setup the C Compiler configuration. + if (cCompilerConfig == null) { + String? unparseKey(String key) => + 'DART_HOOK_TESTING_${key.replaceAll('.', '__').toUpperCase()}'; + + final env = Platform.environment; + String? lookup(String key) => env[unparseKey(key)]; + + final cc = lookup(CCompilerConfigImpl.ccConfigKeyFull); + final ar = lookup(CCompilerConfigImpl.arConfigKeyFull); + final ld = lookup(CCompilerConfigImpl.ldConfigKeyFull); + final envScript = lookup(CCompilerConfigImpl.envScriptConfigKeyFull); + final envScriptArgs = + lookup(CCompilerConfigImpl.envScriptArgsConfigKeyFull) + ?.split(' ') + .map((arg) => arg.trim()) + .where((arg) => arg.isNotEmpty) + .toList(); + final hasEnvScriptArgs = + envScriptArgs != null && envScriptArgs.isNotEmpty; + + if (cc != null || + ar != null || + ld != null || + envScript != null || + hasEnvScriptArgs) { + cCompilerConfig = CCompilerConfigImpl( + archiver: ar != null ? Uri.file(ar) : null, + compiler: cc != null ? Uri.file(cc) : null, + envScript: envScript != null ? Uri.file(envScript) : null, + envScriptArgs: hasEnvScriptArgs ? envScriptArgs : null, + linker: ld != null ? Uri.file(ld) : null, + ); + } + } + packageLayout ??= await PackageLayout.fromRootPackageRoot(workingDirectory); final (buildPlan, packageGraph, planSuccess) = await _makePlan( hook: hook, diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.dart index a7f02557f..ecfa8b28b 100644 --- a/pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.dart @@ -13,7 +13,9 @@ import 'helpers.dart'; const Timeout longTimeout = Timeout(Duration(minutes: 5)); void main() async { - String unparseKey(String key) => key.replaceAll('.', '__').toUpperCase(); + String unparseKey(String key) => + 'DART_HOOK_TESTING_${key.replaceAll('.', '__').toUpperCase()}'; + final arKey = unparseKey(CCompilerConfigImpl.arConfigKeyFull); final ccKey = unparseKey(CCompilerConfigImpl.ccConfigKeyFull); final ldKey = unparseKey(CCompilerConfigImpl.ldConfigKeyFull); diff --git a/pkgs/native_assets_builder/test/helpers.dart b/pkgs/native_assets_builder/test/helpers.dart index f7f6f122e..fa6fc9638 100644 --- a/pkgs/native_assets_builder/test/helpers.dart +++ b/pkgs/native_assets_builder/test/helpers.dart @@ -123,7 +123,8 @@ final pkgNativeAssetsBuilderUri = findPackageRoot('native_assets_builder'); final testDataUri = pkgNativeAssetsBuilderUri.resolve('test_data/'); -String unparseKey(String key) => key.replaceAll('.', '__').toUpperCase(); +String unparseKey(String key) => + 'DART_HOOK_TESTING_${key.replaceAll('.', '__').toUpperCase()}'; /// Archiver provided by the environment. /// diff --git a/pkgs/native_assets_cli/CHANGELOG.md b/pkgs/native_assets_cli/CHANGELOG.md index b39a0dfe4..913579781 100644 --- a/pkgs/native_assets_cli/CHANGELOG.md +++ b/pkgs/native_assets_cli/CHANGELOG.md @@ -12,6 +12,8 @@ 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) +- Use `DART_HOOK_TESTING` prefix for environment variables used for testing on + Dart CI ## 0.8.0 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 ded300194..59de1e5bd 100644 --- a/pkgs/native_assets_cli/lib/src/model/hook_config.dart +++ b/pkgs/native_assets_cli/lib/src/model/hook_config.dart @@ -411,57 +411,15 @@ abstract class HookConfigImpl implements HookConfig { final cCompilerJson = config.getOptional>(CCompilerConfigImpl.configKey); + if (cCompilerJson == null) return CCompilerConfigImpl(); - Uri? archiver; - Uri? compiler; - Uri? linker; - Uri? envScript; - List? envScriptArgs; - if (cCompilerJson != null) { - compiler = _parseCompiler(baseUri, cCompilerJson); - archiver = _parseArchiver(baseUri, cCompilerJson); - envScript = _parseEnvScript(baseUri, cCompilerJson, compiler); - envScriptArgs = _parseEnvScriptArgs(cCompilerJson); - linker = _parseLinker(baseUri, cCompilerJson); - } - - // If the bundling tool didn't specify a C compiler we fallback to - // identifying the C compiler based on specific environment variables. - { - final env = Platform.environment; - String? unparseKey(String key) => key.replaceAll('.', '__').toUpperCase(); - String? lookup(String key) => env[unparseKey(key)]; - Uri? lookupUri(String key) { - final value = lookup(key); - return value != null ? Uri.file(value) : null; - } - - List? lookupList(String key) { - final value = lookup(key); - if (value == null) return null; - final list = value - .split(' ') - .map((arg) => arg.trim()) - .where((arg) => arg.isNotEmpty) - .toList(); - if (list.isEmpty) return null; - return list; - } - - archiver ??= lookupUri(CCompilerConfigImpl.arConfigKeyFull); - compiler ??= lookupUri(CCompilerConfigImpl.ccConfigKeyFull); - linker ??= lookupUri(CCompilerConfigImpl.ldConfigKeyFull); - envScript ??= lookupUri(CCompilerConfigImpl.envScriptConfigKeyFull); - envScriptArgs ??= - lookupList(CCompilerConfigImpl.envScriptArgsConfigKeyFull); - } - + final compiler = _parseCompiler(baseUri, cCompilerJson); return CCompilerConfigImpl( - archiver: archiver, + archiver: _parseArchiver(baseUri, cCompilerJson), compiler: compiler, - envScript: envScript, - envScriptArgs: envScriptArgs, - linker: linker, + envScript: _parseEnvScript(baseUri, cCompilerJson, compiler), + envScriptArgs: _parseEnvScriptArgs(cCompilerJson), + linker: _parseLinker(baseUri, cCompilerJson), ); } diff --git a/pkgs/native_assets_cli/test/helpers.dart b/pkgs/native_assets_cli/test/helpers.dart index b4aac85ab..d3771ed9e 100644 --- a/pkgs/native_assets_cli/test/helpers.dart +++ b/pkgs/native_assets_cli/test/helpers.dart @@ -80,7 +80,8 @@ extension on Uri { String get name => pathSegments.where((e) => e != '').last; } -String unparseKey(String key) => key.replaceAll('.', '__').toUpperCase(); +String unparseKey(String key) => + 'DART_HOOK_TESTING_${key.replaceAll('.', '__').toUpperCase()}'; /// Archiver provided by the environment. /// diff --git a/pkgs/native_toolchain_c/test/helpers.dart b/pkgs/native_toolchain_c/test/helpers.dart index 88db4640a..0756ba198 100644 --- a/pkgs/native_toolchain_c/test/helpers.dart +++ b/pkgs/native_toolchain_c/test/helpers.dart @@ -118,7 +118,8 @@ extension on Uri { String get name => pathSegments.where((e) => e != '').last; } -String unparseKey(String key) => key.replaceAll('.', '__').toUpperCase(); +String unparseKey(String key) => + 'DART_HOOK_TESTING_${key.replaceAll('.', '__').toUpperCase()}'; /// Archiver provided by the environment. ///