Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose rules from language backends with an application to python_dist() creation #5747

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
8fd04c2
add some simple examples to demonstrate how to use backend rules
cosmicexplorer Apr 25, 2018
258c767
...actually make the changes to consume backend rules in register.py
cosmicexplorer Apr 25, 2018
c3dc339
expand a couple docstrings
cosmicexplorer Apr 25, 2018
2b7c33d
revert accidental change to a test target
cosmicexplorer Apr 25, 2018
5620ce6
remove extraneous log statement
cosmicexplorer Apr 25, 2018
d421e5a
fix lint errors
cosmicexplorer Apr 25, 2018
9ac7e0f
respond to review comments
cosmicexplorer Apr 25, 2018
8f5ee6a
add unit test and change field name
cosmicexplorer Apr 25, 2018
5d847a4
fix import order
cosmicexplorer Apr 25, 2018
efe75b8
respond to review comments
cosmicexplorer Apr 26, 2018
c0b4b36
add native backend to release.sh
cosmicexplorer Apr 26, 2018
6a86987
isolate native toolchain path and hope for the best
cosmicexplorer Apr 26, 2018
a39f21d
add backend plugin dependency (intentionally named differently)
cosmicexplorer Apr 26, 2018
26cc5dd
explicitly set CC and CXX because distutils
cosmicexplorer Apr 27, 2018
4ff80a2
add LDSHARED to setup.py environment for fun
cosmicexplorer Apr 27, 2018
1c5218b
try using gcc on travis
cosmicexplorer Apr 27, 2018
94f68f5
fix datatype() declarations
cosmicexplorer Apr 27, 2018
1c7e08d
fix old datatype usages
cosmicexplorer Apr 27, 2018
152e322
revert whitespace edit
cosmicexplorer Apr 27, 2018
514c1de
unclear why these are necessary
cosmicexplorer Apr 27, 2018
1cfe416
replicate the previous behavior
cosmicexplorer Apr 27, 2018
32b39e3
try again to isolate the path, this time using ARCHFLAGS
cosmicexplorer Apr 27, 2018
14eeaff
fix arguments for edited setup py env rule
cosmicexplorer Apr 27, 2018
7236837
restore the previous functionality again
cosmicexplorer Apr 27, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build-support/bin/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ function execute_packaged_pants_with_internal_backends() {
'pants.backend.docgen',\
'pants.backend.graph_info',\
'pants.backend.jvm',\
'pants.backend.native',\
'pants.backend.project_info',\
'pants.backend.python',\
'internal_backend.repositories',\
Expand Down
12 changes: 12 additions & 0 deletions src/python/pants/backend/native/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_library(
dependencies=[
'src/python/pants/backend/native/subsystems',
'src/python/pants/engine:rules',
'src/python/pants/util:objects',
'src/python/pants/util:osutil',
],
)
24 changes: 24 additions & 0 deletions src/python/pants/backend/native/register.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 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)

from pants.backend.native.subsystems.native_toolchain import NativeToolchain
from pants.engine.rules import RootRule, SingletonRule
from pants.util.objects import datatype
from pants.util.osutil import get_normalized_os_name


class Platform(datatype(['normalized_os_name'])):

def __new__(cls):
return super(Platform, cls).__new__(cls, get_normalized_os_name())


def rules():
return [
RootRule(NativeToolchain),
SingletonRule(Platform, Platform()),
]
6 changes: 5 additions & 1 deletion src/python/pants/backend/python/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
from pants.backend.python.tasks.python_run import PythonRun
from pants.backend.python.tasks.resolve_requirements import ResolveRequirements
from pants.backend.python.tasks.select_interpreter import SelectInterpreter
from pants.backend.python.tasks.setup_py import SetupPy
from pants.backend.python.tasks.setup_py import SetupPy, create_setup_py_rules
from pants.build_graph.build_file_aliases import BuildFileAliases
from pants.build_graph.resources import Resources
from pants.goal.task_registrar import TaskRegistrar as task
Expand Down Expand Up @@ -65,3 +65,7 @@ def register_goals():
task(name='setup-py', action=SetupPy).install()
task(name='py', action=PythonBinaryCreate).install('binary')
task(name='isort', action=IsortPythonTask).install('fmt')


