Merge pull request #3352 from mwichmann/java-vers

Fix problems with jdk detection

10 files changed, 235 insertions, 81 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 372c2d0..c80af4b 100755
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -20,16 +20,16 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Use importlib to dynamically load tool and platform modules instead of imp module
     - sconsign: default to .sconsign.dblite if no filename is specified.
       Be more informative in case of unsupported pickle protocol (py2 only).
-    - assorted fixups for pylint: exception types, redefined functions, 
+    - More fixes for newer Java versions (since 9): handle new jdk directory
+      naming (jdk-X.Y instead of jdkX.Y) on Windows; handle two-digit major
+      version. Docstrings improved.
+    - Fixups for pylint: exception types, redefined functions, 
       globals, etc.  Some old code removed to resolve issues (hashlib is
       always present on modern Pythons; no longer need the code for
       2.5-and-earlier optparse). cmp is not a builtin function in Py3,
       drop one (unused) use; replace one.  Fix another instance of
       renaming to SConsEnvironmentError.
 
-  From John Doe:
-
-    - Whatever John Doe did.
 
 
 RELEASE 3.0.5 - Mon, 26 Mar 2019 15:04:42 -0700
diff --git a/src/engine/SCons/Tool/JavaCommon.py b/src/engine/SCons/Tool/JavaCommon.py
index 853f7f2..f1c1b4f 100644
--- a/src/engine/SCons/Tool/JavaCommon.py
+++ b/src/engine/SCons/Tool/JavaCommon.py
@@ -42,6 +42,33 @@ default_java_version = '1.4'
 # anonymous inner class parsing.
 scopeStateVersions = ('1.8')
 
+# Glob patterns for use in finding where the JDK is.
+# These are pairs, *dir_glob used in the general case,
+# *version_dir_glob if matching only a specific version.
+# For now only used for Windows.
+java_win32_dir_glob = 'C:/Program Files*/Java/jdk*/bin'
+# On windows, since Java 9, there is a dash between 'jdk' and the version
+# string that wasn't there before. this glob should catch either way.
+java_win32_version_dir_glob = 'C:/Program Files*/Java/jdk*%s*/bin'
+
+# Glob patterns for use in finding where the JDK headers are.
+# These are pairs, *dir_glob used in the general case,
+# *version_dir_glob if matching only a specific version.
+java_macos_include_dir_glob = '/System/Library/Frameworks/JavaVM.framework/Headers/'
+java_macos_version_include_dir_glob = '/System/Library/Frameworks/JavaVM.framework/Versions/%s*/Headers/'
+
+java_linux_include_dirs_glob = [
+    '/usr/lib/jvm/default-java/include',
+    '/usr/lib/jvm/java-*/include'
+]
+# Need to match path like below (from Centos 7)
+# /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.191.b12-0.el7_5.x86_64/include/
+java_linux_version_include_dirs_glob = [
+    '/usr/lib/jvm/java-*-sun-%s*/include',
+    '/usr/lib/jvm/java-%s*-openjdk*/include',
+    '/usr/java/jdk%s*/include'
+]
+
 if java_parsing:
     # Parse Java files for class names.
     #
@@ -64,14 +91,15 @@ if java_parsing:
                           r'\d*\.\d*|[A-Za-z_][\w\$\.]*|<[A-Za-z_]\w+>|' +
                           r'/\*|\*/|\[\])')
 
+
     class OuterState(object):
         """The initial state for parsing a Java file for classes,
         interfaces, and anonymous inner classes."""
+
         def __init__(self, version=default_java_version):
 
             if not version in ('1.1', '1.2', '1.3', '1.4', '1.5', '1.6', '1.7',
-                               '1.8', '5', '6', '9.0', '10.0', '11.0'):
-
+                               '1.8', '5', '6', '9.0', '10.0', '11.0', '12.0'):
                 msg = "Java version %s not supported" % version
                 raise NotImplementedError(msg)
 
