Skip to content

Commit 62f0754

Browse files
dcharkesdcharkes
committed
[native_assets_builder] Don't cache if env vars change
1 parent 9c33168 commit 62f0754

10 files changed

+94
-147
lines changed

‎pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart

+30-25
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'dart:async';
66
import 'dart:convert';
77
import 'dart:io';
88

9+
import 'package:collection/collection.dart';
910
import 'package:logging/logging.dart';
1011
import 'package:native_assets_cli/native_assets_cli_internal.dart';
1112
import 'package:package_config/package_config.dart';
@@ -96,7 +97,6 @@ class NativeAssetsBuildRunner {
9697
required OS targetOS,
9798
required BuildMode buildMode,
9899
required Uri workingDirectory,
99-
required bool includeParentEnvironment,
100100
PackageLayout? packageLayout,
101101
String? runPackageName,
102102
required List<String> supportedAssetTypes,
@@ -165,7 +165,6 @@ class NativeAssetsBuildRunner {
165165
buildValidator(config as BuildConfig, output as BuildOutput),
166166
packageLayout.packageConfigUri,
167167
workingDirectory,
168-
includeParentEnvironment,
169168
null,
170169
packageLayout,
171170
);
@@ -205,7 +204,6 @@ class NativeAssetsBuildRunner {
205204
required BuildMode buildMode,
206205
required Uri workingDirectory,
207206
required ApplicationAssetValidator applicationAssetValidator,
208-
required bool includeParentEnvironment,
209207
PackageLayout? packageLayout,
210208
Uri? resourceIdentifiers,
211209
String? runPackageName,
@@ -269,7 +267,6 @@ class NativeAssetsBuildRunner {
269267
linkValidator(config as LinkConfig, output as LinkOutput),
270268
packageLayout.packageConfigUri,
271269
workingDirectory,
272-
includeParentEnvironment,
273270
resourceIdentifiers,
274271
packageLayout,
275272
);
@@ -330,7 +327,6 @@ class NativeAssetsBuildRunner {
330327
required OS targetOS,
331328
required Uri workingDirectory,
332329
required bool linkingEnabled,
333-
required bool includeParentEnvironment,
334330
PackageLayout? packageLayout,
335331
String? runPackageName,
336332
required List<String> supportedAssetTypes,
@@ -341,7 +337,6 @@ class NativeAssetsBuildRunner {
341337
validator: (HookConfig config, HookOutput output) =>
342338
buildValidator(config as BuildConfig, output as BuildOutput),
343339
workingDirectory: workingDirectory,
344-
includeParentEnvironment: includeParentEnvironment,
345340
packageLayout: packageLayout,
346341
runPackageName: runPackageName,
347342
supportedAssetTypes: supportedAssetTypes,
@@ -353,7 +348,6 @@ class NativeAssetsBuildRunner {
353348
required _HookValidator validator,
354349
required OS targetOS,
355350
required Uri workingDirectory,
356-
required bool includeParentEnvironment,
357351
PackageLayout? packageLayout,
358352
String? runPackageName,
359353
required bool linkingEnabled,
@@ -420,7 +414,6 @@ class NativeAssetsBuildRunner {
420414
config.packageRoot.resolve('hook/${hook.scriptName}'),
421415
packageConfigUri,
422416
workingDirectory,
423-
includeParentEnvironment,
424417
);
425418
if (!compileSuccess) return null;
426419

@@ -438,7 +431,6 @@ class NativeAssetsBuildRunner {
438431
validator,
439432
packageConfigUri,
440433
workingDirectory,
441-
includeParentEnvironment,
442434
null,
443435
hookKernelFile,
444436
packageLayout!,
@@ -458,7 +450,6 @@ class NativeAssetsBuildRunner {
458450
_HookValidator validator,
459451
Uri packageConfigUri,
460452
Uri workingDirectory,
461-
bool includeParentEnvironment,
462453
Uri? resources,
463454
PackageLayout packageLayout,
464455
) async {
@@ -481,7 +472,6 @@ class NativeAssetsBuildRunner {
481472
config.packageRoot.resolve('hook/${hook.scriptName}'),
482473
packageConfigUri,
483474
workingDirectory,
484-
includeParentEnvironment,
485475
);
486476
if (!compileSuccess) {
487477
return null;
@@ -496,7 +486,12 @@ class NativeAssetsBuildRunner {
496486
final dependenciesHashes =
497487
DependenciesHashFile(file: dependenciesHashFile);
498488
final lastModifiedCutoffTime = DateTime.now();
499-
if (buildOutputFile.existsSync() && dependenciesHashFile.existsSync()) {
489+
final environmentFile = File.fromUri(
490+
config.outputDirectory.resolve('../environment.json'),
491+
);
492+
if (buildOutputFile.existsSync() &&
493+
dependenciesHashFile.existsSync() &&
494+
environmentFile.existsSync()) {
500495
late final HookOutput output;
501496
try {
502497
output = _readHookOutputFromUri(hook, buildOutputFile);
@@ -510,9 +505,15 @@ ${e.message}
510505
''');
511506
return null;
512507
}
508+
513509
final outdatedFile =
514510
await dependenciesHashes.findOutdatedFileSystemEntity();
515-
if (outdatedFile == null) {
511+
final environmentChanged = (!await environmentFile.exists()) ||
512+
!const MapEquality<String, String>().equals(
513+
(json.decode(await environmentFile.readAsString()) as Map)
514+
.cast<String, String>(),
515+
Platform.environment);
516+
if (outdatedFile == null && !environmentChanged) {
516517
logger.info(
517518
'Skipping ${hook.name} for ${config.packageName}'
518519
' in ${outDir.toFilePath()}.'
@@ -522,11 +523,19 @@ ${e.message}
522523
// check here whether the config is equal.
523524
return output;
524525
}
525-
logger.info(
526-
'Rerunning ${hook.name} for ${config.packageName}'
527-
' in ${outDir.toFilePath()}.'
528-
' ${outdatedFile.toFilePath()} changed.',
529-
);
526+
if (outdatedFile != null) {
527+
logger.info(
528+
'Rerunning ${hook.name} for ${config.packageName}'
529+
' in ${outDir.toFilePath()}.'
530+
' ${outdatedFile.toFilePath()} changed.',
531+
);
532+
} else {
533+
logger.info(
534+
'Rerunning ${hook.name} for ${config.packageName}'
535+
' in ${outDir.toFilePath()}.'
536+
' The environment variables changed.',
537+
);
538+
}
530539
}
531540

532541
final result = await _runHookForPackage(
@@ -535,7 +544,6 @@ ${e.message}
535544
validator,
536545
packageConfigUri,
537546
workingDirectory,
538-
includeParentEnvironment,
539547
resources,
540548
hookKernelFile,
541549
packageLayout,
@@ -545,6 +553,9 @@ ${e.message}
545553
await dependenciesHashFile.delete();
546554
}
547555
} else {
556+
await environmentFile.writeAsString(
557+
json.encode(Platform.environment),
558+
);
548559
final modifiedDuringBuild =
549560
await dependenciesHashes.hashFilesAndDirectories(
550561
[
@@ -569,7 +580,6 @@ ${e.message}
569580
_HookValidator validator,
570581
Uri packageConfigUri,
571582
Uri workingDirectory,
572-
bool includeParentEnvironment,
573583
Uri? resources,
574584
File hookKernelFile,
575585
PackageLayout packageLayout,
@@ -597,7 +607,6 @@ ${e.message}
597607
executable: dartExecutable,
598608
arguments: arguments,
599609
logger: logger,
600-
includeParentEnvironment: includeParentEnvironment,
601610
);
602611

603612
var deleteOutputIfExists = false;
@@ -680,7 +689,6 @@ ${e.message}
680689
Uri scriptUri,
681690
Uri packageConfigUri,
682691
Uri workingDirectory,
683-
bool includeParentEnvironment,
684692
) async {
685693
final kernelFile = File.fromUri(
686694
outputDirectory.resolve('../hook.dill'),
@@ -717,7 +725,6 @@ ${e.message}
717725
scriptUri,
718726
packageConfigUri,
719727
workingDirectory,
720-
includeParentEnvironment,
721728
kernelFile,
722729
depFile,
723730
);
@@ -760,7 +767,6 @@ ${e.message}
760767
Uri scriptUri,
761768
Uri packageConfigUri,
762769
Uri workingDirectory,
763-
bool includeParentEnvironment,
764770
File kernelFile,
765771
File depFile,
766772
) async {
@@ -777,7 +783,6 @@ ${e.message}
777783
executable: dartExecutable,
778784
arguments: compileArguments,
779785
logger: logger,
780-
includeParentEnvironment: includeParentEnvironment,
781786
);
782787
var success = true;
783788
if (compileResult.exitCode != 0) {

‎pkgs/native_assets_builder/lib/src/utils/run_process.dart

-13
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,6 @@ Future<RunProcessResult> runProcess({
2525
int expectedExitCode = 0,
2626
bool throwOnUnexpectedExitCode = false,
2727
}) async {
28-
if (Platform.isWindows && !includeParentEnvironment) {
29-
const winEnvKeys = [
30-
'SYSTEMROOT',
31-
'TEMP',
32-
'TMP',
33-
];
34-
environment = {
35-
for (final winEnvKey in winEnvKeys)
36-
winEnvKey: Platform.environment[winEnvKey]!,
37-
...?environment,
38-
};
39-
}
40-
4128
final printWorkingDir =
4229
workingDirectory != null && workingDirectory != Directory.current.uri;
4330
final commandString = [

‎pkgs/native_assets_builder/test/build_runner/build_runner_caching_test.dart

+63-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5+
import 'dart:convert';
56
import 'dart:io';
67

78
import 'package:test/test.dart';
@@ -62,7 +63,6 @@ void main() async {
6263
applicationAssetValidator: validateCodeAssetInApplication,
6364
))!;
6465
final hookUri = packageUri.resolve('hook/build.dart');
65-
print(logMessages.join('\n'));
6666
expect(
6767
logMessages.join('\n'),
6868
isNot(contains('Recompiling ${hookUri.toFilePath()}')),
@@ -220,4 +220,66 @@ void main() async {
220220
});
221221
},
222222
);
223+
224+
test(
225+
'change environment',
226+
timeout: longTimeout,
227+
() async {
228+
await inTempDir((tempUri) async {
229+
await copyTestProjects(targetUri: tempUri);
230+
final packageUri = tempUri.resolve('native_add/');
231+
232+
final logMessages = <String>[];
233+
final logger = createCapturingLogger(logMessages);
234+
235+
await runPubGet(workingDirectory: packageUri, logger: logger);
236+
logMessages.clear();
237+
238+
final result = (await build(
239+
packageUri,
240+
logger,
241+
dartExecutable,
242+
supportedAssetTypes: [CodeAsset.type],
243+
configValidator: validateCodeAssetBuildConfig,
244+
buildValidator: validateCodeAssetBuildOutput,
245+
applicationAssetValidator: validateCodeAssetInApplication,
246+
))!;
247+
logMessages.clear();
248+
249+
// Simulate that the environment variables changed by augmenting the
250+
// persisted environment from the last invocation.
251+
final environmentFile = File.fromUri(
252+
CodeAsset.fromEncoded(result.encodedAssets.single)
253+
.file!
254+
.parent
255+
.parent
256+
.resolve('environment.json'),
257+
);
258+
expect(await environmentFile.exists(), true);
259+
await environmentFile.writeAsString(jsonEncode({
260+
...Platform.environment,
261+
'SOME_KEY_THAT_IS_NOT_ALREADY_IN_THE_ENVIRONMENT': 'some_value',
262+
}));
263+
264+
(await build(
265+
packageUri,
266+
logger,
267+
dartExecutable,
268+
supportedAssetTypes: [CodeAsset.type],
269+
configValidator: validateCodeAssetBuildConfig,
270+
buildValidator: validateCodeAssetBuildOutput,
271+
applicationAssetValidator: validateCodeAssetInApplication,
272+
))!;
273+
expect(
274+
logMessages.join('\n'),
275+
contains('hook.dill'),
276+
);
277+
expect(
278+
logMessages.join('\n'),
279+
isNot(contains('Skipping build for native_add')),
280+
);
281+
logMessages.clear();
282+
});
283+
},
284+
);
223285
}

‎pkgs/native_assets_builder/test/build_runner/build_runner_reusability_test.dart

-4
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ void main() async {
3737
configCreator: configCreator,
3838
targetOS: Target.current.os,
3939
workingDirectory: packageUri,
40-
includeParentEnvironment: true,
4140
linkingEnabled: false,
4241
supportedAssetTypes: [],
4342
buildValidator: (config, output) async => [],
@@ -46,7 +45,6 @@ void main() async {
4645
configCreator: configCreator,
4746
targetOS: Target.current.os,
4847
workingDirectory: packageUri,
49-
includeParentEnvironment: true,
5048
linkingEnabled: false,
5149
supportedAssetTypes: [],
5250
buildValidator: (config, output) async => [],
@@ -56,7 +54,6 @@ void main() async {
5654
targetOS: OS.current,
5755
buildMode: BuildMode.release,
5856
workingDirectory: packageUri,
59-
includeParentEnvironment: true,
6057
linkingEnabled: false,
6158
supportedAssetTypes: [],
6259
configValidator: (config) async => [],
@@ -68,7 +65,6 @@ void main() async {
6865
buildMode: BuildMode.release,
6966
targetOS: OS.current,
7067
workingDirectory: packageUri,
71-
includeParentEnvironment: true,
7268
linkingEnabled: false,
7369
supportedAssetTypes: [],
7470
configValidator: (config) async => [],

0 commit comments

Comments
 (0)