def rules():
return create_setup_py_rules()
2 changes: 2 additions & 0 deletions src/python/pants/backend/python/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ python_library(
'src/python/pants/base:fingerprint_strategy',
'src/python/pants/base:hash_utils',
'src/python/pants/base:specs',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/build_graph',
'src/python/pants/invalidation',
'src/python/pants/python',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
from pants.backend.python.python_requirement import PythonRequirement
from pants.backend.python.targets.python_distribution import PythonDistribution
from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
from pants.backend.python.tasks.setup_py import SetupPyRunner
from pants.backend.python.tasks.setup_py import SetupPyInvocationEnvironment, SetupPyRunner
from pants.base.build_environment import get_buildroot
from pants.base.exceptions import TargetDefinitionException, TaskError
from pants.base.fingerprint_strategy import DefaultFingerprintStrategy
from pants.build_graph.address import Address
from pants.task.task import Task
from pants.util.contextutil import environment_as, get_joined_path
from pants.util.contextutil import environment_as
from pants.util.dirutil import safe_mkdir
from pants.util.memo import memoized_method

Expand Down Expand Up @@ -103,6 +103,11 @@ def _copy_sources(self, dist_tgt, dist_target_dir):
src_relative_to_target_base)
shutil.copyfile(abs_src_path, src_rel_to_results_dir)

def _request_single(self, product, subject):
# This is not supposed to be exposed to Tasks yet -- see #4769 to track the
# status of exposing v2 products in v1 tasks.
return self.context._scheduler.product_request(product, [subject])[0]

# FIXME(cosmicexplorer): We should be isolating the path to just our provided
# toolchain, but this causes errors in Travis because distutils looks for
# "x86_64-linux-gnu-gcc" when linking native extensions. We almost definitely
Expand All @@ -111,11 +116,9 @@ def _copy_sources(self, dist_tgt, dist_target_dir):
# compiler installed. Right now we just put our tools at the end of the PATH.
@contextmanager
def _setup_py_invocation_environment(self):
native_toolchain = self._native_toolchain_instance()
native_toolchain_path_entries = native_toolchain.path_entries()
appended_native_toolchain_path = get_joined_path(
native_toolchain_path_entries, os.environ.copy())
with environment_as(PATH=appended_native_toolchain_path):
setup_py_env = self._request_single(
SetupPyInvocationEnvironment, self._native_toolchain_instance())
with environment_as(**setup_py_env.as_env_dict()):
yield

def _create_dist(self, dist_tgt, dist_target_dir, interpreter):
Expand Down
26 changes: 26 additions & 0 deletions src/python/pants/backend/python/tasks/setup_py.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from twitter.common.collections import OrderedSet
from twitter.common.dirutil.chroot import Chroot

from pants.backend.native.subsystems.native_toolchain import NativeToolchain
from pants.backend.python.targets.python_binary import PythonBinary
from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
from pants.backend.python.targets.python_target import PythonTarget
Expand All @@ -29,10 +30,14 @@
from pants.build_graph.address_lookup_error import AddressLookupError
from pants.build_graph.build_graph import sort_targets
from pants.build_graph.resources import Resources
from pants.engine.rules import rule
from pants.engine.selectors import Select
from pants.task.task import Task
from pants.util.contextutil import get_joined_path
from pants.util.dirutil import safe_rmtree, safe_walk
from pants.util.memo import memoized_property
from pants.util.meta import AbstractClass
from pants.util.objects import datatype


