Skip to content
This repository was archived by the owner on Feb 18, 2026. It is now read-only.

Fixes for android 24 emulator#476

Merged
fjricci merged 1 commit intofacebookarchive:masterfrom
fjricci:travis_aarch64
Apr 6, 2017
Merged

Fixes for android 24 emulator#476
fjricci merged 1 commit intofacebookarchive:masterfrom
fjricci:travis_aarch64

Conversation

@fjricci
Copy link
Contributor

@fjricci fjricci commented Apr 5, 2017

No description provided.

Copy link
Contributor

@sas sas left a comment

Choose a reason for hiding this comment

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

The change is fine if you re-add the quotes. Feel free to fix and submit without another review.


# This is required due to dependency issues in the sdk. Hopefully this can be removed in future
# versions of the sdk.
if [ "${api_level}" -gt 23 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure you should remove these quotes. If the argument is actually a numeric argument, it won't make a difference, but if the argument is not (error case), the quotes will prevent [ from failing with stupid errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this didn't work with the quotes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure language-wise it'll make no difference unless ${api_level} contains something with spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The language spec says you're right. I'll double check that it works tomorrow.

case "${TARGET}" in
"Android-ARM") emulator_arch="arm";;
"Android-ARM64") emulator_arch="aarch64";;
"Android-ARM64") emulator_arch="ranchu-arm64";;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little bit pathetic :D

Copy link
Contributor

@bulbazord bulbazord Apr 6, 2017

Choose a reason for hiding this comment

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

Ranchu is the machine type used when invoking qemu-android-aarch64 or whatever it's called. I wish it was just emulator64-arm64 as -machine type=ranchu is already the default. I guess they wanted to make that extra clear. Including it in the name of the emulator binary feels strange. :/

Copy link
Contributor

@bulbazord bulbazord left a comment

Choose a reason for hiding this comment

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

I don't know if I would say this "fixes" the android 24 emulator, but these changes are without a doubt necessary for when the emulator isn't totally broken.

Looks good to me! :D

case "${TARGET}" in
"Android-ARM") emulator_arch="arm";;
"Android-ARM64") emulator_arch="aarch64";;
"Android-ARM64") emulator_arch="ranchu-arm64";;
Copy link
Contributor

@bulbazord bulbazord Apr 6, 2017

Choose a reason for hiding this comment

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

Ranchu is the machine type used when invoking qemu-android-aarch64 or whatever it's called. I wish it was just emulator64-arm64 as -machine type=ranchu is already the default. I guess they wanted to make that extra clear. Including it in the name of the emulator binary feels strange. :/

@fjricci fjricci merged commit 54b3518 into facebookarchive:master Apr 6, 2017
@fjricci fjricci deleted the travis_aarch64 branch April 6, 2017 14:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants