Added JvmPlatform subsystem and added platform arg to JvmTarget.

The global compile strategy partitions targets by their platform
settings.

The partitioning also respects target dependencies, so two dependent
targets with the same compile settings may be compiled in different
chunks if they are separated by an intermediate dependency with
different settings. An example and test of this behavior can be
found in test_java_compile_settings_partitioning#test_chunks_respect_dependencies.

Testing Done:
Added lots of integration tests and unit tests under:
tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_java_compile_jvm_platform_integration.py
tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_java_compile_settings_partitioning.py
tests/python/pants_test/backend/jvm/tasks/jvm_compile/java/test_java_zinc_compile_jvm_platform_integration.py

CI went green here: https://travis-ci.org/pantsbuild/pants/builds/71341662
And here: https://travis-ci.org/pantsbuild/pants/builds/71817136
And here: https://travis-ci.org/pantsbuild/pants/builds/72320050
And here: https://travis-ci.org/pantsbuild/pants/builds/72346982
And here: https://travis-ci.org/pantsbuild/pants/builds/72854625
And here: https://travis-ci.org/pantsbuild/pants/builds/72875285
And here: https://travis-ci.org/pantsbuild/pants/builds/72923807
And here: https://travis-ci.org/pantsbuild/pants/builds/73068044
And here: https://travis-ci.org/pantsbuild/pants/builds/73070488

Bugs closed: 1826

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

migrations/options/src/python/migrate_config.py

@@ -272,9 +272,14 @@
                          'move to a subsystem, which will fix this requirement.',
   ('jvm', 'debug_args'): 'For now must be defined for each JvmTask subtask separately.  Will soon '
                          'move to a subsystem, which will fix this requirement.',
-  ('java-compile', 'javac_args'): 'source and target args should be moved to separate source: and '
-                                  'target: options. Other args should be placed in args: and '
-                                  'prefixed with -C.',
+  ('java-compile', 'javac_args'): 'Source, target, and bootclasspath args should be specified in '
+                                  'the jvm-platform subsystem. Other args can be placed in args: '
+                                  'and prefixed with -C, or also be included in the jvm-platform '
+                                  'args.',
+  ('java-compile', 'source'): 'source and target args should be defined using the jvm-platform '
+                              'subsystem, rathern than as arguments to java-compile.',
+  ('java-compile', 'target'): 'source and target args should be defined using the jvm-platform '
+                              'subsystem, rathern than as arguments to java-compile.',
   ('jar-tool', 'bootstrap_tools'): 'Each JarTask sub-task can define this in its own section. or '
                                    'this can be defined for everyone in the DEFAULT section.',
   ('ivy-resolve', 'jvm_args'): 'If needed, this should be repeated in resolve.ivy, '

pants.ini

@@ -94,8 +94,6 @@ options: ["-Xmx1g", "-XX:MaxPermSize=256m", "-Dscala.usejavacp=true" ]
 
 [compile.java]
 partition_size_hint: 1000000000
-source: 6
-target: 6
 compiler-bootstrap-tools: ["//:java-compiler"]
 jvm_options: ["-Xmx2G"]
 
@@ -120,6 +118,15 @@ 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).
+[jvm-platform]
+default_platform: java6
+platforms: {
+    'java6': {'source': '6', 'target': '6', 'args': [] },
+    'java7': {'source': '7', 'target': '7', 'args': [] },
+    'java8': {'source': '8', 'target': '8', 'args': [] },
+  }
 
 [idea]
 python_source_paths: ["src/python"]

src/python/pants/backend/core/tasks/task.py

@@ -10,6 +10,7 @@
 import sys
 from abc import abstractmethod
 from contextlib import contextmanager
+from hashlib import sha1
 
 from twitter.common.collections.orderedset import OrderedSet
 
@@ -21,7 +22,6 @@
 from pants.cache.artifact_cache import UnreadableArtifact, call_insert, call_use_cached_files
 from pants.cache.cache_setup import CacheSetup
 from pants.option.optionable import Optionable
-from pants.option.options import Options
 from pants.option.scope import ScopeInfo
 from pants.reporting.reporting_utils import items_to_report_element
 from pants.util.meta import AbstractClass
