From 01d6444490ae9af71b30f7916e5e44a514bb4ec5 Mon Sep 17 00:00:00 2001 From: Greg Spencer Date: Sat, 11 Aug 2018 19:10:14 -0700 Subject: [PATCH] Switch from infinite retries on upgrade to 10 retries. (#20450) This changes the flutter tool to just try 10 times before giving up when running "flutter upgrade". Infinite retries can hang bots, and really don't provide a lot of help: if we've failed to upgrade for for nearly a minute, trying every five seconds, then something is just not responding. Also, changed the bot default warning level to "normal" from "all", because the solver messages are VERY verbose: several megs of output for doing packages get on Flutter. "normal" will give warnings, user messages and errors, which should be sufficient to diagnose problems on the bots without spamming the log. I removed the retrying for building the snapshot on flutter.bat because we don't do that on the other platforms, and because I can't imagine how running it again would give a different answer. I also fixed a problem in the whitespace detection when no files matched the type of file that it is looking for, and removed the code that waits until failure to print the logs on setup, since reducing the log output made a huge difference. --- bin/flutter | 27 ++++++++++++++++++++------- bin/flutter.bat | 30 ++++++++++++++++++++---------- dev/bots/cirrus_setup.sh | 30 +++++++++--------------------- dev/bots/test.dart | 2 +- 4 files changed, 50 insertions(+), 39 deletions(-) diff --git a/bin/flutter b/bin/flutter index f5c4da1392..e7cf407ba8 100755 --- a/bin/flutter +++ b/bin/flutter @@ -39,6 +39,23 @@ function _rmlock () { [ -n "$FLUTTER_UPGRADE_LOCK" ] && rm -f "$FLUTTER_UPGRADE_LOCK" } +function retry_upgrade { + local total_tries="10" + local remaining_tries=$(($total_tries - 1)) + while [[ "$remaining_tries" > 0 ]]; do + (cd "$FLUTTER_TOOLS_DIR" && "$PUB" upgrade "$VERBOSITY" --no-packages-dir) && break + echo "Error: Unable to 'pub upgrade' flutter tool. Retrying in five seconds... ($remaining_tries tries left)" + remaining_tries=$(($remaining_tries - 1)) + sleep 5 + done + + if [[ "$remaining_tries" == 0 ]]; then + echo "Command 'pub upgrade' still failed after $total_tries tries, giving up." + return 1 + fi + return 0 +} + function upgrade_flutter () { mkdir -p "$FLUTTER_ROOT/bin/cache" @@ -94,7 +111,7 @@ function upgrade_flutter () { echo Building flutter tool... if [[ "$CI" == "true" || "$BOT" == "true" || "$CONTINUOUS_INTEGRATION" == "true" || "$CHROME_HEADLESS" == "1" ]]; then PUB_ENVIRONMENT="$PUB_ENVIRONMENT:flutter_bot" - VERBOSITY="--verbosity=all" + VERBOSITY="--verbosity=normal" fi export PUB_ENVIRONMENT="$PUB_ENVIRONMENT:flutter_install" @@ -102,12 +119,8 @@ function upgrade_flutter () { export PUB_CACHE="${PUB_CACHE:-"$FLUTTER_ROOT/.pub-cache"}" fi - while : ; do - cd "$FLUTTER_TOOLS_DIR" - "$PUB" upgrade "$VERBOSITY" --no-packages-dir && break - echo "Error: Unable to 'pub upgrade' flutter tool. Retrying in five seconds..." - sleep 5 - done + retry_upgrade + "$DART" --snapshot="$SNAPSHOT_PATH" --packages="$FLUTTER_TOOLS_DIR/.packages" "$SCRIPT_PATH" echo "$revision" > "$STAMP_PATH" fi diff --git a/bin/flutter.bat b/bin/flutter.bat index fd458cfba7..43af48be67 100644 --- a/bin/flutter.bat +++ b/bin/flutter.bat @@ -126,29 +126,38 @@ GOTO :after_subroutine GOTO not_on_bot :on_bot SET PUB_ENVIRONMENT=%PUB_ENVIRONMENT%:flutter_bot - SET VERBOSITY=--verbosity=all + SET VERBOSITY=--verbosity=normal :not_on_bot SET PUB_ENVIRONMENT=%PUB_ENVIRONMENT%:flutter_install IF "%PUB_CACHE%" == "" ( IF EXIST "%pub_cache_path%" SET PUB_CACHE=%pub_cache_path% ) + + SET /A total_tries=10 + SET /A remaining_tries=%total_tries%-1 :retry_pub_upgrade - CALL "%pub%" upgrade "%VERBOSITY%" --no-packages-dir - IF "%ERRORLEVEL%" NEQ "0" ( - ECHO Error: Unable to 'pub upgrade' flutter tool. Retrying in five seconds... - timeout /t 5 /nobreak + ECHO Running pub upgrade... + CALL "%pub%" upgrade "%VERBOSITY%" --no-packages-dir + IF "%ERRORLEVEL%" EQU "0" goto :upgrade_succeeded + ECHO Error Unable to 'pub upgrade' flutter tool. Retrying in five seconds... (%remaining_tries% tries left) + timeout /t 5 /nobreak 2>NUL + SET /A remaining_tries-=1 + IF "%remaining_tries%" EQU "0" GOTO upgrade_retries_exhausted GOTO :retry_pub_upgrade - ) + :upgrade_retries_exhausted + SET exit_code=%ERRORLEVEL% + ECHO Error: 'pub upgrade' still failing after %total_tries% tries, giving up. + GOTO final_exit + :upgrade_succeeded ENDLOCAL POPD - :retry_dart_snapshot CALL "%dart%" --snapshot="%snapshot_path%" --packages="%flutter_tools_dir%\.packages" "%script_path%" IF "%ERRORLEVEL%" NEQ "0" ( - ECHO Error: Unable to create dart snapshot for flutter tool. Retrying... - timeout /t 5 /nobreak - GOTO :retry_dart_snapshot + ECHO Error: Unable to create dart snapshot for flutter tool. + SET exit_code=%ERRORLEVEL% + GOTO :final_exit ) >"%stamp_path%" ECHO %revision% @@ -172,4 +181,5 @@ IF "%exit_code%" EQU "253" ( SET exit_code=%ERRORLEVEL% ) +:final_exit EXIT /B %exit_code% diff --git a/dev/bots/cirrus_setup.sh b/dev/bots/cirrus_setup.sh index f7c292023a..451bcbc281 100755 --- a/dev/bots/cirrus_setup.sh +++ b/dev/bots/cirrus_setup.sh @@ -1,28 +1,15 @@ #!/bin/bash set -e +function error() { + echo "$@" 1>&2 +} + # This script is only meant to be run by the Cirrus CI system, not locally. # It must be run from the root of the Flutter repo. -# Collects log output in a tmpfile, but only prints it if the command fails. -function log_on_fail() { - local COMMAND="$@" - local TMPDIR="$(mktemp -d)" - local TMPFILE="$TMPDIR/command.log" - local EXIT=0 - if ("$@" > "$TMPFILE" 2>&1); then - echo "'$COMMAND' succeeded." - else - EXIT=$? - cat "$TMPFILE" 1>&2 - echo "FAIL: '$COMMAND' exited with code $EXIT" 1>&2 - fi - rm -rf "$TMPDIR" - return "$EXIT" -} - function accept_android_licenses() { - yes "y" | flutter doctor --android-licenses + yes "y" | flutter doctor --android-licenses > /dev/null 2>&1 } echo "Flutter SDK directory is: $PWD" @@ -30,14 +17,15 @@ echo "Flutter SDK directory is: $PWD" # Run flutter to download dependencies and precompile things, and to disable # analytics on the bots. echo "Downloading build dependencies and pre-compiling Flutter snapshot" -log_on_fail ./bin/flutter config --no-analytics +./bin/flutter config --no-analytics # Run doctor, to print it to the log for debugging purposes. ./bin/flutter doctor -v # Accept licenses. -log_on_fail accept_android_licenses && echo "Android licenses accepted." +echo "Accepting Android licenses." +accept_android_licenses || (error "Accepting Android licenses failed." && false) # Run pub get in all the repo packages. echo "Updating packages for Flutter." -log_on_fail ./bin/flutter update-packages +./bin/flutter update-packages diff --git a/dev/bots/test.dart b/dev/bots/test.dart index b29bebaf1c..942dfdbdf2 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -134,7 +134,7 @@ Future _checkForTrailingSpaces() async { 'git', ['diff', '-U0', '--no-color', '--name-only', commitRange, '--'] + fileTypes, workingDirectory: flutterRoot, ); - if (changedFilesResult.stdout == null) { + if (changedFilesResult.stdout == null || changedFilesResult.stdout.trim().isEmpty) { print('No files found that need to be checked for trailing whitespace.'); return; }