Merge branch 'master' into dmcclanahan/python-dist-c++-sources

contrib/go/src/python/pants/contrib/go/targets/go_thrift_library.py

@@ -5,7 +5,6 @@
 from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
                         unicode_literals, with_statement)
 
-from pants.base.deprecated import deprecated_conditional
 from pants.base.payload import Payload
 from pants.build_graph.target import Target
 
@@ -19,7 +18,6 @@ class GoThriftLibrary(Target):
   def __init__(self,
                address=None,
                payload=None,
-               import_path=None,
                sources=None,
                **kwargs):
     """
@@ -28,11 +26,6 @@ def __init__(self,
                    are relative to the BUILD file's directory.
     :param import_path: Deprecated: unused.
     """
-    deprecated_conditional(lambda: import_path is not None,
-                           removal_version='1.6.0.dev0',
-                           entity_description='import_path',
-                           hint_message='Remove this unused `{}` parameter'.format(self.alias()))
-
     payload = payload or Payload()
     payload.add_field('sources',
                       self.create_sources_field(sources, address.spec_path, key_arg='sources'))

contrib/scrooge/src/python/pants/contrib/scrooge/tasks/java_thrift_library_fingerprint_strategy.py

@@ -12,7 +12,8 @@
 
 
 class JavaThriftLibraryFingerprintStrategy(FingerprintStrategy):
-  """Scrooge cares about a Thrift target's `language` and `rpc_style` in addition to its payload.
+  """Scrooge cares about a Thrift target's `language` and `compiler_args` and more in addition to
+  its payload.
 
   As such this strategy ensures new code is generated by Scrooge whenever any option that effects
   codegen changes.
@@ -29,8 +30,6 @@ def compute_fingerprint(self, target):
     hasher = hashlib.sha1()
     hasher.update(fp)
     hasher.update(self._thrift_defaults.language(target))
-    hasher.update(self._thrift_defaults.rpc_style(target))
-
     hasher.update(str(self._thrift_defaults.compiler_args(target)))
 
     namespace_map = self._thrift_defaults.namespace_map(target)

contrib/scrooge/src/python/pants/contrib/scrooge/tasks/scrooge_gen.py

@@ -27,17 +27,14 @@
 from pants.contrib.scrooge.tasks.thrift_util import calculate_compile_sources
 
 
-_RPC_STYLES = frozenset(['sync', 'finagle', 'ostrich'])
-
-
 class ScroogeGen(SimpleCodegenTask, NailgunTask):
 
   DepInfo = namedtuple('DepInfo', ['service', 'structs'])
   PartialCmd = namedtuple('PartialCmd', [
-    'language', 
-    'namespace_map', 
-    'default_java_namespace', 
-    'include_paths', 
+    'language',
+    'namespace_map',
+    'default_java_namespace',
+    'include_paths',
     'compiler_args'
   ])
 
@@ -115,18 +112,10 @@ def _validate_language(self, target):
     language = self._thrift_defaults.language(target)
     if language not in self._registered_language_aliases():
       raise TargetDefinitionException(
-          target,
-          'language {} not supported: expected one of {}.'.format(language, self._registered_language_aliases().keys()))
+        target,
+        'language {} not supported: expected one of {}.'.format(language, self._registered_language_aliases().keys()))
     return language
 
-  def _validate_rpc_style(self, target):
-    rpc_style = self._thrift_defaults.rpc_style(target)
-    if rpc_style not in _RPC_STYLES:
-      raise TargetDefinitionException(
-          target,
-          'rpc_style {} not supported: expected one of {}.'.format(rpc_style, _RPC_STYLES))
-    return rpc_style
-
   @memoized_method
   def _registered_language_aliases(self):
     return self.get_options().target_types
@@ -143,34 +132,16 @@ def _target_type_for_language(self, language):
     return next(iter(target_types))
 
   def execute_codegen(self, target, target_workdir):
-    self._validate_compiler_configs([target])
+    self._validate_compiler_configs(target)
     self._must_have_sources(target)
 
-    def compiler_args_has_rpc_style(compiler_args):
-      return "--finagle" in compiler_args or "--ostrich" in compiler_args
-
-    def merge_rpc_style_with_compiler_args(compiler_args, rpc_style):
-      new_compiler_args = list(compiler_args)
-      # ignore rpc_style if we think compiler_args is setting it
-      if not compiler_args_has_rpc_style(compiler_args):
-        if rpc_style == 'ostrich':
-          new_compiler_args.append('--finagle')
-          new_compiler_args.append('--ostrich')
-        elif rpc_style == 'finagle':
-          new_compiler_args.append('--finagle')
-      return new_compiler_args
-
     namespace_map = self._thrift_defaults.namespace_map(target)
