From c4119ffcce33158a8ff54eaba9b8a0e6a1091c71 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Mon, 13 Nov 2023 16:21:57 -0500 Subject: [PATCH 1/4] Protect sdk upload script from missing ndk, add documentation for checking write access, improve comments to add context --- tools/android_sdk/create_cipd_packages.sh | 30 ++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tools/android_sdk/create_cipd_packages.sh b/tools/android_sdk/create_cipd_packages.sh index 2887cc1679f0f..68c5b99f93150 100755 --- a/tools/android_sdk/create_cipd_packages.sh +++ b/tools/android_sdk/create_cipd_packages.sh @@ -15,6 +15,7 @@ print_usage () { echo "" echo "This script downloads the packages specified in packages.txt and uploads" echo "them to CIPD for linux, mac, and windows." + echo "To confirm you have write permissions run 'cipd acl-check flutter/android/sdk/all/ -writer'." echo "" echo "Manage the packages to download in 'packages.txt'. You can use" echo "'sdkmanager --list --include_obsolete' in cmdline-tools to list all available packages." @@ -26,13 +27,13 @@ print_usage () { echo "and should only be run on linux or macos hosts." } -# Validate version is provided +# Validate version or argument is provided. if [[ $1 == "" ]]; then print_usage exit 1 fi -#Validate version contains only lower case letters and numbers +# Validate version contains only lower case letters and numbers. if ! [[ $1 =~ ^[[:lower:][:digit:]]+$ ]]; then echo "Version tag can only consist of lower case letters and digits."; print_usage @@ -54,12 +55,22 @@ if [[ ! -d "$sdk_path" ]]; then print_usage exit 1 fi + +# Validate caller has cipd. if [[ ! -d "$sdk_path/cmdline-tools" ]]; then echo "SDK directory does not contain $sdk_path/cmdline-tools." print_usage exit 1 fi +# Validate cipd credentials exist. +# cipdAclCheck=("cipd acl-check flutter/android/sdk/all/") +# if [[ ! cipdAclCheck =~ 'WRITE']]; then +# echo "Visit go/flutter-luci-cipd to request write access" +# print_usage +# exit 1 +# fi + platforms=("linux" "macosx" "windows") package_file_name="packages.txt" @@ -116,12 +127,20 @@ for platform in "${platforms[@]}"; do done # Special treatment for NDK to move to expected directory. + # This is because the engine imports darts GN files which also inlcude + # a copy of the ndk. See https://github.com/flutter/flutter/issues/136666#issuecomment-1805467578 + # for more information. mv $upload_dir/sdk/ndk $upload_dir/ndk-bundle ndk_sub_paths=`find $upload_dir/ndk-bundle -maxdepth 1 -type d` ndk_sub_paths_arr=($ndk_sub_paths) mv ${ndk_sub_paths_arr[1]} $upload_dir/ndk rm -rf $upload_dir/ndk-bundle + if [[ ! -d "$upload_dir/ndk" ]]; then + echo "Failure to bundle ndk for platform" + exit 1 + fi + # Accept all licenses to ensure they are generated and uploaded. yes "y" | $sdkmanager_path --licenses --sdk_root=$sdk_root cp -r "$sdk_root/licenses" "$upload_dir/sdk" @@ -137,11 +156,16 @@ for platform in "${platforms[@]}"; do if [[ $platform == "macosx" ]]; then cipd_name="mac-$arch" fi - echo "Uploading $cipd_name to CIPD" + + echo "Uploading $upload_dir as $cipd_name to CIPD" cipd create -in $upload_dir -name "flutter/android/sdk/all/$cipd_name" -install-mode copy -tag version:$version_tag done rm -rf $sdk_root rm -rf $upload_dir + + # This variable changes the behvaior of sdkmanager. + # Unset to clean up after script. + unset REPO_OS_OVERRIDE done rm -rf $temp_dir From f3102f1f4828484c079b3c5cf5598328addff8b8 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Mon, 13 Nov 2023 16:29:27 -0500 Subject: [PATCH 2/4] Remove commented out write check --- tools/android_sdk/create_cipd_packages.sh | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tools/android_sdk/create_cipd_packages.sh b/tools/android_sdk/create_cipd_packages.sh index 68c5b99f93150..cdcc42a34954c 100755 --- a/tools/android_sdk/create_cipd_packages.sh +++ b/tools/android_sdk/create_cipd_packages.sh @@ -63,14 +63,6 @@ if [[ ! -d "$sdk_path/cmdline-tools" ]]; then exit 1 fi -# Validate cipd credentials exist. -# cipdAclCheck=("cipd acl-check flutter/android/sdk/all/") -# if [[ ! cipdAclCheck =~ 'WRITE']]; then -# echo "Visit go/flutter-luci-cipd to request write access" -# print_usage -# exit 1 -# fi - platforms=("linux" "macosx" "windows") package_file_name="packages.txt" From 6ca01ffcdeca1f223c9e81e271b3797f708915a0 Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Mon, 13 Nov 2023 16:54:28 -0500 Subject: [PATCH 3/4] Spacing --- tools/android_sdk/create_cipd_packages.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/android_sdk/create_cipd_packages.sh b/tools/android_sdk/create_cipd_packages.sh index dfcc392c49a67..2656258ec9417 100755 --- a/tools/android_sdk/create_cipd_packages.sh +++ b/tools/android_sdk/create_cipd_packages.sh @@ -131,8 +131,8 @@ for platform in "${platforms[@]}"; do rm -rf $upload_dir/ndk-bundle if [[ ! -d "$upload_dir/ndk" ]]; then - echo "Failure to bundle ndk for platform" - exit 1 + echo "Failure to bundle ndk for platform" + exit 1 fi # Accept all licenses to ensure they are generated and uploaded. From 8ddae88567d6269726ceed752d5b7ebac042a5cb Mon Sep 17 00:00:00 2001 From: Reid Baker Date: Tue, 14 Nov 2023 10:53:24 -0500 Subject: [PATCH 4/4] Code review feedback --- tools/android_sdk/create_cipd_packages.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/android_sdk/create_cipd_packages.sh b/tools/android_sdk/create_cipd_packages.sh index 2656258ec9417..94d71bb5cabb3 100755 --- a/tools/android_sdk/create_cipd_packages.sh +++ b/tools/android_sdk/create_cipd_packages.sh @@ -27,27 +27,27 @@ print_usage () { echo "and should only be run on linux or macos hosts." } +first_argument=$1 # Validate version or argument is provided. -if [[ $1 == "" ]]; then +if [[ $first_argument == "" ]]; then print_usage exit 1 fi # Validate version contains only lower case letters and numbers. -if ! [[ $1 =~ ^[[:lower:][:digit:]]+$ ]]; then +if ! [[ $first_argument =~ ^[[:lower:][:digit:]]+$ ]]; then echo "Version tag can only consist of lower case letters and digits."; print_usage exit 1 fi -# Validate path contains depot_tools +# Validate environment has cipd installed. if [[ `which cipd` == "" ]]; then echo "'cipd' command not found. depot_tools should be on the path." exit 1 fi sdk_path=${2:-$ANDROID_SDK_ROOT} -version_tag=$1 # Validate directory contains all SDK packages if [[ ! -d "$sdk_path" ]]; then @@ -84,7 +84,7 @@ while [ ! -f "$sdkmanager_path" ]; do done # list available packages -if [ $version_tag == "list" ]; then +if [ $first_argument == "list" ]; then $sdkmanager_path --list --include_obsolete exit 0 fi @@ -152,7 +152,7 @@ for platform in "${platforms[@]}"; do fi echo "Uploading $upload_dir as $cipd_name to CIPD" - cipd create -in $upload_dir -name "flutter/android/sdk/all/$cipd_name" -install-mode copy -tag version:$version_tag + cipd create -in $upload_dir -name "flutter/android/sdk/all/$cipd_name" -install-mode copy -tag version:$first_argument done rm -rf $sdk_root