Refactor the thrift codegen task. (pantsbuild#4155)

Almost all the logic is now in a base class that thrift gen 
for other languages can share.

To prove that this works, created a python thrift gen task
in only a handful of lines of code.

3rdparty/python/requirements.txt

@@ -25,5 +25,5 @@ scandir==1.2
 setproctitle==1.1.10
 setuptools==30.0.0
 six>=1.9.0,<2
-thrift==0.9.1
+thrift>=0.9.1
 wheel==0.29.0

contrib/python/src/python/pants/contrib/python/checks/tasks/checkstyle/checker.py

@@ -74,7 +74,8 @@ def supports_passthru_args(cls):
     return True
 
   def _is_checked(self, target):
-    return isinstance(target, PythonTarget) and target.has_sources(self._PYTHON_SOURCE_EXTENSION)
+    return (not target.is_synthetic and isinstance(target, PythonTarget) and
+            target.has_sources(self._PYTHON_SOURCE_EXTENSION))
 
   @classmethod
   def clear_plugins(cls):
@@ -168,7 +169,7 @@ def checkstyle(self, sources):
     return failure_count
 
   def execute(self):
-    """Run Checkstyle on all found source files."""
+    """Run Checkstyle on all found non-synthetic source files."""
     if self.options.skip:
       return
 

contrib/python/src/python/pants/contrib/python/checks/tasks/python_eval.py

@@ -104,6 +104,9 @@ def _compile_target(self, target):
       if isinstance(target, PythonBinary):
         source = 'entry_point {}'.format(target.entry_point)
         components = target.entry_point.rsplit(':', 1)
+        if not all([x.strip() for x in components]):
+          raise TaskError('Invalid entry point {} for target {}'.format(
+            target.entry_point, target.address.spec))
         module = components[0]
         if len(components) == 2:
           function = components[1]
@@ -121,8 +124,9 @@ def _compile_target(self, target):
               module_path, _ = os.path.splitext(path)
             source = 'file {}'.format(os.path.join(target.target_base, path))
             module = module_path.replace(os.path.sep, '.')
-            data = TemplateData(source=source, import_statement='import {}'.format(module))
-            modules.append(data)
+            if module:
+              data = TemplateData(source=source, import_statement='import {}'.format(module))
+              modules.append(data)
 
       if not modules:
         # Nothing to eval, so a trivial compile success.

migrations/options/src/python/migrate_config.py

@@ -19,7 +19,7 @@
   #('backends', 'packages'): ('DEFAULT', 'backend_packages'),
   #('unknown-arguments', 'ignored'): None,
 
-  ('unknown-arguments', 'ignored'): ('target-arguments', 'ignored'),
+  ('gen', 'thrift'): ('gen', 'thrift-java'),
 }
 
 

pants.ini

@@ -115,11 +115,15 @@ structs_deps: {
   }
 
 
-[gen.thrift]
-gen_options: hashcode
+[gen.thrift-java]
+gen_options_map: {'hashcode': ''}
 deps: ["3rdparty:thrift-0.9.2"]
 
 
+[gen.thrift-py]
+deps: ["3rdparty/python:thrift"]
+
+
 [compile.checkstyle]
 configuration: %(pants_supportdir)s/checkstyle/coding_style.xml
 

src/python/pants/backend/codegen/register.py

@@ -14,8 +14,9 @@
 from pants.backend.codegen.protobuf.java.protobuf_gen import ProtobufGen
 from pants.backend.codegen.ragel.java.java_ragel_library import JavaRagelLibrary
 from pants.backend.codegen.ragel.java.ragel_gen import RagelGen
-from pants.backend.codegen.thrift.java.apache_thrift_gen import ApacheThriftGen
+from pants.backend.codegen.thrift.java.apache_thrift_java_gen import ApacheThriftJavaGen
 from pants.backend.codegen.thrift.java.java_thrift_library import JavaThriftLibrary
+from pants.backend.codegen.thrift.python.apache_thrift_py_gen import ApacheThriftPyGen
 from pants.backend.codegen.thrift.python.python_thrift_library import PythonThriftLibrary
 from pants.backend.codegen.wire.java.java_wire_library import JavaWireLibrary
 from pants.backend.codegen.wire.java.wire_gen import WireGen
@@ -39,7 +40,8 @@ def build_file_aliases():
 
 
 def register_goals():
-  task(name='thrift', action=ApacheThriftGen).install('gen')
+  task(name='thrift-java', action=ApacheThriftJavaGen).install('gen')
+  task(name='thrift-py', action=ApacheThriftPyGen).install('gen')
 
   # TODO(Garrett Malmquist): 'protoc' depends on a nonlocal goal (imports is in the jvm register).
   # This should be cleaned up, with protobuf stuff moved to its own backend. (See John's comment on

