Shopify / tophat

Easily install and test mobile applications with a single click.
MIT License
658 stars 19 forks source link

Require Android `cmdline-tools` package during onboarding #2

Open jaredh opened 3 months ago

jaredh commented 3 months ago

Description

During onboarding Tophat validates that Android Studio is installed, which it uses to for various Android tooling. Tophat however requires a bit more than a standard Android Studio installation, it expects cmdline-tools to be installed for avdmanager, apkanalyzer, etc.

Falling back to versions of the tools which are installed with Android Studio in ~/Library/Android/sdk/tools/bin are unsupported; cmdline-tools is required.

Steps to reproduce

  1. Use an environment without cmdline-tools (ie. fresh Android Studio install)
  2. Run tophat, onboarding reports that Android tooling is fully satisfied
  3. Notice Tophat failing to discover Android AVDs, etc. avdmanager and other tools are not available in the exec path.

Expected behavior

  1. Onboarding screen does not report a satisfied Android environment
  2. Onboarding provides the ability to install cmdline-tools and satisfy Tophat requirements
  3. AVDs are then available in Tophat
jaredh commented 3 months ago

@lfroms Looking for some feedback on how to handle the UX of a missing cmdline-tools gracefully. Not sure the level of which Tophat wants to manage development environments, but I've got a couple options.

  1. Indicate Android Studio dependency as unsatisfied. Provide sdkmanager --install "cmdline-tools;latest" as the Copy Install Command (assumes functioning sdkmanager avail), or maybe just instruct to install through Android Studio. Image

  2. Logically group individual onboarding items together Image

  3. Logically grouped item, but allow single-click installation of cmdline-tools if Android Studio is installed

    Image

Beyond that, Tophat could move to only checking for command-line tools package (maybe even bundling to bootstrap) and verifying build-tools, emulator, etc. is installed. Android Studio itself isn't technically required. Thoughts?

lfroms commented 3 months ago

I think option 2 might be a good place to start because installing cmdline-tools is an explicit manual step. I feel like it would be easier to tell whether you've completed the step successfully that way.

I like the install button of option 3 but it does hide how the install is taking place and in case of error, might not be easy to tell how it failed.

Logical grouping looks great—I think we may need to introduce a scroll view around the onboarding items though as we were already pretty close to running out of room on a 14" display.

Beyond that, Tophat could move to only checking for command-line tools package (maybe even bundling to bootstrap) and verifying build-tools, emulator, etc. is installed. Android Studio itself isn't technically required. Thoughts?

Definitely. The main reason Android Studio is a requirement today is because it gets us 90% of the way through setup in one step (and also because it bundles the JDK internally). I'd love for Tophat to eventually require fewer dependencies and be a bit more self-contained.

bartekpacia commented 3 months ago

Why is Android studio required btw? Shouldn't Android Command-line Tools be enough?

lfroms commented 3 months ago

Why is Android studio required btw? Shouldn't Android Command-line Tools be enough?

@bartekpacia Requiring Android Studio gets the user through most of the setup process in one step, see my previous comment:

The main reason Android Studio is a requirement today is because it gets us 90% of the way through setup in one step (and also because it bundles the JDK internally). I'd love for Tophat to eventually require fewer dependencies and be a bit more self-contained.

outadoc commented 3 months ago

Falling back to versions of the tools which are installed with Android Studio in ~/Library/Android/sdk/tools/bin are unsupported; cmdline-tools is required.

👋 I'm trying out Tophat, and I find this behavior confusing. I have Android Studio installed, and Tophat auto-detected my Android SDK path correctly.

Tools such as avdmanager, apkanalyzer, etc, are present in my SDK installation:

~ took 3s
❯ which avdmanager
/Users/<username>/Library/Android/sdk/tools/bin/avdmanager

~
❯ which apkanalyzer
/Users/<username>/Library/Android/sdk/tools/bin/apkanalyzer

However, when I try to install an APK, I get a bunch of errors (not just for apkanalyzer, others are omitted for the sake of brevity):

An error occurred while executing command: /bin/bash: apkanalyzer: command not found -- code=127 command=apkanalyzer manifest application-id "/var/folders/vj/d6qjmkk95_13jzwtw3swbydw0000gp/T/com.shopify.tophat/downloads/BBCCF139-D083-4727-9094-CD4638A6A7E9/<apk-name>.apk"

Why does Tophat not try to find the tools in the SDK's path, when it explicitly asks for this SDK and knows about it? That feels like the more correct way to look at this issue to me.

lfroms commented 3 months ago

@outadoc, Tophat looks for avdmanager in ~/Library/Android/sdk/cmdline-tools/<version>/bin/avdmanager because it assumes that you have the cmdline-tools package installed. This is exactly the bug that this issue is reporting. That is, installing Android Studio alone is not enough.

A temporary workaround would be to manually install cmdline-tools in addition to Android Studio.

We can either support falling back to Android Studio paths, or explicitly require cmdline-tools during onboarding as potential solutions. The latter may be favourable for a future where installing Android Studio wouldn't be required.

jaredh commented 3 months ago

Tools such as avdmanager, apkanalyzer, etc, are present in my SDK installation:

IIRC those tools aren't the current versions and require JDK8, specifically JAXB, which is no longer bundled with newer versions of the JDK which Android Studio is shipped with.

I'll poke around to see if JAXB is available somewhere in the AS install.