Consider the FLUTTER_TOOL_ARGS as part of the compilation stamp (#94951)

Author: Hixie

bin/flutter

@@ -13,6 +13,11 @@
 
 set -e
 
+# To debug the tool, you can uncomment the following lines to enable debug
+# mode and set an observatory port:
+# FLUTTER_TOOL_ARGS="--enable-asserts $FLUTTER_TOOL_ARGS"
+# FLUTTER_TOOL_ARGS="$FLUTTER_TOOL_ARGS --observe=65432"
+
 # Needed because if it is set, cd may print the path it changed to.
 unset CDPATH
 

bin/flutter.bat

@@ -13,6 +13,9 @@ REM --------------------------------------------------------------------------
 
 SETLOCAL ENABLEDELAYEDEXPANSION
 
+REM To debug the tool, you can uncomment the following line to enable debug mode:
+REM SET FLUTTER_TOOL_ARGS="--enable-asserts %FLUTTER_TOOL_ARGS%"
+
 FOR %%i IN ("%~dp0..") DO SET FLUTTER_ROOT=%%~fi
 
 REM If available, add location of bundled mingit to PATH
@@ -40,9 +43,6 @@ SET snapshot_path=%cache_dir%\flutter_tools.snapshot
 SET dart_sdk_path=%cache_dir%\dart-sdk
 SET dart=%dart_sdk_path%\bin\dart.exe
 
-REM To debug the tool, you can uncomment the following lines to enable checked mode and set an observatory port:
-REM SET FLUTTER_TOOL_ARGS="--enable-asserts %FLUTTER_TOOL_ARGS%"
-
 REM Chaining the call to 'dart' and 'exit' with an ampersand ensures that
 REM Windows reads both commands into memory once before executing them. This
 REM avoids nasty errors that may otherwise occur when the dart command (e.g. as

bin/internal/shared.bat

@@ -69,6 +69,14 @@ GOTO :after_subroutine
   PUSHD "%flutter_root%"
   FOR /f %%r IN ('git rev-parse HEAD') DO SET revision=%%r
   POPD
+  SET compilekey="%revision%:%FLUTTER_TOOL_ARGS%"
+
+  REM Invalidate cache if:
+  REM  * SNAPSHOT_PATH is not a file, or
+  REM  * STAMP_PATH is not a file, or
+  REM  * STAMP_PATH is an empty file, or
+  REM  * Contents of STAMP_PATH is not what we are going to compile, or
+  REM  * pubspec.yaml last modified after pubspec.lock
 
   REM The following IF conditions are all linked with a logical OR. However,
   REM there is no OR operator in batch and a GOTO construct is used as replacement.
@@ -80,7 +88,7 @@ GOTO :after_subroutine
   IF NOT EXIST "%snapshot_path%" GOTO do_snapshot
   IF NOT EXIST "%stamp_path%" GOTO do_snapshot
   SET /P stamp_value=<"%stamp_path%"
-  IF !stamp_value! NEQ !revision! GOTO do_snapshot
+  IF !stamp_value! NEQ !compilekey! GOTO do_snapshot
   SET pubspec_yaml_path=%flutter_tools_dir%\pubspec.yaml
   SET pubspec_lock_path=%flutter_tools_dir%\pubspec.lock
   FOR /F %%i IN ('DIR /B /O:D "%pubspec_yaml_path%" "%pubspec_lock_path%"') DO SET newer_file=%%i
@@ -164,7 +172,7 @@ GOTO :after_subroutine
       SET exit_code=%ERRORLEVEL%
       GOTO :final_exit
     )
-    >"%stamp_path%" ECHO %revision%
+    >"%stamp_path%" ECHO %compilekey%
 
   REM Exit Subroutine
   EXIT /B

bin/internal/shared.sh

@@ -17,7 +17,7 @@ set -e
 # Needed because if it is set, cd may print the path it changed to.
 unset CDPATH
 
-function retry_upgrade {
+function pub_upgrade_with_retry {
   local total_tries="10"
   local remaining_tries=$((total_tries - 1))
   while [[ "$remaining_tries" -gt 0 ]]; do
@@ -115,45 +115,48 @@ function upgrade_flutter () (
   mkdir -p "$FLUTTER_ROOT/bin/cache"
 
   local revision="$(cd "$FLUTTER_ROOT"; git rev-parse HEAD)"
+  local compilekey="$revision:$FLUTTER_TOOL_ARGS"
 
   # Invalidate cache if:
   #  * SNAPSHOT_PATH is not a file, or
-  #  * STAMP_PATH is not a file with nonzero size, or
-  #  * Contents of STAMP_PATH is not our local git HEAD revision, or
+  #  * STAMP_PATH is not a file, or
+  #  * STAMP_PATH is an empty file, or
+  #  * Contents of STAMP_PATH is not what we are going to compile, or
   #  * pubspec.yaml last modified after pubspec.lock
-  if [[ ! -f "$SNAPSHOT_PATH" || ! -s "$STAMP_PATH" || "$(cat "$STAMP_PATH")" != "$revision" || "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]]; then
+  if [[ ! -f "$SNAPSHOT_PATH" || ! -s "$STAMP_PATH" || "$(cat "$STAMP_PATH")" != "$compilekey" || "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]]; then
     # Waits for the update lock to be acquired. Placing this check inside the
     # conditional allows the majority of flutter/dart installations to bypass
     # the lock entirely, but as a result this required a second verification that
     # the SDK is up to date.
     _wait_for_lock
 
     # A different shell process might have updated the tool/SDK.
-    if [[ -f "$SNAPSHOT_PATH" && -s "$STAMP_PATH" && "$(cat "$STAMP_PATH")" == "$revision" && "$FLUTTER_TOOLS_DIR/pubspec.yaml" -ot "$FLUTTER_TOOLS_DIR/pubspec.lock" ]]; then
+    if [[ -f "$SNAPSHOT_PATH" && -s "$STAMP_PATH" && "$(cat "$STAMP_PATH")" == "$compilekey" && "$FLUTTER_TOOLS_DIR/pubspec.yaml" -ot "$FLUTTER_TOOLS_DIR/pubspec.lock" ]]; then
       exit $?
     fi
 
+    # Fetch Dart...
     rm -f "$FLUTTER_ROOT/version"
     touch "$FLUTTER_ROOT/bin/cache/.dartignore"
     "$FLUTTER_ROOT/bin/internal/update_dart_sdk.sh"
-    VERBOSITY="--verbosity=error"
 
     >&2 echo Building flutter tool...
+
+    # Prepare packages...
+    VERBOSITY="--verbosity=error"
     if [[ "$CI" == "true" || "$BOT" == "true" || "$CONTINUOUS_INTEGRATION" == "true" || "$CHROME_HEADLESS" == "1" ]]; then
       PUB_ENVIRONMENT="$PUB_ENVIRONMENT:flutter_bot"
       VERBOSITY="--verbosity=normal"
     fi
-
     export PUB_ENVIRONMENT="$PUB_ENVIRONMENT:flutter_install"
-
     if [[ -d "$FLUTTER_ROOT/.pub-cache" ]]; then
       export PUB_CACHE="${PUB_CACHE:-"$FLUTTER_ROOT/.pub-cache"}"
     fi
+    pub_upgrade_with_retry
 
-    retry_upgrade
-
+    # Compile...
     "$DART" --verbosity=error --disable-dart-dev $FLUTTER_TOOL_ARGS --snapshot="$SNAPSHOT_PATH" --packages="$FLUTTER_TOOLS_DIR/.packages" --no-enable-mirrors "$SCRIPT_PATH"
-    echo "$revision" > "$STAMP_PATH"
+    echo "$compilekey" > "$STAMP_PATH"
   fi
   # The exit here is extraneous since the function is run in a subshell, but
   # this serves as documentation that running the function in a subshell is
@@ -212,11 +215,6 @@ function shared::execute() {
     exit 1
   fi
 
-  # To debug the tool, you can uncomment the following lines to enable checked
-  # mode and set an observatory port:
-  # FLUTTER_TOOL_ARGS="--enable-asserts $FLUTTER_TOOL_ARGS"
-  # FLUTTER_TOOL_ARGS="$FLUTTER_TOOL_ARGS --observe=65432"
-
   upgrade_flutter 7< "$PROG_NAME"
 
   BIN_NAME="$(basename "$PROG_NAME")"

dev/bots/test.dart

@@ -849,18 +849,17 @@ Future<void> _runFrameworkTests() async {
       '--sound-null-safety',
       'test_private.dart',
     ];
-    final Map<String, String> pubEnvironment = <String, String>{
+    final Map<String, String> environment = <String, String>{
       'FLUTTER_ROOT': flutterRoot,
+      if (Directory(pubCache).existsSync())
+        'PUB_CACHE': pubCache,
     };
-    if (Directory(pubCache).existsSync()) {
-      pubEnvironment['PUB_CACHE'] = pubCache;
-    }
-    recompileFlutterToolWithAsserts(pubEnvironment);
+    adjustEnvironmentToEnableFlutterAsserts(environment);
     await runCommand(
       pub,
       args,
       workingDirectory: path.join(flutterRoot, 'packages', 'flutter', 'test_private'),
-      environment: pubEnvironment,
+      environment: environment,
     );
   }
 
@@ -1530,7 +1529,7 @@ Future<void> _runWebDebugTest(String target, {
   final Map<String, String> environment = <String, String>{
     'FLUTTER_WEB': 'true',
   };
-  recompileFlutterToolWithAsserts(environment);
+  adjustEnvironmentToEnableFlutterAsserts(environment);
   final CommandResult result = await runCommand(
     flutter,
     <String>[
@@ -1648,21 +1647,21 @@ Future<void> _pubRunTest(String workingDirectory, {
       for (final String testPath in testPaths)
         testPath,
   ];
-  final Map<String, String> pubEnvironment = <String, String>{
+  final Map<String, String> environment = <String, String>{
     'FLUTTER_ROOT': flutterRoot,
-    if (includeLocalEngineEnv) ...localEngineEnv,
+    if (includeLocalEngineEnv)
+      ...localEngineEnv,
+    if (Directory(pubCache).existsSync())
+      'PUB_CACHE': pubCache,
   };
-  if (Directory(pubCache).existsSync()) {
-    pubEnvironment['PUB_CACHE'] = pubCache;
-  }
   if (enableFlutterToolAsserts) {
-    recompileFlutterToolWithAsserts(pubEnvironment);
+    adjustEnvironmentToEnableFlutterAsserts(environment);
   }
   if (ensurePrecompiledTool) {
     // We rerun the `flutter` tool here just to make sure that it is compiled
     // before tests run, because the tests might time out if they have to rebuild
     // the tool themselves.
-    await runCommand(flutter, <String>['--version'], environment: pubEnvironment);
+    await runCommand(flutter, <String>['--version'], environment: environment);
   }
   if (useFlutterTestFormatter) {
     final FlutterCompactFormatter formatter = FlutterCompactFormatter();
@@ -1672,7 +1671,7 @@ Future<void> _pubRunTest(String workingDirectory, {
         pub,
         args,
         workingDirectory: workingDirectory,
-        environment: pubEnvironment,
+        environment: environment,
       );
     } finally {
       formatter.finish();
@@ -1683,7 +1682,7 @@ Future<void> _pubRunTest(String workingDirectory, {
       pub,
       args,
       workingDirectory: workingDirectory,
-      environment: pubEnvironment,
+      environment: environment,
       removeLine: useBuildRunner ? (String line) => line.startsWith('[INFO]') : null,
     );
   }
@@ -1784,20 +1783,16 @@ Future<void> _runFlutterTest(String workingDirectory, {
   }
 }
 
-/// This will force the next run of the Flutter tool (if it uses the provided environment) to
-/// have asserts enabled, by setting an environment variable and deleting the cache.
-void recompileFlutterToolWithAsserts(Map<String, String> pubEnvironment) {
+/// This will force the next run of the Flutter tool (if it uses the provided
+/// environment) to have asserts enabled, by setting an environment variable.
+void adjustEnvironmentToEnableFlutterAsserts(Map<String, String> environment) {
   // If an existing env variable exists append to it, but only if
   // it doesn't appear to already include enable-asserts.
   String toolsArgs = Platform.environment['FLUTTER_TOOL_ARGS'] ?? '';
   if (!toolsArgs.contains('--enable-asserts')) {
     toolsArgs += ' --enable-asserts';
   }
-  pubEnvironment['FLUTTER_TOOL_ARGS'] = toolsArgs.trim();
-  // The flutter_tool will originally have been snapshotted without asserts.
-  // We need to force it to be regenerated with them enabled.
-  deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.snapshot'));
-  deleteFile(path.join(flutterRoot, 'bin', 'cache', 'flutter_tools.stamp'));
+  environment['FLUTTER_TOOL_ARGS'] = toolsArgs.trim();
 }
 
 Map<String, String> _initGradleEnvironment() {