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

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

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,

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);

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.
 ///

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
 

pkgs/native_assets_cli/lib/src/model/hook_config.dart

@@ -411,57 +411,15 @@ abstract class HookConfigImpl implements HookConfig {
 
     final cCompilerJson =
         config.getOptional<Map<String, Object?>>(CCompilerConfigImpl.configKey);
+    if (cCompilerJson == null) return CCompilerConfigImpl();
 
-    Uri? archiver;
-    Uri? compiler;
-    Uri? linker;
-    Uri? envScript;
-    List<String>? 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<String>? 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),
     );
   }
 

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.
 ///

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.
 ///