@@ -197,6 +197,9 @@ def workdir(self):
     """
     return self._workdir
 
+  def _options_fingerprint(self, scope):
+    return str(self.context.options.payload_for_scope(scope).fingerprint(context=self.context))
+
   @property
   def fingerprint(self):
     """Returns a fingerprint for the identity of the task.
@@ -208,8 +211,12 @@ def fingerprint(self):
     A task's fingerprint is only valid afer the task has been fully initialized.
     """
     if not self._fingerprint:
-      payload = self.context.options.payload_for_scope(self.options_scope)
-      self._fingerprint = payload.fingerprint(context=self.context)
+      hasher = sha1()
+      hasher.update(self._options_fingerprint(self.options_scope))
+      for subsystem in self.task_subsystems():
+        hasher.update(self._options_fingerprint(subsystem.subscope(self.options_scope)))
+        hasher.update(self._options_fingerprint(subsystem.options_scope))
+      self._fingerprint = str(hasher.hexdigest())
     return self._fingerprint
 
   def artifact_cache_reads_enabled(self):

src/python/pants/backend/jvm/subsystems/BUILD

@@ -32,6 +32,17 @@ python_library(
   ],
 )
 
+python_library(
+  name = 'jvm_platform',
+  sources = ['jvm_platform.py'],
+  dependencies = [
+    'src/python/pants/base:revision',
+    'src/python/pants/option',
+    'src/python/pants/subsystem',
+    'src/python/pants/util:memo',
+    'src/python/pants/java/distribution',
+  ],
+)
 
 python_library(
   name = 'scala_platform',
@@ -41,4 +52,4 @@ python_library(
     'src/python/pants/option',
     'src/python/pants/subsystem',
   ],
-)
+)

src/python/pants/backend/jvm/subsystems/jvm_platform.py

@@ -0,0 +1,196 @@
+# coding=utf-8
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
+                        unicode_literals, with_statement)
+
+import logging
+
+from pants.base.exceptions import TaskError
+from pants.base.revision import Revision
+from pants.java.distribution.distribution import Distribution
+from pants.option.options import Options
+from pants.subsystem.subsystem import Subsystem
+from pants.util.memo import memoized_method, memoized_property
+
+
+logger = logging.getLogger(__name__)
+
+
+class JvmPlatform(Subsystem):
+  """Used to keep track of repo compile settings."""
+
+  # NB(gmalmquist): These assume a java version number N can be specified as either 'N' or '1.N'
+  # (eg, '7' is equivalent to '1.7'). New versions should only be added to this list
+  # if they follow this convention. If this convention is ever not followed for future
+  # java releases, they can simply be omitted from this list and they will be parsed
+  # strictly (eg, if Java 10 != 1.10, simply leave it out).
+  SUPPORTED_CONVERSION_VERSIONS = (6, 7, 8,)
+
+  class IllegalDefaultPlatform(TaskError):
+    """The --default-platform option was set, but isn't defined in --platforms."""
+
+  class UndefinedJvmPlatform(TaskError):
+    """Platform isn't defined."""
+    def __init__(self, target, platform_name, platforms_by_name):
+      scope_name = JvmPlatform.options_scope
+      messages = ['Undefined jvm platform "{}" (referenced by {}).'
+                    .format(platform_name, target.address.spec if target else 'unknown target')]
+      if not platforms_by_name:
+        messages.append('In fact, no platforms are defined under {0}. These should typically be'
+                        ' specified in [{0}] in pants.ini.'.format(scope_name))
+      else:
+        messages.append('Perhaps you meant one of:{}'.format(
+          ''.join('\n  {}'.format(name) for name in sorted(platforms_by_name.keys()))
+        ))
+        messages.append('\nThese are typically defined under [{}] in pants.ini.'
+                        .format(scope_name))
+      super(JvmPlatform.UndefinedJvmPlatform, self).__init__(' '.join(messages))
+
+  options_scope = 'jvm-platform'
+
+  # Mapping to keep version numbering consistent for ease of comparison.
+
+  @classmethod
+  def register_options(cls, register):
+    super(JvmPlatform, cls).register_options(register)
+    register('--platforms', advanced=True, type=Options.dict, default={}, fingerprint=True,
+             help='Compile settings that can be referred to by name in jvm_targets.')
+    register('--default-platform', advanced=True, type=str, default=None, fingerprint=True,
+             help='Name of the default platform to use if none are specified.')
+
+  def _parse_platform(self, name, platform):
+    return JvmPlatformSettings(platform.get('source', platform.get('target')),
+                               platform.get('target', platform.get('source')),
+                               platform.get('args', ()),
+                               name=name)
+
+  @memoized_property
+  def platforms_by_name(self):
+    platforms = self.get_options().platforms or {}
+    return { name: self._parse_platform(name, platform) for name, platform in platforms.items() }
+
+  @property
+  def _fallback_platform(self):
+    logger.warn('No default jvm platform is defined.')
+    source_level = JvmPlatform.parse_java_version(Distribution.cached().version)
+    target_level = source_level
+    platform_name = '(Distribution.cached().version {})'.format(source_level)
+    return JvmPlatformSettings(source_level, target_level, [], name=platform_name)
+
+  @memoized_property
+  def default_platform(self):
+    name = self.get_options().default_platform
+    if not name:
+      return self._fallback_platform
+    platforms_by_name = self.platforms_by_name
+    if name not in platforms_by_name:
+      raise self.IllegalDefaultPlatform(
+          "The default platform was set to '{0}', but no platform by that name has been "
+          "defined. Typically, this should be defined under [{1}] in pants.ini."
+          .format(name, self.options_scope)
+      )
+    return JvmPlatformSettings(*platforms_by_name[name], name='{} (by default)'.format(name))
+
+  @memoized_method
+  def get_platform_by_name(self, name, for_target=None):
+    """Finds the platform with the given name.
+
+    If the name is empty or None, returns the default platform.
+    If not platform with the given name is defined, raises an error.
+    :param str name: name of the platform.
+    :param JvmTarget for_target: optionally specified target we're looking up the platform for.
+      Only used in error message generation.
+    :return: The jvm platform object.
+    :rtype: JvmPlatformSettings
+    """
+    if not name:
+      return self.default_platform
+    if name not in self.platforms_by_name:
+      raise self.UndefinedJvmPlatform(for_target, name, self.platforms_by_name)
+    return self.platforms_by_name[name]
+
+  def get_platform_for_target(self, target):
+    if not target.payload.platform and target.is_synthetic:
+      derived_from = target.derived_from
+      platform = derived_from and getattr(derived_from, 'platform', None)
+      if platform:
+        return platform
+    return self.get_platform_by_name(target.payload.platform, target)
+
+  @classmethod
+  def parse_java_version(cls, version):
+    """Parses the java version (given a string or Revision object).
+
+    Handles java version-isms, converting things like '7' -> '1.7' appropriately.
+
+    Truncates input versions down to just the major and minor numbers (eg, 1.6), ignoring extra
+    versioning information after the second number.
+
+    :param version: the input version, given as a string or Revision object.
+    :return: the parsed and cleaned version, suitable as a javac -source or -target argument.
+    :rtype: Revision
+    """
+    conversion = { str(i): '1.{}'.format(i) for i in cls.SUPPORTED_CONVERSION_VERSIONS }
+    if str(version) in conversion:
+      return Revision.lenient(conversion[str(version)])
+
+    if not hasattr(version, 'components'):
+      version = Revision.lenient(version)
+    if len(version.components) <= 2:
+      return version
+    return Revision(*version.components[:2])
+
+
+class JvmPlatformSettings(object):
+  """Simple information holder to keep track of common arguments to java compilers."""
+
+  class IllegalSourceTargetCombination(TaskError):
+    """Illegal pair of -source and -target flags to compile java."""
+
+  def __init__(self, source_level, target_level, args, name=None):
+    """
+    :param source_level: Revision object or string for the java source level.
+    :param target_level: Revision object or string for the java target level.
+    :param list args: Additional arguments to pass to the java compiler.
+    :param str name: optional name to identify this platform; not used for anything but logging.
+    """
+    self.source_level = JvmPlatform.parse_java_version(source_level)
+    self.target_level = JvmPlatform.parse_java_version(target_level)
+    self.args = tuple(args or ())
+    self.name = name
+    self._validate_source_target()
+
+  def _validate_source_target(self):
+    if self.source_level > self.target_level:
+      raise self.IllegalSourceTargetCombination(
+        'Platform {platform} has java source level {source_level} but target level {target_level}.'
+        .format(platform=self.name,
+                source_level=self.source_level,
+                target_level=self.target_level)
+      )
+
+  def __iter__(self):
+    yield self.source_level
+    yield self.target_level
+    yield self.args
+
+  def __eq__(self, other):
+    return tuple(self) == tuple(other)
+
+  def __ne__(self, other):
+    return not self.__eq__(other)
+
+  def __hash__(self):
+    return hash(tuple(self))
+
+  def __cmp__(self, other):
+    return cmp(tuple(self), tuple(other))
+
+  def __str__(self):
+    return 'source={source},target={target},args=({args})'.format(
+      source=self.source_level,
+      target=self.target_level,
+      args=' '.join(self.args)
+    )