-    compiler_args = merge_rpc_style_with_compiler_args(
-        self._thrift_defaults.compiler_args(target), 
-        self._validate_rpc_style(target))
-
     partial_cmd = self.PartialCmd(
-        language=self._validate_language(target),
-        namespace_map=tuple(sorted(namespace_map.items())) if namespace_map else (),
-        default_java_namespace=self._thrift_defaults.default_java_namespace(target),
-        include_paths=target.include_paths,
-        compiler_args=compiler_args)
+      language=self._validate_language(target),
+      namespace_map=tuple(sorted(namespace_map.items())) if namespace_map else (),
+      default_java_namespace=self._thrift_defaults.default_java_namespace(target),
+      include_paths=target.include_paths,
+      compiler_args=(self._thrift_defaults.compiler_args(target)))
 
     self.gen(partial_cmd, target, target_workdir)
 
@@ -254,29 +225,24 @@ def is_gentarget(self, target):
     # Apache thrift compiler
     return self._thrift_defaults.compiler(target) == 'scrooge'
 
-  def _validate_compiler_configs(self, targets):
-    assert len(targets) == 1, ("TODO: This method now only ever receives one target. Simplify.")
-    ValidateCompilerConfig = namedtuple('ValidateCompilerConfig', ['language', 'rpc_style'])
+  _ValidateCompilerConfig = namedtuple('ValidateCompilerConfig',
+                                       ['language', 'compiler', 'compiler_args'])
+
+  def _validate_compiler_configs(self, target):
 
     def compiler_config(tgt):
-      # Note compiler is not present in this signature. At this time
-      # Scrooge and the Apache thrift generators produce identical
-      # java sources, and the Apache generator does not produce scala
-      # sources. As there's no permutation allowing the creation of
-      # incompatible sources with the same language+rpc_style we omit
-      # the compiler from the signature at this time.
-      return ValidateCompilerConfig(language=self._thrift_defaults.language(tgt),
-                                    rpc_style=self._thrift_defaults.rpc_style(tgt))
+      return self._ValidateCompilerConfig(language=self._thrift_defaults.language(tgt),
+                                          compiler=self._thrift_defaults.compiler(tgt),
+                                          compiler_args=self._thrift_defaults.compiler_args(tgt))
 
     mismatched_compiler_configs = defaultdict(set)
+    mycompilerconfig = compiler_config(target)
 
-    for target in filter(lambda t: isinstance(t, JavaThriftLibrary), targets):
-      mycompilerconfig = compiler_config(target)
+    def collect(dep):
+      if mycompilerconfig != compiler_config(dep):
+        mismatched_compiler_configs[target].add(dep)
 
-      def collect(dep):
-        if mycompilerconfig != compiler_config(dep):
-          mismatched_compiler_configs[target].add(dep)
-      target.walk(collect, predicate=lambda t: isinstance(t, JavaThriftLibrary))
+    target.walk(collect, predicate=lambda t: isinstance(t, JavaThriftLibrary))
 
     if mismatched_compiler_configs:
       msg = ['Thrift dependency trees must be generated with a uniform compiler configuration.\n\n']

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_java_thrift_library_fingerprint_strategy.py

@@ -17,7 +17,6 @@ class JavaThriftLibraryFingerprintStrategyTest(BaseTest):
 
   options1 = {'compiler': 'scrooge',
               'language': 'java',
-              'rpc_style': 'async',
               'compiler_args': []}
 
   def create_strategy(self, option_values):
@@ -29,25 +28,17 @@ def create_strategy(self, option_values):
   def test_fp_diffs_due_to_option(self):
     option_values = {'compiler': 'scrooge',
                      'language': 'java',
-                     'rpc_style': 'finagle'}
+                     'compiler_args': ['--foo']}
 
-    a = self.make_target(':a', target_type=JavaThriftLibrary, dependencies=[])
+    a = self.make_target(':a', target_type=JavaThriftLibrary)
 
     fp1 = self.create_strategy(self.options1).compute_fingerprint(a)
     fp2 = self.create_strategy(option_values).compute_fingerprint(a)
     self.assertNotEquals(fp1, fp2)
 
