Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[native_assets_builder] Don't pass in the whole environment #1764

Merged
merged 7 commits into from
Dec 4, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ class NativeAssetsBuildRunner {
null,
hookKernelFile,
packageLayout!,
_filteredEnvironment(_environmentVariablesFilter),
),
);
if (buildOutput == null) return null;
Expand All @@ -450,7 +451,7 @@ class NativeAssetsBuildRunner {
Uri? resources,
PackageLayout packageLayout,
) async {
final environment = Platform.environment;
final environment = _filteredEnvironment(_environmentVariablesFilter);
final outDir = config.outputDirectory;
return await runUnderDirectoriesLock(
[
Expand Down Expand Up @@ -526,6 +527,7 @@ ${e.message}
resources,
hookKernelFile,
packageLayout,
environment,
);
if (result == null) {
if (await dependenciesHashFile.exists()) {
Expand All @@ -550,6 +552,22 @@ ${e.message}
);
}

/// Limit the environment that hook invocations get to see.
///
/// This allowlist lists environment variables needed to run mainstream
/// compilers.
static const _environmentVariablesFilter = {
'ANDROID_HOME', // Needed for the NDK.
'HOME', // Needed to find tools in default install locations.
'PATH', // Needed to invoke native tools.
'PROGRAMDATA', // Needed for vswhere.exe.
'SYSTEMROOT', // Needed for process invocations on Windows.
'TEMP', // Needed for temp dirs in Dart process.
'TMP', // Needed for temp dirs in Dart process.
'TMPDIR', // Needed for temp dirs in Dart process.
'USER_PROFILE', // Needed to find tools in default install locations.
};

Future<HookOutput?> _runHookForPackage(
Hook hook,
HookConfig config,
Expand All @@ -559,6 +577,7 @@ ${e.message}
Uri? resources,
File hookKernelFile,
PackageLayout packageLayout,
Map<String, String> environment,
) async {
final configFile = config.outputDirectory.resolve('../config.json');
final configFileContents =
Expand All @@ -583,6 +602,8 @@ ${e.message}
executable: dartExecutable,
arguments: arguments,
logger: logger,
includeParentEnvironment: false,
environment: environment,
);

var deleteOutputIfExists = false;
Expand Down Expand Up @@ -639,6 +660,12 @@ ${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
Expand Down Expand Up @@ -666,7 +693,8 @@ ${e.message}
Uri packageConfigUri,
Uri workingDirectory,
) async {
final environment = Platform.environment;
// Don't invalidate cache with environment changes.
final environmentForCaching = <String, String>{};
final kernelFile = File.fromUri(
outputDirectory.resolve('../hook.dill'),
);
Expand All @@ -682,8 +710,8 @@ ${e.message}
if (!await dependenciesHashFile.exists()) {
mustCompile = true;
} else {
final outdatedDependency =
await dependenciesHashes.findOutdatedDependency(environment);
final outdatedDependency = await dependenciesHashes
.findOutdatedDependency(environmentForCaching);
if (outdatedDependency != null) {
mustCompile = true;
logger.info(
Expand Down Expand Up @@ -717,7 +745,7 @@ ${e.message}
dartExecutable.resolve('../version'),
],
lastModifiedCutoffTime,
environment,
environmentForCaching,
);
if (modifiedDuringBuild != null) {
logger.severe('File modified during build. Build must be rerun.');
Expand Down Expand Up @@ -759,6 +787,7 @@ ${e.message}
executable: dartExecutable,
arguments: compileArguments,
logger: logger,
includeParentEnvironment: true,
);
var success = true;
if (compileResult.exitCode != 0) {
Expand Down
Loading