[GitHub] [cordova-android] breautek commented on a change in pull request #1130: refactor: java checks / get java version

[GitBox <gi...@apache.org>]

breautek commented on a change in pull request #1130:
URL: https://github.com/apache/cordova-android/pull/1130#discussion_r602712183



##########
File path: bin/templates/cordova/lib/check_reqs.js
##########
@@ -143,72 +137,20 @@ module.exports.check_gradle = function () {
                             'in your path, or install Android Studio'));
 };
 
-// Returns a promise.
-module.exports.check_java = function () {
-    var javacPath = forgivingWhichSync('javac');
-    var hasJavaHome = !!process.env.JAVA_HOME;
-    return Promise.resolve().then(function () {
-        if (hasJavaHome) {
-            // Windows java installer doesn't add javac to PATH, nor set JAVA_HOME (ugh).
-            if (!javacPath) {
-                process.env.PATH += path.delimiter + path.join(process.env.JAVA_HOME, 'bin');
-            }
-        } else {
-            if (javacPath) {
-                // OS X has a command for finding JAVA_HOME.
-                var find_java = '/usr/libexec/java_home';
-                var default_java_error_msg = 'Failed to find \'JAVA_HOME\' environment variable. Try setting it manually.';
-                if (fs.existsSync(find_java)) {
-                    return execa(find_java).then(({ stdout }) => {
-                        process.env.JAVA_HOME = stdout;
-                    }).catch(function (err) {
-                        if (err) {
-                            throw new CordovaError(default_java_error_msg);
-                        }
-                    });
-                } else {
-                    // See if we can derive it from javac's location.
-                    var maybeJavaHome = path.dirname(path.dirname(javacPath));
-                    if (fs.existsSync(path.join(maybeJavaHome, 'lib', 'tools.jar'))) {
-                        process.env.JAVA_HOME = maybeJavaHome;
-                    } else {
-                        throw new CordovaError(default_java_error_msg);
-                    }
-                }
-            } else if (module.exports.isWindows()) {
-                const { env } = process;
-                const baseDirs = [env.ProgramFiles, env['ProgramFiles(x86)']];
-                const globOpts = { absolute: true, onlyDirectories: true };
-                const flatMap = (arr, f) => [].concat(...arr.map(f));
-
-                const jdkDir = flatMap(baseDirs, cwd =>
-                    glob.sync('java/jdk*', { cwd, ...globOpts })
-                )[0];
-
-                if (jdkDir) {
-                    env.PATH += path.delimiter + path.join(jdkDir, 'bin');
-                    env.JAVA_HOME = path.normalize(jdkDir);
-                }
-            }
-        }
-    }).then(function () {
-        return execa('javac', ['-version'], { all: true })
-            .then(({ all: output }) => {
-                // Java <= 8 writes version info to stderr, Java >= 9 to stdout
-                const match = /javac\s+([\d.]+)/i.exec(output);
-                return match && match[1];
-            }, () => {
-                var msg =
-                'Failed to run "javac -version", make sure that you have a JDK version 8 installed.\n' +
-                'You can get it from the following location:\n' +
-                'https://www.oracle.com/technetwork/java/javase/downloads/jdk8-downloads-2133151.html';
-                if (process.env.JAVA_HOME) {
-                    msg += '\n\n';
-                    msg += 'Your JAVA_HOME is invalid: ' + process.env.JAVA_HOME;
-                }
-                throw new CordovaError(msg);
-            });
-    });
+/**
+ * Checks for the java installation and correct version
+ */
+module.exports.check_java = async function () {
+    const javaVersion = await java.getVersion();
+
+    if (!semver.satisfies(javaVersion.version, `= ${EXPECTED_JAVA_VERSION}`)) {
+        throw new CordovaError(
+            `Requirements check failed for JDK ${EXPECTED_JAVA_VERSION}! Detected version: ${javaVersion.version}\n` +
+            'Check your ANDROID_SDK_ROOT / JAVA_HOME / PATH environment variables.'
+        );
+    }
+
+    return javaVersion.string;

Review comment:
       Not really sure what I was thinking at the time of writing, but a "check" method shouldn't really be returning anything, so I'll remove this, which would have matched the old behaviour as well.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org