SETUP_BOILERPLATE = """
Expand Down Expand Up @@ -74,6 +79,21 @@ def _setup_command(self):
return self.__setup_command


class SetupPyInvocationEnvironment(datatype(['joined_path'])):

def as_env_dict(self):
return {
'PATH': self.joined_path,
}


@rule(SetupPyInvocationEnvironment, [Select(NativeToolchain)])
def get_setup_py_env(native_toolchain):
joined_path = get_joined_path(
native_toolchain.path_entries(), os.environ.copy())
return SetupPyInvocationEnvironment(joined_path)


class TargetAncestorIterator(object):
"""Supports iteration of target ancestor lineages."""

Expand Down Expand Up @@ -640,3 +660,9 @@ def create(exported_python_target):
setup_runner = SetupPyRunner(setup_dir, self._run, interpreter=interpreter)
setup_runner.run()
python_dists[exported_python_target] = setup_dir


def create_setup_py_rules():
return [
get_setup_py_env,
]
15 changes: 12 additions & 3 deletions src/python/pants/bin/engine_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,19 @@ def create_build_graph(self, target_roots, build_root=None):
class EngineInitializer(object):
"""Constructs the components necessary to run the v2 engine with v1 BuildGraph compatibility."""

@staticmethod
def get_default_build_file_aliases():
_, build_config = OptionsInitializer(OptionsBootstrapper()).setup(init_logging=False)
return build_config.registered_aliases()

@staticmethod
def setup_legacy_graph(pants_ignore_patterns,
workdir,
build_file_imports_behavior,
build_root=None,
native=None,
build_file_aliases=None,
rules=None,
build_ignore_patterns=None,
exclude_target_regexps=None,
subproject_roots=None,
Expand Down Expand Up @@ -146,8 +152,10 @@ def setup_legacy_graph(pants_ignore_patterns,
scm = get_scm()

if not build_file_aliases:
_, build_config = OptionsInitializer(OptionsBootstrapper()).setup(init_logging=False)
build_file_aliases = build_config.registered_aliases()
build_file_aliases = EngineInitializer.get_default_build_file_aliases()

if not rules:
rules = []

symbol_table = LegacySymbolTable(build_file_aliases)

Expand All @@ -173,7 +181,8 @@ def setup_legacy_graph(pants_ignore_patterns,
create_legacy_graph_tasks(symbol_table) +
create_fs_rules() +
create_graph_rules(address_mapper, symbol_table) +
create_process_rules()
create_process_rules() +
rules
)

scheduler = LocalScheduler(workdir, dict(), tasks, project_tree, native, include_trace_on_error=include_trace_on_error)
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/bin/goal_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ def _init_graph(self,
self._global_options.build_file_imports,
native=native,
build_file_aliases=self._build_config.registered_aliases(),
rules=self._build_config.rules(),
build_ignore_patterns=build_ignore_patterns,
exclude_target_regexps=exclude_target_regexps,
subproject_roots=subproject_build_roots,
Expand Down
20 changes: 20 additions & 0 deletions src/python/pants/build_graph/build_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ def __init__(self):
self._exposed_object_by_alias = {}
self._exposed_context_aware_object_factory_by_alias = {}
self._subsystems = set()
self._rules = []

def registered_aliases(self):
"""Return the registered aliases exposed in BUILD files.
Expand Down Expand Up @@ -129,6 +130,25 @@ def subsystems(self):
"""
return self._subsystems

def register_rules(self, rules):
"""Registers the given rules.

:param rules: The rules to register.
:type rules: :class:`collections.Iterable` containing
:class:`pants.engine.rules.Rule` instances.
"""

if not isinstance(rules, Iterable):
raise TypeError('The rules must be an iterable, given {!r}'.format(rules))
self._rules.extend(rules)

def rules(self):
"""Returns the registered rules.

