Updated CLI scripts to support Android SDK Tools 25.3.1

[filmaj]

Platforms affected

Android

What does this PR do?

Adds support for using the Android SDK Tools version 25.3.1.

Relevant issues are CB-12546 and CB-12554.

In general, the CLI scripts will now attempt to use the old tools first (specifically the android command), and if that command fails with a specific exit code and message (tested on Windows 10 and Mac OS 10.12), it will then try to use the new tools (specifically the avdmanager and sdkmanager commands).

I have also renamed a bunch of methods in an attempt to try to make it clearer what they are doing.

I have added a bunch of jasmine unit tests to all of this as well. Hoping the coverage report reflects that!

What testing has been done on this change?

I tested on two OSes: Mac OS 10.12 and Windows 10. For each OS, I tested four environment setups:

1. Setting ANDROID_HOME environment variable to point to a new installation of the Android SDK.
2. Setting PATH environment variable to contain the tools, tools/bin and platform-tools Android SDK subdirectories.
3. Having neither of the above environment variables set.
4. Having both of the above environment variables set.

For each environment, I then ran the following test scenarios:

1. Running npm test from the repo. In all eight environments, this passed.
2. Running bin/check_reqs from the repo. If neither environment variables are set, you would receive an error telling you to set ANDROID_HOME. In the other 6 cases, check_reqs tells you you are all good.
3. Running bin/android_sdk_version from the repo. In the four environments I tested where you didn't have the android command on your PATH, you would get a "android cannot be found" error. In the other four, it would return the id of the latest android target you have installed on your system.
4. Running bin/create from the repo. This passed in all eight environments.
5. I checked running the following commands in a generated cordova-android project:

• cordova/android_sdk_version: same behaviour as test scenario 3 above (as expected).
• cordova/check_reqs: same behaviour as test scenario 2 above (as expected).
• cordova/build: it would error out if you didn't have your ANDROID_HOME set and tell you to set it. Otherwise, passed fine.
• cordova/clean: same as build above ✅
• cordova/run --emulator: same as build above ✅

NOTE: This has not been tested on an Android SDK installation with SDK Tools older than 25.3.1! I would love for someone with SDK Tools 25.2.x to give this a go.

Other Notes

These are extensive changes! I would love a bunch of people to review this, so pinging @infil00p, @purplecabbage, @shazron, @stevengill, @timkim, @macdonst, @dpogue. Review party!

Checklist

• Reported an issue: CB-12546, CB-12554
• Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths",
• Added automated test coverage as appropriate for this change.

[codecov-io]

Codecov Report

Merging #369 into master will increase coverage by 7.84%.
The diff coverage is 71%.

@@            Coverage Diff             @@
##           master     #369      +/-   ##
==========================================
+ Coverage   35.58%   43.42%   +7.84%     
==========================================
  Files          12       14       +2     
  Lines        1037     1347     +310     
  Branches      173      247      +74     
==========================================
+ Hits          369      585     +216     
- Misses        668      762      +94

