Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upInstall android tools r25.2.3 concurrently #639
Merged
+719
−20
Conversation
aneeshusa
force-pushed the
aneeshusa:install-android-tools-r25.2.3-concurrently
branch
7 times, most recently
from
d18c648
to
6215be2
aneeshusa
changed the title
|
I got this working! r? @larsbergstrom |
aneeshusa
force-pushed the
aneeshusa:install-android-tools-r25.2.3-concurrently
branch
from
6215be2
to
a99d2c4
|
Amazing! |
The newer version of this module will prefer to use the unzip command instead of the python zipfile module for unzipping. This works around https://bugs.python.org/issue15795, namely a bug where the zipfile module does not respect permissions. This will fix our Android toolchain `archive.extracted` states. Note that this version of the module does not include saltstack/salt#36552, so keep the `archive_user` argument to `archive.extracted` states. `archive.extracted` sets ownership only on the `if_missing` path, so ensure Salt sets ownership on all the extracted files by unsetting `if_missing`, which defaults to `name`. This now refers to the top level extracted directory, so all extracted files will be properly owned by `servo/servo`, which ensures we are able to run the `android` command as the `servo` user to download the platform and build tools in the `cmd.run` state. Because the `unzip` binary will be used to extract the zip files, require the `android-dependencies` `pkg.installed` state from `archive.extracted` states that extract zip archives to ensure that the `unzip` binary is available.
Install a new version of the Android tools along with the old one. Keep the old version as the current version for now to avoid breaking builds, but still make the new version available. This will also make future upgrades more smooth as well. Switch to the new URL scheme for SDK downloads, which also has switched to using zip files instead of .tar.gz files. Note that the new packaging format no longer has a top-level `android-sdk-linux` folder. Move the `build_tools` and `platform` attributes under `sdk`, as they are only used with the SDK-related states. Reinstate `if_missing` for the SDK `archive.extracted` state to ensure that the new archive will be extracted if the old one is present, so that the SDK has the correct directory layout. This also means ownership will only be updated for that file (the `android` binary), so add a `file.directory` state to handle fixing the ownership for the rest of the extracted files, and fixup the requisites as necessary.
The SDK layout has changed due to the new packaging format, so simply extracting the new archive over the old one will leave extraneous files around. To fix this, if we expect to make changes in the `archive.extracted` state, then completely remove the existing version of the SDK. Also, unlink the `current` symlink before making changes to an SDK to avoid processes inadverently using the SDK while it is being changed (the symlink will be restored once the SDK is done being updated.)
Unfortunately, the `android` tool always returns a 0 exit code even in case of failure, so this does not yet catch failures. However, the new `sdkmanager` tool seems to return appropriate exit codes, so this will kick in in the future. This will prevent errors from passing silently, such as bad executable bits or incorrect file permissions.
The Android SDK installation states often have tricky changes due to upstream changes by Google, and additionally often fail silently in various different ways e.g. because the `android` tool always has a zero exit status. Additionally, some failures are not evident until Servo builds themselves start failing. Add a somke test in saltfs to catch common problems early on, and automatically run it on Travis.
This will clear the entire cache in between runs when running with SALT_FROM_SCRATCH=false. Originally, only sls and other files in the state tree were cleared, but since external modules and other files also need to be cleared, just clear everything for robustness. Additionally, move this to after we checkout the new code, since we sync everything after invalidating the cache, and we want to sync the new configuration instead of the old one.
aneeshusa
force-pushed the
aneeshusa:install-android-tools-r25.2.3-concurrently
branch
from
a99d2c4
to
c82043c
|
BTW, I would appreciate a highstate/deploy to roll out #638 and confirm that it's working before we apply this PR, to make it easier in case we need to revert something. |
|
LGTM ;) |
|
@aneeshusa Deployed and confirmed set in: |
|
@aneeshusa Wow, this is an epic PR! I think I'm ready to |
|
@larsbergstrom Looks like the |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Apr 21, 2017
…ntly, r=larsbergstrom Install android tools r25.2.3 concurrently Install the new Android tools concurrently with the old Android tools, including automatically cleaning up the old version of the tools and handling the directory layout change. (Keep the default tool version the same.) Backport the `archive.py` state module to fix zip file permissions extraction. Fix ownership setting on extracted files. Add testing for the Android SDK due to repeated issues and the `android` tool always exiting with a 0 return code even in case of error. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/639) <!-- Reviewable:end -->
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
aneeshusa commentedApr 19, 2017
•
edited
Install the new Android tools concurrently with the old Android tools, including automatically cleaning up the old version of the tools and handling the directory layout change. (Keep the default tool version the same.)
Backport the
archive.pystate module to fix zip file permissions extraction.Fix ownership setting on extracted files.
Add testing for the Android SDK due to repeated issues and the
androidtool always exiting with a 0 return code even in case of error.This change is