Adding a special '$JAVA_HOME' symbol for use in jvm platforms args.

This addresses a long-standing TODO in pants.ini:

    # TODO(gmalmquist): Find a way to resolve the -Xbootclasspath
    # automatically, either by putting rt.jars up on the nexus or
    # using some kind of special variable name (see discussion on
    # RB 2494).

Now, at compile-time, the args from the jvm platform settings will
be pre-processed to replace all instances of `$JAVA_HOME` with the
java home determined by the platform's preferred jvm distribution.

The code will attempt to look up the distribution strictly, but if
that fails it will fallback on a non-strict lookup. "Strict" here
means a version X such that

    target_level <= X <= target_level.9999

Whereas unstrict is simply:

    target_level <= X

Testing Done:
Manual testing + added a test to `tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/jvm_platform_integration_mixin.py`.

Travis went green: https://travis-ci.org/pantsbuild/pants/builds/132398495
Jenkins went green: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3490/1/

Travis went green again: https://travis-ci.org/pantsbuild/pants/builds/132867977

Bugs closed: 3490

Reviewed at https://rbcommons.com/s/twitter/r/3924/

examples/src/java/org/pantsbuild/example/README.md

@@ -261,6 +261,25 @@ You can also override these on the cli:
     :::bash
     ./pants compile --jvm-platform-default-platform=java8 examples/src/java/org/pantsbuild/example/hello/main
 
+If you want to set the `-bootclasspath` (or `-Xbootclasspath`) to use the
+appropriate java distribution, you can use the `$JAVA_HOME` symbol in the
+`args` list. For example:
+
+    :::ini
+    [jvm-platform]
+    default_platform: java6
+    platforms: {
+        'java7': {'source': '7', 'target': '7', 'args': ["-C-bootclasspath:$JAVA_HOME/jre/lib/resources.jar:$JAVA_HOME/jre/lib/rt.jar:$JAVA_HOME/jre/lib/sunrsasign.jar:$JAVA_HOME/jre/lib/jsse.jar:$JAVA_HOME/jre/lib/jce.jar:$JAVA_HOME/jre/lib/charsets.jar:$JAVA_HOME/jre/lib/jfr.jar:$JAVA_HOME/jre/classes"] },
+      }
+
+Your `-bootclasspath` should be designed to work with any compatible version of
+the JVM that might be used. If you make use of `[jvm-distributions]` and have
+strict control over what jvm installations are used by developers, this means you
+probably only have to make it work for one version of the JDK. Otherwise, you
+should design your bootclasspath to reference the union of all possible jars
+you might need to pull in from different JVMs (any paths that aren't available
+will simply be ignored by java).
+
 **Note:** Currently, pants is known to work with OpenJDK version 7 or greater,
 and Oracle JDK version 6 or greater.
 

pants.ini

