Update Java support for more versions

* on Windows, detect more possible JDK locations.
* On all platforms, more Java versions (up to 17.0 now).
* Add some docu on version selection and on JavaH tool in light of javah
  command dropped since 10.0.
* Try to be better about preserving user's passed-in JAVA* consvars.

Signed-off-by: Mats Wichmann <mats@linux.com>

6 files changed, 238 insertions, 107 deletions

diff --git a/SCons/Tool/JavaCommon.py b/SCons/Tool/JavaCommon.py
index d869b38..dac7d50 100644
--- a/SCons/Tool/JavaCommon.py
+++ b/SCons/Tool/JavaCommon.py
@@ -1,11 +1,6 @@
-"""SCons.Tool.JavaCommon
-
-Stuff for processing Java.
-
-"""
-
+# MIT License
 #
-# __COPYRIGHT__
+# Copyright The SCons Foundation
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and associated documentation files (the
@@ -25,16 +20,16 @@ Stuff for processing Java.
 # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-#
 
-__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
+"""Common routines for processing Java. """
 
 import os
-import os.path
 import re
 import glob
+from pathlib import Path
+from typing import List
 
-java_parsing = 1
+java_parsing = True
 
 default_java_version = '1.4'
 
@@ -43,13 +38,18 @@ default_java_version = '1.4'
 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'
+#
+# These are pairs, (*dir_glob, *version_dir_glob) depending on whether
+# a JDK version was requested or not.
+# For now only used for Windows, which doesn't install JDK in a
+# path that would be in env['ENV']['PATH'].  The specific tool will
+# add the discovered path to this.  Since Oracle changed the rules,
+# there are many possible vendors, we can't guess them all, but take a shot.
+java_win32_dir_glob = 'C:/Program Files*/*/*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'
+java_win32_version_dir_glob = 'C:/Program Files*/*/*jdk*%s*/bin'
 
 # Glob patterns for use in finding where the JDK headers are.
 # These are pairs, *dir_glob used in the general case,
@@ -98,9 +98,27 @@ if java_parsing:
         interfaces, and anonymous inner classes."""
 
         def __init__(self, version=default_java_version):
-
-            if version not 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', '12.0'):
+            if version not 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',
+                '12.0',
+                '13.0',
+                '14.0',
+                '15.0',
+                '16.0',
+                '17.0',
+            ):
                 msg = "Java version %s not supported" % version
                 raise NotImplementedError(msg)
 
@@ -207,7 +225,24 @@ 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', '12.0'):
+            # TODO: shouldn't need to repeat versions here and in OuterState
+            elif self.version in (
+                '1.5',
+                '1.6',
+                '1.7',
+                '1.8',
+                '5',
+                '6',
+                '9.0',
+                '10.0',
+                '11.0',
+                '12.0',
+                '13.0',
+                '14.0',
+                '15.0',
+                '16.0',
+                '17.0',
+            ):
                 self.stackAnonClassBrackets.append(self.brackets)
                 className = []
                 className.extend(self.listClasses)
@@ -443,49 +478,82 @@ else:
         return os.path.split(fn)
 
 
-def get_java_install_dirs(platform, version=None):
-    """
-    Find the java jdk installation directories.
+def get_java_install_dirs(platform, version=None) -> List[str]:
+    """ Find possible java jdk installation directories.
+
+    Returns a list for use as `default_paths` when looking up actual
+    java binaries with :meth:`SCons.Tool.find_program_path`.
+    The paths are sorted by version, latest first.
 
-    This list is intended to supply as "default paths" for use when looking
-    up actual java binaries.
+    Args:
+        platform: selector for search algorithm.
+        version: if not None, restrict the search to this version.
 
-    :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.
+    Returns:
+        list of default paths for jdk.
     """
 
-    paths = []
     if platform == 'win32':
+        paths = []
         if version:
             paths = glob.glob(java_win32_version_dir_glob % version)
         else:
             paths = glob.glob(java_win32_dir_glob)
-    else:
-        # other platforms, do nothing for now
-        pass
 
-    return sorted(paths)
+        def win32getvnum(java):
+            """ Generates a sort key for win32 jdk versions.
 
+            We'll have gotten a path like ...something/*jdk*/bin because
+            that is the pattern we glob for. To generate the sort key,
+            extracts the next-to-last component, then trims it further if
+            it had a complex name, like 'java-1.8.0-openjdk-1.8.0.312-1',
+            to try and put it on a common footing with the more common style,
+            which looks like 'jdk-11.0.2'. 
+
+            This is certainly fragile, and if someone has a 9.0 it won't
+            sort right since this will still be alphabetic, BUT 9.0 was
+            not an LTS release and is 30 mos out of support as this note
+            is written so just assume it will be okay.
+            """
+            d = Path(java).parts[-2]
+            if not d.startswith('jdk'):
+                d = 'jdk' + d.rsplit('jdk', 1)[-1]
+            return d
+
+        return sorted(paths, key=win32getvnum, reverse=True)
+
+    # other platforms, do nothing for now: we expect the standard
+    # paths to be enough to find a jdk (e.g. use alternatives system)
+    return []
 
-def get_java_include_paths(env, javac, version):
-    """
-    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.
+def get_java_include_paths(env, javac, version) -> List[str]:
+    """Find java include paths for JNI building.
+
+    Cannot be called in isolation - `javac` refers to an already detected
+    compiler. Normally would would call :func:`get_java_install_dirs` first
+    and then do lookups on the paths it returns before calling us.
+
+    Args:
+        env: construction environment, used to extract platform.
+        javac: path to detected javac.
+        version: if not None, restrict the search to this version.
+
+    Returns:
+        list of include directory 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
+        return []
+
+    # on Windows, we have a path to the actual javac, so look locally
+    if env['PLATFORM'] == 'win32':
         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')]
+
+    # for the others, we probably found something which isn't in the JDK dir,
+    # so use the predefined patterns to glob for an include directory.
     elif env['PLATFORM'] == 'darwin':
         if not version:
             paths = [java_macos_include_dir_glob]
@@ -500,10 +568,10 @@ def get_java_include_paths(env, javac, version):
             for p in java_linux_version_include_dirs_glob:
                 base_paths.extend(glob.glob(p % version))
 
+        paths = []
         for p in base_paths:
             paths.extend([p, os.path.join(p, 'linux')])
 
-    # print("PATHS:%s"%paths)
     return paths
 
 # Local Variables:
diff --git a/SCons/Tool/JavaCommonTests.py b/SCons/Tool/JavaCommonTests.py
index f35cb9e..75e75ef 100644
--- a/SCons/Tool/JavaCommonTests.py
+++ b/SCons/Tool/JavaCommonTests.py
@@ -1,5 +1,6 @@
+# MIT License
 #
-# __COPYRIGHT__
+# Copyright The SCons Foundation
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and associated documentation files (the
@@ -19,9 +20,6 @@
 # LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
-#
-
-__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 import os.path
 import unittest
@@ -674,24 +672,43 @@ public class AnonDemo {
         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
+        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')
+            (r'C:/Program Files/Java/jdk1.8.0_201/bin', '1.8.0'),
+            (r'C:/Program Files/Java/jdk-11.0.2/bin', '11.0.2'),
+            (r'C:/Program Files/AdoptOpenJDK/jdk-16.0.1.9-hotspot/bin', '16.0.1'),
+            (r'C:/Program Files/Microsoft/jdk-17.0.0.35-hotspot/bin', '17.0.0'),
+            (r'C:/Program Files/OpenJDK/openjdk-11.0.13_8/bin', '11.0.13'),
+            (r'C:/Program Files/RedHat/java-1.8.0-openjdk-1.8.0.312-1/bin', '1.8.0'),
+            (r'C:/Program Files/RedHat/java-11-openjdk-11.0.13-1/bin', '11.0.13'),
         ]
 
         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))
