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.

Sign up for GitHub

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

Jump to bottom

Quick fix to support ANDROID_SDK_ROOT #656

Merged
merged 2 commits into from
Feb 12, 2019
Merged

Quick fix to support ANDROID_SDK_ROOT #656

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f1bfe1
Quick fix to support ANDROID_SDK_ROOT
Feb 8, 2019
bedaf90
Update bin/templates/cordova/lib/check_reqs.js
janpio Feb 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions bin/templates/cordova/lib/check_reqs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,14 @@ module.exports.check_android = function () {
// First ensure ANDROID_HOME is set
// If we have no hints (nothing in PATH), try a few default locations
if (!hasAndroidHome && !androidCmdPath && !adbInPath && !avdmanagerInPath) {
if (process.env['ANDROID_SDK_ROOT']) {
// Quick fix to set ANDROID_HOME according to ANDROID_SDK_ROOT
// if ANDROID_HOME is **not** defined and
// ANDROID_SDK_ROOT **is** defined
// according to environment variables as documented in:
// https://developer.android.com/studio/command-line/variables
maybeSetAndroidHome(path.join(process.env['ANDROID_SDK_ROOT']));
}
if (module.exports.isWindows()) {
// Android Studio 1.0 installer
maybeSetAndroidHome(path.join(process.env['LOCALAPPDATA'], 'Android', 'sdk'));
Expand Down Expand Up @@ -356,12 +364,13 @@ module.exports.check_android_target = function (originalError) {
module.exports.run = function () {
return Q.all([this.check_java(), this.check_android()]).then(function (values) {
console.log('Checking Java JDK and Android SDK versions');
console.log('ANDROID_HOME=' + process.env['ANDROID_HOME']);
console.log('ANDROID_SDK_ROOT=' + process.env['ANDROID_SDK_ROOT'] + ' (recommended setting)');
console.log('ANDROID_HOME=' + process.env['ANDROID_HOME'] + '- DEPRECATED');
brodybits marked this conversation as resolved.
Show resolved Hide resolved

if (!String(values[0]).startsWith('1.8.')) {
throw new CordovaError(
'Requirements check failed for JDK 8 (\'1.8.*\')! Detected version: ' + values[0] + '\n' +
'Check your JAVA_HOME / ANDROID_HOME / PATH environment variables.'
'Check your ANDROID_SDK_ROOT / JAVA_HOME / PATH environment variables.'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'Check your ANDROID_SDK_ROOT / JAVA_HOME / PATH environment variables.'
'Check your `ANDROID_SDK_ROOT` and `JAVA_HOME` PATH environment variables.'

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 am not 100% convinced by this suggestion. It is possible that a developer could set a PATH environment variable that already includes the Android SDK bin directory.

Copy link
Member

Choose a reason for hiding this comment

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

ah, then I misread the meaning.

Suggested change
'Check your ANDROID_SDK_ROOT / JAVA_HOME / PATH environment variables.'
'Check your `ANDROID_SDK_ROOT`, `JAVA_HOME` and `PATH` environment variables.'

);
}

Expand Down