src/python/pants/backend/jvm/targets/BUILD

@@ -29,6 +29,7 @@ python_library(
     '3rdparty/python/twitter/commons:twitter.common.collections',
     '3rdparty/python/twitter/commons:twitter.common.dirutil',
     'src/python/pants/backend/core/targets:common',
+    'src/python/pants/backend/jvm/subsystems:jvm_platform',
     'src/python/pants/base:address_lookup_error',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:build_manual',

src/python/pants/backend/jvm/targets/jvm_target.py

@@ -6,18 +6,23 @@
                         unicode_literals, with_statement)
 
 from pants.backend.core.targets.resources import Resources
+from pants.backend.jvm.subsystems.jvm_platform import JvmPlatform
 from pants.backend.jvm.targets.exclude import Exclude
 from pants.backend.jvm.targets.jar_library import JarLibrary
 from pants.backend.jvm.targets.jarable import Jarable
 from pants.base.payload import Payload
-from pants.base.payload_field import ConfigurationsField, ExcludesField
+from pants.base.payload_field import ConfigurationsField, ExcludesField, PrimitiveField
 from pants.base.target import Target
 from pants.util.memo import memoized_property
 
 
 class JvmTarget(Target, Jarable):
   """A base class for all java module targets that provides path and dependency translation."""
 
