Remove the JVM global compile strategy, switch default java compiler …

…to zinc On the assumption that Foursquare's switch to the isolated compile strategy will stick, this patch deletes the global strategy. - Inline all of the isolated strategy into jvm_compile - Merge pants.ini files - Delete the global strategy and strategy interface - Make `zinc` the default for java compiles since jmake does not support partitioned analysis - Update `migrate_config` to indicate the global-related options will be ignored Testing Done: pantsbuild#2227 Reviewed at https://rbcommons.com/s/twitter/r/2852/
michaelttran · Sep 23, 2015 · c4d964d · c4d964d
1 parent 07f0235
commit c4d964d
Show file tree

Hide file tree

Showing 48 changed files with 860 additions and 1,813 deletions.
diff --git a/3rdparty/BUILD b/3rdparty/BUILD
@@ -96,8 +96,13 @@ jar_library(name='slf4j-api',
 jar_library(name = 'spindle-runtime',
             jars = [
               jar(org = 'com.foursquare', name = 'spindle-runtime_2.10', rev = '3.0.0-M7'),
+            ],
+            dependencies = [
+              'contrib/spindle/3rdparty:rogue',
             ])
 
+# NB: we have two versions of thrift here due to scrooge requiring one version while apache
+# thrift requires another. this is not usually recommended
 jar_library(name='libthrift-0.9.2',
             jars = [
               jar(org='org.apache.thrift', name='libthrift', rev='0.9.2')
@@ -110,7 +115,17 @@ target(name='thrift-0.9.2',
          ':slf4j-api',
        ])
 
-target(name='thrift', dependencies = [ ':thrift-0.9.2' ])
+jar_library(name='libthrift-0.6.1',
+            jars = [
+              jar(org='org.apache.thrift', name='libthrift', rev='0.6.1')
+            ])
+
+target(name='thrift-0.6.1',
+       dependencies = [
+         ':commons-lang',
+         ':libthrift-0.6.1',
+         ':slf4j-api',
+       ])
 
 
 ###############

diff --git a/3rdparty/jvm/com/twitter/BUILD b/3rdparty/jvm/com/twitter/BUILD
@@ -0,0 +1,22 @@
+# Copyright 2015 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+jar_library(name='finagle-thrift',
+  jars=[
+    jar(org='com.twitter', name='finagle-thrift_2.10', rev='6.28.0')
+      .exclude(org = 'org.apache.thrift', name = 'libthrift'),
+  ],
+  dependencies=[
+    '3rdparty:thrift-0.6.1',
+  ],
+)
+
+jar_library(name='scrooge-core',
+  jars=[
+    jar(org='com.twitter', name='scrooge-core_2.10', rev='3.20.0')
+      .exclude(org = 'org.apache.thrift', name = 'libthrift'),
+  ],
+  dependencies=[
+    '3rdparty:thrift-0.6.1',
+  ],
+)
diff --git a/BUILD.tools b/BUILD.tools
@@ -32,12 +32,12 @@ jar_library(name = 'scrooge-gen',
             jars = [
               jar(org='com.twitter', name='scrooge-generator_2.10', rev='3.20.0')
                 # scrooge requires libthrift 0.5.0-1 which is not available on
-                # the default maven repos. Force scrooge to use the lib thirft used
+                # the default maven repos. Force scrooge to use the lib thrift used
                 # by pants: 3rdparty:thrift
                 .exclude(org = 'org.apache.thrift', name = 'libthrift')
             ],
             dependencies = [
-              '3rdparty:thrift',
+              '3rdparty:thrift-0.6.1',
             ])
 
 jar_library(name = 'scrooge-linter',
@@ -46,5 +46,5 @@ jar_library(name = 'scrooge-linter',
                 .exclude(org = 'org.apache.thrift', name = 'libthrift')
             ],
             dependencies = [
-              '3rdparty:thrift',
+              '3rdparty:thrift-0.6.1',
             ])
diff --git a/contrib/scrooge/tests/thrift/org/pantsbuild/contrib/scrooge/android_generator/BUILD b/contrib/scrooge/tests/thrift/org/pantsbuild/contrib/scrooge/android_generator/BUILD
@@ -4,7 +4,7 @@ java_thrift_library(
   compiler='scrooge',
   language='android',
   rpc_style='finagle',
-  dependencies=['3rdparty:thrift'],
+  dependencies=['3rdparty:thrift-0.6.1'],
   provides=artifact(
     org='org.archimedes',
     name='android_generator',

diff --git a/examples/src/python/example/pants_publish_plugin/extra_test_jar_example.py b/examples/src/python/example/pants_publish_plugin/extra_test_jar_example.py
@@ -25,6 +25,7 @@ class ExtraTestJarExample(JarTask):
   will create an 'example.txt' file, which will be placed in an additional jar. During publishing,
   this additional jar will be published along with the target.
   """
+
   def __init__(self, context, workdir):
     # Constructor for custom task. Setup things that you need at pants initialization time.
     super(ExtraTestJarExample, self).__init__(context, workdir)

diff --git a/examples/tests/python/example_test/usethriftpy/use_thrift_test.py b/examples/tests/python/example_test/usethriftpy/use_thrift_test.py
@@ -12,8 +12,6 @@
 
 
 # Illustrate using Thrift-generated code from Python.
-
-
 class UseThriftTest(unittest.TestCase):
   def test_make_it_rain(self):
     distance = Distance()

diff --git a/migrations/options/src/python/migrate_config.py b/migrations/options/src/python/migrate_config.py
@@ -325,8 +325,22 @@
   ('test.junit', 'junit'): None,
   ('thrift-linter', 'nailgun-server'): None,
   ('thrift-linter', 'scrooge-linter'): None,
+
+  # Global strategy removal.
+  ('compile.apt', 'changed-targets-heuristic-limit'): None,
+  ('compile.apt', 'partition-size-hint'): None,
+  ('compile.apt', 'strategy'): None,
+  ('compile.java', 'changed-targets-heuristic-limit'): None,
+  ('compile.java', 'partition-size-hint'): None,
+  ('compile.java', 'strategy'): None,
+  ('compile.zinc', 'changed-targets-heuristic-limit'): None,
+  ('compile.zinc', 'partition-size-hint'): None,
+  ('compile.zinc', 'strategy'): None,
 }
 
+jvm_global_strategy_removal = ('The JVM global compile strategy was removed in favor of the '
+                               'isolated strategy, which uses a different set of options.')
+
 ng_daemons_note = ('The global "ng_daemons" option has been replaced by a "use_nailgun" option '
                    'local to each task that can use a nailgun.  A default can no longer be '
                    'specified at intermediate scopes; ie: "compile" when the option is present in '
@@ -499,6 +513,17 @@
   ('test.junit', 'junit'): jvm_tool_spec_override,
   ('thrift-linter', 'nailgun-server'): jvm_tool_spec_override,
   ('thrift-linter', 'scrooge-linter'): jvm_tool_spec_override,
+
+  # Global strategy removal.
+  ('compile.apt', 'changed-targets-heuristic-limit'): jvm_global_strategy_removal,
+  ('compile.apt', 'partition-size-hint'): jvm_global_strategy_removal,
+  ('compile.apt', 'strategy'): jvm_global_strategy_removal,
+  ('compile.java', 'changed-targets-heuristic-limit'): jvm_global_strategy_removal,
+  ('compile.java', 'partition-size-hint'): jvm_global_strategy_removal,
+  ('compile.java', 'strategy'): jvm_global_strategy_removal,
+  ('compile.zinc', 'changed-targets-heuristic-limit'): jvm_global_strategy_removal,
+  ('compile.zinc', 'partition-size-hint'): jvm_global_strategy_removal,
+  ('compile.zinc', 'strategy'): jvm_global_strategy_removal,
 }
 
 

diff --git a/pants.ini b/pants.ini
@@ -74,9 +74,36 @@ ivy_settings: %(pants_supportdir)s/ivy/ivysettings.xml
 ivy_profile: %(pants_supportdir)s/ivy/ivy.xml
 
 
+[gen.scrooge]
+service_deps: {
+    'java': [
+      '//:scala-library',
+      '3rdparty:slf4j-api',
+      '3rdparty:thrift-0.6.1',
+      '3rdparty/jvm/com/twitter:finagle-thrift',
+      '3rdparty/jvm/com/twitter:scrooge-core',
+    ],
+    'scala': [
+      '3rdparty:thrift-0.6.1',
+      '3rdparty/jvm/com/twitter:finagle-thrift',
+      '3rdparty/jvm/com/twitter:scrooge-core',
+    ],
+  }
+structs_deps: {
+    'java': [
+      '3rdparty:thrift-0.6.1',
+      '3rdparty/jvm/com/twitter:scrooge-core',
+    ],
+    'scala': [
+      '3rdparty:thrift-0.6.1',
+      '3rdparty/jvm/com/twitter:scrooge-core',
+    ],
+  }
+
+
 [gen.thrift]
 gen_options: hashcode
-deps: ["3rdparty:thrift"]
+deps: ["3rdparty:thrift-0.9.2"]
 
 
 [compile.checkstyle]
@@ -116,14 +143,28 @@ skip: True
 [pycheck-except-statement]
 skip: True
 
-[compile.java]
-partition_size_hint: 1000000000
-compiler-bootstrap-tools: ["//:java-compiler"]
-jvm_options: ["-Xmx2G"]
-
 
 [compile.zinc]
-jvm_options: ["-Xmx2g", "-XX:MaxPermSize=256m", "-Dzinc.analysis.cache.limit=0"]
+worker_count: 4
+jvm_options: [
+    '-Xmx4g', '-XX:MaxPermSize=512m', '-XX:+UseConcMarkSweepGC', '-XX:ParallelGCThreads=4',
+    # bigger cache size for our big projects (default is just 5)
+    '-Dzinc.analysis.cache.limit=1000',
+  ]
+
+args: [
+    '-S-encoding', '-SUTF-8',
+    '-S-g:vars',
+  ]
+warning_args: [
+    '-S-deprecation',
+    '-S-unchecked',
+    # request warnings for http://www.scala-lang.org/api/2.10.4/index.html#scala.language$
+    '-S-feature',
+  ]
+no_warning_args: [
+    '-S-nowarn',
+  ]
 
 
 [jvm.run.jvm]

diff --git a/pants.ini.isolated b/pants.ini.isolated
diff --git a/src/python/pants/backend/codegen/tasks/BUILD b/src/python/pants/backend/codegen/tasks/BUILD
@@ -132,6 +132,7 @@ python_library(
     'src/python/pants/base:address',
     'src/python/pants/base:address_lookup_error',
     'src/python/pants/base:build_environment',
+    'src/python/pants/base:dep_lookup_error',
     'src/python/pants/base:workunit',
     'src/python/pants/util:memo',
   ],

diff --git a/src/python/pants/backend/codegen/tasks/simple_codegen_task.py b/src/python/pants/backend/codegen/tasks/simple_codegen_task.py
@@ -16,6 +16,7 @@
 from pants.base.address_lookup_error import AddressLookupError
 from pants.base.build_environment import get_buildroot
 from pants.base.build_graph import sort_targets
+from pants.base.dep_lookup_error import DepLookupError
 from pants.base.exceptions import TaskError
 from pants.base.workunit import WorkUnitLabel
 from pants.util.dirutil import safe_rmtree, safe_walk
@@ -284,7 +285,7 @@ def resolve_deps(self, unresolved_deps):
       try:
         deps.update(self.context.resolve(dep))
       except AddressLookupError as e:
-        raise self.DepLookupError('{message}\n  on dependency {dep}'.format(message=e, dep=dep))
+        raise DepLookupError('{message}\n  on dependency {dep}'.format(message=e, dep=dep))
     return deps
 
   class CodegenStrategy(AbstractClass):

diff --git a/src/python/pants/backend/jvm/subsystems/BUILD b/src/python/pants/backend/jvm/subsystems/BUILD
@@ -5,7 +5,7 @@ python_library(
   name = 'jvm_tool_mixin',
   sources = ['jvm_tool_mixin.py'],
   dependencies = [
-    'src/python/pants/base:address_lookup_error',
+    'src/python/pants/base:dep_lookup_error',
     'src/python/pants/base:exceptions',
   ],
 )

diff --git a/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py b/src/python/pants/backend/jvm/subsystems/jvm_tool_mixin.py
@@ -8,7 +8,7 @@
 from collections import namedtuple
 from textwrap import dedent
 
-from pants.base.address_lookup_error import AddressLookupError
+from pants.base.dep_lookup_error import DepLookupError
 from pants.base.exceptions import TaskError
 
 
@@ -17,10 +17,6 @@ class JvmToolMixin(object):
 
   Must be mixed in to something that can register and use options, e.g., a Task or a Subsystem.
   """
-  class DepLookupError(AddressLookupError):
-    """Thrown when a dependency can't be found."""
-    pass
-
   class InvalidToolClasspath(TaskError):
     """Indicates an invalid jvm tool classpath."""
 

diff --git a/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD b/src/python/pants/backend/jvm/tasks/jvm_compile/BUILD
@@ -66,54 +66,25 @@ python_library(
 python_library(
   name = 'jvm_compile',
   sources = ['jvm_compile.py'],
-  dependencies = [
-    ':jvm_compile_global_strategy',
-    ':jvm_compile_isolated_strategy',
-    '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/base:exceptions',
-    'src/python/pants/base:fingerprint_strategy',
-    'src/python/pants/base:workunit',
-    'src/python/pants/goal:products',
-    'src/python/pants/option',
-    'src/python/pants/reporting',
-  ],
-)
-
-python_library(
-  name = 'jvm_compile_global_strategy',
-  sources = ['jvm_compile_global_strategy.py'],
   dependencies = [
     '3rdparty/python/twitter/commons:twitter.common.collections',
     ':compile_context',
-    ':jvm_compile_strategy',
+    ':execution_graph',
     ':resource_mapping',
-    'src/python/pants/backend/jvm/targets:jvm',
+    'src/python/pants/backend/core/tasks:group_task',
+    'src/python/pants/backend/jvm/subsystems:jvm_platform',
     'src/python/pants/backend/jvm/tasks:classpath_util',
+    'src/python/pants/backend/jvm/tasks:nailgun_task',
     'src/python/pants/base:build_environment',
     'src/python/pants/base:exceptions',
+    'src/python/pants/base:fingerprint_strategy',
     'src/python/pants/base:target',
     'src/python/pants/base:worker_pool',
+    'src/python/pants/goal:products',
     'src/python/pants/option',
+    'src/python/pants/reporting',
     'src/python/pants/util:contextutil',
     'src/python/pants/util:dirutil',
-  ],
-)
-
-python_library(
-  name = 'jvm_compile_isolated_strategy',
-  sources = ['jvm_compile_isolated_strategy.py'],
-  dependencies = [
-    ':compile_context',
-    ':execution_graph',
-    ':jvm_compile_strategy',
-    ':resource_mapping',
-    'src/python/pants/backend/jvm/tasks:classpath_util',
-    'src/python/pants/base:build_environment',
-    'src/python/pants/base:target',
-    'src/python/pants/base:worker_pool',
-    'src/python/pants/util:dirutil',
     'src/python/pants/util:fileutil',
   ],
 )
@@ -126,18 +97,6 @@ python_library(
   ],
 )
 
-python_library(
-  name = 'jvm_compile_strategy',
-  sources = ['jvm_compile_strategy.py'],
-  dependencies = [
-    '3rdparty/python/twitter/commons:twitter.common.collections',
-    'src/python/pants/base:exceptions',
-    'src/python/pants/base:build_environment',
-    'src/python/pants/util:dirutil',
-    'src/python/pants/util:contextutil',
-  ],
-)
-
 python_library(
   name = 'resource_mapping',
   sources = ['resource_mapping.py'],