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

build-support/bin/native/bootstrap.sh

@@ -45,13 +45,16 @@ esac
 readonly CACHE_ROOT=${XDG_CACHE_HOME:-$HOME/.cache}/pants
 readonly NATIVE_ENGINE_CACHE_DIR=${CACHE_ROOT}/bin/native-engine
 
+readonly RUST_TOOLCHAIN="1.24.0"
+
 function calculate_current_hash() {
   # Cached and unstaged files, with ignored files excluded.
   # NB: We fork a subshell because one or both of `ls-files`/`hash-object` are
   # sensitive to the CWD, and the `--work-tree` option doesn't seem to resolve that.
   (
    cd ${REPO_ROOT}
    (echo "${MODE_FLAG}"
+    echo "${RUST_TOOLCHAIN}"
     git ls-files -c -o --exclude-standard \
      "${NATIVE_ROOT}" \
      "${REPO_ROOT}/src/python/pants/engine/native.py" \
@@ -70,11 +73,9 @@ function _ensure_cffi_sources() {
 function ensure_native_build_prerequisites() {
   # Control a pants-specific rust toolchain.
 
-  local rust_toolchain_root="${CACHE_ROOT}/rust"
-  export CARGO_HOME="${rust_toolchain_root}/cargo"
-  export RUSTUP_HOME="${rust_toolchain_root}/rustup"
-
-  local rust_toolchain="1.23.0"
+  local RUST_TOOLCHAIN_root="${CACHE_ROOT}/rust"
+  export CARGO_HOME="${RUST_TOOLCHAIN_root}/cargo"
+  export RUSTUP_HOME="${RUST_TOOLCHAIN_root}/rustup"
 
   # NB: rustup installs itself into CARGO_HOME, but fetches toolchains into RUSTUP_HOME.
   if [[ ! -x "${CARGO_HOME}/bin/rustup" ]]
@@ -83,13 +84,13 @@ function ensure_native_build_prerequisites() {
         "https://www.rustup.rs ..."
     local readonly rustup=$(mktemp -t pants.rustup.XXXXXX)
     curl https://sh.rustup.rs -sSf > ${rustup}
-    sh ${rustup} -y --no-modify-path --default-toolchain "${rust_toolchain}" 1>&2
+    sh ${rustup} -y --no-modify-path --default-toolchain "${RUST_TOOLCHAIN}" 1>&2
     rm -f ${rustup}
   fi
 
   # Make sure rust is pinned at the correct version.
   # We sincerely hope that no one ever runs `rustup override set` in a subdirectory of the working directory.
-  "${CARGO_HOME}/bin/rustup" override set "${rust_toolchain}" >&2
+  "${CARGO_HOME}/bin/rustup" override set "${RUST_TOOLCHAIN}" >&2
 
   # Sometimes fetching a large git repo dependency can take more than 10 minutes.
   # This times out on travis, because nothing is printed to stdout/stderr in that time.

contrib/buildrefactor/src/python/pants/contrib/buildrefactor/BUILD

@@ -4,24 +4,17 @@
 contrib_plugin(
   name='plugin',
   dependencies=[
-    ':buildozer'
+    ':buildrefactor'
   ],
   distribution_name='pantsbuild.pants.contrib.buildrefactor',
   description='Plugin to manipulate and refactor BUILD files and targets',
   register_goals=True,
 )
 
-
 python_library(
-  name='buildozer',
-  sources=['buildozer.py'],
   dependencies = [
+    'src/python/pants/base:build_environment',
+    'src/python/pants/base:exceptions',
     'src/python/pants/binaries:binary_util'
   ]
 )
-
-
-python_library(
-  name='meta_rename',
-  sources=['meta_rename.py']
-)

contrib/buildrefactor/src/python/pants/contrib/buildrefactor/buildozer.py

@@ -9,10 +9,11 @@
 
 from pants.base.build_environment import get_buildroot
 from pants.base.exceptions import TaskError
-from pants.binaries.binary_util import BinaryUtil
 from pants.task.task import Task
 from pants.util.process_handler import subprocess
 
+from pants.contrib.buildrefactor.buildozer_binary import BuildozerBinary
+
 
 logger = logging.getLogger(__name__)
 
@@ -38,44 +39,36 @@ class Buildozer(Task):
 
   @classmethod
   def subsystem_dependencies(cls):
-    return super(Buildozer, cls).subsystem_dependencies() + (BinaryUtil.Factory,)
+    return super(Buildozer, cls).subsystem_dependencies() + (BuildozerBinary.scoped(cls),)
 
   @classmethod
   def register_options(cls, register):
-    register('--version', default='0.6.0.dce8b3c287652cbcaf43c8dd076b3f48c92ab44c', help='Version of buildozer.')
+    register('--version', type=str, advanced=True,
+             removal_version='1.7.0.dev0',
+             removal_hint='Use --version in scope buildozer-binary instead.',
+             help='Version of buildozer.')
     register('--add-dependencies', type=str, help='The dependency or dependencies to add')
     register('--remove-dependencies', type=str, help='The dependency or dependencies to remove')
     register('--command', type=str, help='A custom buildozer command to execute')
 
-  def __init__(self, *args, **kwargs):
-    super(Buildozer, self).__init__(*args, **kwargs)
-
-    self.options = self.get_options()
-    self._executable = BinaryUtil.Factory.create().select_binary('scripts/buildozer', self.options.version, 'buildozer')
-
   def execute(self):
-    if self.options.command:
-      if self.options.add_dependencies or self.options.remove_dependencies:
+    options = self.get_options()
+    if options.command:
+      if options.add_dependencies or options.remove_dependencies:
         raise TaskError('Buildozer custom command cannot be used together with ' +
                         '--add-dependencies or --remove-dependencies.')
-      self._execute_buildozer_script(self.options.command)
+      self._execute_buildozer_script(options.command)
 
-    if self.options.add_dependencies:
-      self._execute_buildozer_script('add dependencies {}'.format(self.options.add_dependencies))
+    if options.add_dependencies:
+      self._execute_buildozer_script('add dependencies {}'.format(options.add_dependencies))
 
-    if self.options.remove_dependencies:
-      self._execute_buildozer_script('remove dependencies {}'.format(self.options.remove_dependencies))
+    if options.remove_dependencies:
+      self._execute_buildozer_script('remove dependencies {}'.format(options.remove_dependencies))
 
   def _execute_buildozer_script(self, command):
+    binary = BuildozerBinary.scoped_instance(self)
     for root in self.context.target_roots:
-      address = root.address
-      Buildozer.execute_binary(command, address.spec, binary=self._executable)
-
-  @classmethod
-  def execute_binary(cls, command, spec, binary=None, version='0.6.0.dce8b3c287652cbcaf43c8dd076b3f48c92ab44c'):
-    binary = binary if binary else BinaryUtil.Factory.create().select_binary('scripts/buildozer', version, 'buildozer')
-
-    Buildozer._execute_buildozer_command([binary, command, spec])
+      binary.execute(command, root.address.spec, context=self.context)
 
   @classmethod
   def _execute_buildozer_command(cls, buildozer_command):

contrib/buildrefactor/src/python/pants/contrib/buildrefactor/buildozer_binary.py

@@ -0,0 +1,39 @@
+# coding=utf-8
+# Copyright 2018 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.build_environment import get_buildroot
+from pants.base.exceptions import TaskError
+from pants.binaries.binary_tool import NativeTool
+from pants.util.process_handler import subprocess
+
+
+logger = logging.getLogger(__name__)
+
+
+class BuildozerBinary(NativeTool):
+  options_scope = 'buildozer-binary'
+  name = 'buildozer'
+  default_version = '0.6.0.dce8b3c287652cbcaf43c8dd076b3f48c92ab44c'
+
+  replaces_scope = 'buildozer'
+  replaces_name = 'version'
+
+  # TODO: Move this to bin/buildozer - buildozer is a native binary.
+  @classmethod
+  def get_support_dir(cls):
+    return 'scripts/buildozer'
+
+  def execute(self, buildozer_command, spec, context=None):
+    try:
+      subprocess.check_call([self.select(context), buildozer_command, spec], cwd=get_buildroot())
+    except subprocess.CalledProcessError as err:
+      if err.returncode == 3:
+        logger.warn('{} ... no changes were made'.format(buildozer_command))
+      else:
+        raise TaskError('{} ... exited non-zero ({}).'.format(buildozer_command, err.returncode))

contrib/buildrefactor/src/python/pants/contrib/buildrefactor/meta_rename.py

@@ -6,15 +6,14 @@
                         unicode_literals, with_statement)
 
 import logging
-
 from collections import defaultdict
 
 from pants.base.specs import DescendantAddresses
 from pants.build_graph.address import Address
 from pants.build_graph.target import Target
 from pants.task.task import Task
 
-from pants.contrib.buildrefactor.buildozer import Buildozer
+from pants.contrib.buildrefactor.buildozer_binary import BuildozerBinary
 
 
 logger = logging.getLogger(__name__)
@@ -27,6 +26,10 @@ class MetaRename(Task):
   Also renames the target wherever it's specified as a dependency.
   """
 
+  @classmethod
+  def subsystem_dependencies(cls):
+    return super(MetaRename, cls).subsystem_dependencies() + (BuildozerBinary.scoped(cls),)
+
   @classmethod
   def register_options(cls, register):
     super(MetaRename, cls).register_options(register)
@@ -46,25 +49,33 @@ def execute(self):
 
   def update_dependee_references(self):
     dependee_targets = self.dependency_graph()[
+      # TODO: Target() expects build_graph to be an instance of BuildGraph, not a list.
+      # TODO: The **{} seems unnecessary.
       Target(name=self._from_address.target_name, address=self._from_address, build_graph=[], **{})
     ]
 
     logging.disable(logging.WARNING)
 
+    buildozer_binary = BuildozerBinary.scoped_instance(self)
     for concrete_target in dependee_targets:
       for formats in [
         { 'from': self._from_address.spec, 'to': self._to_address.spec },
-        { 'from': ':{}'.format(self._from_address.target_name), 'to': ':{}'.format(self._to_address.target_name) }
+        { 'from': ':{}'.format(self._from_address.target_name), 'to': ':{}'.format(
+          self._to_address.target_name) }
       ]:
-        Buildozer.execute_binary(
+        buildozer_binary.execute(
           'replace dependencies {} {}'.format(formats['from'], formats['to']),
-          spec=concrete_target.address.spec
+          spec=concrete_target.address.spec,
+          context=self.context
         )
 
     logging.disable(logging.NOTSET)
 
   def update_original_build_name(self):
-    Buildozer.execute_binary('set name {}'.format(self._to_address.target_name), spec=self._from_address.spec)
+    BuildozerBinary.scoped_instance(self).execute(
+      'set name {}'.format(self._to_address.target_name),
+      spec=self._from_address.spec,
+      context=self.context)
 
   def dependency_graph(self, scope=''):
     dependency_graph = defaultdict(set)

contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor/BUILD

@@ -12,8 +12,8 @@ python_tests(
   name='buildozer',
   sources=['test_buildozer.py'],
   dependencies=[
-    'contrib/buildrefactor/src/python/pants/contrib/buildrefactor:buildozer',
-    'contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor:buildozer_util',
+    ':buildozer_util',
+    'contrib/buildrefactor/src/python/pants/contrib/buildrefactor',
     'src/python/pants/backend/jvm/targets:java',
     'src/python/pants/util:contextutil',
     'tests/python/pants_test/subsystem:subsystem_utils',
@@ -26,9 +26,8 @@ python_tests(
   name='meta_rename',
   sources=['test_meta_rename.py'],
   dependencies=[
-    'contrib/buildrefactor/src/python/pants/contrib/buildrefactor:buildozer',
-    'contrib/buildrefactor/src/python/pants/contrib/buildrefactor:meta_rename',
-    'contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor:buildozer_util',
+    ':buildozer_util',
+    'contrib/buildrefactor/src/python/pants/contrib/buildrefactor',
     'src/python/pants/backend/jvm/targets:java',
     'src/python/pants/base:build_environment',
     'tests/python/pants_test/subsystem:subsystem_utils',
@@ -42,7 +41,8 @@ python_tests(
   sources=['test_meta_rename_integration.py'],
   dependencies=[
     'tests/python/pants_test:int-test',
-  ]
+  ],
+  tags={'integration'},
 )
 
 
@@ -51,5 +51,6 @@ python_tests(
   sources=['test_buildozer_integration.py'],
   dependencies=[
     'tests/python/pants_test:int-test',
-  ]
+  ],
+  tags={'integration'},
 )

contrib/buildrefactor/tests/python/pants_test/contrib/buildrefactor/test_buildozer.py

@@ -10,11 +10,8 @@
 
 from pants.backend.jvm.targets.java_library import JavaLibrary
 from pants.base.exceptions import TaskError
-from pants.binaries.binary_util import BinaryUtil
-from pants.build_graph.address import Address
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants_test.contrib.buildrefactor.buildozer_util import prepare_dependencies
-from pants_test.subsystem.subsystem_util import init_subsystem
 from pants_test.tasks.task_test_base import TaskTestBase
 
 from pants.contrib.buildrefactor.buildozer import Buildozer
@@ -58,15 +55,6 @@ def test_custom_command(self):
     self._run_buildozer({ 'command': 'set name {}'.format(new_build_name) })
     self.assertInFile(new_build_name, os.path.join(self.build_root, 'b/BUILD'))
 
-  def test_execute_binary(self):
-    init_subsystem(BinaryUtil.Factory)
-
-    new_build_name = 'b_2'
-
-    Buildozer.execute_binary('set name {}'.format(new_build_name), spec=Address.parse('b').spec)
-
-    self.assertInFile(new_build_name, os.path.join(self.build_root, 'b/BUILD'))
-
   def test_multiple_addresses(self):
     roots = ['b', 'c']
     dependency_to_add = '/l/m/n'
@@ -97,7 +85,8 @@ def _test_add_dependencies(self, spec, dependencies_to_add):
     self._run_buildozer({ 'add_dependencies': ' '.join(dependencies_to_add) })
 
     for dependency in dependencies_to_add:
-      self.assertIn(dependency, self._build_file_dependencies(os.path.join(self.build_root, spec, 'BUILD')))
+      self.assertIn(dependency, self._build_file_dependencies(
+        os.path.join(self.build_root, spec, 'BUILD')))
 
   def _test_add_dependencies_with_targets(self, dependencies_to_add, roots, targets):
     """
@@ -107,36 +96,37 @@ def _test_add_dependencies_with_targets(self, dependencies_to_add, roots, target
     for dependency_to_add in dependencies_to_add:
       self._run_buildozer({ 'add_dependencies': dependency_to_add }, roots=roots, targets=targets)
 
-    for root in roots:
-      self.assertInFile(dependency_to_add, os.path.join(self.build_root, root, 'BUILD'))
+      for root in roots:
+        self.assertInFile(dependency_to_add, os.path.join(self.build_root, root, 'BUILD'))
 
   def _test_remove_dependencies(self, spec, dependencies_to_remove):
     self._run_buildozer({ 'remove_dependencies': ' '.join(dependencies_to_remove) }, roots=[spec])
 
     for dependency in dependencies_to_remove:
-      self.assertNotIn(dependency, self._build_file_dependencies(os.path.join(self.build_root, spec, 'BUILD')))
+      self.assertNotIn(dependency, self._build_file_dependencies(
+        os.path.join(self.build_root, spec, 'BUILD')))
 
-  def _run_buildozer(self, options, roots=['b'], targets=None):
+  def _run_buildozer(self, options, roots=('b',), targets=None):
     """Run buildozer on the specified context roots and target objects.
 
     roots -- the context roots supplied to buildozer (default ['b'])
     targets -- the targets buildozer will run on (defaults to self.targets)
     """
-    targets = self.targets if targets is None else targets
-
     self.set_options(**options)
 
+    targets = self.targets if targets is None else targets
     target_roots = []
-
     for root in roots:
       target_roots.append(targets[root])
 
     self.create_task(self.context(target_roots=target_roots)).execute()
 
-  def _build_file_dependencies(self, build_file):
+  @staticmethod
+  def _build_file_dependencies(build_file):
     with open(build_file) as f:
       source = f.read()
 
     dependencies = re.compile('dependencies+.?=+.?\[([^]]*)').findall(source)
 
-    return ''.join(dependencies[0].replace('\"', '').split()).split(',') if len(dependencies) > 0 else dependencies
+    return (''.join(dependencies[0].replace('\"', '').split()).split(',')
+            if len(dependencies) > 0 else dependencies)