From 0c99a3ba749bbdc57b0d8895cdfaeef835119613 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Mon, 10 May 2021 12:36:21 -0400 Subject: [PATCH] Simplify the tooling bash scripts (#3865) - Rename incremental_build.sh to tool_runner.sh since it has been used for things other than building for a while, and also isn't always incremental. - Replace check_publish.sh with tool_runner.sh to reduce the number of special-case scripts. This is a small behavioral change in that now PRs that don't change any packages will test all packages, as with our other scripts. - Eliminate check_changed_packages. All critical uses have now been replaced by the Dart version of the logic, and the one remaining use was purely for an error message that says something that should be relatively easy to figure out from context anyway. This means we have less bash code to maintain (without unit tests) and understand. --- .cirrus.yml | 40 +++++++++--------- .../video_player/video_player/CONTRIBUTING.md | 2 +- script/build_all_plugins_app.sh | 14 +------ script/check_publish.sh | 22 ---------- script/common.sh | 42 ------------------- .../{incremental_build.sh => tool_runner.sh} | 0 6 files changed, 23 insertions(+), 97 deletions(-) delete mode 100755 script/check_publish.sh rename script/{incremental_build.sh => tool_runner.sh} (100%) diff --git a/.cirrus.yml b/.cirrus.yml index d269f901a180..fe36ec1d5eae 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -48,10 +48,10 @@ task: - CIRRUS_BUILD_ID=null pub run test - name: publishable script: - - ./script/incremental_build.sh version-check - - ./script/check_publish.sh + - ./script/tool_runner.sh version-check + - ./script/tool_runner.sh publish-check - name: format - format_script: ./script/incremental_build.sh format --fail-on-change + format_script: ./script/tool_runner.sh format --fail-on-change license_script: - dart script/tool/lib/src/main.dart license-check - name: test @@ -60,7 +60,7 @@ task: CHANNEL: "master" CHANNEL: "stable" test_script: - - ./script/incremental_build.sh test + - ./script/tool_runner.sh test - name: analyze_master env: matrix: @@ -68,8 +68,8 @@ task: tool_script: - cd script/tool - dart analyze --fatal-infos - plugins_script: - - ./script/incremental_build.sh analyze + script: + - ./script/tool_runner.sh analyze ## TODO(cyanglaz): ## Combing stable and master analyze jobs when integration test null safety is ready on flutter stable. - name: analyze_stable @@ -78,7 +78,7 @@ task: CHANNEL: "stable" script: - find . -depth -type d -wholename '*_web/example' -exec rm -rf {} \; - - ./script/incremental_build.sh analyze + - ./script/tool_runner.sh analyze ### Android tasks ### - name: build_all_plugins_apk env: @@ -111,9 +111,9 @@ task: CHANNEL: "stable" build_script: - flutter config --enable-linux-desktop - - ./script/incremental_build.sh build-examples --linux + - ./script/tool_runner.sh build-examples --linux test_script: - - xvfb-run ./script/incremental_build.sh drive-examples --linux + - xvfb-run ./script/tool_runner.sh drive-examples --linux # Heavy-workload Linux tasks. # These use machines with more CPUs and memory, so will reduce parallelization @@ -154,11 +154,11 @@ task: - echo "$CIRRUS_COMMIT_MESSAGE" > /tmp/cirrus_commit_message.txt - export CIRRUS_CHANGE_MESSAGE="" - export CIRRUS_COMMIT_MESSAGE="" - - ./script/incremental_build.sh build-examples --apk - - ./script/incremental_build.sh java-test # must come after apk build + - ./script/tool_runner.sh build-examples --apk + - ./script/tool_runner.sh java-test # must come after apk build - if [[ -n "$GCLOUD_FIREBASE_TESTLAB_KEY" ]]; then - echo $GCLOUD_FIREBASE_TESTLAB_KEY > ${HOME}/gcloud-service-key.json - - ./script/incremental_build.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26 + - ./script/tool_runner.sh firebase-test-lab --device model=flame,version=29 --device model=starqlteue,version=26 - else - echo "This user does not have permission to run Firebase Test Lab tests." - fi @@ -177,11 +177,11 @@ task: - dart lib/web_driver_installer.dart chromedriver --install-only - ./chromedriver/chromedriver --port=4444 & build_script: - - ./script/incremental_build.sh build-examples --web + - ./script/tool_runner.sh build-examples --web test_script: # TODO(stuartmorgan): Eliminate this check once 2.1 reaches stable. - if [[ "$CHANNEL" == "master" ]]; then - - ./script/incremental_build.sh drive-examples --web + - ./script/tool_runner.sh drive-examples --web - else - echo "Requires null-safe integration_test; skipping." - fi @@ -216,13 +216,13 @@ task: - xcrun simctl list - xcrun simctl create Flutter-iPhone com.apple.CoreSimulator.SimDeviceType.iPhone-11 com.apple.CoreSimulator.SimRuntime.iOS-14-3 | xargs xcrun simctl boot build_script: - - ./script/incremental_build.sh build-examples --ipa + - ./script/tool_runner.sh build-examples --ipa test_script: - - ./script/incremental_build.sh xctest --skip $PLUGINS_TO_SKIP_XCTESTS --ios-destination "platform=iOS Simulator,name=iPhone 11,OS=latest" + - ./script/tool_runner.sh xctest --skip $PLUGINS_TO_SKIP_XCTESTS --ios-destination "platform=iOS Simulator,name=iPhone 11,OS=latest" # `drive-examples` contains integration tests, which changes the UI of the application. # This UI change sometimes affects `xctest`. # So we run `drive-examples` after `xctest`, changing the order will result ci failure. - - ./script/incremental_build.sh drive-examples --ios + - ./script/tool_runner.sh drive-examples --ios ### macOS desktop tasks ### - name: build_all_plugins_macos env: @@ -240,9 +240,9 @@ task: PATH: $PATH:/usr/local/bin build_script: - flutter config --enable-macos-desktop - - ./script/incremental_build.sh build-examples --macos --no-ipa + - ./script/tool_runner.sh build-examples --macos --no-ipa test_script: - - ./script/incremental_build.sh drive-examples --macos + - ./script/tool_runner.sh drive-examples --macos task: # Don't use FLUTTER_UPGRADE_TEMPLATE, Flutter tooling not needed. @@ -253,4 +253,4 @@ task: script: # TODO(jmagman): Lint macOS podspecs but skip any that fail library validation. - find . -name "*.podspec" | xargs grep -l "osx" | xargs rm - - ./script/incremental_build.sh podspecs + - ./script/tool_runner.sh podspecs diff --git a/packages/video_player/video_player/CONTRIBUTING.md b/packages/video_player/video_player/CONTRIBUTING.md index 32c9d1b791d1..15c48038f6fc 100644 --- a/packages/video_player/video_player/CONTRIBUTING.md +++ b/packages/video_player/video_player/CONTRIBUTING.md @@ -8,7 +8,7 @@ dependencies in the examples directory): flutter pub upgrade flutter pub run pigeon --dart_null_safety --input pigeons/messages.dart # git commit your changes so that your working environment is clean -(cd ../../../; ./script/incremental_build.sh format --travis --clang-format=clang-format-7) +(cd ../../../; ./script/tool_runner.sh format --clang-format=clang-format-7) ``` If you update pigeon itself and want to test the changes here, diff --git a/script/build_all_plugins_app.sh b/script/build_all_plugins_app.sh index 06566f059a54..3b3416021a42 100755 --- a/script/build_all_plugins_app.sh +++ b/script/build_all_plugins_app.sh @@ -18,8 +18,6 @@ readonly REPO_DIR="$(dirname "$SCRIPT_DIR")" source "$SCRIPT_DIR/common.sh" -check_changed_packages > /dev/null - # This list should be kept as short as possible, and things should remain here # only as long as necessary, since in general the goal is for all of the latest # versions of plugins to be mutually compatible. @@ -63,18 +61,10 @@ for version in "${BUILD_MODES[@]}"; do if [ $? -eq 0 ]; then echo "Successfully built $version all_plugins app." - echo "All first party plugins compile together." + echo "All first-party plugins compile together." else error "Failed to build $version all_plugins app." - if [[ "${#CHANGED_PACKAGE_LIST[@]}" == 0 ]]; then - error "There was a failure to compile all first party plugins together, but there were no changes detected in packages." - else - error "Changes to the following packages may prevent all first party plugins from compiling together:" - for package in "${CHANGED_PACKAGE_LIST[@]}"; do - error "$package" - done - echo "" - fi + error "This indicates a conflict between two or more first-party plugins." failures=$(($failures + 1)) fi done diff --git a/script/check_publish.sh b/script/check_publish.sh deleted file mode 100755 index a08df7a0b5d8..000000000000 --- a/script/check_publish.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/bin/bash -# Copyright 2013 The Flutter Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -set -e - -# This script checks to make sure that each of the plugins *could* be published. -# It doesn't actually publish anything. - -# So that users can run this script from anywhere and it will work as expected. -readonly SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null && pwd)" -readonly REPO_DIR="$(dirname "$SCRIPT_DIR")" - -source "$SCRIPT_DIR/common.sh" - -# Sets CHANGED_PACKAGE_LIST and CHANGED_PACKAGES -check_changed_packages - -if [[ "${#CHANGED_PACKAGE_LIST[@]}" != 0 ]]; then - plugin_tools publish-check --plugins="${CHANGED_PACKAGES}" -fi diff --git a/script/common.sh b/script/common.sh index 52eeefa6e9ff..0e5290e33d8c 100644 --- a/script/common.sh +++ b/script/common.sh @@ -7,48 +7,6 @@ function error() { echo "$@" 1>&2 } -function get_branch_base_sha() { - local branch_base_sha="$(git merge-base --fork-point FETCH_HEAD HEAD || git merge-base FETCH_HEAD HEAD)" - echo "$branch_base_sha" -} - -function check_changed_packages() { - # Try get a merge base for the branch and calculate affected packages. - # We need this check because some CIs can do a single branch clones with a limited history of commits. - local packages - local branch_base_sha="$(get_branch_base_sha)" - if [[ "$branch_base_sha" != "" ]]; then - echo "Checking for changed packages from $branch_base_sha" - IFS=$'\n' packages=( $(git diff --name-only "$branch_base_sha" HEAD | grep -o "packages/[^/]*" | sed -e "s/packages\///g" | sort | uniq) ) - else - error "Cannot find a merge base for the current branch to run an incremental build..." - error "Please rebase your branch onto the latest master!" - return 1 - fi - - CHANGED_PACKAGES="" - CHANGED_PACKAGE_LIST=() - - # Filter out packages that have been deleted. - for package in "${packages[@]}"; do - if [ -d "$REPO_DIR/packages/$package" ]; then - CHANGED_PACKAGES="${CHANGED_PACKAGES},$package" - CHANGED_PACKAGE_LIST=("${CHANGED_PACKAGE_LIST[@]}" "$package") - fi - done - - if [[ "${#CHANGED_PACKAGE_LIST[@]}" == 0 ]]; then - echo "No changes detected in packages." - else - echo "Detected changes in the following ${#CHANGED_PACKAGE_LIST[@]} package(s):" - for package in "${CHANGED_PACKAGE_LIST[@]}"; do - echo "$package" - done - echo "" - fi - return 0 -} - # Runs the plugin tools from the plugin_tools git submodule. function plugin_tools() { (pushd "$REPO_DIR/script/tool" && dart pub get && popd) >/dev/null diff --git a/script/incremental_build.sh b/script/tool_runner.sh similarity index 100% rename from script/incremental_build.sh rename to script/tool_runner.sh