@@ -131,15 +159,15 @@ if java_parsing:
         def closeBracket(self):
             self.brackets = self.brackets - 1
             if len(self.stackBrackets) and \
-                            self.brackets == self.stackBrackets[-1]:
+                    self.brackets == self.stackBrackets[-1]:
                 self.listOutputs.append('$'.join(self.listClasses))
                 self.localClasses.pop()
                 self.listClasses.pop()
                 self.anonStacksStack.pop()
                 self.stackBrackets.pop()
             if len(self.stackAnonClassBrackets) and \
-                            self.brackets == self.stackAnonClassBrackets[-1] and \
-                            self.version not in scopeStateVersions:
+                    self.brackets == self.stackAnonClassBrackets[-1] and \
+                    self.version not in scopeStateVersions:
                 self._getAnonStack().pop()
                 self.stackAnonClassBrackets.pop()
 
@@ -152,13 +180,13 @@ if java_parsing:
                 self.openBracket()
             elif token == '}':
                 self.closeBracket()
-            elif token in [ '"', "'" ]:
+            elif token in ['"', "'"]:
                 return IgnoreState(token, self)
             elif token == "new":
                 # anonymous inner class
                 if len(self.listClasses) > 0:
                     return self.__getAnonClassState()
-                return self.__getSkipState() # Skip the class name
+                return self.__getSkipState()  # Skip the class name
             elif token in ['class', 'interface', 'enum']:
                 if len(self.listClasses) == 0:
                     self.nextAnon = 1
@@ -178,7 +206,7 @@ if java_parsing:
             if self.version in ('1.1', '1.2', '1.3', '1.4'):
                 clazz = self.listClasses[0]
                 self.listOutputs.append('%s$%d' % (clazz, self.nextAnon))
-            elif self.version in ('1.5', '1.6', '1.7', '1.8', '5', '6', '9.0', '10.0', '11.0'):
+            elif self.version in ('1.5', '1.6', '1.7', '1.8', '5', '6', '9.0', '10.0', '11.0', '12.0'):
                 self.stackAnonClassBrackets.append(self.brackets)
                 className = []
                 className.extend(self.listClasses)
@@ -193,11 +221,13 @@ if java_parsing:
         def setPackage(self, package):
             self.package = package
 
+
     class ScopeState(object):
         """
         A state that parses code within a scope normally,
         within the confines of a scope.
         """
+
         def __init__(self, old_state):
             self.outer_state = old_state.outer_state
             self.old_state = old_state
@@ -259,13 +289,16 @@ if java_parsing:
                 return self.__getSkipState()
             return self
 
+
     class AnonClassState(object):
         """A state that looks for anonymous inner classes."""
+
         def __init__(self, old_state):
             # outer_state is always an instance of OuterState
             self.outer_state = old_state.outer_state
             self.old_state = old_state
             self.brace_level = 0
+
         def parseToken(self, token):
             # This is an anonymous class if and only if the next
             # non-whitespace token is a bracket. Everything between
@@ -293,26 +326,32 @@ if java_parsing:
             if token == '{':
                 self.outer_state.addAnonClass()
                 if self.outer_state.version in scopeStateVersions:
-                    return ScopeState(old_state = self.old_state).parseToken(token)
+                    return ScopeState(old_state=self.old_state).parseToken(token)
             return self.old_state.parseToken(token)
 
+
     class SkipState(object):
         """A state that will skip a specified number of tokens before
         reverting to the previous state."""
+
         def __init__(self, tokens_to_skip, old_state):
             self.tokens_to_skip = tokens_to_skip
             self.old_state = old_state
+
         def parseToken(self, token):
             self.tokens_to_skip = self.tokens_to_skip - 1
             if self.tokens_to_skip < 1:
                 return self.old_state
             return self
 
+
     class ClassState(object):
         """A state we go into when we hit a class or interface keyword."""
+
         def __init__(self, outer_state):
             # outer_state is always an instance of OuterState
             self.outer_state = outer_state
+
         def parseToken(self, token):
             # the next non-whitespace token should be the name of the class
             if token == '\n':
