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

Android builds are failing on servo-linux-cross3 #661

Closed
aneeshusa opened this issue May 8, 2017 · 8 comments
Closed

Android builds are failing on servo-linux-cross3 #661

aneeshusa opened this issue May 8, 2017 · 8 comments

Comments

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented May 8, 2017

We've tried re-highstating servo-linux-cross3; jury is still out on the last highstate, but previous attempts have not fixed the problem.

I'm not well versed with Android, but I have a new idea about what might be going wrong:

  • The relevant builds (e.g. http://build.servo.org/builders/android/builds/6778/steps/compile/logs/stdio) are currently failing in the /mach build --android --dev step
  • In buildbot_steps.yml, we only override the ANDROID_SDK variable for the ./mach package step, not the ./mach build step, so the current SDK symlink takes precedence
  • #644 has not yet been deployed to -cross1 and -cross2, only -cross3:
    • Both -cross1 and -cross2 still have the older r24.4.1 SDKs on disk, while all three have the new r25.2.3 SDK on disk.
    • -cross1 and -cross2 have the old SDK as current, while -cross3 has the new SDK as current.

Is it feasible that ./mach build --android --dev is failing due to getting the new r25.2.3 SDK on -cross3, while working OK with the r24.4.1 SDK on -cross1 and -cross2?

cc @larsbergstrom @MortimerGoro

aneeshusa added a commit to aneeshusa/servo that referenced this issue May 8, 2017
bors-servo added a commit to servo/servo that referenced this issue May 8, 2017
…s, r=<try>

[DO NOT MERGE] Test hypothesis for failing Android builds on cross3

See servo/saltfs#661 (comment).

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
bors-servo added a commit to servo/servo that referenced this issue May 8, 2017
…s, r=<try>

[DO NOT MERGE] Test hypothesis for failing Android builds on cross3

See servo/saltfs#661 (comment).

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16772)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this issue May 8, 2017
…s, r=<try>

[DO NOT MERGE] Test hypothesis for failing Android builds on cross3

See servo/saltfs#661 (comment).

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16772)
<!-- Reviewable:end -->
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 8, 2017

OK, reproduced the failure on servo-linux-cross2 with the above test PR (see http://build.servo.org/builders/android/builds/6786), SDK 25.2.3 breaks ./mach build --android --dev.

@MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented May 8, 2017

This line in blurdroid dependency is the one causing the problem:

https://github.com/szeged/blurdroid/blob/master/build.rs#L64

The problem is that when SDK 25.2.3 is enabled, only android-25 platform is installed. When r24.4.1 is enabled only android-18 platform is installled. Blurdroid dependency has android-18 path hardcoded...

Possible solutions:

  • Add an array of platforms to saltfs instead of using a unique android-platform
  • Fix hardcoded path in blurdroid dependency
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 8, 2017

Making platforms into an array in saltfs is a fairly easy change. However, IMO it makes more sense to update blurdroid, then send a servo/servo PR with that change (and also fixing buildbot_steps.yml to pass the ANDROID_SDK var for ./mach build) for a few reasons:

  • Easier to test servo/servo changes since there's no need to revert as with saltfs changes
  • It doesn't seem like anyone else depends on blurdroid: https://crates.io/crates/blurdroid/reverse_dependencies
  • This avoid having multiple platforms, which seems like it could easily cause problems later

Also, I just noticed we should update the servobuild.example file as well at some point.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 8, 2017

It might also make sense to use an environment variable for the platform version in blurdroid, and set that from buildbot_steps.yml / saltfs.

@MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented May 8, 2017

Yes, good reasons to update blurdroid.

I'll do the PR to blurdroid, it's a easy change too.

bors-servo added a commit to servo/servo that referenced this issue May 11, 2017
Update blurdroid and set ANDROID_SDK in build_steps.yml

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc @aneeshusa @larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16812)
<!-- Reviewable:end -->
@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 11, 2017

Thanks for the PRs @MortimerGoro! I just grepped servo/servo and I still see android-18 in a few places and saw it used in the build log for servo/servo#16812, so I think we need to update this in more places if you want to take a look. I'm going to run a force build on cross3 to check in the meantime.

@MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented May 11, 2017

@aneeshusa the android-18 in the log are related to Android NDK (C++) which is a different SDK (it has all the platforms included). I think that there aren't more dependencies in Servo that use Android SDK (java). So the cross3 build should work

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 11, 2017

I didn't know that about SDK vs. NDK, thanks for the explanation. Indeed, the force build succeeded: http://build.servo.org/builders/android/builds/6858

I'm going to roll out #644 to cross2 and cross1 now, since this appears to be working. Closing this issue.

@aneeshusa aneeshusa closed this May 11, 2017
moz-v2v-gh pushed a commit to mozilla/gecko-projects that referenced this issue May 12, 2017
…teps.yml (from MortimerGoro:update_blurdroid); r=jdm

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc @aneeshusa @larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 060a651f8847189e8d0830e35bbf4497014f01d5

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : c70047d684d85626b0fe84ce8df83d7336df02a5
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this issue May 15, 2017
…teps.yml (from MortimerGoro:update_blurdroid); r=jdm

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc @aneeshusa @larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 060a651f8847189e8d0830e35bbf4497014f01d5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…teps.yml (from MortimerGoro:update_blurdroid); r=jdm

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc aneeshusa larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 060a651f8847189e8d0830e35bbf4497014f01d5

UltraBlame original commit: 2f5a221eedf65d950fb932f5eb6949635a76adb5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…teps.yml (from MortimerGoro:update_blurdroid); r=jdm

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc aneeshusa larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 060a651f8847189e8d0830e35bbf4497014f01d5

UltraBlame original commit: 2f5a221eedf65d950fb932f5eb6949635a76adb5
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…teps.yml (from MortimerGoro:update_blurdroid); r=jdm

<!-- Please describe your changes on the following line: -->

See servo/saltfs#661. Update blurdroid and set ANDROID_SDK in build_steps.yml

cc aneeshusa larsbergstrom

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 060a651f8847189e8d0830e35bbf4497014f01d5

UltraBlame original commit: 2f5a221eedf65d950fb932f5eb6949635a76adb5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.