src/python/pants/backend/codegen/subsystems/thrift_defaults.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.thrift.java.thrift_defaults instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.thrift.java.thrift_defaults instead')
 
 ThriftDefaults = ThriftDefaults

src/python/pants/backend/codegen/targets/java_antlr_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.antlr.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.antlr.java instead')
 
 JavaAntlrLibrary = JavaAntlrLibrary

src/python/pants/backend/codegen/targets/java_protobuf_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.protobuf.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.protobuf.java instead')
 
 JavaProtobufLibrary = JavaProtobufLibrary

src/python/pants/backend/codegen/targets/java_ragel_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.ragel.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.ragel.java instead')
 
 JavaRagelLibrary = JavaRagelLibrary

src/python/pants/backend/codegen/targets/java_thrift_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.thrift.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.thrift.java instead')
 
 JavaThriftLibrary = JavaThriftLibrary

src/python/pants/backend/codegen/targets/java_wire_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.wire.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.wire.java instead')
 
 JavaWireLibrary = JavaWireLibrary

src/python/pants/backend/codegen/targets/jaxb_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.jaxb instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.jaxb instead')
 
 JaxbLibrary = JaxbLibrary

src/python/pants/backend/codegen/targets/python_antlr_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.antlr.python instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.antlr.python instead')
 
 PythonAntlrLibrary = PythonAntlrLibrary

src/python/pants/backend/codegen/targets/python_thrift_library.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.thrift.python instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.thrift.python instead')
 
 PythonThriftLibrary = PythonThriftLibrary

src/python/pants/backend/codegen/tasks/antlr_gen.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.antlr.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.antlr.java instead')
 
 AntlrGen = AntlrGen

src/python/pants/backend/codegen/tasks/apache_thrift_gen.py

@@ -5,10 +5,10 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-from pants.backend.codegen.thrift.java.apache_thrift_gen import ApacheThriftGen
+from pants.backend.codegen.thrift.java.apache_thrift_java_gen import ApacheThriftJavaGen
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.thrift.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.thrift.java instead')
 
-ApacheThriftGen = ApacheThriftGen
+ApacheThriftGen = ApacheThriftJavaGen

src/python/pants/backend/codegen/tasks/jaxb_gen.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.jaxb instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.jaxb instead')
 
 JaxbGen = JaxbGen

src/python/pants/backend/codegen/tasks/protobuf_gen.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.protobuf.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.protobuf.java instead')
 
 ProtobufGen = ProtobufGen

src/python/pants/backend/codegen/tasks/ragel_gen.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.ragel.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.ragel.java instead')
 
 RagelGen = RagelGen

src/python/pants/backend/codegen/tasks/simple_codegen_task.py

@@ -9,6 +9,6 @@
 from pants.task.simple_codegen_task import SimpleCodegenTask
 
 
-deprecated_module('1.5.0', 'Use pants.task.simple_codegen_task instead')
+deprecated_module('1.5.0dev0', 'Use pants.task.simple_codegen_task instead')
 
 SimpleCodegenTask = SimpleCodegenTask

src/python/pants/backend/codegen/tasks/wire_gen.py

@@ -9,6 +9,6 @@
 from pants.base.deprecated import deprecated_module
 
 
-deprecated_module('1.5.0', 'Use pants.backend.codegen.wire.java instead')
+deprecated_module('1.5.0dev0', 'Use pants.backend.codegen.wire.java instead')
 
 WireGen = WireGen

src/python/pants/backend/codegen/thrift/java/BUILD

@@ -4,16 +4,13 @@
 python_library(
   dependencies = [
     '3rdparty/python/twitter/commons:twitter.common.collections',
+    'src/python/pants/backend/codegen/thrift/lib',
     'src/python/pants/backend/jvm/targets:java',
-    'src/python/pants/base:build_environment',
     'src/python/pants/base:exceptions',
-    'src/python/pants/base:workunit',
-    'src/python/pants/binaries:thrift_util',
     'src/python/pants/build_graph',
     'src/python/pants/goal:task_registrar',
     'src/python/pants/option',
     'src/python/pants/subsystem',
     'src/python/pants/task',
-    'src/python/pants/util:memo',
   ],
 )

src/python/pants/backend/codegen/thrift/java/apache_thrift_java_gen.py