-  def test_fp_diffs_due_to_target_change(self):
-    a = self.make_target(':a', target_type=JavaThriftLibrary, rpc_style='sync', dependencies=[])
-    b = self.make_target(':b', target_type=JavaThriftLibrary, rpc_style='finagle', dependencies=[])
-
-    fp1 = self.create_strategy(self.options1).compute_fingerprint(a)
-    fp2 = self.create_strategy(self.options1).compute_fingerprint(b)
-    self.assertNotEquals(fp1, fp2)
-
   def test_fp_diffs_due_to_compiler_args_change(self):
-    a = self.make_target(':a', target_type=JavaThriftLibrary, compiler_args=['--foo'], dependencies=[])
-    b = self.make_target(':b', target_type=JavaThriftLibrary, compiler_args=['--bar'], dependencies=[])
+    a = self.make_target(':a', target_type=JavaThriftLibrary, compiler_args=['--foo'])
+    b = self.make_target(':b', target_type=JavaThriftLibrary, compiler_args=['--bar'])
 
     fp1 = self.create_strategy(self.options1).compute_fingerprint(a)
     fp2 = self.create_strategy(self.options1).compute_fingerprint(b)
@@ -60,7 +51,7 @@ def test_hash_and_equal(self):
 
     option_values = {'compiler': 'scrooge',
                      'language': 'java',
-                     'rpc_style': 'finagle'}
+                     'compiler_args': ['--baz']}
     self.assertNotEqual(self.create_strategy(self.options1), self.create_strategy(option_values))
     self.assertNotEqual(hash(self.create_strategy(self.options1)),
                         hash(self.create_strategy(option_values)))

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen.py

@@ -12,7 +12,7 @@
 from pants.backend.codegen.thrift.java.java_thrift_library import JavaThriftLibrary
 from pants.backend.jvm.targets.java_library import JavaLibrary
 from pants.backend.jvm.targets.scala_library import ScalaLibrary
-from pants.base.exceptions import TargetDefinitionException, TaskError
+from pants.base.exceptions import TargetDefinitionException
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.goal.context import Context
 from pants.source.wrapped_globs import EagerFilesetWithSpec
@@ -42,7 +42,6 @@ def test_validate_compiler_configs(self):
     self.set_options_for_scope('thrift-defaults',
                                compiler='unchecked',
                                language='uniform',
-                               rpc_style='async',
                                service_deps='service_deps',
                                structs_deps='structs_deps')
 
