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 upAndroid refactor #263
Android refactor #263
Conversation
|
Also, the reason this is WIP is that we should version the URL that we download B2G from so I can give it a similar treatment as the Android states, preferably something like or |
|
(Also, if we're updating the B2G URL: I always find capital letters in URLs to look weird, so something like |
|
I would not invest too much effort into the B2G rules - I suspect that once we have Aarch64 working (requires a servo fix before we can gate on it) there will be limited need to keep it running. Between Android & Arm32/Aarch64, we'll have most of the coverage we want for those platforms. I don't know when we would remove it, but I can promise there won't be a v2 of the binaries :-) |
|
Do we need to install the |
|
The only thing that I see in that list that we need is |
|
What would we need |
|
@aneeshusa Maybe we're doing it wrong, but |
|
Salt actually has built-in support for using git as a fileserver backend: https://docs.saltstack.com/en/2015.5/topics/tutorials/gitfs.html. Our current workflow is OK for now, so I'll add a state to install |
|
Hmm, it looks like we're hitting the Travis log length limit due to how many files we extract: each extracted file shows up twice, once in the INFO logging and once at the end. How do you feel about dropping down the log level to warning? We'll miss out on all the lines starting with (We're really close, too! Travis kills us in the middle of outputting the highstate summary at the very end.) |
|
I think that dropping it down to warning is fine. I get the most out of seeing which task failed and what the error message was, personally. |
|
Reviewed 9 of 9 files at r1, 5 of 5 files at r2, 3 of 3 files at r3, 4 of 4 files at r4, 2 of 2 files at r5, 1 of 1 files at r6, 2 of 2 files at r7, 1 of 1 files at r8. Comments from the review on Reviewable.io |
|
|
This change groups the various sls files that set up the Servo build environment into a single folder to: - make their purpose more clear in the top.sls file - make it easier to add auxiliary files (i.e. map.jinja) Also, don't install the Servo build dependencies on the Buildbot master, but make sure to keep git installed to update the Salt file tree.
Use a SHA512 hash instead of a SHA1 hash to verify the B2G download. Update the style guide.
Use separate directories for separate versions of the SDK, NDK, and toolchain, and use symlinks to point to the current versions. This is more robust for a few reasons: - Partially-completed upgrades to new versions won't touch existing versions on the disk - The symlinks aren't updated until the respective new version is completely installed, allowing for more transactional updates. - The symlinks also allow for constant paths in the bash_profile file and the buildbot config, which means less moving parts to break. In particular, the relevant buildbot config is on the master, and using symlinks makes it unnecessary to use Salt orchestration to gate changes to the master buildbot config on changes on the cross builders. Ideally, these states would also use file.directory with clean: True to clean out old versions of the SDK, NDK, and toolchain, but I wasn't able to get this working properly yet (the just-downloaded files would keep getting cleaned away). This also eliminates our use of cmd.wait, which was recently put on the deprecation path in Salt, and replaces it with cmd.run + creates: True.
The buildbot configuration already sets the correct environment variables for Android builds, so it's unnecessary to set them via .bash_profile.
archive.extracted is fairly loud because each extracted file is listed both in INFO log level output and the highstate summary. This gets around exceeding Travis's 4MB log length limit.
|
@edunham I needed to rebase to handle the fixup and squash commits anyways, so I rebased on top of the latest master. |
|
Reviewable did not handle that rebase very gracefully. Still good, though. Reviewed 3 of 9 files at r1, 11 of 11 files at r9, 5 of 5 files at r10, 3 of 3 files at r11, 4 of 4 files at r12, 2 of 2 files at r13, 1 of 1 files at r14. Comments from the review on Reviewable.io |
|
@bors-servo r+ |
|
|
Android refactor Main highlights: - Update to SHA512 hashes and HTTPS URLs - Use multiple directories + symlinks to be more robust during version updates for Android - Moves states around to make their purpose more clear Helps with #209. Supersedes #259, #260. cc @larsbergstrom @edunham I recommend reviewing this commit by commit and reading the commit messages - feel free to ask questions. Also, we should check that this doesn't break buildbot; I don't know how to do that. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/saltfs/263) <!-- Reviewable:end -->
|
|
aneeshusa commentedMar 22, 2016
Main highlights:
Helps with #209.
Supersedes #259, #260.
cc @larsbergstrom @edunham
I recommend reviewing this commit by commit and reading the commit messages - feel free to ask questions.
Also, we should check that this doesn't break buildbot; I don't know how to do that.
This change is