@@ -192,8 +192,10 @@ options: [
 [jvm.bench]
 options: ["-Xmx1g", "-XX:MaxPermSize=256m"]
 
-# TODO(gmalmquist): Find a way to resolve the -Xbootclasspath automatically, either by putting
-# rt.jars up on the nexus or using some kind of special variable name (see discussion on RB 2494).
+# NB(gmalmquist): You can set the bootclasspath relative to the
+# appropriate java home (inferred from the target level) by setting
+# an arg like:
+# "-C-Xbootclasspath:$JAVA_HOME/jre/lib/resources.jar:$JAVA_HOME/jre/lib/rt.jar:$JAVA_HOME/jre/lib/sunrsasign.jar:$JAVA_HOME/jre/lib/jsse.jar:$JAVA_HOME/jre/lib/jce.jar:$JAVA_HOME/jre/lib/charsets.jar:$JAVA_HOME/jre/lib/jfr.jar:$JAVA_HOME/jre/classes"
 [jvm-platform]
 default_platform: java6
 platforms: {

src/python/pants/backend/jvm/tasks/jvm_compile/zinc/zinc_compile.py

@@ -5,12 +5,14 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
+import logging
 import os
 import re
 import textwrap
 from contextlib import closing
 from xml.etree import ElementTree
 
+from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform
 from pants.backend.jvm.subsystems.scala_platform import ScalaPlatform
 from pants.backend.jvm.subsystems.shader import Shader
 from pants.backend.jvm.targets.annotation_processor import AnnotationProcessor
@@ -26,7 +28,7 @@
 from pants.base.exceptions import TaskError
 from pants.base.hash_utils import hash_file
 from pants.base.workunit import WorkUnitLabel
-from pants.java.distribution.distribution import Distribution
+from pants.java.distribution.distribution import Distribution, DistributionLocator
 from pants.util.contextutil import open_zip
 from pants.util.dirutil import safe_open
 from pants.util.memo import memoized_method, memoized_property
@@ -42,6 +44,9 @@
 _PROCESSOR_INFO_FILE = 'META-INF/services/javax.annotation.processing.Processor'
 
 
+logger = logging.getLogger(__name__)
+
+
 class BaseZincCompile(JvmCompile):
   """An abstract base class for zinc compilation tasks."""
 
@@ -83,6 +88,33 @@ def validate(idx):
     while arg_index < len(args):
       arg_index += validate(arg_index)
 
+  @staticmethod
+  def _get_zinc_arguments(settings):
+    """Extracts and formats the zinc arguments given in the jvm platform settings.
+
+    This is responsible for the symbol substitution which replaces $JAVA_HOME with the path to an
+    appropriate jvm distribution.
+
+    :param settings: The jvm platform settings from which to extract the arguments.
+    :type settings: :class:`JvmPlatformSettings`
+    """
+    zinc_args = [
+      '-C-source', '-C{}'.format(settings.source_level),
+      '-C-target', '-C{}'.format(settings.target_level),
+    ]
+    if settings.args:
+      settings_args = settings.args
+      if any('$JAVA_HOME' in a for a in settings.args):
+        try:
+          distribution = JvmPlatform.preferred_jvm_distribution([settings], strict=True)
+        except DistributionLocator.Error:
+          distribution = JvmPlatform.preferred_jvm_distribution([settings], strict=False)
+        logger.debug('Substituting "$JAVA_HOME" with "{}" in jvm-platform args.'
+                     .format(distribution.home))
+        settings_args = (a.replace('$JAVA_HOME', distribution.home) for a in settings.args)
+      zinc_args.extend(settings_args)
+    return zinc_args
+
   @classmethod
   def compiler_plugin_types(cls):
     """A tuple of target types which are compiler plugins."""
@@ -299,19 +331,14 @@ def compile(self, args, classpath, sources, classes_output_dir, upstream_analysi
     zinc_args.extend(['-sbt-interface', self.tool_jar('sbt-interface')])
     zinc_args.extend(['-scala-path', ':'.join(self.compiler_classpath())])
 
-    zinc_args += self.javac_plugin_args(javac_plugins_to_exclude)
-    zinc_args += self.scalac_plugin_args
+    zinc_args.extend(self.javac_plugin_args(javac_plugins_to_exclude))
+    zinc_args.extend(self.scalac_plugin_args)
     if upstream_analysis:
       zinc_args.extend(['-analysis-map',
                         ','.join('{}:{}'.format(*kv) for kv in upstream_analysis.items())])
 
-    zinc_args += args
-
-    zinc_args.extend([
-      '-C-source', '-C{}'.format(settings.source_level),
-      '-C-target', '-C{}'.format(settings.target_level),
-    ])
-    zinc_args.extend(settings.args)
+    zinc_args.extend(args)
+    zinc_args.extend(self._get_zinc_arguments(settings))
 
     if fatal_warnings:
       zinc_args.extend(self.get_options().fatal_warnings_enabled_args)

src/python/pants/java/distribution/distribution.py

@@ -516,7 +516,9 @@ def _locate(self, minimum_version=None, maximum_version=None, jdk=False):
         logger.debug('Located {} for constraints: minimum_version {}, maximum_version {}, jdk {}'
                      .format(dist, minimum_version, maximum_version, jdk))
         return dist
-      except (ValueError, Distribution.Error):
+      except (ValueError, Distribution.Error) as e:
+        logger.debug('{} is not a valid distribution because: {}'
+                     .format(location.home_path, str(e)))
         pass
 
     if (minimum_version is not None

tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/BUILD

@@ -67,6 +67,7 @@ python_tests(
     'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',
     'tests/python/pants_test:base_test',
+    'tests/python/pants_test/java/distribution',
   ],
 )
 

tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_java_compile_settings_partitioning.py

@@ -6,12 +6,17 @@
                         unicode_literals, with_statement)
 
 from collections import defaultdict
+from contextlib import contextmanager
 
 from pants.backend.jvm.subsystems.jvm_platform import JvmPlatformSettings
 from pants.backend.jvm.targets.java_library import JavaLibrary
 from pants.backend.jvm.tasks.jvm_compile.zinc.zinc_compile import ZincCompile
 from pants.base.revision import Revision
+from pants.java.distribution.distribution import DistributionLocator
 from pants.util.memo import memoized_method
+from pants.util.osutil import get_os_name, normalize_os_name
+from pants_test.java.distribution.test_distribution import EXE, distribution
+from pants_test.subsystem.subsystem_util import subsystem_instance
 from pants_test.tasks.task_test_base import TaskTestBase
 
 
@@ -158,3 +163,107 @@ def test_compile_setting_inequivalence(self):
 
     self.assertNotEqual(JvmPlatformSettings('1.4', '1.6', ['-Xfoo:bar']),
                         JvmPlatformSettings('1.6', '1.6', ['-Xfoo:bar']))
+
+  def test_java_home_extraction(self):
+    _, source, _, target, foo, bar, composite, single = tuple(ZincCompile._get_zinc_arguments(
+      JvmPlatformSettings('1.7', '1.7', [
+        'foo', 'bar', 'foo:$JAVA_HOME/bar:$JAVA_HOME/foobar', '$JAVA_HOME',
+      ])
+    ))
+
+    self.assertEquals('-C1.7', source)
+    self.assertEquals('-C1.7', target)
+    self.assertEquals('foo', foo)
+    self.assertEquals('bar', bar)
+    self.assertNotEqual('$JAVA_HOME', single)
+    self.assertNotIn('$JAVA_HOME', composite)
+    self.assertEquals('foo:{0}/bar:{0}/foobar'.format(single), composite)
+
+  def test_java_home_extraction_empty(self):
+    result = tuple(ZincCompile._get_zinc_arguments(
+      JvmPlatformSettings('1.7', '1.7', [])
+    ))
+    self.assertEquals(4, len(result),
+                      msg='_get_zinc_arguments did not correctly handle empty args.')
+
+  def test_java_home_extraction_missing_distributions(self):
+    # This will need to be bumped if java ever gets to major version one million.
+    far_future_version = '999999.1'
+    farer_future_version = '999999.2'
+
+    os_name = normalize_os_name(get_os_name())
+
+    @contextmanager
+    def fake_distributions(versions):
+      """Create a fake JDK for each java version in the input, and yield the list of java_homes.
+
+      :param list versions: List of java version strings.
+      """
+      fakes = []
+      for version in versions:
+        fakes.append(distribution(
+          executables=[EXE('bin/java', version), EXE('bin/javac', version)],
+        ))
+      yield [d.__enter__() for d in fakes]
+      for d in fakes:
+        d.__exit__(None, None, None)
+
+    @contextmanager
+    def fake_distribution_locator(*versions):
+      """Sets up a fake distribution locator with fake distributions.
+
+      Creates one distribution for each java version passed as an argument, and yields a list of
+      paths to the java homes for each distribution.
+      """
+      with fake_distributions(versions) as paths:
+        path_options = {
+          'jvm-distributions': {
+            'paths': {
+              os_name: paths,
+            }
+          }
+        }
+        with subsystem_instance(DistributionLocator, **path_options) as locator:
+          yield paths
+          locator._reset()
+
+    # Completely missing a usable distribution.
+    with fake_distribution_locator(far_future_version):
+      with self.assertRaises(DistributionLocator.Error):
+        ZincCompile._get_zinc_arguments(JvmPlatformSettings(
+          source_level=farer_future_version,
+          target_level=farer_future_version,
+          args=['$JAVA_HOME/foo'],
+        ))
+
+    # Missing a strict distribution.
+    with fake_distribution_locator(farer_future_version) as paths:
+      results = ZincCompile._get_zinc_arguments(JvmPlatformSettings(
+        source_level=far_future_version,
+        target_level=far_future_version,
+        args=['$JAVA_HOME/foo', '$JAVA_HOME'],
+      ))
+      self.assertEquals(paths[0], results[-1])
+      self.assertEquals('{}/foo'.format(paths[0]), results[-2])
+
+    # Make sure we pick up the strictest possible distribution.
+    with fake_distribution_locator(farer_future_version, far_future_version) as paths:
+      farer_path, far_path = paths
+      results = ZincCompile._get_zinc_arguments(JvmPlatformSettings(
+        source_level=far_future_version,
+        target_level=far_future_version,
+        args=['$JAVA_HOME/foo', '$JAVA_HOME'],
+      ))
+      self.assertEquals(far_path, results[-1])
+      self.assertEquals('{}/foo'.format(far_path), results[-2])
+
+    # Make sure we pick the higher distribution when the lower one doesn't work.
+    with fake_distribution_locator(farer_future_version, far_future_version) as paths:
+      farer_path, far_path = paths
+      results = ZincCompile._get_zinc_arguments(JvmPlatformSettings(
+        source_level=farer_future_version,
+        target_level=farer_future_version,
+        args=['$JAVA_HOME/foo', '$JAVA_HOME'],
+      ))
+      self.assertEquals(farer_path, results[-1])
+      self.assertEquals('{}/foo'.format(farer_path), results[-2])