+                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))
+                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-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'),
         ]
@@ -699,7 +716,7 @@ public class AnonDemo {
         # Test non-windows/non-macos globs
         for (wjd, version) in non_win_java_include_dirs:
             match = False
-            globs_tried =[]
+            globs_tried = []
             for jlig in java_linux_include_dirs_glob:
                 globs_tried.append(jlig)
 
@@ -708,36 +725,52 @@ public class AnonDemo {
                     break
 
             if not match:
-                self.fail("Didn't properly match %s with pattern %s" % (wjd, globs_tried))
+                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)
+                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))
+                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'),
+            (
+                '/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))
+        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))
-
-
+                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__":
diff --git a/SCons/Tool/javac.py b/SCons/Tool/javac.py
index ff206c3..0c4f535 100644
--- a/SCons/Tool/javac.py
+++ b/SCons/Tool/javac.py
@@ -220,23 +220,24 @@ def generate(env):
     else:
         javac = SCons.Tool.find_program_path(env, 'javac')
 
-    env['JAVAINCLUDES'] = get_java_include_paths(env, javac, version)
-
-
-    env['JAVAC']                    = 'javac'
-    env['JAVACFLAGS']               = SCons.Util.CLVar('')
-    env['JAVABOOTCLASSPATH']        = []
-    env['JAVACLASSPATH']            = []
-    env['JAVASOURCEPATH']           = []
-    env['_javapathopt']             = pathopt
-    env['_JAVABOOTCLASSPATH']       = '${_javapathopt("-bootclasspath", "JAVABOOTCLASSPATH")} '
-    env['_JAVACLASSPATH']           = '${_javapathopt("-classpath", "JAVACLASSPATH")} '
-    env['_JAVASOURCEPATH']          = '${_javapathopt("-sourcepath", "JAVASOURCEPATH", "_JAVASOURCEPATHDEFAULT")} '
-    env['_JAVASOURCEPATHDEFAULT']   = '${TARGET.attributes.java_sourcedir}'
-    env['_JAVACCOM']                = '$JAVAC $JAVACFLAGS $_JAVABOOTCLASSPATH $_JAVACLASSPATH -d ${TARGET.attributes.java_classdir} $_JAVASOURCEPATH $SOURCES'
-    env['JAVACCOM']                 = "${TEMPFILE('$_JAVACCOM','$JAVACCOMSTR')}"
-    env['JAVACLASSSUFFIX']          = '.class'
-    env['JAVASUFFIX']               = '.java'
+
+    env.SetDefault(
+        JAVAC='javac',
+        JAVACFLAGS=SCons.Util.CLVar(''),
+        JAVAINCLUDES=get_java_include_paths(env, javac, version),
+        JAVACLASSSUFFIX='.class',
+        JAVASUFFIX='.java',
+        JAVABOOTCLASSPATH=[],
+        JAVACLASSPATH=[],
+        JAVASOURCEPATH=[],
+    )
+    env['_javapathopt'] = pathopt
+    env['_JAVABOOTCLASSPATH'] = '${_javapathopt("-bootclasspath", "JAVABOOTCLASSPATH")} '
+    env['_JAVACLASSPATH'] = '${_javapathopt("-classpath", "JAVACLASSPATH")} '
+    env['_JAVASOURCEPATH'] = '${_javapathopt("-sourcepath", "JAVASOURCEPATH", "_JAVASOURCEPATHDEFAULT")} '
+    env['_JAVASOURCEPATHDEFAULT'] = '${TARGET.attributes.java_sourcedir}'
+    env['_JAVACCOM'] = '$JAVAC $JAVACFLAGS $_JAVABOOTCLASSPATH $_JAVACLASSPATH -d ${TARGET.attributes.java_classdir} $_JAVASOURCEPATH $SOURCES'
+    env['JAVACCOM'] = "${TEMPFILE('$_JAVACCOM','$JAVACCOMSTR')}"
 
 def exists(env):
     return 1