@@ -322,12 +361,12 @@ if java_parsing:
             # 'Foo$1Inner'
             # https://github.com/SCons/scons/issues/2087
             if self.outer_state.localClasses and \
-                            self.outer_state.stackBrackets[-1] > \
-                                    self.outer_state.stackBrackets[-2]+1:
+                    self.outer_state.stackBrackets[-1] > \
+                    self.outer_state.stackBrackets[-2] + 1:
                 locals = self.outer_state.localClasses[-1]
                 try:
                     idx = locals[token]
-                    locals[token] = locals[token]+1
+                    locals[token] = locals[token] + 1
                 except KeyError:
                     locals[token] = 1
                 token = str(locals[token]) + token
@@ -336,32 +375,40 @@ if java_parsing:
             self.outer_state.anonStacksStack.append([0])
             return self.outer_state
 
+
     class IgnoreState(object):
         """A state that will ignore all tokens until it gets to a
         specified token."""
+
         def __init__(self, ignore_until, old_state):
             self.ignore_until = ignore_until
             self.old_state = old_state
+
         def parseToken(self, token):
             if self.ignore_until == token:
                 return self.old_state
             return self
 
+
     class PackageState(object):
         """The state we enter when we encounter the package keyword.
         We assume the next token will be the package name."""
+
         def __init__(self, outer_state):
             # outer_state is always an instance of OuterState
             self.outer_state = outer_state
+
         def parseToken(self, token):
             self.outer_state.setPackage(token)
             return self.outer_state
 
+
     def parse_java_file(fn, version=default_java_version):
         with open(fn, 'r') as f:
             data = f.read()
         return parse_java(data, version)
 
+
     def parse_java(contents, version=default_java_version, trace=None):
         """Parse a .java file and return a double of package directory,
         plus a list of .class files that compiling that .java file will
@@ -395,81 +442,68 @@ else:
         return os.path.split(fn)
 
 
-
-java_win32_version_dir_glob = 'C:/Program Files*/Java/jdk%s*/bin'
-java_win32_dir_glob = 'C:/Program Files*/Java/jdk*/bin'
-
-java_macos_include_dir = '/System/Library/Frameworks/JavaVM.framework/Headers/'
-java_macos_version_include_dir = '/System/Library/Frameworks/JavaVM.framework/Versions/%s*/Headers/'
-
-java_linux_include_dirs = ['/usr/lib/jvm/default-java/include',
-                        '/usr/lib/jvm/java-*/include']
-# Need to match path like below (from Centos 7)
-# /usr/lib/jvm/java-1.8.0-openjdk-1.8.0.191.b12-0.el7_5.x86_64/include/
-java_linux_version_include_dirs = ['/usr/lib/jvm/java-*-sun-%s*/include',
-                                   '/usr/lib/jvm/java-%s*-openjdk*/include',
-                                   '/usr/java/jdk%s*/include']
-
-
-
 def get_java_install_dirs(platform, version=None):
     """
-    Using patterns above find the java jdk install dir
-    :param platform:
+    Find the java jdk installation directories.
+
+    This list is intended to supply as "default paths" for use when looking
+    up actual java binaries.
+
+    :param platform: selector for search algorithm.
     :param version: If specified, only look for java sdk's of this version
     :return: list of default paths for java.
     """
+
     paths = []
     if platform == 'win32':
         if version:
-            paths = glob.glob(java_win32_version_dir_glob%version)
+            paths = glob.glob(java_win32_version_dir_glob % version)
         else:
             paths = glob.glob(java_win32_dir_glob)
     else:
-        # do nothing for now
+        # other platforms, do nothing for now
         pass
 
-    paths=sorted(paths)
+    return sorted(paths)
 
-    return paths
 
 def get_java_include_paths(env, javac, version):
     """
