[native_assets_builder] Make hook environment configurable (#1853)

dcharkes · web-flow · commit 225164637d2d · 2025-01-02T11:05:44.000+01:00
diff --git a/pkgs/native_assets_builder/CHANGELOG.md b/pkgs/native_assets_builder/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.10.1
+
+- Pass in the environment for hook invocations.
+
 ## 0.10.0
 
 - Removed support for dry run (Flutter no long requires it).
diff --git a/pkgs/native_assets_builder/lib/native_assets_builder.dart b/pkgs/native_assets_builder/lib/native_assets_builder.dart
@@ -2,7 +2,14 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-export 'package:native_assets_builder/src/build_runner/build_runner.dart';
+export 'package:native_assets_builder/src/build_runner/build_runner.dart'
+    show
+        ApplicationAssetValidator,
+        BuildConfigValidator,
+        BuildValidator,
+        LinkConfigValidator,
+        LinkValidator,
+        NativeAssetsBuildRunner;
 export 'package:native_assets_builder/src/model/build_result.dart';
 export 'package:native_assets_builder/src/model/kernel_assets.dart';
 export 'package:native_assets_builder/src/model/link_result.dart';
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
@@ -68,14 +68,18 @@ class NativeAssetsBuildRunner {
   final Logger logger;
   final Uri dartExecutable;
   final Duration singleHookTimeout;
+  final Map<String, String> hookEnvironment;
 
   NativeAssetsBuildRunner({
     required this.logger,
     required this.dartExecutable,
     required FileSystem fileSystem,
     Duration? singleHookTimeout,
+    Map<String, String>? hookEnvironment,
   })  : _fileSystem = fileSystem,
-        singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5);
+        singleHookTimeout = singleHookTimeout ?? const Duration(minutes: 5),
+        hookEnvironment = hookEnvironment ??
+            filteredEnvironment(hookEnvironmentVariablesFilter);
 
   /// [workingDirectory] is expected to contain `.dart_tool`.
   ///
@@ -321,7 +325,6 @@ class NativeAssetsBuildRunner {
     Uri? resources,
     PackageLayout packageLayout,
   ) async {
-    final environment = _filteredEnvironment(_environmentVariablesFilter);
     final outDir = config.outputDirectory;
     return await runUnderDirectoriesLock(
       _fileSystem,
@@ -367,7 +370,7 @@ ${e.message}
           }
 
           final outdatedDependency =
-              await dependenciesHashes.findOutdatedDependency(environment);
+              await dependenciesHashes.findOutdatedDependency(hookEnvironment);
           if (outdatedDependency == null) {
             logger.info(
               'Skipping ${hook.name} for ${config.packageName}'
@@ -393,7 +396,7 @@ ${e.message}
           resources,
           hookKernelFile,
           packageLayout,
-          environment,
+          hookEnvironment,
         );
         if (result == null) {
           if (await dependenciesHashes.exists()) {
@@ -409,7 +412,7 @@ ${e.message}
               hookKernelFile.uri,
             ],
             lastModifiedCutoffTime,
-            environment,
+            hookEnvironment,
           );
           if (modifiedDuringBuild != null) {
             logger.severe('File modified during build. Build must be rerun.');
@@ -420,11 +423,11 @@ ${e.message}
     );
   }
 
-  /// Limit the environment that hook invocations get to see.
-  ///
+  /// The list of environment variables used if [hookEnvironment] is not passed
+  /// in.
   /// This allowlist lists environment variables needed to run mainstream
   /// compilers.
-  static const _environmentVariablesFilter = {
+  static const hookEnvironmentVariablesFilter = {
     'ANDROID_HOME', // Needed for the NDK.
     'HOME', // Needed to find tools in default install locations.
     'PATH', // Needed to invoke native tools.
@@ -530,12 +533,6 @@ ${e.message}
     }
   }
 
-  Map<String, String> _filteredEnvironment(Set<String> allowList) => {
-        for (final entry in Platform.environment.entries)
-          if (allowList.contains(entry.key.toUpperCase()))
-            entry.key: entry.value,
-      };
-
   /// Compiles the hook to kernel and caches the kernel.
   ///
   /// If any of the Dart source files, or the package config changed after
@@ -842,3 +839,8 @@ Future<List<Uri>> _readDepFile(File depFile) async {
   final dartSources = parseDepFileInputs(depFileContents);
   return dartSources.map(Uri.file).toList();
 }
+
+Map<String, String> filteredEnvironment(Set<String> allowList) => {
+      for (final entry in Platform.environment.entries)
+        if (allowList.contains(entry.key.toUpperCase())) entry.key: entry.value,
+    };
diff --git a/pkgs/native_assets_builder/pubspec.yaml b/pkgs/native_assets_builder/pubspec.yaml
@@ -1,7 +1,7 @@
 name: native_assets_builder
 description: >-
   This package is the backend that invokes build hooks.
-version: 0.10.0
+version: 0.10.1
 repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder
 
 # publish_to: none
diff --git a/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart
@@ -5,6 +5,7 @@
 import 'dart:convert';
 import 'dart:io';
 
+import 'package:native_assets_builder/src/build_runner/build_runner.dart';
 import 'package:test/test.dart';
 
 import '../helpers.dart';
@@ -221,75 +222,81 @@ void main() async {
     },
   );
 
-  test(
-    'change environment',
-    timeout: longTimeout,
-    () async {
-      await inTempDir((tempUri) async {
-        await copyTestProjects(targetUri: tempUri);
-        final packageUri = tempUri.resolve('native_add/');
+  for (final modifiedEnvKey in ['PATH', 'CUSTOM_KEY_123']) {
+    test(
+      'change environment $modifiedEnvKey',
+      timeout: longTimeout,
+      () async {
+        await inTempDir((tempUri) async {
+          await copyTestProjects(targetUri: tempUri);
+          final packageUri = tempUri.resolve('native_add/');
 
-        final logMessages = <String>[];
-        final logger = createCapturingLogger(logMessages);
+          final logMessages = <String>[];
+          final logger = createCapturingLogger(logMessages);
 
-        await runPubGet(workingDirectory: packageUri, logger: logger);
-        logMessages.clear();
+          await runPubGet(workingDirectory: packageUri, logger: logger);
+          logMessages.clear();
 
-        final result = (await build(
-          packageUri,
-          logger,
-          dartExecutable,
-          buildAssetTypes: [CodeAsset.type],
-          configValidator: validateCodeAssetBuildConfig,
-          buildValidator: validateCodeAssetBuildOutput,
-          applicationAssetValidator: validateCodeAssetInApplication,
-        ))!;
-        logMessages.clear();
+          final result = (await build(
+            packageUri,
+            logger,
+            dartExecutable,
+            buildAssetTypes: [CodeAsset.type],
+            configValidator: validateCodeAssetBuildConfig,
+            buildValidator: validateCodeAssetBuildOutput,
+            applicationAssetValidator: validateCodeAssetInApplication,
+            hookEnvironment: modifiedEnvKey == 'PATH'
+                ? null
+                : filteredEnvironment(
+                    NativeAssetsBuildRunner.hookEnvironmentVariablesFilter,
+                  ),
+          ))!;
+          logMessages.clear();
 
-        // Simulate that the environment variables changed by augmenting the
-        // persisted environment from the last invocation.
-        final dependenciesHashFile = File.fromUri(
-          CodeAsset.fromEncoded(result.encodedAssets.single)
-              .file!
-              .parent
-              .parent
-              .resolve('dependencies.dependencies_hash_file.json'),
-        );
-        expect(await dependenciesHashFile.exists(), true);
-        final dependenciesContent =
-            jsonDecode(await dependenciesHashFile.readAsString())
-                as Map<Object, Object?>;
-        const modifiedEnvKey = 'PATH';
-        (dependenciesContent['environment'] as List<dynamic>).add({
-          'key': modifiedEnvKey,
-          'hash': 123456789,
-        });
-        await dependenciesHashFile
-            .writeAsString(jsonEncode(dependenciesContent));
+          // Simulate that the environment variables changed by augmenting the
+          // persisted environment from the last invocation.
+          final dependenciesHashFile = File.fromUri(
+            CodeAsset.fromEncoded(result.encodedAssets.single)
+                .file!
+                .parent
+                .parent
+                .resolve('dependencies.dependencies_hash_file.json'),
+          );
+          expect(await dependenciesHashFile.exists(), true);
+          final dependenciesContent =
+              jsonDecode(await dependenciesHashFile.readAsString())
+                  as Map<Object, Object?>;
+          (dependenciesContent['environment'] as List<dynamic>).add({
+            'key': modifiedEnvKey,
+            'hash': 123456789,
+          });
+          await dependenciesHashFile
+              .writeAsString(jsonEncode(dependenciesContent));
 
-        (await build(
-          packageUri,
-          logger,
-          dartExecutable,
-          buildAssetTypes: [CodeAsset.type],
-          configValidator: validateCodeAssetBuildConfig,
-          buildValidator: validateCodeAssetBuildOutput,
-          applicationAssetValidator: validateCodeAssetInApplication,
-        ))!;
-        expect(
-          logMessages.join('\n'),
-          contains('hook.dill'),
-        );
-        expect(
-          logMessages.join('\n'),
-          isNot(contains('Skipping build for native_add')),
-        );
-        expect(
-          logMessages.join('\n'),
-          contains('Environment variable changed: $modifiedEnvKey.'),
-        );
-        logMessages.clear();
-      });
-    },
-  );
+          (await build(
+            packageUri,
+            logger,
+            dartExecutable,
+            buildAssetTypes: [CodeAsset.type],
+            configValidator: validateCodeAssetBuildConfig,
+            buildValidator: validateCodeAssetBuildOutput,
+            applicationAssetValidator: validateCodeAssetInApplication,
+          ))!;
+          expect(
+            logMessages.join('\n'),
+            contains('hook.dill'),
+          );
+          expect(
+            logMessages.join('\n'),
+            isNot(contains('Skipping build for native_add')),
+          );
+          expect(
+            logMessages.join('\n'),
+            contains('Environment variable changed: $modifiedEnvKey.'),
+          );
+          logMessages.clear();
+        });
+      },
+    );
+  }
 }
diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart
@@ -48,13 +48,15 @@ Future<BuildResult?> build(
   Target? target,
   bool linkingEnabled = false,
   required List<String> buildAssetTypes,
+  Map<String, String>? hookEnvironment,
 }) async {
   final targetOS = target?.os ?? OS.current;
   return await runWithLog(capturedLogs, () async {
     final result = await NativeAssetsBuildRunner(
       logger: logger,
       dartExecutable: dartExecutable,
       fileSystem: const LocalFileSystem(),
+      hookEnvironment: hookEnvironment,
     ).build(
       configCreator: () {
         final configBuilder = BuildConfigBuilder();
