Add script to upload unified Android SDK CIPD archives and switch DEPS to use it.#29776
Add script to upload unified Android SDK CIPD archives and switch DEPS to use it.#29776GaryQian merged 6 commits intoflutter:mainfrom
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Gold has detected about 231 new digest(s) on patchset 2. |
|
Gold has detected about 132 new digest(s) on patchset 4. |
| ], | ||
| 'condition': 'download_android_deps', | ||
| 'dep_type': 'cipd', | ||
| }, |
| @@ -0,0 +1,5 @@ | |||
| platforms;android-31:platforms | |||
There was a problem hiding this comment.
nice. minor nit: what about moving to a folder. Maybe /tools/android_sdk then this file becomes packages.txt?
| # This script requires depot_tools to be on path. | ||
|
|
||
| print_usage () { | ||
| echo "Usage: create_sdk_cipd_united_package.sh <PATH_TO_SDK_DIR> <VERSION_TAG>" |
| echo "Usage: create_sdk_cipd_united_package.sh <PATH_TO_SDK_DIR> <VERSION_TAG>" | ||
| echo " where:" | ||
| echo " - PATH_TO_SDK_DIR is the path to the sdk folder" | ||
| echo " - VERSION_TAG is the version of the package, e.g. 28r6 or 31v1" |
There was a problem hiding this comment.
| echo " - VERSION_TAG is the version of the package, e.g. 28r6 or 31v1" | |
| echo " - VERSION_TAG is the tag of the cipd package, e.g. 28r6 or 31v1" |
| echo "Unable to find sdkmanager in the SDK directory. Please ensure cmdline-tools is installed." | ||
| exit 1 | ||
| fi | ||
| sdkmanager_path="${find_results[$i]}" |
There was a problem hiding this comment.
how do we know that this is the desired sdkmanager? I think supporting the latest one from line 45 is probably sufficient
There was a problem hiding this comment.
I want this script to not explicitly only support latest. For example, currently, latest is equivalent to 5.0. It is valid to use the 5.0 version or just simply use latest. But, even though these are equivalent, sdkmanager stores them under different subdirs (latest and 5.0 respectively).
Also, when 6.0 is released, latest would point to 6.0 under the latest subdir while people who explicitly use the 5.0 package would still have the old package under the 5.0 subdir. I want this tool to not break for people who use explicit version numbers.
| temp_dir="$1/temp_cipd_android_sdk" | ||
| rm -rf $temp_dir | ||
| mkdir $temp_dir |
There was a problem hiding this comment.
it could use mktemp -d -t android_sdk to create a temp directory with the android_sdk prefix.
| # We copy only the relevant directories to a temporary dir | ||
| # for upload. sdkmanager creates extra files that we don't need. | ||
| array_length=${#split[@]} | ||
| for (( i=1; i<${array_length}; i++ )); do |
There was a problem hiding this comment.
wouldn't this always be 2?. Assuming we are splitting by : and there's only one per line in android_sdk_packages.txt.
There was a problem hiding this comment.
This actually supports multiple directories which can be specified with additional : delimiters. For example, platforms;android-31:platforms:licenses would copy both the platforms dir and the licenses dir that is generated by the platforms;android-31 package.
| @@ -0,0 +1,98 @@ | |||
| #!/bin/bash | |||
|
|
|||
| # This script requires depot_tools to be on path. | |||
There was a problem hiding this comment.
could we check that? sanity checks like this are time savers :)
There was a problem hiding this comment.
We could, but since depot-tools can be installed in any directory, it would not be a robust check.
There was a problem hiding this comment.
It could look for which cipd and exit with code 1 if the string is empty
|
This pull request is not suitable for automatic merging in its current state.
|
…itch DEPS to use it. (flutter/engine#29776)
|
It looks like this commit causes failures on the head-head-head bot: |
Consolidate core Android SDK to one CIPD repo.
New script cold downloads all of the specified packages and uploads them to CIPD for all platforms. This script no longer requires your Android studio managed SDK directory to be modified.