-    Return java include paths
-    :param platform:
-    :param javac:
-    :return:
+    Find java include paths for JNI building.
+
+    :param env: construction environment, used to extract platform.
+    :param javac: path to detected javac.
+    :return: list of paths.
     """
+
     paths = []
     if not javac:
         # there are no paths if we've not detected javac.
         pass
     elif env['PLATFORM'] == 'win32':
+        # on Windows, we have the right path to javac, so look locally
         javac_bin_dir = os.path.dirname(javac)
         java_inc_dir = os.path.normpath(os.path.join(javac_bin_dir, '..', 'include'))
         paths = [java_inc_dir, os.path.join(java_inc_dir, 'win32')]
     elif env['PLATFORM'] == 'darwin':
         if not version:
-            paths = [java_macos_include_dir]
+            paths = [java_macos_include_dir_glob]
         else:
-            paths = sorted(glob.glob(java_macos_version_include_dir%version))
+            paths = sorted(glob.glob(java_macos_version_include_dir_glob % version))
     else:
-        base_paths=[]
+        base_paths = []
         if not version:
-            for p in java_linux_include_dirs:
+            for p in java_linux_include_dirs_glob:
                 base_paths.extend(glob.glob(p))
         else:
-            for p in java_linux_version_include_dirs:
-                base_paths.extend(glob.glob(p%version))
+            for p in java_linux_version_include_dirs_glob:
+                base_paths.extend(glob.glob(p % version))
 
         for p in base_paths:
-            paths.extend([p, os.path.join(p,'linux')])
-            
-    #print("PATHS:%s"%paths)
-    return paths
-
-
+            paths.extend([p, os.path.join(p, 'linux')])
 
+    # print("PATHS:%s"%paths)
+    return paths
 
 # Local Variables:
 # tab-width:4
diff --git a/src/engine/SCons/Tool/JavaCommonTests.py b/src/engine/SCons/Tool/JavaCommonTests.py
index 10be671..9242624 100644
--- a/src/engine/SCons/Tool/JavaCommonTests.py
+++ b/src/engine/SCons/Tool/JavaCommonTests.py
@@ -26,10 +26,12 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 import os.path
 import sys
 import unittest
+import fnmatch
 
 import SCons.Scanner.IDL
 import SCons.Tool.JavaCommon
 
+import TestSCons
 
 # Adding trace=trace to any of the parse_jave() calls below will cause
 # the parser to spit out trace messages of the tokens it sees and the
@@ -607,6 +609,76 @@ public class AnonDemo {
         pkg_dir, classes = SCons.Tool.JavaCommon.parse_java(input, '1.8')
         assert expect == classes, (expect, classes)
 
+    def test_jdk_globs(self):
+        """
+        Verify that the java path globs work with specific examples.
+        :return:
+        """
+        from SCons.Tool.JavaCommon import java_linux_include_dirs_glob, java_linux_version_include_dirs_glob, java_win32_dir_glob, java_win32_version_dir_glob, java_macos_include_dir_glob, java_macos_version_include_dir_glob
+
+        # Test windows globs
+        win_java_dirs = [
+            ('C:/Program Files/Java/jdk1.8.0_201/bin', '1.8.0'),
+            ('C:/Program Files/Java/jdk-11.0.2/bin', '11.0.2'),
+            ('C:/Program Files/Java/jdk1.7.0_80/bin', '1.7.0')
+        ]
+
+        for (wjd, version) in win_java_dirs:
+            if not fnmatch.fnmatch(wjd, java_win32_dir_glob):
+                self.fail("Didn't properly match %s with pattern %s" % (wjd, java_win32_dir_glob))
+            if not fnmatch.fnmatch(wjd, java_win32_version_dir_glob % version):
+                self.fail("Didn't properly match %s with version (%s) specific pattern %s" % (
+                wjd, version, java_win32_version_dir_glob % version))
+
+        non_win_java_include_dirs = [
+            ('/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.191.b12-0.el7_5.x86_64/include', '1.8.0'),
+            ('/usr/lib/jvm/java-1.8.0-openjdk-amd64/include', '1.8.0'),
+            ('/usr/lib/jvm/java-8-openjdk-amd64/include', '8'),
+        ]
+
+        # Test non-windows/non-macos globs
+        for (wjd, version) in non_win_java_include_dirs:
+            match = False
+            globs_tried =[]
+            for jlig in java_linux_include_dirs_glob:
+                globs_tried.append(jlig)
+
+                if fnmatch.fnmatch(wjd, jlig):
+                    match = True
+                    break
+
+            if not match:
+                self.fail("Didn't properly match %s with pattern %s" % (wjd, globs_tried))
+
+            match = False
+            globs_tried = []
+            for jlvig in java_linux_version_include_dirs_glob:
+                globs_tried.append(jlvig%version)
+                if fnmatch.fnmatch(wjd, jlvig % version):
+                    match = True
+                    break
+
+            if not match:
+                self.fail("Didn't properly match %s with version (%s) specific pattern %s" % (
+                    wjd, version, globs_tried))
+
+        # Test macos globs
+        # Test windows globs
+        macos_java_dirs = [
+            # ('/System/Library/Frameworks/JavaVM.framework/Headers/', None),
+            ('/System/Library/Frameworks/JavaVM.framework/Versions/11.0.2/Headers/', '11.0.2'),
+        ]
+
+        if not fnmatch.fnmatch('/System/Library/Frameworks/JavaVM.framework/Headers/', java_macos_include_dir_glob):
+            self.fail("Didn't properly match %s with pattern %s" % ('/System/Library/Frameworks/JavaVM.framework/Headers/', java_macos_include_dir_glob))
+
+        for (wjd, version) in macos_java_dirs:
+            if not fnmatch.fnmatch(wjd, java_macos_version_include_dir_glob % version):
+                self.fail("Didn't properly match %s with version (%s) specific pattern %s" % (
+                wjd, version, java_macos_version_include_dir_glob % version))
+
+
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/src/engine/SCons/Tool/jar.py b/src/engine/SCons/Tool/jar.py
index e0a6a69..02bda57 100644
--- a/src/engine/SCons/Tool/jar.py
+++ b/src/engine/SCons/Tool/jar.py
@@ -209,9 +209,9 @@ def generate(env):
     env.AddMethod(Jar)
 
     if env['PLATFORM'] == 'win32':
-        # Ensure that we have a proper path for clang
-        jar = SCons.Tool.find_program_path(env, 'jar',
-                                             default_paths=get_java_install_dirs(env['PLATFORM']))
+        # Ensure that we have a proper path for jar
+        paths = get_java_install_dirs('win32')
+        jar = SCons.Tool.find_program_path(env, 'jar', default_paths=paths)
         if jar:
             jar_bin_dir = os.path.dirname(jar)
             env.AppendENVPath('PATH', jar_bin_dir)
diff --git a/src/engine/SCons/Tool/javac.py b/src/engine/SCons/Tool/javac.py
index 8d98b54..537913f 100644
--- a/src/engine/SCons/Tool/javac.py
+++ b/src/engine/SCons/Tool/javac.py
@@ -210,15 +210,15 @@ def generate(env):
 
     version = env.get('JAVAVERSION', None)
 
-    javac = SCons.Tool.find_program_path(env, 'javac')
     if env['PLATFORM'] == 'win32':
         # Ensure that we have a proper path for javac
-        paths=get_java_install_dirs(env['PLATFORM'], version=version)
-        javac = SCons.Tool.find_program_path(env, 'javac',
-                                             default_paths=paths)
+        paths = get_java_install_dirs('win32', version=version)
+        javac = SCons.Tool.find_program_path(env, 'javac', default_paths=paths)
         if javac:
             javac_bin_dir = os.path.dirname(javac)
             env.AppendENVPath('PATH', javac_bin_dir)
+    else:
+        javac = SCons.Tool.find_program_path(env, 'javac')
 
     env['JAVAINCLUDES'] = get_java_include_paths(env, javac, version)
 
diff --git a/src/engine/SCons/Tool/javah.py b/src/engine/SCons/Tool/javah.py
index f514479..80f8a6b 100644
--- a/src/engine/SCons/Tool/javah.py
+++ b/src/engine/SCons/Tool/javah.py
@@ -123,9 +123,9 @@ def generate(env):
     java_javah.emitter = emit_java_headers
 
     if env['PLATFORM'] == 'win32':
-        # Ensure that we have a proper path for clang
-        javah = SCons.Tool.find_program_path(env, 'javah',
-                                             default_paths=get_java_install_dirs(env['PLATFORM']))
+        # Ensure that we have a proper path for javah
+        paths = get_java_install_dirs('win32')
+        javah = SCons.Tool.find_program_path(env, 'javah', default_paths=paths)
         if javah:
             javah_bin_dir = os.path.dirname(javah)
             env.AppendENVPath('PATH', javah_bin_dir)
diff --git a/src/engine/SCons/Tool/rmic.py b/src/engine/SCons/Tool/rmic.py
index 173ef5f..5c7a040 100644
--- a/src/engine/SCons/Tool/rmic.py
+++ b/src/engine/SCons/Tool/rmic.py
@@ -110,11 +110,9 @@ def generate(env):
 
     if env['PLATFORM'] == 'win32':
         version = env.get('JAVAVERSION', None)
-        default_paths=get_java_install_dirs(env['PLATFORM'], version=version)
-
         # Ensure that we have a proper path for rmic
-        rmic = SCons.Tool.find_program_path(env, 'rmic', default_paths=default_paths)
-
+        paths = get_java_install_dirs('win32', version=version)
+        rmic = SCons.Tool.find_program_path(env, 'rmic', default_paths=paths)
         # print("RMIC: %s"%rmic)
         if rmic:
             rmic_bin_dir = os.path.dirname(rmic)
diff --git a/test/Java/RMIC.py b/test/Java/RMIC.py
index 021f666..c8848e9 100644
--- a/test/Java/RMIC.py
+++ b/test/Java/RMIC.py
@@ -102,13 +102,13 @@ if java_version.count('.') == 1:
     major, minor = java_version.split('.')
     try:
         curver = (int(major), int(minor))
-    except:
+    except ValueError:
         pass
 elif java_version.count('.') == 0:
     # java 11?
     try:
         curver = (int(java_version), 0)
-    except:
+    except ValueError:
         pass
 
 # Check the version of the found Java compiler.
diff --git a/test/Repository/RMIC.py b/test/Repository/RMIC.py
index 433890f..aa4a57e 100644
--- a/test/Repository/RMIC.py
+++ b/test/Repository/RMIC.py
@@ -44,7 +44,13 @@ if java_version.count('.') == 1:
     major, minor = java_version.split('.')
     try:
         curver = (int(major), int(minor))
-    except:
+    except ValueError:
+        pass
+elif java_version.count('.') == 0:
+    # java 11?
+    try:
+        curver = (int(java_version), 0)
+    except ValueError:
         pass
 
 # Check the version of the found Java compiler.
diff --git a/testing/framework/TestSCons.py b/testing/framework/TestSCons.py
index 91a443e..23e8753 100644
--- a/testing/framework/TestSCons.py
+++ b/testing/framework/TestSCons.py
@@ -682,6 +682,9 @@ class TestSCons(TestCommon):
         """
         Initialize with a default external environment that uses a local
         Java SDK in preference to whatever's found in the default PATH.
