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

Replace the use of the ANDROID_SDK_ROOT env variable with ANDROID_HOME #84316

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented Nov 1, 2023

Replace the use of the ANDROID_SDK_ROOT env variable with ANDROID_HOME as the former is deprecated.

See https://developer.android.com/tools/variables#android_home for more details.

Fixes #80910

def get_env_android_sdk_root():
return os.environ.get("ANDROID_SDK_ROOT", -1)
return os.environ.get("ANDROID_HOME", os.environ.get("ANDROID_SDK_ROOT"))
Copy link
Member

Choose a reason for hiding this comment

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

Should the fallback also have a fallback? Not sure what -1 achieved, should maybe be an empty string? Otherwise it may be None and lead to 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 thought None was preferred, so we can do things like if get_env_android_sdk_root():.

I'm no python expert so happy to defer to the recommended practice.

Copy link
Member

Choose a reason for hiding this comment

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

Well currently the only use of get_env_android_sdk_root() is here:

("ANDROID_HOME", "Path to the Android SDK", get_env_android_sdk_root()),

Which means env["ANDROID_HOME"] would be None, and thus code like env["ANDROID_HOME"] + "/ndk/" + get_ndk_version() will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the empty string as a fallback. We have various os.path.exists(...) checks so they should quickly detect if env["ANDROID_HOME"] has an invalid value / path.

@m4gr3d m4gr3d force-pushed the update_android_env_variables branch from 3b26a71 to 2b71d73 Compare November 1, 2023 22:36
@m4gr3d m4gr3d force-pushed the update_android_env_variables branch from 2b71d73 to a1ca4ba Compare November 1, 2023 22:50
@akien-mga akien-mga merged commit da0b1eb into godotengine:master Nov 2, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@m4gr3d m4gr3d deleted the update_android_env_variables branch November 2, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ANDROID_SDK_ROOT is deprecated, use ANDROID_HOME instead
2 participants