Skip to content

Commit d56a5b5

Browse files
authored
[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 (#1764), this PR is about caching correctness. Bug: * #32
1 parent aa82b97 commit d56a5b5

File tree

12 files changed

+235
-200
lines changed

12 files changed

+235
-200
lines changed

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

+17-36
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ class NativeAssetsBuildRunner {
9696
required OS targetOS,
9797
required BuildMode buildMode,
9898
required Uri workingDirectory,
99-
required bool includeParentEnvironment,
10099
PackageLayout? packageLayout,
101100
String? runPackageName,
102101
required List<String> supportedAssetTypes,
@@ -165,7 +164,6 @@ class NativeAssetsBuildRunner {
165164
buildValidator(config as BuildConfig, output as BuildOutput),
166165
packageLayout.packageConfigUri,
167166
workingDirectory,
168-
includeParentEnvironment,
169167
null,
170168
packageLayout,
171169
);
@@ -205,7 +203,6 @@ class NativeAssetsBuildRunner {
205203
required BuildMode buildMode,
206204
required Uri workingDirectory,
207205
required ApplicationAssetValidator applicationAssetValidator,
208-
required bool includeParentEnvironment,
209206
PackageLayout? packageLayout,
210207
Uri? resourceIdentifiers,
211208
String? runPackageName,
@@ -269,7 +266,6 @@ class NativeAssetsBuildRunner {
269266
linkValidator(config as LinkConfig, output as LinkOutput),
270267
packageLayout.packageConfigUri,
271268
workingDirectory,
272-
includeParentEnvironment,
273269
resourceIdentifiers,
274270
packageLayout,
275271
);
@@ -330,7 +326,6 @@ class NativeAssetsBuildRunner {
330326
required OS targetOS,
331327
required Uri workingDirectory,
332328
required bool linkingEnabled,
333-
required bool includeParentEnvironment,
334329
PackageLayout? packageLayout,
335330
String? runPackageName,
336331
required List<String> supportedAssetTypes,
@@ -341,7 +336,6 @@ class NativeAssetsBuildRunner {
341336
validator: (HookConfig config, HookOutput output) =>
342337
buildValidator(config as BuildConfig, output as BuildOutput),
343338
workingDirectory: workingDirectory,
344-
includeParentEnvironment: includeParentEnvironment,
345339
packageLayout: packageLayout,
346340
runPackageName: runPackageName,
347341
supportedAssetTypes: supportedAssetTypes,
@@ -353,7 +347,6 @@ class NativeAssetsBuildRunner {
353347
required _HookValidator validator,
354348
required OS targetOS,
355349
required Uri workingDirectory,
356-
required bool includeParentEnvironment,
357350
PackageLayout? packageLayout,
358351
String? runPackageName,
359352
required bool linkingEnabled,
@@ -420,7 +413,6 @@ class NativeAssetsBuildRunner {
420413
config.packageRoot.resolve('hook/${hook.scriptName}'),
421414
packageConfigUri,
422415
workingDirectory,
423-
includeParentEnvironment,
424416
);
425417
if (!compileSuccess) return null;
426418

@@ -438,7 +430,6 @@ class NativeAssetsBuildRunner {
438430
validator,
439431
packageConfigUri,
440432
workingDirectory,
441-
includeParentEnvironment,
442433
null,
443434
hookKernelFile,
444435
packageLayout!,
@@ -450,18 +441,16 @@ class NativeAssetsBuildRunner {
450441
return hookResult;
451442
}
452443

453-
// TODO(https://github.com/dart-lang/native/issues/32): Rerun hook if
454-
// environment variables change.
455444
Future<HookOutput?> _runHookForPackageCached(
456445
Hook hook,
457446
HookConfig config,
458447
_HookValidator validator,
459448
Uri packageConfigUri,
460449
Uri workingDirectory,
461-
bool includeParentEnvironment,
462450
Uri? resources,
463451
PackageLayout packageLayout,
464452
) async {
453+
final environment = Platform.environment;
465454
final outDir = config.outputDirectory;
466455
return await runUnderDirectoriesLock(
467456
[
@@ -481,7 +470,6 @@ class NativeAssetsBuildRunner {
481470
config.packageRoot.resolve('hook/${hook.scriptName}'),
482471
packageConfigUri,
483472
workingDirectory,
484-
includeParentEnvironment,
485473
);
486474
if (!compileSuccess) {
487475
return null;
@@ -510,9 +498,10 @@ ${e.message}
510498
''');
511499
return null;
512500
}
513-
final outdatedFile =
514-
await dependenciesHashes.findOutdatedFileSystemEntity();
515-
if (outdatedFile == null) {
501+
502+
final outdatedDependency =
503+
await dependenciesHashes.findOutdatedDependency(environment);
504+
if (outdatedDependency == null) {
516505
logger.info(
517506
'Skipping ${hook.name} for ${config.packageName}'
518507
' in ${outDir.toFilePath()}.'
@@ -524,8 +513,7 @@ ${e.message}
524513
}
525514
logger.info(
526515
'Rerunning ${hook.name} for ${config.packageName}'
527-
' in ${outDir.toFilePath()}.'
528-
' ${outdatedFile.toFilePath()} changed.',
516+
' in ${outDir.toFilePath()}. $outdatedDependency',
529517
);
530518
}
531519

@@ -535,7 +523,6 @@ ${e.message}
535523
validator,
536524
packageConfigUri,
537525
workingDirectory,
538-
includeParentEnvironment,
539526
resources,
540527
hookKernelFile,
541528
packageLayout,
@@ -545,14 +532,14 @@ ${e.message}
545532
await dependenciesHashFile.delete();
546533
}
547534
} else {
548-
final modifiedDuringBuild =
549-
await dependenciesHashes.hashFilesAndDirectories(
535+
final modifiedDuringBuild = await dependenciesHashes.hashDependencies(
550536
[
551537
...result.dependencies,
552538
// Also depend on the hook source code.
553539
hookHashesFile.uri,
554540
],
555-
validBeforeLastModified: lastModifiedCutoffTime,
541+
lastModifiedCutoffTime,
542+
environment,
556543
);
557544
if (modifiedDuringBuild != null) {
558545
logger.severe('File modified during build. Build must be rerun.');
@@ -569,7 +556,6 @@ ${e.message}
569556
_HookValidator validator,
570557
Uri packageConfigUri,
571558
Uri workingDirectory,
572-
bool includeParentEnvironment,
573559
Uri? resources,
574560
File hookKernelFile,
575561
PackageLayout packageLayout,
@@ -597,7 +583,6 @@ ${e.message}
597583
executable: dartExecutable,
598584
arguments: arguments,
599585
logger: logger,
600-
includeParentEnvironment: includeParentEnvironment,
601586
);
602587

603588
var deleteOutputIfExists = false;
@@ -680,8 +665,8 @@ ${e.message}
680665
Uri scriptUri,
681666
Uri packageConfigUri,
682667
Uri workingDirectory,
683-
bool includeParentEnvironment,
684668
) async {
669+
final environment = Platform.environment;
685670
final kernelFile = File.fromUri(
686671
outputDirectory.resolve('../hook.dill'),
687672
);
@@ -697,13 +682,12 @@ ${e.message}
697682
if (!await dependenciesHashFile.exists()) {
698683
mustCompile = true;
699684
} else {
700-
final outdatedFile =
701-
await dependenciesHashes.findOutdatedFileSystemEntity();
702-
if (outdatedFile != null) {
685+
final outdatedDependency =
686+
await dependenciesHashes.findOutdatedDependency(environment);
687+
if (outdatedDependency != null) {
703688
mustCompile = true;
704689
logger.info(
705-
'Recompiling ${scriptUri.toFilePath()}, '
706-
'${outdatedFile.toFilePath()} changed.',
690+
'Recompiling ${scriptUri.toFilePath()}. $outdatedDependency',
707691
);
708692
}
709693
}
@@ -717,7 +701,6 @@ ${e.message}
717701
scriptUri,
718702
packageConfigUri,
719703
workingDirectory,
720-
includeParentEnvironment,
721704
kernelFile,
722705
depFile,
723706
);
@@ -727,14 +710,14 @@ ${e.message}
727710
}
728711

729712
final dartSources = await _readDepFile(depFile);
730-
final modifiedDuringBuild =
731-
await dependenciesHashes.hashFilesAndDirectories(
713+
final modifiedDuringBuild = await dependenciesHashes.hashDependencies(
732714
[
733715
...dartSources,
734716
// If the Dart version changed, recompile.
735717
dartExecutable.resolve('../version'),
736718
],
737-
validBeforeLastModified: lastModifiedCutoffTime,
719+
lastModifiedCutoffTime,
720+
environment,
738721
);
739722
if (modifiedDuringBuild != null) {
740723
logger.severe('File modified during build. Build must be rerun.');
@@ -760,7 +743,6 @@ ${e.message}
760743
Uri scriptUri,
761744
Uri packageConfigUri,
762745
Uri workingDirectory,
763-
bool includeParentEnvironment,
764746
File kernelFile,
765747
File depFile,
766748
) async {
@@ -777,7 +759,6 @@ ${e.message}
777759
executable: dartExecutable,
778760
arguments: compileArguments,
779761
logger: logger,
780-
includeParentEnvironment: includeParentEnvironment,
781762
);
782763
var success = true;
783764
if (compileResult.exitCode != 0) {

0 commit comments

Comments
 (0)