+
+        :param version: if set, match only that version
+        :return: the new env.
         """
         if not self.external:
             try:
@@ -698,11 +701,11 @@ class TestSCons(TestCommon):
             if version:
                 if sys.platform == 'win32':
                     patterns = [
-                        'C:/Program Files*/Java/jdk%s*/bin'%version,
+                        'C:/Program Files*/Java/jdk*%s*/bin' % version,
                     ]
                 else:
                     patterns = [
-                        '/usr/java/jdk%s*/bin'    % version,
+                        '/usr/java/jdk%s*/bin' % version,
                         '/usr/lib/jvm/*-%s*/bin' % version,
                         '/usr/local/j2sdk%s*/bin' % version,
                     ]
@@ -727,7 +730,10 @@ class TestSCons(TestCommon):
 
     def java_where_includes(self,version=None):
         """
-        Return java include paths compiling java jni code
+        Find include path needed for compiling java jni code.
+
+        :param version: if set, match only that version
+        :return: path to java headers
         """
         import sys
 
@@ -761,6 +767,14 @@ class TestSCons(TestCommon):
         return result
 
     def java_where_java_home(self, version=None):
+        """
+        Find path to what would be JAVA_HOME.
+
+        SCons does not read JAVA_HOME from the environment, so deduce it.
+
+        :param version: if set, match only that version
+        :return: path where JDK components live
+        """
         if sys.platform[:6] == 'darwin':
             # osx 10.11, 10.12
             home_tool = '/usr/libexec/java_home'
@@ -807,6 +821,12 @@ class TestSCons(TestCommon):
             self.skip_test("Could not find Java " + java_bin_name + ", skipping test(s).\n")
 
     def java_where_jar(self, version=None):
+        """
+        Find java archiver jar.
+
+        :param version: if set, match only that version
+        :return: path to jar
+        """
         ENV = self.java_ENV(version)
         if self.detect_tool('jar', ENV=ENV):
             where_jar = self.detect('JAR', 'jar', ENV=ENV)
@@ -821,7 +841,10 @@ class TestSCons(TestCommon):
 
     def java_where_java(self, version=None):
         """
