Skip to content
Permalink
Browse files

Canonicalize enum pattern matching for execution strategy, platform, …

…and elsewhere (pantsbuild#7226)

### Problem

In pantsbuild#7092 we added [`NailgunTask#do_for_execution_strategy_variant()`](https://github.com/cosmicexplorer/pants/blob/70977ef064305b78406a627e07f4dae3a60e4ae4/src/python/pants/backend/jvm/tasks/nailgun_task.py#L31-L43), which allowed performing more declarative execution strategy-specific logic in nailgunnable tasks. Further work with rsc will do even more funky things with our nailgunnable task logic, and while we will eventually have a unified story again for nailgun and subprocess invocations with the v2 engine (see pantsbuild#7079), for now having this check that we have performed the logic we expect all execution strategy variants is very useful.

This PR puts that pattern matching logic into `enum()`: https://github.com/pantsbuild/pants/blob/84cf9a75dbf68cf7126fe8372ab9b2f48720464d/src/python/pants/util/objects.py#L173-L174, among other things.

**Note:** `TypeCheckError` and other exceptions are moved up from further down in `objects.py`.

### Solution

- add `resolve_for_enum_variant()` method to `enum` which does the job of the previous `do_for_execution_strategy_variant()`
- make the native backend's `Platform` into an enum.
- stop silently converting a `None` argument to the enum's `create()` classmethod into its`default_value`.
- add `register_enum_option()` helper method to register options based on enum types.

### Result

We have a low-overhead way to convert potentially-tricky conditional logic into a checked pattern matching-style interface with `enum()`, and it is easier to register enum options.
  • Loading branch information...
cosmicexplorer committed Feb 12, 2019
1 parent d0432df commit e382541d70c5b57e682dce998d361c96aed4d8e9
@@ -201,11 +201,11 @@ def _nailgunnable_combined_classpath(self):

# Overrides the normal zinc compiler classpath, which only contains zinc.
def get_zinc_compiler_classpath(self):
return self.do_for_execution_strategy_variant({
return self.execution_strategy_enum.resolve_for_enum_variant({
self.HERMETIC: lambda: super(RscCompile, self).get_zinc_compiler_classpath(),
self.SUBPROCESS: lambda: super(RscCompile, self).get_zinc_compiler_classpath(),
self.NAILGUN: lambda: self._nailgunnable_combined_classpath,
})
})()

def register_extra_products_from_contexts(self, targets, compile_contexts):
super(RscCompile, self).register_extra_products_from_contexts(targets, compile_contexts)
@@ -861,15 +861,15 @@ def _runtool_nonhermetic(self, parent_workunit, classpath, main, tool_name, args
def _runtool(self, main, tool_name, args, distribution,
tgt=None, input_files=tuple(), input_digest=None, output_dir=None):
with self.context.new_workunit(tool_name) as wu:
return self.do_for_execution_strategy_variant({
return self.execution_strategy_enum.resolve_for_enum_variant({
self.HERMETIC: lambda: self._runtool_hermetic(
main, tool_name, args, distribution,
tgt=tgt, input_files=input_files, input_digest=input_digest, output_dir=output_dir),
self.SUBPROCESS: lambda: self._runtool_nonhermetic(
wu, self.tool_classpath(tool_name), main, tool_name, args, distribution),
self.NAILGUN: lambda: self._runtool_nonhermetic(
wu, self._nailgunnable_combined_classpath, main, tool_name, args, distribution),
})
})()

def _run_metai_tool(self,
distribution,
@@ -15,6 +15,7 @@
from pants.process.subprocess import Subprocess
from pants.task.task import Task, TaskBase
from pants.util.memo import memoized_property
from pants.util.objects import enum, register_enum_option


class NailgunTaskBase(JvmToolTaskMixin, TaskBase):
@@ -24,30 +25,16 @@ class NailgunTaskBase(JvmToolTaskMixin, TaskBase):
SUBPROCESS = 'subprocess'
HERMETIC = 'hermetic'

class InvalidExecutionStrategyMapping(Exception): pass

_all_execution_strategies = frozenset([NAILGUN, SUBPROCESS, HERMETIC])

def do_for_execution_strategy_variant(self, mapping):
"""Invoke the method in `mapping` with the key corresponding to the execution strategy.
`mapping` is a dict mapping execution strategy -> zero-argument lambda.
"""
variants = frozenset(mapping.keys())
if variants != self._all_execution_strategies:
raise self.InvalidExecutionStrategyMapping(
'Must specify a mapping with exactly the keys {} (was: {})'
.format(self._all_execution_strategies, variants))
method_for_variant = mapping[self.execution_strategy]
# The methods need not return a value, but we pass it along if they do.
return method_for_variant()
class ExecutionStrategy(enum([NAILGUN, SUBPROCESS, HERMETIC])): pass

@classmethod
def register_options(cls, register):
super(NailgunTaskBase, cls).register_options(register)
register('--execution-strategy', choices=[cls.NAILGUN, cls.SUBPROCESS, cls.HERMETIC], default=cls.NAILGUN,
help='If set to nailgun, nailgun will be enabled and repeated invocations of this '
'task will be quicker. If set to subprocess, then the task will be run without nailgun.')
register_enum_option(
register, cls.ExecutionStrategy, '--execution-strategy',
help='If set to nailgun, nailgun will be enabled and repeated invocations of this '
'task will be quicker. If set to subprocess, then the task will be run without nailgun. '
'Hermetic execution is an experimental subprocess execution framework.')
register('--nailgun-timeout-seconds', advanced=True, default=10, type=float,
help='Timeout (secs) for nailgun startup.')
register('--nailgun-connect-attempts', advanced=True, default=5, type=int,
@@ -60,6 +47,13 @@ def register_options(cls, register):
rev='0.9.1'),
])

@memoized_property
def execution_strategy_enum(self):
# TODO: This .create() call can be removed when the enum interface is more stable as the option
# is converted into an instance of self.ExecutionStrategy via the `type` argument through
# register_enum_option().
return self.ExecutionStrategy.create(self.get_options().execution_strategy)

@classmethod
def subsystem_dependencies(cls):
return super(NailgunTaskBase, cls).subsystem_dependencies() + (Subprocess.Factory,)
@@ -76,9 +70,10 @@ def __init__(self, *args, **kwargs):
self._executor_workdir = os.path.join(self.context.options.for_global_scope().pants_workdir,
*id_tuple)

@memoized_property
# TODO: eventually deprecate this when we can move all subclasses to use the enum!
@property
def execution_strategy(self):
return self.get_options().execution_strategy
return self.execution_strategy_enum.value

def create_java_executor(self, dist=None):
"""Create java executor that uses this task's ng daemon, if allowed.
@@ -9,37 +9,14 @@

from pants.engine.rules import SingletonRule
from pants.util.meta import AbstractClass
from pants.util.objects import datatype
from pants.util.objects import datatype, enum
from pants.util.osutil import all_normalized_os_names, get_normalized_os_name
from pants.util.strutil import create_path_env_var


class Platform(datatype(['normalized_os_name'])):
class Platform(enum('normalized_os_name', all_normalized_os_names())):

class UnsupportedPlatformError(Exception):
"""Thrown if pants is running on an unrecognized platform."""

@classmethod
def create(cls):
return Platform(get_normalized_os_name())

_NORMALIZED_OS_NAMES = frozenset(all_normalized_os_names())

def resolve_platform_specific(self, platform_specific_funs):
arg_keys = frozenset(platform_specific_funs.keys())
unknown_plats = self._NORMALIZED_OS_NAMES - arg_keys
if unknown_plats:
raise self.UnsupportedPlatformError(
"platform_specific_funs {} must support platforms {}"
.format(platform_specific_funs, list(unknown_plats)))
extra_plats = arg_keys - self._NORMALIZED_OS_NAMES
if extra_plats:
raise self.UnsupportedPlatformError(
"platform_specific_funs {} has unrecognized platforms {}"
.format(platform_specific_funs, list(extra_plats)))

fun_for_platform = platform_specific_funs[self.normalized_os_name]
return fun_for_platform()
default_value = get_normalized_os_name()


class Executable(AbstractClass):
@@ -89,9 +66,9 @@ def as_invocation_environment_dict(self):
:rtype: dict of string -> string
"""
lib_env_var = self._platform.resolve_platform_specific({
'darwin': lambda: 'DYLD_LIBRARY_PATH',
'linux': lambda: 'LD_LIBRARY_PATH',
lib_env_var = self._platform.resolve_for_enum_variant({
'darwin': 'DYLD_LIBRARY_PATH',
'linux': 'LD_LIBRARY_PATH',
})
return {
'PATH': create_path_env_var(self.path_entries),
@@ -44,9 +44,9 @@ def path_entries(self):

@memoized_method
def _common_lib_dirs(self, platform):
lib64_tuples = platform.resolve_platform_specific({
'darwin': lambda: [],
'linux': lambda: [('lib64',)],
lib64_tuples = platform.resolve_for_enum_variant({
'darwin': [],
'linux': [('lib64',)],
})
return self._filemap(lib64_tuples + [
('lib',),
@@ -80,16 +80,13 @@ def _filemap(self, all_components_list):
def path_entries(self):
return self._filemap([('bin',)])

_PLATFORM_SPECIFIC_LINKER_NAME = {
'darwin': lambda: 'ld64.lld',
'linux': lambda: 'lld',
}

def linker(self, platform):
return Linker(
path_entries=self.path_entries,
exe_filename=platform.resolve_platform_specific(
self._PLATFORM_SPECIFIC_LINKER_NAME),
exe_filename=platform.resolve_for_enum_variant({
'darwin': 'ld64.lld',
'linux': 'lld',
}),
library_dirs=[],
linking_library_dirs=[],
extra_args=[],
@@ -10,14 +10,10 @@
from pants.subsystem.subsystem import Subsystem
from pants.util.memo import memoized_property
from pants.util.meta import classproperty
from pants.util.objects import enum
from pants.util.objects import enum, register_enum_option


class ToolchainVariant(enum('descriptor', ['gnu', 'llvm'])):

@property
def is_gnu(self):
return self.descriptor == 'gnu'
class ToolchainVariant(enum(['gnu', 'llvm'])): pass


class NativeBuildStep(CompilerOptionSetsMixin, MirroredTargetOptionMixin, Subsystem):
@@ -39,11 +35,10 @@ def register_options(cls, register):
help='The default for the "compiler_option_sets" argument '
'for targets of this language.')

register('--toolchain-variant', type=str, fingerprint=True, advanced=True,
choices=ToolchainVariant.allowed_values,
default=ToolchainVariant.default_value,
help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
"linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")
register_enum_option(
register, ToolchainVariant, '--toolchain-variant', advanced=True,
help="Whether to use gcc (gnu) or clang (llvm) to compile C and C++. Currently all "
"linking is done with binutils ld on Linux, and the XCode CLI Tools on MacOS.")

def get_compiler_option_sets_for_target(self, target):
return self.get_target_mirrored_option('compiler_option_sets', target)
@@ -87,10 +87,11 @@ class LLVMCppToolchain(datatype([('cpp_toolchain', CppToolchain)])): pass

@rule(LibcObjects, [Select(Platform), Select(NativeToolchain)])
def select_libc_objects(platform, native_toolchain):
paths = platform.resolve_platform_specific({
# We use lambdas here to avoid searching for libc on osx, where it will fail.
paths = platform.resolve_for_enum_variant({
'darwin': lambda: [],
'linux': lambda: native_toolchain._libc_dev.get_libc_objects(),
})
})()
yield LibcObjects(paths)


@@ -343,8 +344,12 @@ class ToolchainVariantRequest(datatype([
@rule(CToolchain, [Select(ToolchainVariantRequest)])
def select_c_toolchain(toolchain_variant_request):
native_toolchain = toolchain_variant_request.toolchain
# TODO: make an enum exhaustiveness checking method that works with `yield Get(...)` statements!
if toolchain_variant_request.variant.is_gnu:
# TODO(#5933): make an enum exhaustiveness checking method that works with `yield Get(...)`!
use_gcc = toolchain_variant_request.variant.resolve_for_enum_variant({
'gnu': True,
'llvm': False,
})
if use_gcc:
toolchain_resolved = yield Get(GCCCToolchain, NativeToolchain, native_toolchain)
else:
toolchain_resolved = yield Get(LLVMCToolchain, NativeToolchain, native_toolchain)
@@ -354,7 +359,12 @@ def select_c_toolchain(toolchain_variant_request):
@rule(CppToolchain, [Select(ToolchainVariantRequest)])
def select_cpp_toolchain(toolchain_variant_request):
native_toolchain = toolchain_variant_request.toolchain
if toolchain_variant_request.variant.is_gnu:
# TODO(#5933): make an enum exhaustiveness checking method that works with `yield Get(...)`!
use_gcc = toolchain_variant_request.variant.resolve_for_enum_variant({
'gnu': True,
'llvm': False,
})
if use_gcc:
toolchain_resolved = yield Get(GCCCppToolchain, NativeToolchain, native_toolchain)
else:
toolchain_resolved = yield Get(LLVMCppToolchain, NativeToolchain, native_toolchain)
@@ -22,9 +22,9 @@ def alias(cls):

def as_shared_lib(self, platform):
# TODO: check that the name conforms to some format in the constructor (e.g. no dots?).
return platform.resolve_platform_specific({
'darwin': lambda: 'lib{}.dylib'.format(self.lib_name),
'linux': lambda: 'lib{}.so'.format(self.lib_name),
return platform.resolve_for_enum_variant({
'darwin': 'lib{}.dylib'.format(self.lib_name),
'linux': 'lib{}.so'.format(self.lib_name),
})

def _compute_fingerprint(self):
@@ -124,9 +124,9 @@ def _conan_user_home(self, conan, in_workdir=False):

@memoized_property
def _conan_os_name(self):
return Platform.create().resolve_platform_specific({
'darwin': lambda: 'Macos',
'linux': lambda: 'Linux',
return Platform.create().resolve_for_enum_variant({
'darwin': 'Macos',
'linux': 'Linux',
})

@property
@@ -142,11 +142,6 @@ def _make_link_request(self, vt, compiled_objects_product):

return link_request

_SHARED_CMDLINE_ARGS = {
'darwin': lambda: ['-Wl,-dylib'],
'linux': lambda: ['-shared'],
}

def _execute_link_request(self, link_request):
object_files = link_request.object_files

@@ -163,7 +158,10 @@ def _execute_link_request(self, link_request):
self.context.log.debug("resulting_shared_lib_path: {}".format(resulting_shared_lib_path))
# We are executing in the results_dir, so get absolute paths for everything.
cmd = ([linker.exe_filename] +
self.platform.resolve_platform_specific(self._SHARED_CMDLINE_ARGS) +
self.platform.resolve_for_enum_variant({
'darwin': ['-Wl,-dylib'],
'linux': ['-shared'],
}) +
linker.extra_args +
['-o', os.path.abspath(resulting_shared_lib_path)] +
['-L{}'.format(lib_dir) for lib_dir in link_request.external_lib_dirs] +
@@ -105,9 +105,9 @@ def _name_and_platform(whl):

@memoized_classproperty
def _current_platform_abbreviation(cls):
return NativeBackendPlatform.create().resolve_platform_specific({
'darwin': lambda: 'macosx',
'linux': lambda: 'linux',
return NativeBackendPlatform.create().resolve_for_enum_variant({
'darwin': 'macosx',
'linux': 'linux',
})

@classmethod
@@ -57,8 +57,9 @@ def __new__(cls, include, exclude=(), glob_match_error_behavior=None, conjunctio
cls,
include=tuple(include),
exclude=tuple(exclude),
glob_match_error_behavior=GlobMatchErrorBehavior.create(glob_match_error_behavior),
conjunction=GlobExpansionConjunction.create(conjunction))
glob_match_error_behavior=GlobMatchErrorBehavior.create(glob_match_error_behavior,
none_is_default=True),
conjunction=GlobExpansionConjunction.create(conjunction, none_is_default=True))


class PathGlobsAndRoot(datatype([('path_globs', PathGlobs), ('root', text_type)])):
@@ -346,7 +346,8 @@ def setup_legacy_graph_extended(
rules = (
[
RootRule(Console),
SingletonRule.from_instance(GlobMatchErrorBehavior.create(glob_match_error_behavior)),
SingletonRule.from_instance(GlobMatchErrorBehavior.create(glob_match_error_behavior,
none_is_default=True)),
SingletonRule.from_instance(build_configuration),
SingletonRule(SymbolTable, symbol_table),
] +
@@ -16,7 +16,7 @@
from pants.option.optionable import Optionable
from pants.option.scope import ScopeInfo
from pants.subsystem.subsystem_client_mixin import SubsystemClientMixin
from pants.util.objects import datatype, enum
from pants.util.objects import datatype, enum, register_enum_option


class GlobMatchErrorBehavior(enum('failure_behavior', ['ignore', 'warn', 'error'])):
@@ -26,8 +26,6 @@ class GlobMatchErrorBehavior(enum('failure_behavior', ['ignore', 'warn', 'error'
be aware of any changes to this object's definition.
"""

default_option_value = 'warn'


class ExecutionOptions(datatype([
'remote_store_server',
@@ -197,12 +195,12 @@ def register_bootstrap_options(cls, register):
help='Paths to ignore for all filesystem operations performed by pants '
'(e.g. BUILD file scanning, glob matching, etc). '
'Patterns use the gitignore syntax (https://git-scm.com/docs/gitignore).')
register('--glob-expansion-failure', type=str,
choices=GlobMatchErrorBehavior.allowed_values,
default=GlobMatchErrorBehavior.default_option_value,
advanced=True,
help="Raise an exception if any targets declaring source files "
"fail to match any glob provided in the 'sources' argument.")
register_enum_option(
# TODO: allow using the attribute `GlobMatchErrorBehavior.warn` for more safety!
register, GlobMatchErrorBehavior, '--glob-expansion-failure', default='warn',
advanced=True,
help="Raise an exception if any targets declaring source files "
"fail to match any glob provided in the 'sources' argument.")

register('--exclude-target-regexp', advanced=True, type=list, default=[], daemon=False,
metavar='<regexp>', help='Exclude target roots that match these regexes.')
Oops, something went wrong.

0 comments on commit e382541

Please sign in to comment.
You can’t perform that action at this time.