From 2835c3f1161afcd93894324475c72eb66f67a05e Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Mon, 10 Jan 2022 15:20:05 -0800 Subject: [PATCH] Consider the FLUTTER_TOOL_ARGS as part of the compilation stamp (#94951) --- bin/flutter | 5 +++++ bin/flutter.bat | 6 +++--- bin/internal/shared.bat | 12 ++++++++++-- bin/internal/shared.sh | 30 ++++++++++++++-------------- dev/bots/test.dart | 43 ++++++++++++++++++----------------------- 5 files changed, 51 insertions(+), 45 deletions(-) diff --git a/bin/flutter b/bin/flutter index a6ca46d48f..591b0107cb 100755 --- a/bin/flutter +++ b/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 diff --git a/bin/flutter.bat b/bin/flutter.bat index 7c3ba962d6..06c9a353c9 100644 --- a/bin/flutter.bat +++ b/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 diff --git a/bin/internal/shared.bat b/bin/internal/shared.bat index d4ee4552aa..05cbac1c9f 100644 --- a/bin/internal/shared.bat +++ b/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 diff --git a/bin/internal/shared.sh b/bin/internal/shared.sh index 05cba4393b..50ffcd4c28 100644 --- a/bin/internal/shared.sh +++ 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,13 +115,15 @@ 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 @@ -129,31 +131,32 @@ function upgrade_flutter () ( _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")" diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 4a8bf38d37..46aca344ce 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -849,18 +849,17 @@ Future _runFrameworkTests() async { '--sound-null-safety', 'test_private.dart', ]; - final Map pubEnvironment = { + final Map environment = { '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 _runWebDebugTest(String target, { final Map environment = { 'FLUTTER_WEB': 'true', }; - recompileFlutterToolWithAsserts(environment); + adjustEnvironmentToEnableFlutterAsserts(environment); final CommandResult result = await runCommand( flutter, [ @@ -1648,21 +1647,21 @@ Future _pubRunTest(String workingDirectory, { for (final String testPath in testPaths) testPath, ]; - final Map pubEnvironment = { + final Map environment = { '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, ['--version'], environment: pubEnvironment); + await runCommand(flutter, ['--version'], environment: environment); } if (useFlutterTestFormatter) { final FlutterCompactFormatter formatter = FlutterCompactFormatter(); @@ -1672,7 +1671,7 @@ Future _pubRunTest(String workingDirectory, { pub, args, workingDirectory: workingDirectory, - environment: pubEnvironment, + environment: environment, ); } finally { formatter.finish(); @@ -1683,7 +1682,7 @@ Future _pubRunTest(String workingDirectory, { pub, args, workingDirectory: workingDirectory, - environment: pubEnvironment, + environment: environment, removeLine: useBuildRunner ? (String line) => line.startsWith('[INFO]') : null, ); } @@ -1784,20 +1783,16 @@ Future _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 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 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 _initGradleEnvironment() {