-        Return a path to the java executable.
+        Find java executable.
+
+        :param version: if set, match only that version
+        :return: path to the java rutime
         """
         ENV = self.java_ENV(version)
         where_java = self.where_is('java', ENV['PATH'])
@@ -835,7 +858,10 @@ class TestSCons(TestCommon):
 
     def java_where_javac(self, version=None):
         """
-        Return a path to the javac compiler.
+        Find java compiler.
+
+        :param version: if set, match only that version
+        :return: path to javac
         """
         ENV = self.java_ENV(version)
         if self.detect_tool('javac'):
@@ -851,15 +877,17 @@ class TestSCons(TestCommon):
                  arguments = '-version',
                  stderr=None,
                  status=None)
+        # Note recent versions output version info to stdout instead of stderr
         if version:
-            if self.stderr().find('javac %s' % version) == -1:
+            verf = 'javac %s' % version
+            if self.stderr().find(verf) == -1 and self.stdout().find(verf) == -1:
                 fmt = "Could not find javac for Java version %s, skipping test(s).\n"
                 self.skip_test(fmt % version)
         else:
-            m = re.search(r'javac (\d\.*\d)', self.stderr())
-            # Java 11 outputs this to stdout
+            version_re = r'javac (\d*\.*\d)'
+            m = re.search(version_re, self.stderr())
             if not m:
-                m = re.search(r'javac (\d\.*\d)', self.stdout())
+                m = re.search(version_re, self.stdout())
 
             if m:
                 version = m.group(1)
@@ -873,6 +901,16 @@ class TestSCons(TestCommon):
         return where_javac, version
 
     def java_where_javah(self, version=None):
+        """
+        Find java header generation tool.
+
+        TODO issue #3347 since JDK10, there is no separate javah command,
+        'javac -h' is used. We should not return a javah from a different
+        installed JDK - how to detect and what to return in this case?
+
+        :param version: if set, match only that version
+        :return: path to javah
+        """
         ENV = self.java_ENV(version)
         if self.detect_tool('javah'):
             where_javah = self.detect('JAVAH', 'javah', ENV=ENV)
@@ -883,6 +921,12 @@ class TestSCons(TestCommon):
         return where_javah
 
     def java_where_rmic(self, version=None):
+        """
+        Find java rmic tool.
+
+        :param version: if set, match only that version
+        :return: path to rmic
+        """
         ENV = self.java_ENV(version)
         if self.detect_tool('rmic'):
             where_rmic = self.detect('RMIC', 'rmic', ENV=ENV)