:rtype list
"""
return self._rules

@memoized_method
def _get_addressable_factory(self, target_type, alias):
return TargetAddressable.factory(target_type=target_type, alias=alias)
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/init/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ target(
'src/python/pants/backend/jvm:plugin',
'src/python/pants/backend/project_info:plugin',
'src/python/pants/backend/python:plugin',
'src/python/pants/backend/native',
],
)
4 changes: 4 additions & 0 deletions src/python/pants/init/extension_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,8 @@ def invoke_entrypoint(name):
if subsystems:
build_configuration.register_subsystems(subsystems)

rules = invoke_entrypoint('rules')
if rules:
build_configuration.register_rules(rules)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see some unit tests for this in test_extension_loader.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's only 3 lines long... seems like overkill?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair. My thought was that as this is a plugin-writer facing change, it should have tests. If we're currently thinking of it as being experimental, then it's fine without. It'd be nice if that were noted somehow though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's definitely covered by tests, in that the rest of this change relies on it working... it just doesn't seem worth a unit test. It's definitely not code with high cyclomatic complexity, for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree that it's important to test anything that goes out to plugin writers, but as far as I am aware this change only affects backends within the pants codebase. I did however write an extremely small test anyway (see here). The natural extension of this PR is to do the same for plugins, and I absolutely think that change be paired with testprojects using it and integration tests as well, probably, but for this PR I think passing the python dist testing that exists is sufficient to prove it works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!


invoke_entrypoint('register_goals')
1 change: 1 addition & 0 deletions src/python/pants/option/global_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def register_bootstrap_options(cls, register):
default=['pants.backend.graph_info',
'pants.backend.python',
'pants.backend.jvm',
'pants.backend.native',
'pants.backend.codegen.antlr.java',
'pants.backend.codegen.antlr.python',
'pants.backend.codegen.jaxb',
Expand Down
8 changes: 8 additions & 0 deletions src/python/pants/util/osutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,13 @@ def normalize_os_name(os_name):
return os_name


def get_normalized_os_name():
return normalize_os_name(get_os_name())


def all_normalized_os_names():
return OS_ALIASES.keys()


def known_os_names():
return reduce(set.union, OS_ALIASES.values())
3 changes: 3 additions & 0 deletions tests/python/pants_test/backend/python/tasks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ python_tests(
'3rdparty/python:coverage',
'3rdparty/python:mock',
'3rdparty/python:pex',
'src/python/pants/backend/native',
'src/python/pants/backend/python:plugin',
'src/python/pants/backend/python/subsystems',
'src/python/pants/backend/python/targets',
'src/python/pants/backend/python/tasks',
Expand All @@ -42,6 +44,7 @@ python_tests(
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'src/python/pants/util:process_handler',
'tests/python/pants_test/engine:scheduler_test_base',
'tests/python/pants_test/subsystem:subsystem_utils',
'tests/python/pants_test/tasks:task_test_base',
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@

from textwrap import dedent

from pants.backend.native.register import rules as native_backend_rules
from pants.backend.python.register import rules as python_backend_rules
from pants.backend.python.targets.python_distribution import PythonDistribution
from pants.backend.python.tasks.build_local_python_distributions import \
BuildLocalPythonDistributions
from pants_test.backend.python.tasks.python_task_test_base import PythonTaskTestBase
from pants_test.engine.scheduler_test_base import SchedulerTestBase


class TestBuildLocalPythonDistributions(PythonTaskTestBase):
class TestBuildLocalPythonDistributions(PythonTaskTestBase, SchedulerTestBase):
@classmethod
def task_type(cls):
return BuildLocalPythonDistributions
Expand Down Expand Up @@ -43,9 +46,18 @@ def setUp(self):
target_type=PythonDistribution,
sources=sources)

def _scheduling_context(self, **kwargs):
rules = (
native_backend_rules() +
python_backend_rules()
)
scheduler = self.mk_scheduler(rules=rules)
return self.context(scheduler=scheduler, **kwargs)

def test_python_create_distributions(self):
context = self.context(target_roots=[self.python_dist_tgt],
for_task_types=[BuildLocalPythonDistributions])
context = self._scheduling_context(
target_roots=[self.python_dist_tgt],
for_task_types=[BuildLocalPythonDistributions])
self.assertEquals([self.python_dist_tgt], context.build_graph.targets())
python_create_distributions_task = self.create_task(context)
python_create_distributions_task.execute()
Expand Down
5 changes: 3 additions & 2 deletions tests/python/pants_test/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ def set_options_for_scope(self, scope, **kwargs):

def context(self, for_task_types=None, for_subsystems=None, options=None,
target_roots=None, console_outstream=None, workspace=None,
**kwargs):
scheduler=None, **kwargs):
"""
:API: public

Expand Down Expand Up @@ -330,7 +330,8 @@ def context(self, for_task_types=None, for_subsystems=None, options=None,
build_file_parser=self.build_file_parser,
address_mapper=self.address_mapper,
console_outstream=console_outstream,
workspace=workspace)
workspace=workspace,
scheduler=scheduler)
return context

def tearDown(self):
Expand Down
3 changes: 3 additions & 0 deletions tests/python/pants_test/engine/scheduler_test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ def mk_scheduler(self,
self._native,
include_trace_on_error=include_trace_on_error)

def context_with_scheduler(self, scheduler, *args, **kwargs):
return self.context(*args, scheduler=scheduler, **kwargs)

def execute(self, scheduler, product, *subjects):
"""Runs an ExecutionRequest for the given product and subjects, and returns the result value."""
request = scheduler.execution_request([product], subjects)
Expand Down
5 changes: 5 additions & 0 deletions tests/python/pants_test/init/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,19 @@ python_tests(
'3rdparty/python:pex',
'3rdparty/python:setuptools',
'src/python/pants/base:exceptions',
'src/python/pants/bin',
'src/python/pants/build_graph',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/goal',
'src/python/pants/goal:task_registrar',
'src/python/pants/init',
'src/python/pants/option',
'src/python/pants/pantsd:pants_daemon',
'src/python/pants/subsystem',
'src/python/pants/util:contextutil',
'src/python/pants/util:dirutil',
'src/python/pants/util:objects',
'tests/python/pants_test:base_test',
'tests/python/pants_test/subsystem:subsystem_utils'
],
Expand Down
Loading