Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NDK installation should run whenever it's not installed #259

Closed
wants to merge 1 commit into from

Conversation

edunham
Copy link
Contributor

@edunham edunham commented Mar 21, 2016

According to
https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cmd.html#should-i-use-cmd-run-or-cmd-wait,
ndk installation will only try to run if the ndk was just downloaded.

In the old implementation, if you succeed at downloading the ndk but fail to
install it, subsequent highstates won't attempt to install the ndk because the
downloaded file will be unchanged.

With these changes, we decide whether to install the ndk based on whether the directory created by installation exists, rather than based on whether we downloaded the file this run.


This change is Reviewable

According to
https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cmd.html#should-i-use-cmd-run-or-cmd-wait,
ndk installation will only try to run if the ndk was just downloaded.

In the old implementation, if you succeed at downloading the ndk but fail to
install it, subsequent highstates won't attempt to install the ndk because the
downloaded file will be unchanged.
@edunham
Copy link
Contributor Author

edunham commented Mar 21, 2016

r? @larsbergstrom

@larsbergstrom
Copy link
Contributor

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit e2990dd has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

⌛ Testing commit e2990dd with merge 0171303...

bors-servo pushed a commit that referenced this pull request Mar 21, 2016
NDK installation should run whenever it's not installed

According to
https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cmd.html#should-i-use-cmd-run-or-cmd-wait,
ndk installation will only try to run if the ndk was just downloaded.

In the old implementation, if you succeed at downloading the ndk but fail to
install it, subsequent highstates won't attempt to install the ndk because the
downloaded file will be unchanged.

With these changes, we decide whether to install the ndk based on whether the directory created by installation exists, rather than based on whether we downloaded the file this run.

<!-- 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/259)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - travis

@larsbergstrom
Copy link
Contributor

@bors-servo retry

�[0;31m----------�[0m
    �[0;31m      ID: android-dependencies�[0m
    �[0;31mFunction: pkg.installed�[0m
    �[0;31m  Result: False�[0m
    �[0;31m Comment: An error was encountered while installing package(s): W: Failed to fetch http://us.archive.ubuntu.com/ubuntu/dists/trusty-updates/main/binary-i386/Packages  Hash Sum mismatch

              W: Failed to fetch http://us.archive.ubuntu.com/ubuntu/dists/trusty-updates/universe/binary-i386/Packages  Hash Sum mismatch

              E: Some index files failed to download. They have been ignored, or old ones used instead.�[0m
    �[0;31m Started: 19:47:49.282790�[0m
    �[0;31mDuration: 6150.757 ms�[0m

@bors-servo
Copy link
Contributor

⌛ Testing commit e2990dd with merge 550bd97...

bors-servo pushed a commit that referenced this pull request Mar 21, 2016
NDK installation should run whenever it's not installed

According to
https://docs.saltstack.com/en/latest/ref/states/all/salt.states.cmd.html#should-i-use-cmd-run-or-cmd-wait,
ndk installation will only try to run if the ndk was just downloaded.

In the old implementation, if you succeed at downloading the ndk but fail to
install it, subsequent highstates won't attempt to install the ndk because the
downloaded file will be unchanged.

With these changes, we decide whether to install the ndk based on whether the directory created by installation exists, rather than based on whether we downloaded the file this run.

<!-- 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/259)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - travis

@bors-servo
Copy link
Contributor

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@aneeshusa
Copy link
Contributor

I'm almost ready to put in the PR I mentioned in #239 which touches the Android stuff heavily, and it's going to have merge conflicts with this/includes some of this work already. Can we hold off on this PR please?

Also, this isn't quite right - we still need to keep a requisite to android-ndk for ordering purposes being the biggest thing.

@aneeshusa aneeshusa mentioned this pull request Mar 22, 2016
bors-servo pushed a commit that referenced this pull request Mar 22, 2016
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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants