From d56a5b5fbd9a0eba48b4b0ca34e808034b3a29d9 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Wed, 4 Dec 2024 11:01:59 +0100 Subject: [PATCH] [native_assets_builder] Don't cache hook invocations if env vars change (#1759) This PR makes the native assets builder save the `Platform.environment` a hook is run in. Subsequent invocations check if the environment didn't change. This removes the `includeParentEnvironment` parameter everywhere. It was always set to true in `dartdev` and `flutter_tools` (This will require a manual roll into the Dart SDK and Flutter tools.) A follow up PR should restrict the list of environment variables (https://github.com/dart-lang/native/pull/1764), this PR is about caching correctness. Bug: * https://github.com/dart-lang/native/issues/32 --- .../lib/src/build_runner/build_runner.dart | 53 +++----- .../dependencies_hash_file.dart | 126 ++++++++++++++---- .../lib/src/utils/run_process.dart | 13 -- .../build_runner_caching_test.dart | 75 ++++++++++- .../build_runner_reusability_test.dart | 4 - .../build_runner_run_in_isolation_test.dart | 75 ----------- .../concurrency_shared_test_helper.dart | 1 - .../build_runner/concurrency_test_helper.dart | 1 - .../test/build_runner/helpers.dart | 9 -- .../dependencies_hash_file_test.dart | 59 +++++--- pkgs/native_assets_builder/test/helpers.dart | 2 - .../lib/src/utils/run_process.dart | 17 +-- 12 files changed, 235 insertions(+), 200 deletions(-) delete mode 100644 pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.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 index 1559ec803..f5a900280 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 @@ -96,7 +96,6 @@ class NativeAssetsBuildRunner { required OS targetOS, required BuildMode buildMode, required Uri workingDirectory, - required bool includeParentEnvironment, PackageLayout? packageLayout, String? runPackageName, required List supportedAssetTypes, @@ -165,7 +164,6 @@ class NativeAssetsBuildRunner { buildValidator(config as BuildConfig, output as BuildOutput), packageLayout.packageConfigUri, workingDirectory, - includeParentEnvironment, null, packageLayout, ); @@ -205,7 +203,6 @@ class NativeAssetsBuildRunner { required BuildMode buildMode, required Uri workingDirectory, required ApplicationAssetValidator applicationAssetValidator, - required bool includeParentEnvironment, PackageLayout? packageLayout, Uri? resourceIdentifiers, String? runPackageName, @@ -269,7 +266,6 @@ class NativeAssetsBuildRunner { linkValidator(config as LinkConfig, output as LinkOutput), packageLayout.packageConfigUri, workingDirectory, - includeParentEnvironment, resourceIdentifiers, packageLayout, ); @@ -330,7 +326,6 @@ class NativeAssetsBuildRunner { required OS targetOS, required Uri workingDirectory, required bool linkingEnabled, - required bool includeParentEnvironment, PackageLayout? packageLayout, String? runPackageName, required List supportedAssetTypes, @@ -341,7 +336,6 @@ class NativeAssetsBuildRunner { validator: (HookConfig config, HookOutput output) => buildValidator(config as BuildConfig, output as BuildOutput), workingDirectory: workingDirectory, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, runPackageName: runPackageName, supportedAssetTypes: supportedAssetTypes, @@ -353,7 +347,6 @@ class NativeAssetsBuildRunner { required _HookValidator validator, required OS targetOS, required Uri workingDirectory, - required bool includeParentEnvironment, PackageLayout? packageLayout, String? runPackageName, required bool linkingEnabled, @@ -420,7 +413,6 @@ class NativeAssetsBuildRunner { config.packageRoot.resolve('hook/${hook.scriptName}'), packageConfigUri, workingDirectory, - includeParentEnvironment, ); if (!compileSuccess) return null; @@ -438,7 +430,6 @@ class NativeAssetsBuildRunner { validator, packageConfigUri, workingDirectory, - includeParentEnvironment, null, hookKernelFile, packageLayout!, @@ -450,18 +441,16 @@ class NativeAssetsBuildRunner { return hookResult; } - // TODO(https://github.com/dart-lang/native/issues/32): Rerun hook if - // environment variables change. Future _runHookForPackageCached( Hook hook, HookConfig config, _HookValidator validator, Uri packageConfigUri, Uri workingDirectory, - bool includeParentEnvironment, Uri? resources, PackageLayout packageLayout, ) async { + final environment = Platform.environment; final outDir = config.outputDirectory; return await runUnderDirectoriesLock( [ @@ -481,7 +470,6 @@ class NativeAssetsBuildRunner { config.packageRoot.resolve('hook/${hook.scriptName}'), packageConfigUri, workingDirectory, - includeParentEnvironment, ); if (!compileSuccess) { return null; @@ -510,9 +498,10 @@ ${e.message} '''); return null; } - final outdatedFile = - await dependenciesHashes.findOutdatedFileSystemEntity(); - if (outdatedFile == null) { + + final outdatedDependency = + await dependenciesHashes.findOutdatedDependency(environment); + if (outdatedDependency == null) { logger.info( 'Skipping ${hook.name} for ${config.packageName}' ' in ${outDir.toFilePath()}.' @@ -524,8 +513,7 @@ ${e.message} } logger.info( 'Rerunning ${hook.name} for ${config.packageName}' - ' in ${outDir.toFilePath()}.' - ' ${outdatedFile.toFilePath()} changed.', + ' in ${outDir.toFilePath()}. $outdatedDependency', ); } @@ -535,7 +523,6 @@ ${e.message} validator, packageConfigUri, workingDirectory, - includeParentEnvironment, resources, hookKernelFile, packageLayout, @@ -545,14 +532,14 @@ ${e.message} await dependenciesHashFile.delete(); } } else { - final modifiedDuringBuild = - await dependenciesHashes.hashFilesAndDirectories( + final modifiedDuringBuild = await dependenciesHashes.hashDependencies( [ ...result.dependencies, // Also depend on the hook source code. hookHashesFile.uri, ], - validBeforeLastModified: lastModifiedCutoffTime, + lastModifiedCutoffTime, + environment, ); if (modifiedDuringBuild != null) { logger.severe('File modified during build. Build must be rerun.'); @@ -569,7 +556,6 @@ ${e.message} _HookValidator validator, Uri packageConfigUri, Uri workingDirectory, - bool includeParentEnvironment, Uri? resources, File hookKernelFile, PackageLayout packageLayout, @@ -597,7 +583,6 @@ ${e.message} executable: dartExecutable, arguments: arguments, logger: logger, - includeParentEnvironment: includeParentEnvironment, ); var deleteOutputIfExists = false; @@ -680,8 +665,8 @@ ${e.message} Uri scriptUri, Uri packageConfigUri, Uri workingDirectory, - bool includeParentEnvironment, ) async { + final environment = Platform.environment; final kernelFile = File.fromUri( outputDirectory.resolve('../hook.dill'), ); @@ -697,13 +682,12 @@ ${e.message} if (!await dependenciesHashFile.exists()) { mustCompile = true; } else { - final outdatedFile = - await dependenciesHashes.findOutdatedFileSystemEntity(); - if (outdatedFile != null) { + final outdatedDependency = + await dependenciesHashes.findOutdatedDependency(environment); + if (outdatedDependency != null) { mustCompile = true; logger.info( - 'Recompiling ${scriptUri.toFilePath()}, ' - '${outdatedFile.toFilePath()} changed.', + 'Recompiling ${scriptUri.toFilePath()}. $outdatedDependency', ); } } @@ -717,7 +701,6 @@ ${e.message} scriptUri, packageConfigUri, workingDirectory, - includeParentEnvironment, kernelFile, depFile, ); @@ -727,14 +710,14 @@ ${e.message} } final dartSources = await _readDepFile(depFile); - final modifiedDuringBuild = - await dependenciesHashes.hashFilesAndDirectories( + final modifiedDuringBuild = await dependenciesHashes.hashDependencies( [ ...dartSources, // If the Dart version changed, recompile. dartExecutable.resolve('../version'), ], - validBeforeLastModified: lastModifiedCutoffTime, + lastModifiedCutoffTime, + environment, ); if (modifiedDuringBuild != null) { logger.severe('File modified during build. Build must be rerun.'); @@ -760,7 +743,6 @@ ${e.message} Uri scriptUri, Uri packageConfigUri, Uri workingDirectory, - bool includeParentEnvironment, File kernelFile, File depFile, ) async { @@ -777,7 +759,6 @@ ${e.message} executable: dartExecutable, arguments: compileArguments, logger: logger, - includeParentEnvironment: includeParentEnvironment, ); var success = true; if (compileResult.exitCode != 0) { diff --git a/pkgs/native_assets_builder/lib/src/dependencies_hash_file/dependencies_hash_file.dart b/pkgs/native_assets_builder/lib/src/dependencies_hash_file/dependencies_hash_file.dart index 259be5793..2b1c49259 100644 --- a/pkgs/native_assets_builder/lib/src/dependencies_hash_file/dependencies_hash_file.dart +++ b/pkgs/native_assets_builder/lib/src/dependencies_hash_file/dependencies_hash_file.dart @@ -33,23 +33,24 @@ class DependenciesHashFile { void _reset() => _hashes = FileSystemHashes(); /// Populate the hashes and persist file with entries from - /// [fileSystemEntities]. + /// [fileSystemEntities] and [environment]. /// - /// If [validBeforeLastModified] is provided, any entities that were modified - /// after [validBeforeLastModified] will get a dummy hash so that they will - /// show up as outdated. If any such entity exists, its uri will be returned. - Future hashFilesAndDirectories( - List fileSystemEntities, { - DateTime? validBeforeLastModified, - }) async { + /// Any file system entities that were modified after + /// [fileSystemValidBeforeLastModified] will get a dummy hash so that they + /// will show up as outdated. If any such entity exists, its uri will be + /// returned. + Future hashDependencies( + List fileSystemEntities, + DateTime fileSystemValidBeforeLastModified, + Map environment, + ) async { _reset(); Uri? modifiedAfterTimeStamp; for (final uri in fileSystemEntities) { int hash; - if (validBeforeLastModified != null && - (await uri.fileSystemEntity.lastModified()) - .isAfter(validBeforeLastModified)) { + if ((await uri.fileSystemEntity.lastModified()) + .isAfter(fileSystemValidBeforeLastModified)) { hash = _hashLastModifiedAfterCutoff; modifiedAfterTimeStamp = uri; } else { @@ -61,30 +62,55 @@ class DependenciesHashFile { } _hashes.files.add(FilesystemEntityHash(uri, hash)); } + for (final entry in environment.entries) { + _hashes.environment.add(EnvironmentVariableHash( + entry.key, _hashEnvironmentValue(entry.value))); + } await _persist(); return modifiedAfterTimeStamp; } Future _persist() => _file.writeAsString(json.encode(_hashes.toJson())); - /// Reads the file with hashes and finds an outdated file or directory if it - /// exists. - Future findOutdatedFileSystemEntity() async { + /// Reads the file with hashes and reports if there is an outdated file, + /// directory or environment variable. + Future findOutdatedDependency( + Map environment, + ) async { await _readFile(); for (final savedHash in _hashes.files) { final uri = savedHash.path; final savedHashValue = savedHash.hash; - final int hashValue; if (_isDirectoryPath(uri.path)) { - hashValue = await _hashDirectory(uri); + final hashValue = await _hashDirectory(uri); + if (savedHashValue != hashValue) { + return 'Directory contents changed: ${uri.toFilePath()}.'; + } } else { - hashValue = await _hashFile(uri); + final hashValue = await _hashFile(uri); + if (savedHashValue != hashValue) { + return 'File contents changed: ${uri.toFilePath()}.'; + } + } + } + + // Check if env vars changed or were removed. + for (final savedHash in _hashes.environment) { + final hashValue = _hashEnvironmentValue(environment[savedHash.key]); + if (savedHash.hash != hashValue) { + return 'Environment variable changed: ${savedHash.key}.'; } - if (savedHashValue != hashValue) { - return uri; + } + + // Check if env vars were added. + final savedEnvKeys = _hashes.environment.map((e) => e.key).toSet(); + for (final envKey in environment.keys) { + if (!savedEnvKeys.contains(envKey)) { + return 'Environment variable changed: $envKey.'; } } + return null; } @@ -109,8 +135,14 @@ class DependenciesHashFile { return _hashNotExists; } final children = directory.listSync(followLinks: true, recursive: false); - final childrenNames = children.map((e) => _pathBaseName(e.path)).join(';'); - return _md5int64(utf8.encode(childrenNames)); + final childrenNames = children.map((e) => _pathBaseName(e.path)).toList() + ..sort(); + return _md5int64(utf8.encode(childrenNames.join(';'))); + } + + int _hashEnvironmentValue(String? value) { + if (value == null) return _hashNotExists; + return _md5int64(utf8.encode(value)); } /// Predefined hash for files and directories that do not exist. @@ -135,27 +167,43 @@ class DependenciesHashFile { class FileSystemHashes { FileSystemHashes({ List? files, - }) : files = files ?? []; + List? environment, + }) : files = files ?? [], + environment = environment ?? []; factory FileSystemHashes.fromJson(Map json) { - final rawEntries = (json[_entitiesKey] as List).cast(); + final rawFilesystemEntries = + (json[_filesystemKey] as List?)?.cast() ?? []; final files = [ - for (final rawEntry in rawEntries) + for (final rawEntry in rawFilesystemEntries) FilesystemEntityHash._fromJson((rawEntry as Map).cast()), ]; + final rawEnvironmentEntries = + (json[_environmentKey] as List?)?.cast() ?? []; + final environment = [ + for (final rawEntry in rawEnvironmentEntries) + EnvironmentVariableHash._fromJson((rawEntry as Map).cast()), + ]; return FileSystemHashes( files: files, + environment: environment, ); } final List files; + final List environment; - static const _entitiesKey = 'entities'; + static const _filesystemKey = 'file_system'; + + static const _environmentKey = 'environment'; Map toJson() => { - _entitiesKey: [ + _filesystemKey: [ for (final FilesystemEntityHash file in files) file.toJson(), ], + _environmentKey: [ + for (final EnvironmentVariableHash env in environment) env.toJson(), + ], }; } @@ -190,6 +238,32 @@ class FilesystemEntityHash { }; } +class EnvironmentVariableHash { + EnvironmentVariableHash( + this.key, + this.hash, + ); + + factory EnvironmentVariableHash._fromJson(Map json) => + EnvironmentVariableHash( + json[_keyKey] as String, + json[_hashKey] as int, + ); + + static const _keyKey = 'key'; + static const _hashKey = 'hash'; + + final String key; + + /// A 64 bit hash. + final int hash; + + Object toJson() => { + _keyKey: key, + _hashKey: hash, + }; +} + bool _isDirectoryPath(String path) => path.endsWith(Platform.pathSeparator) || path.endsWith('/'); diff --git a/pkgs/native_assets_builder/lib/src/utils/run_process.dart b/pkgs/native_assets_builder/lib/src/utils/run_process.dart index cb989c7bc..efb712012 100644 --- a/pkgs/native_assets_builder/lib/src/utils/run_process.dart +++ b/pkgs/native_assets_builder/lib/src/utils/run_process.dart @@ -25,19 +25,6 @@ Future runProcess({ int expectedExitCode = 0, bool throwOnUnexpectedExitCode = false, }) async { - if (Platform.isWindows && !includeParentEnvironment) { - const winEnvKeys = [ - 'SYSTEMROOT', - 'TEMP', - 'TMP', - ]; - environment = { - for (final winEnvKey in winEnvKeys) - winEnvKey: Platform.environment[winEnvKey]!, - ...?environment, - }; - } - final printWorkingDir = workingDirectory != null && workingDirectory != Directory.current.uri; final commandString = [ 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 index 5b31cae41..851458b14 100644 --- 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 @@ -2,6 +2,7 @@ // 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. +import 'dart:convert'; import 'dart:io'; import 'package:test/test.dart'; @@ -139,7 +140,7 @@ void main() async { stringContainsInOrder( [ 'Rerunning build for native_add in', - '${cUri.toFilePath()} changed.' + 'File contents changed: ${cUri.toFilePath()}.' ], ), ); @@ -219,4 +220,76 @@ void main() async { }); }, ); + + test( + 'change environment', + timeout: longTimeout, + () async { + await inTempDir((tempUri) async { + await copyTestProjects(targetUri: tempUri); + final packageUri = tempUri.resolve('native_add/'); + + final logMessages = []; + final logger = createCapturingLogger(logMessages); + + await runPubGet(workingDirectory: packageUri, logger: logger); + logMessages.clear(); + + final result = (await build( + packageUri, + logger, + dartExecutable, + supportedAssetTypes: [CodeAsset.type], + configValidator: validateCodeAssetBuildConfig, + buildValidator: validateCodeAssetBuildOutput, + applicationAssetValidator: validateCodeAssetInApplication, + ))!; + 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; + const modifiedEnvKey = 'PATH'; + (dependenciesContent['environment'] as List).add({ + 'key': modifiedEnvKey, + 'hash': 123456789, + }); + await dependenciesHashFile + .writeAsString(jsonEncode(dependenciesContent)); + + (await build( + packageUri, + logger, + dartExecutable, + supportedAssetTypes: [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/build_runner_reusability_test.dart b/pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart index 19e58520d..2f8d34b45 100644 --- a/pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart @@ -37,7 +37,6 @@ void main() async { configCreator: configCreator, targetOS: Target.current.os, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [], buildValidator: (config, output) async => [], @@ -46,7 +45,6 @@ void main() async { configCreator: configCreator, targetOS: Target.current.os, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [], buildValidator: (config, output) async => [], @@ -56,7 +54,6 @@ void main() async { targetOS: OS.current, buildMode: BuildMode.release, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [], configValidator: (config) async => [], @@ -68,7 +65,6 @@ void main() async { buildMode: BuildMode.release, targetOS: OS.current, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [], configValidator: (config) async => [], 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 deleted file mode 100644 index 45f6bbc17..000000000 --- a/pkgs/native_assets_builder/test/build_runner/build_runner_run_in_isolation_test.dart +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file -// 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. - -import 'dart:io'; - -import 'package:test/test.dart'; - -import '../helpers.dart'; -import 'helpers.dart'; - -const Timeout longTimeout = Timeout(Duration(minutes: 5)); - -void main() async { - final env = Platform.environment; - final cc = env['DART_HOOK_TESTING_C_COMPILER__CC']; - final ar = env['DART_HOOK_TESTING_C_COMPILER__AR']; - final ld = env['DART_HOOK_TESTING_C_COMPILER__LD']; - final envScript = env['DART_HOOK_TESTING_C_COMPILER__ENV_SCRIPT']; - final envScriptArgs = - env['DART_HOOK_TESTING_C_COMPILER__ENV_SCRIPT_ARGUMENTS'] - ?.split(' ') - .map((arg) => arg.trim()) - .where((arg) => arg.isNotEmpty) - .toList(); - - if (cc == null) { - // We don't set any compiler paths on the GitHub CI. - // We neither set compiler paths on MacOS on the Dart SDK CI - // in pkg/test_runner/lib/src/configuration.dart - // nativeCompilerEnvironmentVariables. - // - // We could potentially run this test if we default to some compilers - // we find on the path before running the test. However, the logic for - // discovering compilers is currently hidden inside - // package:native_toolchain_c. - return; - } - - test('run in isolation', timeout: longTimeout, () async { - await inTempDir((tempUri) async { - await copyTestProjects(targetUri: tempUri); - final packageUri = tempUri.resolve('native_add/'); - - await runPubGet(workingDirectory: packageUri, logger: logger); - - printOnFailure('cc: $cc'); - - final result = (await build( - packageUri, - logger, - dartExecutable, - // Manually pass in a compiler. - cCompilerConfig: CCompilerConfig( - archiver: ar?.fileUri, - compiler: cc.fileUri, - envScript: envScript?.fileUri, - envScriptArgs: envScriptArgs, - linker: ld?.fileUri, - ), - // Prevent any other environment variables. - includeParentEnvironment: false, - supportedAssetTypes: [CodeAsset.type], - configValidator: validateCodeAssetBuildConfig, - buildValidator: validateCodeAssetBuildOutput, - applicationAssetValidator: validateCodeAssetInApplication, - ))!; - expect(result.encodedAssets.length, 1); - }); - }); -} - -extension on String { - Uri get fileUri => Uri.file(this); -} diff --git a/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart b/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart index cb69d2833..7b9e5d5cf 100644 --- a/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart +++ b/pkgs/native_assets_builder/test/build_runner/concurrency_shared_test_helper.dart @@ -24,7 +24,6 @@ void main(List args) async { buildMode: BuildMode.release, targetOS: target.os, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [DataAsset.type], configValidator: validateDataAssetBuildConfig, diff --git a/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart b/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart index bebf171a7..6d3bf6a85 100644 --- a/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart +++ b/pkgs/native_assets_builder/test/build_runner/concurrency_test_helper.dart @@ -35,7 +35,6 @@ void main(List args) async { buildMode: BuildMode.release, targetOS: OS.current, workingDirectory: packageUri, - includeParentEnvironment: true, linkingEnabled: false, supportedAssetTypes: [CodeAsset.type, DataAsset.type], configValidator: (config) async => [ diff --git a/pkgs/native_assets_builder/test/build_runner/helpers.dart b/pkgs/native_assets_builder/test/build_runner/helpers.dart index c80f3f798..61f6fa343 100644 --- a/pkgs/native_assets_builder/test/build_runner/helpers.dart +++ b/pkgs/native_assets_builder/test/build_runner/helpers.dart @@ -37,7 +37,6 @@ Future build( required ApplicationAssetValidator applicationAssetValidator, LinkModePreference linkModePreference = LinkModePreference.dynamic, CCompilerConfig? cCompilerConfig, - bool includeParentEnvironment = true, List? capturedLogs, PackageLayout? packageLayout, String? runPackageName, @@ -75,7 +74,6 @@ Future build( buildMode: BuildMode.release, targetOS: targetOS, workingDirectory: packageUri, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, runPackageName: runPackageName, linkingEnabled: linkingEnabled, @@ -105,7 +103,6 @@ Future link( required ApplicationAssetValidator applicationAssetValidator, LinkModePreference linkModePreference = LinkModePreference.dynamic, CCompilerConfig? cCompilerConfig, - bool includeParentEnvironment = true, List? capturedLogs, PackageLayout? packageLayout, required BuildResult buildResult, @@ -143,7 +140,6 @@ Future link( buildMode: BuildMode.release, targetOS: target?.os ?? OS.current, workingDirectory: packageUri, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, buildResult: buildResult, resourceIdentifiers: resourceIdentifiers, @@ -171,7 +167,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink( required BuildValidator buildValidator, required LinkValidator linkValidator, required ApplicationAssetValidator applicationAssetValidator, - bool includeParentEnvironment = true, List? capturedLogs, PackageLayout? packageLayout, String? runPackageName, @@ -203,7 +198,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink( buildMode: BuildMode.release, targetOS: target?.os ?? OS.current, workingDirectory: packageUri, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, runPackageName: runPackageName, linkingEnabled: true, @@ -237,7 +231,6 @@ Future<(BuildResult?, LinkResult?)> buildAndLink( buildMode: BuildMode.release, targetOS: target?.os ?? OS.current, workingDirectory: packageUri, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, buildResult: buildResult, resourceIdentifiers: resourceIdentifiers, @@ -279,7 +272,6 @@ Future buildDryRun( required BuildValidator buildValidator, LinkModePreference linkModePreference = LinkModePreference.dynamic, CCompilerConfig? cCompilerConfig, - bool includeParentEnvironment = true, List? capturedLogs, PackageLayout? packageLayout, required bool linkingEnabled, @@ -297,7 +289,6 @@ Future buildDryRun( ), targetOS: Target.current.os, workingDirectory: packageUri, - includeParentEnvironment: includeParentEnvironment, packageLayout: packageLayout, linkingEnabled: linkingEnabled, supportedAssetTypes: supportedAssetTypes, diff --git a/pkgs/native_assets_builder/test/dependencies_hash_file/dependencies_hash_file_test.dart b/pkgs/native_assets_builder/test/dependencies_hash_file/dependencies_hash_file_test.dart index ae4fd2522..c222b31a4 100644 --- a/pkgs/native_assets_builder/test/dependencies_hash_file/dependencies_hash_file_test.dart +++ b/pkgs/native_assets_builder/test/dependencies_hash_file/dependencies_hash_file_test.dart @@ -11,6 +11,8 @@ import 'package:test/test.dart'; import '../helpers.dart'; void main() async { + final environment = Platform.environment; + test('json format', () async { await inTempDir((tempUri) async { final hashes = FileSystemHashes( @@ -43,66 +45,91 @@ void main() async { await tempFile.writeAsString('hello'); await subFile.writeAsString('world'); - await hashes.hashFilesAndDirectories([ - tempFile.uri, - tempSubDir.uri, - ]); + await hashes.hashDependencies( + [ + tempFile.uri, + tempSubDir.uri, + ], + (await tempFile.lastModified()).add(const Duration(minutes: 1)), + environment, + ); } await reset(); // No changes - expect(await hashes.findOutdatedFileSystemEntity(), isNull); + expect(await hashes.findOutdatedDependency(environment), isNull); // Change file contents. await tempFile.writeAsString('asdf'); - expect(await hashes.findOutdatedFileSystemEntity(), tempFile.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempFile.uri.toFilePath()), + ); await reset(); // Delete file. await tempFile.delete(); - expect(await hashes.findOutdatedFileSystemEntity(), tempFile.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempFile.uri.toFilePath()), + ); await reset(); // Add file to tracked directory. final subFile2 = File.fromUri(tempSubDir.uri.resolve('baz.txt')); await subFile2.create(recursive: true); await subFile2.writeAsString('hello'); - expect(await hashes.findOutdatedFileSystemEntity(), tempSubDir.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempSubDir.uri.toFilePath()), + ); await reset(); // Delete file from tracked directory. await subFile.delete(); - expect(await hashes.findOutdatedFileSystemEntity(), tempSubDir.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempSubDir.uri.toFilePath()), + ); await reset(); // Delete tracked directory. await tempSubDir.delete(recursive: true); - expect(await hashes.findOutdatedFileSystemEntity(), tempSubDir.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempSubDir.uri.toFilePath()), + ); await reset(); // Add directory to tracked directory. final subDir2 = Directory.fromUri(tempSubDir.uri.resolve('baz/')); await subDir2.create(recursive: true); - expect(await hashes.findOutdatedFileSystemEntity(), tempSubDir.uri); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempSubDir.uri.toFilePath()), + ); await reset(); // Overwriting a file with identical contents. await tempFile.writeAsString('something something'); await tempFile.writeAsString('hello'); - expect(await hashes.findOutdatedFileSystemEntity(), isNull); + expect(await hashes.findOutdatedDependency(environment), isNull); await reset(); // If a file is modified after the valid timestamp, it should be marked // as changed. - await hashes.hashFilesAndDirectories( + await hashes.hashDependencies( [ tempFile.uri, ], - validBeforeLastModified: (await tempFile.lastModified()) - .subtract(const Duration(seconds: 1)), + (await tempFile.lastModified()).subtract(const Duration(seconds: 1)), + environment, + ); + expect( + await hashes.findOutdatedDependency(environment), + contains(tempFile.uri.toFilePath()), ); - expect(await hashes.findOutdatedFileSystemEntity(), tempFile.uri); }); }); } diff --git a/pkgs/native_assets_builder/test/helpers.dart b/pkgs/native_assets_builder/test/helpers.dart index 3bb4c710a..ba864da71 100644 --- a/pkgs/native_assets_builder/test/helpers.dart +++ b/pkgs/native_assets_builder/test/helpers.dart @@ -69,7 +69,6 @@ Future runProcess({ List arguments = const [], Uri? workingDirectory, Map? environment, - bool includeParentEnvironment = true, required Logger? logger, bool captureOutput = true, int expectedExitCode = 0, @@ -80,7 +79,6 @@ Future runProcess({ arguments: arguments, workingDirectory: workingDirectory, environment: environment, - includeParentEnvironment: includeParentEnvironment, logger: logger, captureOutput: captureOutput, expectedExitCode: expectedExitCode, diff --git a/pkgs/native_toolchain_c/lib/src/utils/run_process.dart b/pkgs/native_toolchain_c/lib/src/utils/run_process.dart index f9204e5b9..989251409 100644 --- a/pkgs/native_toolchain_c/lib/src/utils/run_process.dart +++ b/pkgs/native_toolchain_c/lib/src/utils/run_process.dart @@ -18,25 +18,11 @@ Future runProcess({ List arguments = const [], Uri? workingDirectory, Map? environment, - bool includeParentEnvironment = true, required Logger? logger, bool captureOutput = true, int expectedExitCode = 0, bool throwOnUnexpectedExitCode = false, }) async { - if (Platform.isWindows && !includeParentEnvironment) { - const winEnvKeys = [ - 'SYSTEMROOT', - 'TEMP', - 'TMP', - ]; - environment = { - for (final winEnvKey in winEnvKeys) - winEnvKey: Platform.environment[winEnvKey]!, - ...?environment, - }; - } - final printWorkingDir = workingDirectory != null && workingDirectory != Directory.current.uri; final commandString = [ @@ -55,8 +41,7 @@ Future runProcess({ arguments, workingDirectory: workingDirectory?.toFilePath(), environment: environment, - includeParentEnvironment: includeParentEnvironment, - runInShell: Platform.isWindows && !includeParentEnvironment, + runInShell: Platform.isWindows && workingDirectory != null, ); final stdoutSub = process.stdout