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

Support for Android arm64 compilations #17099

Merged
merged 1 commit into from Jul 8, 2017

Conversation

MortimerGoro
Copy link
Contributor

@MortimerGoro MortimerGoro commented May 30, 2017

This PR is the final step to adds support for Android arm64 compilations. See #11921 for previous work.

Fixes in this PR:


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • [] These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label May 30, 2017
@MortimerGoro
Copy link
Contributor Author

cc @larsbergstrom

with cd(openssl_dir):
status = call(
make_cmd + ["-f", "openssl.makefile"],
env=env,
verbose=verbose)
if status:
return status
openssl_dir = path.join(openssl_dir, "openssl-1.0.1t")
openssl_dir = path.join(openssl_dir, "openssl-1.1.0f")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't suddenly update openssl in the middle of nowhere of a patch that is intended to support arm64, that change is out of scope.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it makes sense to move the OpenSSL update into a separate PR. The reason for the update is that OpenSSL 1.1.0 comes with a compilation target ready for arm64, while 1.0.1 required a lot of hacking (and it's a out of support version)

@wafflespeanut
Copy link
Contributor

r? @larsbergstrom

@MortimerGoro
Copy link
Contributor Author

We have decided to do the OpenSSL update into a separate PR before merging this. Let's wait for #17079 with the mirror and the new version.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #17240) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Jun 12, 2017
@larsbergstrom
Copy link
Contributor

Looks like #17079 landed, so this should be good to go after rebase!

@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented Jul 5, 2017

@larsbergstrom Rebase ready! #17079 used OpenSSL 1.0.2 instead of 1.1. But I found a way to compile for aarch64 in 1.0.x.

I want to add a ci build step for android64. Should I add the ./mach build and ./mach package commands to the android section in https://github.com/servo/servo/blob/master/etc/ci/buildbot_steps.yml or should I create a whole new section for android64?

@larsbergstrom
Copy link
Contributor

@bors-servo r+

@MortimerGoro Please just add them to the existing builders. Adding new build sections requires a significant amount of hacking & deployment on the saltfs side and there is some time to spare in our cross-builder configurations right now.

@bors-servo
Copy link
Contributor

📌 Commit 48fb715 has been approved by larsbergstrom

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Jul 8, 2017
@bors-servo
Copy link
Contributor

⌛ Testing commit 48fb715 with merge 85e5551...

bors-servo pushed a commit that referenced this pull request Jul 8, 2017
Support for Android arm64 compilations

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

This PR is the final step to adds support for Android arm64 compilations. See #11921 for previous work.

Fixes in this PR:
- Fix js dependency compilation: servo/rust-mozjs#360
- Fix skia dependency link error: servo/skia#136
- Fix blurdroid dependency compilation: szeged/blurdroid#4
- Fix mio and net2 dependency compilations: tokio-rs/mio#599
- Fix gcc compiler name in the fake linker
- Compile OpenSSL for aarch64 (update to stable 1.1.0 was required for this)

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

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: larsbergstrom
Pushing 85e5551 to master...

@bors-servo bors-servo merged commit 48fb715 into servo:master Jul 8, 2017
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 8, 2017
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.

None yet

6 participants