diff --git a/SCons/Tool/javac.xml b/SCons/Tool/javac.xml
index 3ec7fea..bda1ef1 100644
--- a/SCons/Tool/javac.xml
+++ b/SCons/Tool/javac.xml
@@ -278,27 +278,32 @@ env = Environment(JAVACCOMSTR="Compiling class files $TARGETS from $SOURCES")
     <cvar name="JAVAVERSION">
         <summary>
             <para>
-                Specifies the Java version being used by the &b-Java; builder.
-                This is <emphasis>not</emphasis> currently used to select one
-                version of the Java compiler vs. another.
-                Instead, you should set this to specify the version of Java
-                supported by your &javac; compiler.
-                The default is <literal>1.4</literal>.
-            </para>
-
-            <para>
+                Specifies the Java version being used by the &b-link-Java;
+                builder. Set this to specify the version of Java targeted
+                by the &javac; compiler.
                 This is sometimes necessary because
                 Java 1.5 changed the file names that are created
                 for nested anonymous inner classes,
                 which can cause a mismatch with the files
                 that &SCons; expects will be generated by the &javac; compiler.
-                Setting &cv-JAVAVERSION; to
-                <literal>1.5</literal>
-                (or <literal>1.6</literal>, as appropriate)
-                can make &SCons; realize that a Java 1.5 or 1.6
-                build is actually up to date.
+                Setting &cv-JAVAVERSION; to a version greater than
+                <literal>1.4</literal> makes &SCons; realize that a build
+                with such a compiler is actually up to date.
+                The default is <literal>1.4</literal>.
+            </para>
+            <para>
+                While this is <emphasis>not</emphasis> primarily intended for
+                selecting one version of the Java compiler vs. another,
+                it does have that effect on the Windows platform. A
+                more precise approach is to set &cv-link-JAVAC; (and related
+                &consvars; for related utilities) to the path to the specific
+                Java compiler you want, if that is not the default compiler.
+                On non-Windows platforms, the
+                <systemitem>alternatives</systemitem> system may provide a
+                way to adjust the default Java compiler without
+                having to specify explicit paths.
             </para>
         </summary>
     </cvar>
 