@@ -0,0 +1,61 @@
+# coding=utf-8
+# Copyright 2014 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)
+
+from pants.backend.codegen.thrift.java.java_thrift_library import JavaThriftLibrary
+from pants.backend.codegen.thrift.java.thrift_defaults import ThriftDefaults
+from pants.backend.codegen.thrift.lib.apache_thrift_gen_base import ApacheThriftGenBase
+from pants.backend.jvm.targets.java_library import JavaLibrary
+from pants.base.exceptions import TargetDefinitionException
+from pants.binaries.thrift_binary import ThriftBinary
+
+
+class ApacheThriftJavaGen(ApacheThriftGenBase):
+  """Generate Java source files from thrift IDL files."""
+  deprecated_options_scope = 'gen.thrift'  # New scope is gen.thrift-java.
+  deprecated_options_scope_removal_version = '1.5.0dev0'
+
+  thrift_library_target_type = JavaThriftLibrary
+  thrift_generator = 'java'
+
+  _COMPILER = 'thrift'
+  _RPC_STYLE = 'sync'
+
+  @classmethod
+  def subsystem_dependencies(cls):
+    return (super(ApacheThriftJavaGen, cls).subsystem_dependencies() +
+            (ThriftDefaults, ThriftBinary.Factory.scoped(cls)))
+
+  @classmethod
+  def implementation_version(cls):
+    return super(ApacheThriftJavaGen, cls).implementation_version() + [('ApacheThriftJavaGen', 2)]
+
+  def __init__(self, *args, **kwargs):
+    super(ApacheThriftJavaGen, self).__init__(*args, **kwargs)
+    self._thrift_defaults = ThriftDefaults.global_instance()
+
+  def synthetic_target_type(self, target):
+    return JavaLibrary
+
+  def is_gentarget(self, target):
+    return (super(ApacheThriftJavaGen, self).is_gentarget(target) and
+            self._thrift_defaults.compiler(target) == self._COMPILER)
+
+  def _validate(self, target):
+    # TODO: Fix ThriftDefaults to only pertain to scrooge (see TODO there) and then
+    # get rid of this spurious validation.
+    if self._thrift_defaults.language(target) != self.thrift_generator:
+      raise TargetDefinitionException(
+          target,
+          'Compiler {} supports only language={}.'.format(self._COMPILER, self.thrift_generator))
+    if self._thrift_defaults.rpc_style(target) != self._RPC_STYLE:
+      raise TargetDefinitionException(
+          target,
+          'Compiler {} supports only rpc_style={}.'.format(self._COMPILER, self._RPC_STYLE))
+
+  def execute_codegen(self, target, target_workdir):
+    self._validate(target)
+    super(ApacheThriftJavaGen, self).execute_codegen(target, target_workdir)

src/python/pants/backend/codegen/thrift/java/register.py

@@ -5,7 +5,7 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-from pants.backend.codegen.thrift.java.apache_thrift_gen import ApacheThriftGen
+from pants.backend.codegen.thrift.java.apache_thrift_java_gen import ApacheThriftJavaGen
 from pants.backend.codegen.thrift.java.java_thrift_library import JavaThriftLibrary
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.goal.task_registrar import TaskRegistrar as task
@@ -20,4 +20,4 @@ def build_file_aliases():
 
 
 def register_goals():
-  task(name='thrift', action=ApacheThriftGen).install('gen')
+  task(name='thrift-java', action=ApacheThriftJavaGen).install('gen')

src/python/pants/backend/codegen/thrift/java/thrift_defaults.py

@@ -10,9 +10,13 @@
 
 
 class ThriftDefaults(Subsystem):
-  """Tracks defaults for thrift target attributes that influence code generation."""
+  """Tracks defaults for java thrift target attributes that influence code generation."""
   options_scope = 'thrift-defaults'
 
+  # TODO: These exist to support alternate compilers (specifically scrooge), but this should
+  # probably be a scrooge-specific subsystem, tied to a scrooge_thrift_library target type.
+  # These options (e.g., rpc_style, language) are relevant to scrooge but may have no meaning
+  # in some other compiler (they certainly don't in the apache thrift compiler).
   @classmethod
   def register_options(cls, register):
     register('--compiler', type=str, advanced=True, default='thrift',

src/python/pants/backend/codegen/thrift/lib/BUILD

@@ -0,0 +1,15 @@
+# Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+python_library(
+  dependencies = [
+    '3rdparty/python/twitter/commons:twitter.common.collections',
+    'src/python/pants/base:build_environment',
+    'src/python/pants/base:exceptions',
+    'src/python/pants/base:workunit',
+    'src/python/pants/binaries:thrift_util',
+    'src/python/pants/option',
+    'src/python/pants/task',
+    'src/python/pants/util:memo',
+  ],
+)