+  @classmethod
+  def subsystems(cls):
+    return super(JvmTarget, cls).subsystems() + (JvmPlatform,)
+
   def __init__(self,
                address=None,
                payload=None,
@@ -29,6 +34,7 @@ def __init__(self,
                configurations=None,
                no_cache=False,
                services=None,
+               platform=None,
                **kwargs):
     """
     :param configurations: One or more ivy configurations to resolve for this target.
@@ -46,6 +52,11 @@ def __init__(self,
                      by this target that implements the service interface and should be
                      discoverable by the jvm service provider discovery mechanism described here:
                      https://docs.oracle.com/javase/6/docs/api/java/util/ServiceLoader.html
+    :param str platform: The name of the platform (defined under the jvm-platform subsystem) to use
+      for compilation (that is, a key into the --jvm-platform-platforms dictionary). If unspecified,
+      the platform will default to the first one of these that exist: (1) the default_platform
+      specified for jvm-platform, (2) a platform constructed from whatever java version is returned
+      by Distribution.cached().version.
     """
     self.address = address  # Set in case a TargetDefinitionException is thrown early
     if sources_rel_path is None:
@@ -56,6 +67,7 @@ def __init__(self,
       'provides': provides,
       'excludes': ExcludesField(self.assert_list(excludes, expected_type=Exclude, key_arg='excludes')),
       'configurations': ConfigurationsField(self.assert_list(configurations, key_arg='configurations')),
+      'platform': PrimitiveField(platform),
     })
     self._resource_specs = self.assert_list(resources, key_arg='resources')
 
@@ -70,6 +82,10 @@ def __init__(self,
     if no_cache:
       self.add_labels('no_cache')
 
+  @property
+  def platform(self):
+    return JvmPlatform.global_instance().get_platform_for_target(self)
+
   @memoized_property
   def jar_dependencies(self):
     return set(self.get_jar_dependencies())

src/python/pants/backend/jvm/tasks/jvm_compile/BUILD

@@ -64,6 +64,7 @@ python_library(
     ':jvm_compile_isolated_strategy',
     ':jvm_dependency_analyzer',
     'src/python/pants/backend/core/tasks:group_task',
+    'src/python/pants/backend/jvm/subsystems:jvm_platform',
     'src/python/pants/backend/jvm/tasks:nailgun_task',
     'src/python/pants/goal:products',
     'src/python/pants/option',