-</sconsdoc>
+    </sconsdoc>
diff --git a/SCons/Tool/javah.py b/SCons/Tool/javah.py
index ca5bca3..c5a7564 100644
--- a/SCons/Tool/javah.py
+++ b/SCons/Tool/javah.py
@@ -128,12 +128,15 @@ def generate(env):
             javah_bin_dir = os.path.dirname(javah)
             env.AppendENVPath('PATH', javah_bin_dir)
 
-    env['_JAVAHOUTFLAG']    = JavaHOutFlagGenerator
-    env['JAVAH']            = 'javah'
-    env['JAVAHFLAGS']       = SCons.Util.CLVar('')
-    env['_JAVAHCLASSPATH']  = getJavaHClassPath
-    env['JAVAHCOM']         = '$JAVAH $JAVAHFLAGS $_JAVAHOUTFLAG $_JAVAHCLASSPATH ${SOURCES.attributes.java_classname}'
-    env['JAVACLASSSUFFIX']  = '.class'
+    env.SetDefault(
+        JAVAH='javah',
+        JAVAHFLAGS=SCons.Util.CLVar(''),
+        JAVACLASSSUFFIX='.class',
+        JAVASUFFIX='.java',
+    )
+    env['_JAVAHOUTFLAG'] = JavaHOutFlagGenerator
+    env['_JAVAHCLASSPATH'] = getJavaHClassPath
+    env['JAVAHCOM'] = '$JAVAH $JAVAHFLAGS $_JAVAHOUTFLAG $_JAVAHCLASSPATH ${SOURCES.attributes.java_classname}'
 
 def exists(env):
     return env.Detect('javah')
diff --git a/SCons/Tool/javah.xml b/SCons/Tool/javah.xml
index ecbf782..0691ae2 100644
--- a/SCons/Tool/javah.xml
+++ b/SCons/Tool/javah.xml
@@ -90,6 +90,27 @@ env.JavaH(
     JAVACLASSDIR="classes",
 )
 </example_commands>
+
+<note>
+<para>
+Java versions starting with 10.0 no longer use the
+<command>javah</command> command for generating JNI
+headers/sources, and indeed have removed the command entirely
+(see Java Enhancement Proposal
+<ulink url="https:openjdk.java.net/jeps/313">JEP 313</ulink>),
+making this tool harder to use for that purpose.
+&SCons; may autodiscover a <command>javah</command>
+belonging to an older release if there are multiple Java
+versions on the system, which will lead to incorrect results.
+To use with a newer Java, override the default values of &cv-link-JAVAH;
+(to contain the path to the <command>javac</command>)
+and &cv-link-JAVAHFLAGS; (to contain at least a <option>-d</option>
+flag) and note that generating headers with
+<command>javac</command> requires supplying source
+<filename>.java</filename> files only,
+not <filename>.class</filename> files.
+</para>
+</note>
 </summary>
 </builder>