Skip to content

/ servo

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

Make armv7-linux-androideabi default target on Android #17008

Merged
merged 1 commit into from May 31, 2017
Merged

Make armv7-linux-androideabi default target on Android #17008

merged 1 commit into from May 31, 2017

Conversation

@MortimerGoro
Copy link
Contributor

MortimerGoro commented May 23, 2017 •
edited by larsbergstrom

See #11921

After fixing problematic dependencies this is the last step to support armv7-linux-androideabi target on Android:

  • Updates skia dependency to fix a linker error. See servo/skia#136
  • Fixes specifying android targets on ./mach package and ./mach install steps:
    -./mach package --release --target=arm-linux-androideabi
    -./mach package --release --target=armv7-linux-androideabi
    -./mach package --release --target=aarch64-linux-android
  • Make armv7-linux-androideabidefault when --android param is used in build, package or install commands
  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #11921 (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

highfive commented May 23, 2017

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/package_commands.py, python/servo/command_base.py
@MortimerGoro MortimerGoro changed the title Make armv7-android-linuxeabi default target on Android Make armv7-linux-androideabi default target on Android May 23, 2017
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:android_default_armv7 branch from 87bb2e1 to b2f8dd5 May 23, 2017
@emilio
Copy link
Member

emilio commented May 23, 2017

what about CI? Does it use the --android flag?

r? @larsbergstrom
@highfive highfive assigned larsbergstrom and unassigned jdm May 23, 2017
@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented May 23, 2017

what about CI? Does it use the --android flag?

Yes, it uses the --android flag, which will translate to armv7-linux-androideabi after this PR is merged.

I also plan to add add aarch64-linux-android target compilation to the CI when all the dependencies are ready (waiting for tokio-rs/mio#607 be ready in crates.io)
@bors-servo
Copy link
Contributor

bors-servo commented May 26, 2017

☔ The latest upstream changes (presumably #17042) made this pull request unmergeable. Please resolve the merge conflicts.
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:android_default_armv7 branch 3 times, most recently from 1ed7777 to 2222777 May 26, 2017
@larsbergstrom
Copy link
Contributor

larsbergstrom commented May 31, 2017

This is r=me after rebase. Sorry for the delay!
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:android_default_armv7 branch 2 times, most recently from 13a8837 to fdbf68e May 31, 2017
@jdm
Copy link
Member

jdm commented May 31, 2017

@bors-servo: r=larsberg
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

📌 Commit fdbf68e has been approved by larsberg
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

⌛ Testing commit fdbf68e with merge b031316...
bors-servo added a commit that referenced this pull request May 31, 2017
@bors-servo
Auto merge of #17008 - MortimerGoro:android_default_armv7, r=larsberg
Loading status checks…
b031316 
Make armv7-linux-androideabi default target on Android

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

See #11921

After fixing problematic dependencies this is the last step to support armv7-linux-androideabi target on Android:

- Updates skia dependency to fix a linker error. See servo/skia#136
- Fixes specifying android targets on `./mach package` and `./mach install` steps:
  -`./mach package --release --target=arm-linux-androideabi`
  -`./mach package --release --target=armv7-linux-androideabi`
  -`./mach package --release --target=aarch64-linux-android`
- Make `armv7-linux-androideabi`default when `--android` param is used in build, package or install commands

---
<!-- 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
- [x] These changes fix #11921 (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/17008)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

💔 Test failed - android
@MortimerGoro
Make armv7-linux-androideabi default target on Android
Loading status checks…
04fb628
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:android_default_armv7 branch from fdbf68e to 04fb628 May 31, 2017
@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented May 31, 2017

Fixed the hardcoded path in one of the a ci steps.

objdump_output = subprocess.check_output([
    os.path.join(
        os.environ['ANDROID_NDK'], 'toolchains', 'arm-linux-androideabi-4.9',
        'prebuilt', 'linux-x86_64', 'bin', 'arm-linux-androideabi-objdump'),
    '-T',
    'target/arm-linux-androideabi/debug/libservo.so']
).split(b'\n')
@jdm
Copy link
Member

jdm commented May 31, 2017

@bors-servo r=larsberg
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

📌 Commit 04fb628 has been approved by larsberg
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

⌛ Testing commit 04fb628 with merge a694f70...
bors-servo added a commit that referenced this pull request May 31, 2017
@bors-servo
Auto merge of #17008 - MortimerGoro:android_default_armv7, r=larsberg
Loading status checks…
a694f70 
Make armv7-linux-androideabi default target on Android

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

See #11921

After fixing problematic dependencies this is the last step to support armv7-linux-androideabi target on Android:

- Updates skia dependency to fix a linker error. See servo/skia#136
- Fixes specifying android targets on `./mach package` and `./mach install` steps:
  -`./mach package --release --target=arm-linux-androideabi`
  -`./mach package --release --target=armv7-linux-androideabi`
  -`./mach package --release --target=aarch64-linux-android`
- Make `armv7-linux-androideabi`default when `--android` param is used in build, package or install commands

---
<!-- 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
- [x] These changes fix #11921 (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/17008)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 31, 2017

☀️ 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: larsberg
Pushing a694f70 to master...
@bors-servo bors-servo merged commit 04fb628 into servo:master May 31, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
This was referenced Jun 12, 2017
Closed
Merged
bors-servo added a commit that referenced this pull request Jun 12, 2017
@bors-servo
Auto merge of #17276 - servo:jdm-patch-3, r=mbrubeck
Loading status checks…
c1ea54d 
Use new ARM target for nightly upload.

This should fix #17275 after the changes in #17008.

<!-- 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/17276)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@larsbergstrom larsbergstrom

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Support new "armv7-android-linuxeabi" target triple
6 participants
@MortimerGoro @highfive @emilio @bors-servo @larsbergstrom @jdm
You can’t perform that action at this time.