@@ -60,81 +59,52 @@ def test_validate_compiler_configs(self):
       )
     '''))
 
-    self.add_to_build_file('test_validate', dedent('''
-      java_thrift_library(name='three',
-        sources=[],
-        dependencies=[':one'],
-        rpc_style='finagle',
-      )
-    '''))
-
     target = self.target('test_validate:one')
     context = self.context(target_roots=[target])
     task = self.prepare_execute(context)
-    task._validate_compiler_configs([self.target('test_validate:one')])
-    task._validate_compiler_configs([self.target('test_validate:two')])
-
-    with self.assertRaises(TaskError):
-      task._validate_compiler_configs([self.target('test_validate:three')])
+    task._validate_compiler_configs(self.target('test_validate:one'))
+    task._validate_compiler_configs(self.target('test_validate:two'))
 
   def test_scala(self):
     sources = [os.path.join(self.test_workdir, 'org/pantsbuild/example/Example.scala')]
-    self._test_help('scala', ScalaLibrary, [GEN_ADAPT], sources, 'finagle')
+    self._test_help('scala', ScalaLibrary, [GEN_ADAPT], sources)
 
-  def test_compiler_args_no_rpc_style(self):
+  def test_compiler_args(self):
     sources = [os.path.join(self.test_workdir, 'org/pantsbuild/example/Example.scala')]
     self._test_help('scala', ScalaLibrary, [GEN_ADAPT], sources)
 
   def test_android(self):
     sources = [os.path.join(self.test_workdir, 'org/pantsbuild/android_example/Example.java')]
-    self._test_help('android', JavaLibrary, [GEN_ADAPT], sources, 'finagle')
+    self._test_help('android', JavaLibrary, [GEN_ADAPT], sources)
 
   def test_invalid_lang(self):
     with self.assertRaises(TargetDefinitionException):
-      self._test_help('not-a-lang', JavaLibrary, [GEN_ADAPT], [], 'finagle')
-
-  def test_invalid_style(self):
-    with self.assertRaises(TargetDefinitionException):
-      self._test_help('scala', JavaLibrary, [GEN_ADAPT], [], 'not-a-style')
+      self._test_help('not-a-lang', JavaLibrary, [GEN_ADAPT], [])
 
   def test_empty_compiler_args(self):
     sources = [os.path.join(self.test_workdir, 'org/pantsbuild/example/Example.scala')]
-    self._test_help('scala', ScalaLibrary, [], sources, 'finagle')
+    self._test_help('scala', ScalaLibrary, [], sources)
 
   def compiler_args_to_string(self, compiler_args):
     quoted = map(lambda x: "'{}'".format(x), compiler_args)
     comma_separated = ', '.join(quoted)
     return '[{}]'.format(comma_separated)
 
-  def _test_create_build_str(self, language, compiler_args, rpc_style=None):
+  def _test_create_build_str(self, language, compiler_args):
     compiler_args_str = self.compiler_args_to_string(compiler_args)
-    if rpc_style is None:
-      return dedent('''
-        java_thrift_library(name='a',
-          sources=['a.thrift'],
-          dependencies=[],
-          compiler='scrooge',
-          language='{language}',
-          compiler_args={compiler_args},
-          strict_deps=True,
-          fatal_warnings=False,
-        )
-      '''.format(language=language, compiler_args=compiler_args_str))
-    else:
-      return dedent('''
-        java_thrift_library(name='a',
-          sources=['a.thrift'],
-          dependencies=[],
-          compiler='scrooge',
-          language='{language}',
-          rpc_style='{rpc_style}',
-          compiler_args={compiler_args},
-          strict_deps=True,
-          fatal_warnings=False,
-        )
-      '''.format(language=language, rpc_style=rpc_style, compiler_args=compiler_args_str))
-
-  def _test_help(self, language, library_type, compiler_args, sources, rpc_style = None):
+    return dedent('''
+      java_thrift_library(name='a',
+        sources=['a.thrift'],
+        dependencies=[],
+        compiler='scrooge',
+        language='{language}',
+        compiler_args={compiler_args},
+        strict_deps=True,
+        fatal_warnings=False,
+      )
+    '''.format(language=language, compiler_args=compiler_args_str))
+
+  def _test_help(self, language, library_type, compiler_args, sources):
     contents = dedent('''#@namespace android org.pantsbuild.android_example
       namespace java org.pantsbuild.example
       struct Example {
@@ -143,7 +113,7 @@ def _test_help(self, language, library_type, compiler_args, sources, rpc_style =
     ''')
 
     self.create_file(relpath='test_smoke/a.thrift', contents=contents)
-    build_string = self._test_create_build_str(language, compiler_args, rpc_style)
+    build_string = self._test_create_build_str(language, compiler_args)
     self.add_to_build_file('test_smoke', build_string)
 
     target = self.target('test_smoke:a')

contrib/scrooge/tests/python/pants_test/contrib/scrooge/tasks/test_scrooge_gen_integration.py

@@ -85,12 +85,6 @@ def test_good(self):
     pants_run = self.run_pants(cmd)
     self.assert_success(pants_run)
 
-  def test_both_compiler_args_and_rpc_style(self):
-    # scrooge_gen should pass when both compiler_args and rpc_style are specified
-    cmd = ['gen', self.thrift_test_target('both-compiler-args-and-rpc-style')]
-    pants_run = self.run_pants(cmd)
-    self.assert_success(pants_run)
-
   def test_exports_of_thrift(self):
     # Compiling against a thrift service with strict_deps=True should work
     # because the necessary transitive dependencies will be exported.

contrib/scrooge/tests/thrift/org/pantsbuild/contrib/scrooge/scrooge_gen/BUILD

@@ -40,11 +40,3 @@ java_thrift_library(
   ],
 )
 
-java_thrift_library(
-  name = 'both-compiler-args-and-rpc-style',
-  sources = ['good.thrift'],
-  compiler='scrooge',
-  language='scala',
-  compiler_args=['--finagle'],
-  rpc_style='finagle',
-)

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

@@ -80,7 +80,7 @@ Use Apache Thrift compiler (the default):
     java_thrift_library(  # Yes, a "java" library to generate Scala
       compiler='scrooge', # default compiler does not gen Scala; Scrooge does
       language='scala',
-      # maybe set an rpc_style
+      # maybe set compiler_args
     )
 
 **Android**

pants.ini

@@ -333,7 +333,6 @@ timeout_default: 60
 chroot: true
 timeouts: true
 timeout_default: 60
-legacy_report_layout: False
 
 
 [buildgen.go]

src/python/pants/VERSION

@@ -1 +1 @@
-1.5.0rc0
+1.6.0.dev0