Impacted Files	Coverage Δ
bin/templates/cordova/lib/check_reqs.js	48.25% <55.42%> (ø)
bin/templates/cordova/lib/android_sdk.js	81.81% <81.81%> (ø)
bin/templates/cordova/lib/emulator.js	38.51% <82.25%> (+24.62%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2af492...2f2e8a5. Read the comment docs.

[dpogue]

Linux with Android SDK 25.2.2

✅ npm test
✅ ./bin/check_reqs
✅ ./bin/android_sdk_version
✅ ./bin/create
✅ ./cordova/check_reqs
✅ ./cordova/android_sdk_version
✅ ./cordova/build
✅ ./cordova/clean
(No emulator images installed, so I didn't try that step)

Linux with Android SDK 25.3.1

✅ npm test
✅ ./bin/check_reqs
❌ ./bin/android_sdk_version

{ Error: sdkmanager: Command failed with exit code ENOENT
    at ChildProcess.whenDone (/mnt/users/dpogue/Coding/cordova-android/node_modules/cordova-common/src/superspawn.js:169:23)
    at emitOne (events.js:96:13)
    at ChildProcess.emit (events.js:191:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:213:12)
    at onErrorNT (internal/child_process.js:367:16)
    at _combinedTickCallback (internal/process/next_tick.js:80:11)
    at process._tickCallback (internal/process/next_tick.js:104:9) code: 'ENOENT' }

✅ ./bin/create
✅ ./cordova/check_reqs
❌ ./cordova/android_sdk_version

{ Error: sdkmanager: Command failed with exit code ENOENT
    at ChildProcess.whenDone (/mnt/users/dpogue/Coding/cordova-android/node_modules/cordova-common/src/superspawn.js:169:23)
    at emitOne (events.js:96:13)
    at ChildProcess.emit (events.js:191:7)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:213:12)
    at onErrorNT (internal/child_process.js:367:16)
    at _combinedTickCallback (internal/process/next_tick.js:80:11)
    at process._tickCallback (internal/process/next_tick.js:104:9) code: 'ENOENT' }

✅ ./cordova/build
✅ ./cordova/clean

It looks like the two failures are due to not having the sdkmanager tool on my path, but it should be able to figure it out based on ANDROID_HOME?

For the record, my PATH contains ~/.android/SDK/tools and ~/.android/SDK/platform-tools

[filmaj]

@dpogue thanks for checking! So I think android_sdk_version failing in this case is kind of expected, as that script, historically, did not do any environment checking, and I tried keeping the same behaviour.

The other commands that are part of Api.js, like build and clean and run, always call through to check_reqs. Part of that functionality in this pull request now automatically adds the new SDK tools to your PATH if they are missing. That's why the build command worked for you, but the android_sdk_version command did not.

We could tweak the behaviour of android_sdk_version so that it calls through to check_reqs as well? This change would fix that command failing in your case, and provide more meaningful errors for people who don't have their environments configured properly (i.e. will check for missing ANDROID_HOME, will munge PATH based on what kinds of Android tooling it will find, etc.). What do you think?

The deeper question that rises out of this is: is android_sdk_version useful? Is it worth keeping around? It is not part of the platform API and is still hanging around due to history more than anything else.

Let me know what y'all think.

[filmaj]

Oh yes, and ping @alsorokin - not sure if the changes to the android CLI scripts affect the CI in any way? But in any case, probably worth getting your eyes on this change too :)

[alsorokin]

I'll create a test job that uses this code when our slaves are back up (some Microsoft infra permutations are underway ATM).

[shazron]

I have the same results as @dpogue. Tested macOS Sierra with Android SDK 25.3.1 and Windows 10 with Android SDK 25.3.1.

For android_sdk_version on Windows I got this error:

> bin\android_sdk_version
{ [Error: cmd: Command failed with exit code 1 Error output:
'android' is not recognized as an internal or external command,
operable program or batch file.]
  stderr: '\'android\' is not recognized as an internal or external command,\r\noperable program or batch file.\r\n',
  code: 1 }

Not sure how to downgrade (possible?) if not I would have tested that.

[filmaj]

@shazron I think that's expected and will happen if you also try it with the master branch. It is because android_sdk_version does not leverage check_reqs, which will modify your PATH and ANDROID_HOME environment variables.

I can update it, though, to use check_reqs? It would then work unless you have neither ANDROID_HOME nor any of the Android SDK tooling on your PATH - just like all the other commands.

[shazron]

@filmaj as usual, perhaps we need to keep it around, and set a deprecation period of three releases (we had a discussion in dev@) then remove. Thats assuming it was documented, if not, it's all up for grabs to remove

[filmaj]

I've posted a DISCUSS for removal of the android_sdk_version script here: http://markmail.org/message/k4oysup6lkfzk4o2

Any opposition to me merging it in? I am hesitant to do so without an explicit +1 from some other committers.

[dpogue]

The android_sdk_version issue doesn't seem to cause any problems with building, and this PR does work for building with both the older and latest SDKs, so that's a 👍 from me

[purplecabbage]

LGTM!

[filmaj]

Rebased on latest master, will wait for appveyor to pass before merging.