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

Reduce duplication of the "android-18" property in the Android build #8348

Closed
larsbergstrom opened this issue Nov 5, 2015 · 2 comments
Closed

Comments

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented Nov 5, 2015

If you look at:
https://github.com/servo/servo/search?utf8=%E2%9C%93&q=android-18

You'll see that we have copies of the string "android-18" in a project properties file, a .mk file, and some Rust code. This can probably be broken out into an environment variable set in the Mach/python driver that is expanded in the appropriate ways for the .mk file and Rust code, and gets generated into the properties file by passing the correct flags to the android project update command.

Instructions for building to target Android are available here:
https://github.com/servo/servo/wiki/Building-for-Android

bors-servo added a commit that referenced this issue Nov 13, 2015
factor out references to the android SDK target into an env variable

for #8348

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8519)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Nov 17, 2015
factor out references to the android SDK target into an env variable

for #8348

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8519)
<!-- Reviewable:end -->
@jdm jdm added the C-assigned label Nov 23, 2015
@jdm
Copy link
Member

@jdm jdm commented Feb 26, 2016

#8519 was proposed to solve this issue. It requires further investigation to figure out precisely why it doesn't work on the build machines. There are comments explaining how to reproduce the issue locally.

@mitchhentges
Copy link
Contributor

@mitchhentges mitchhentges commented May 19, 2016

I think I got this, fam

bors-servo added a commit that referenced this issue May 20, 2016
Reduce 'android-18' duplication by putting $ANDROID_TARGET into env variable

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 --faster` does not report any errors
- [x] These changes fix #8348

Either:
- [X] These changes do not require tests because ~~I'm lazy~~ it's a configuration change, and asserting default values is usually tedious and not very helpful

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

---

Allegedly, this broke the build [back in the day](#8519).
Going to see if it's a piece of cake to land now

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11278)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 20, 2016
Reduce 'android-18' duplication by putting it in $ANDROID_TARGET env variable

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 --faster` does not report any errors
- [x] These changes fix #8348

Either:
- [X] These changes do not require tests because ~~I'm lazy~~ it's a configuration change, and asserting default values is usually tedious and not very helpful

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

---

Allegedly, this broke the build [back in the day](#8519).
Going to see if it's a piece of cake to land now

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11278)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue May 24, 2016
Reduce 'android-18' duplication by putting it in $ANDROID_TARGET env variable

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 --faster` does not report any errors
- [x] These changes fix #8348

Either:
- [X] These changes do not require tests because ~~I'm lazy~~ it's a configuration change, and asserting default values is usually tedious and not very helpful

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

---

Allegedly, this broke the build [back in the day](#8519).
Going to see if it's a piece of cake to land now

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11278)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.