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

Add support for ANDROID_HOME environment variable #463

Closed
ProgrammAbel opened this issue Aug 1, 2020 · 19 comments · Fixed by #1307
Closed

Add support for ANDROID_HOME environment variable #463

ProgrammAbel opened this issue Aug 1, 2020 · 19 comments · Fixed by #1307
Labels
android The issue relates to Android mobile support. bug A crash or error in behavior. good first issue Is this your first time contributing? This could be a good place to start!

Comments

@ProgrammAbel
Copy link

Describe the bug
After creating a project, building for Android and accepting the license, I had to delete it (because of #462). Then, when creating a new, clean project and building it for Android again, it did not prompt me to accept a license but instead gave me an error that the licenses weren't accepted.

To Reproduce
Steps to reproduce the behavior:

  1. Create a project
  2. Build for Android and accept the license
  3. Delete the folder of this project
  4. Create another project
  5. Build for Android
  6. See error

Expected behavior
For the application to build successfully.

Screenshots
Screenshots2020 08 01-19 05 43 screenshot

Environment:

  • Operating System: Linux 5.7.10-arch1-1
  • Python version: Python 3.8.4
  • Software versions:
    • Briefcase: 0.3.3
    • Toga: 0.3.0
@ProgrammAbel ProgrammAbel added the bug A crash or error in behavior. label Aug 1, 2020
@praveen-manohar
Copy link

Hi @ProgrammAbel i got the same error once, you have to accept the license on android sdkmanager(not on project run), once you done that, you're error will be clear now.

@freakboy3742
Copy link
Member

Thanks for the report. The licenses are accepted on a per-computer basis, so the interaction with multiple projects is coincidental.

I assume one of two things happened:

  • The licenses weren't fully accepted on the first execution.
  • The second execution introduced new licenses that needed to be accepted.

The license check at present is very simplistic - it looks for the existence of ~.briefcase/tools/android_sdk/licenses/android-sdk-license. If that file exists, it assumes the licenses have been accepted. However, there are a lot of other license files in that folder (7 on my install); so it's possible we might need to improve that check.

If you're still seeing this error, I'd be interested in knowing what license files (if any) currently exist in that folder (and, if there is an android-sdk-license file, what it contains). Hopefully, we only need to expand the license check to look for another file (or files). Worse case, we need to modify the license check to actually invoke licenses every time (or add a command flag to explicitly perform a license check).

@ProgrammAbel
Copy link
Author

ProgrammAbel commented Aug 2, 2020

Thank you for taking the time to help me 😀 So, I've discovered a few things. I figured out that licenses are stored in ~/.briefcase/tools/android_sdk/licenses, and when I looked, they were all there. For some reason, briefcase build looked in /opt/android-sdk/licenses, and since that folder didn't exist, it thought I hadn't accepted the licenses. I've temporarily solved this by deleting /opt/android-sdk.

I believe this error stems from the fact that I've done other Android development on this computer in the past, and the SDK folder was /opt/android-sdk. So I think Gradle prioritised this folder above ~/.briefcase, causing this error. Is there anything you can do about this or is this all in the hands of Gradle?

@freakboy3742
Copy link
Member

If you've got ANDROID_SDK_ROOT defined in your environment, it will use that version in preference to anything Briefcase managed. That shouldn't impact the commands that are executed; just the location of the SDK manager and ADB instance that it tries to use.

I can't think of any way that would conflict with license management, but I guess anything is possible...

@ProgrammAbel
Copy link
Author

ANDROID_SDK_ROOT isn't defined, though... Either way, I set it to nothing, but the same problem still occurs. It's really weird, if I delete /opt/android-sdk it looks in ~/.briefcase/tools/android_sdk/licenses instead and builds successfully. If /opt/android-sdk exists, even if it's empty, it fails to build. If I copy ~/.briefcase/tools/android_sdk/licenses to /opt/android-sdk/licenses, provided I have write permissions, it builds. 🤔

@freakboy3742
Copy link
Member

I don't know how to put this politely... but... are you absolutely certain that your environment doesn't contain ANDROID_SDK_ROOT? Because Briefcase starts sdkmanager by providing a full literal path to the executable, and the only way I can see that it would find /opt/android-sdk is if there is an ANDROID_SDK_ROOT directing it there.

If you insist that there isn't an ANDROID_SDK_ROOT in your environment, you'll need to set a breakpoint in the verification code (somewhere around here) and walk through the process of verifying the Android toolchain to work out why it's finding /opt/android-sdk.

@ProgrammAbel
Copy link
Author

ProgrammAbel commented Aug 3, 2020

No problem 😁 Here's video proof that my environment doesn't contain ANDROID_SDK_ROOT:

Screen Capture_kitty_20200803132523

How exactly would I set a breakpoint? Running pdb android_sdk.py in the integrations directory doesn't recognise my venv. Sorry for being a noob, I've only ever done debugging in C++ with gdb 😅

@freakboy3742
Copy link
Member

Thats... definitely interesting. Definitely looks like there's no stray Android variable.

(A quick side note - while the animation is definitely pretty, a static dump of terminal content is a lot more practical from a diagnostic perspective).

I guess debugging is our only choice at this point. To set a breakpoint, you edit the android_sdk.py file and add breakpoint() as a function call at the point you want the breakpoint (there are other ways to set a breakpoint, but this is the easiest). Then, run briefcase as you normally would. The code will stop at the breakpoint, and drop you into pdb.

@ProgrammAbel
Copy link
Author

Ah, right, will do next time. Also I'll take a look and update this message if I've found anything. Out of curiosity, are you able to replicate this problem? Because I've tried to reproduce it on my MacBook but it seems to build fine.

@freakboy3742
Copy link
Member

No, I haven't been able to reproduce at all - and the behavior you have described (and demonstrated) doesn't make any sense to me. Briefcase shouldn't be using a version in /opt unless you have explicitly told it to - but based on what you've shown, you're not doing that. So - there is clearly something going on that is specific to your environment - the question is what.

@paulproteus
Copy link
Contributor

paulproteus commented Aug 5, 2020

Hey @ProgrammAbel -- Android also respects ANDROID_HOME. Any change you have that set?

One quick (?) way to show if you do/don't:

$ export | grep -i android_home
$

@freakboy3742
Copy link
Member

@paulproteus waitwat? It does? Well that is something that we need to fix, then...

@freakboy3742
Copy link
Member

Confirmed using https://developer.android.com/studio/command-line/variables . That'll do it, I guess. I thought ANDROID_HOME only impacted on Android Studio operation.

Adding support for ANDROID_HOME should be relatively straightforward - it's essentially reproducing lines 121-150 of android_sdk.py, but testing for ANDROID_HOME, and adjusting the final SDK path to whatever subfolder under ANDROID_HOME is used by Android Studio.

@ProgrammAbel
Copy link
Author

ProgrammAbel commented Aug 5, 2020

Yep, that's definitely the culprit. It's set to /opt/android-sdk. This seems like a simple thing to implement, right? If so, could I be assigned to fix this? I would love to make a contribution!

@freakboy3742
Copy link
Member

Yes - this should be very approachable as a first contribution! I actually gave the details of the implementation required in my previous message; combine that with some tests, and you should be good to go!

@freakboy3742 freakboy3742 added up-for-grabs good first issue Is this your first time contributing? This could be a good place to start! bug A crash or error in behavior. android The issue relates to Android mobile support. and removed bug A crash or error in behavior. labels Mar 25, 2022
@mhsmith
Copy link
Member

mhsmith commented Aug 17, 2022

Going by the Wayback Machine history of this page, Google has gone back and forth on which variable they recommend. They currently recommend ANDROID_HOME, and say:

ANDROID_SDK_ROOT, which also points to the SDK installation directory, is deprecated. If you continue to use it, Android Studio and the Android Gradle plugin will check that the old and new variables are consistent.

So Briefcase should do the following:

  • Accept either variable.
  • If they're both set with different values, fail with an error.
  • When calling subprocesses, set ANDROID_HOME rather than ANDROID_SDK_ROOT.

Also, ANDROID_HOME and JAVA_HOME should be added to the documentation, because they're not currently mentioned.

@freakboy3742: Can you change the issue title to mention ANDROID_HOME, so it's easier to find?

@freakboy3742 freakboy3742 changed the title License doesn't accept for multiple projects in Android Add support for ANDROID_HOME as a local configuration variable. Aug 17, 2022
@mhsmith mhsmith changed the title Add support for ANDROID_HOME as a local configuration variable. Add support for ANDROID_HOME environment variable Feb 3, 2023
@mhsmith
Copy link
Member

mhsmith commented Jun 8, 2023

This has become more urgent now that we've upgraded to a newer Android Gradle plugin version, because as mentioned above, the new version "will check that the old and new variables are consistent". This means that people like me, who follow Google's advice and set an ANDROID_HOME variable in their shell, will now get the following error when running briefcase build android:

[webview] Building Android APK...
Starting a Gradle Daemon, 1 incompatible Daemon could not be reused, use --status for details

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':app:compileDebugJavaWithJavac'.
> Several environment variables and/or system properties contain different paths to the SDK.
  Please correct and use only one way to inject the SDK location.
  
  ANDROID_HOME: /Users/msmith/Library/Android/sdk
  ANDROID_SDK_ROOT: /Users/msmith/Library/Caches/org.beeware.briefcase/tools/android_sdk
  
  It is recommended to use ANDROID_HOME as other methods are deprecated

@rmartin16
Copy link
Member

Oh yeah...that's a good point; I ran in to the same thing trying to disable usage of GitHub's Android SDK in CI.

Do you think we should just switch to only using ANDROID_HOME? Or perhaps use ANDROID_HOME if set and fall back to ANDROID_SDK_ROOT if not? Or since Gradle will complain about this now, perhaps it's best to detect this situation in Briefcase before Gradle bombs out on it.

@mhsmith
Copy link
Member

mhsmith commented Jun 8, 2023

For clarity, in my previous comment, ANDROID_HOME was only the variable I had in my shell environment. Briefcase currently ignores it, and adds ANDROID_SDK_ROOT pointing at its own SDK before running Gradle.

Briefcase uses the SDK for other things than running Gradle (e.g. the emulator), so I think we should have our own consistency check. See this comment for suggested behavior.

mhsmith added a commit that referenced this issue Jun 12, 2023
Add support to specify an Android SDK in `ANDROID_HOME` (fixes #463)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android The issue relates to Android mobile support. bug A crash or error in behavior. good first issue Is this your first time contributing? This could be a good place to start!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants