Skip to content

Commit

Permalink
try defining algebraic Executables in the native backend to compose m…
Browse files Browse the repository at this point in the history
…ore readable toolchains (pantsbuild#6855)

### Problem

As can be seen in `native_toolchain.py` in e.g. pantsbuild#6800, it is often difficult to follow changes to the native backend, especially changes which modify the order of resources such as library and include directories for our linkers and compilers. This is because we have been patching together collections of these resources "by hand", without applying any higher-level structure (explicitly constructing each `path_entries` and `library_dirs` field for every executable, every time, for example). This was done to avoid creating abstractions that might break down due to the rapidly evolving code. We can now take the step of more clearly defining the relationships between the toolchains we construct hierarchically.

### Solution

- Add an `ExtensibleAlgebraic` mixin which allows declaring list fields which can be immutably modified one at a time with `prepend_field` and `append_field`, or all at once with `sequence`.
- Add a `for_compiler` method to `BaseLinker` to wrap the specific process required to prep our linker for a specific compiler.
- Apply all of the above in `native_toolchain.py`.

### Result

The compilers and linkers provided by `@rule`s in `native_toolchain.py` are composed with consistent verbs from `ExtensibleAlgebraic`, leading to increased readability.
  • Loading branch information
cosmicexplorer committed Feb 26, 2019
1 parent cd4c773 commit 26b0179
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 175 deletions.
177 changes: 141 additions & 36 deletions src/python/pants/backend/native/config/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
from __future__ import absolute_import, division, print_function, unicode_literals

import os
from abc import abstractproperty
from abc import abstractmethod, abstractproperty

from pants.engine.rules import SingletonRule
from pants.util.memo import memoized_classproperty
from pants.util.meta import AbstractClass
from pants.util.objects import datatype, enum
from pants.util.osutil import all_normalized_os_names, get_normalized_os_name
Expand All @@ -19,9 +20,108 @@ class Platform(enum('normalized_os_name', all_normalized_os_names())):
default_value = get_normalized_os_name()


class Executable(AbstractClass):
def _list_field(func):
"""A decorator for methods corresponding to list-valued fields of an `ExtensibleAlgebraic`.
@abstractproperty
The result is also wrapped in `abstractproperty`.
"""
wrapped = abstractproperty(func)
wrapped._field_type = 'list'
return wrapped


def _algebraic_data(metaclass):
"""A class decorator to pull out `_list_fields` from a mixin class for use with a `datatype`."""
def wrapper(cls):
cls.__bases__ += (metaclass,)
cls._list_fields = metaclass._list_fields
return cls
return wrapper


# NB: prototypal inheritance seems *deeply* linked with the idea here!
# TODO: since we are calling these methods from other files, we should remove the leading underscore
# and add testing!
class _ExtensibleAlgebraic(AbstractClass):
"""A mixin to make it more concise to coalesce datatypes with related collection fields."""

@memoized_classproperty
def _list_fields(cls):
all_list_fields = []
for field_name in cls.__abstractmethods__:
f = getattr(cls, field_name)
if getattr(f, '_field_type', None) == 'list':
all_list_fields.append(field_name)
return frozenset(all_list_fields)

@abstractmethod
def copy(self, **kwargs):
"""Implementations should have the same behavior as a `datatype()`'s `copy()` method."""

class AlgebraicDataError(Exception): pass

def _single_list_field_operation(self, field_name, list_value, prepend=True):
if field_name not in self._list_fields:
raise self.AlgebraicDataError(
"Field '{}' is not in this object's set of declared list fields: {} (this object is : {})."
.format(field_name, self._list_fields, self))
cur_value = getattr(self, field_name)

if prepend:
new_value = list_value + cur_value
else:
new_value = cur_value + list_value

arg_dict = {field_name: new_value}
return self.copy(**arg_dict)

def prepend_field(self, field_name, list_value):
"""Return a copy of this object with `list_value` prepended to the field named `field_name`."""
return self._single_list_field_operation(field_name, list_value, prepend=True)

def append_field(self, field_name, list_value):
"""Return a copy of this object with `list_value` appended to the field named `field_name`."""
return self._single_list_field_operation(field_name, list_value, prepend=False)

def sequence(self, other, exclude_list_fields=None):
"""Return a copy of this object which combines all the fields common to both `self` and `other`.
List fields will be concatenated.
The return type of this method is the type of `self` (or whatever `.copy()` returns), but the
`other` argument can be any `_ExtensibleAlgebraic` instance.
"""
exclude_list_fields = frozenset(exclude_list_fields or [])
overwrite_kwargs = {}

nonexistent_excluded_fields = exclude_list_fields - self._list_fields
if nonexistent_excluded_fields:
raise self.AlgebraicDataError(
"Fields {} to exclude from a sequence() were not found in this object's list fields: {}. "
"This object is {}, the other object is {}."
.format(nonexistent_excluded_fields, self._list_fields, self, other))

shared_list_fields = (self._list_fields
& other._list_fields
- exclude_list_fields)
if not shared_list_fields:
raise self.AlgebraicDataError(
"Objects to sequence have no shared fields after excluding {}. "
"This object is {}, with list fields: {}. "
"The other object is {}, with list fields: {}."
.format(exclude_list_fields, self, self._list_fields, other, other._list_fields))

for list_field_name in shared_list_fields:
lhs_value = getattr(self, list_field_name)
rhs_value = getattr(other, list_field_name)
overwrite_kwargs[list_field_name] = lhs_value + rhs_value

return self.copy(**overwrite_kwargs)


class _Executable(_ExtensibleAlgebraic):

@_list_field
def path_entries(self):
"""A list of directory paths containing this executable, to be used in a subprocess's PATH.
Expand All @@ -37,32 +137,33 @@ def exe_filename(self):
:rtype: str
"""

# TODO: rename this to 'runtime_library_dirs'!
@abstractproperty
def library_dirs(self):
@_list_field
def runtime_library_dirs(self):
"""Directories containing shared libraries that must be on the runtime library search path.
Note: this is for libraries needed for the current Executable to run -- see LinkerMixin below
Note: this is for libraries needed for the current _Executable to run -- see _LinkerMixin below
for libraries that are needed at link time.
:rtype: list of str
"""

@property
@_list_field
def extra_args(self):
"""Additional arguments used when invoking this Executable.
"""Additional arguments used when invoking this _Executable.
These are typically placed before the invocation-specific command line arguments.
:rtype: list of str
"""
return []

_platform = Platform.create()

@property
def as_invocation_environment_dict(self):
"""A dict to use as this Executable's execution environment.
def invocation_environment_dict(self):
"""A dict to use as this _Executable's execution environment.
This isn't made into an "algebraic" field because its contents (the keys of the dict) are
generally known to the specific class which is overriding this property. Implementations of this
property can then make use of the data in the algebraic fields to populate this dict.
:rtype: dict of string -> string
"""
Expand All @@ -72,28 +173,29 @@ def as_invocation_environment_dict(self):
})
return {
'PATH': create_path_env_var(self.path_entries),
lib_env_var: create_path_env_var(self.library_dirs),
lib_env_var: create_path_env_var(self.runtime_library_dirs),
}


@_algebraic_data(_Executable)
class Assembler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
]), Executable):
pass
'runtime_library_dirs',
'extra_args',
])): pass


class LinkerMixin(Executable):
class _LinkerMixin(_Executable):

@abstractproperty
@_list_field
def linking_library_dirs(self):
"""Directories to search for libraries needed at link time.
:rtype: list of str
"""

@abstractproperty
@_list_field
def extra_object_files(self):
"""A list of object files required to perform a successful link.
Expand All @@ -103,8 +205,8 @@ def extra_object_files(self):
"""

@property
def as_invocation_environment_dict(self):
ret = super(LinkerMixin, self).as_invocation_environment_dict.copy()
def invocation_environment_dict(self):
ret = super(_LinkerMixin, self).invocation_environment_dict.copy()

full_library_path_dirs = self.linking_library_dirs + [
os.path.dirname(f) for f in self.extra_object_files
Expand All @@ -118,63 +220,66 @@ def as_invocation_environment_dict(self):
return ret


@_algebraic_data(_LinkerMixin)
class Linker(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'linking_library_dirs',
'extra_args',
'extra_object_files',
]), LinkerMixin): pass
])): pass


class CompilerMixin(Executable):
class _CompilerMixin(_Executable):

@abstractproperty
@_list_field
def include_dirs(self):
"""Directories to search for header files to #include during compilation.
:rtype: list of str
"""

@property
def as_invocation_environment_dict(self):
ret = super(CompilerMixin, self).as_invocation_environment_dict.copy()
def invocation_environment_dict(self):
ret = super(_CompilerMixin, self).invocation_environment_dict.copy()

if self.include_dirs:
ret['CPATH'] = create_path_env_var(self.include_dirs)

return ret


@_algebraic_data(_CompilerMixin)
class CCompiler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'include_dirs',
'extra_args',
]), CompilerMixin):
])):

@property
def as_invocation_environment_dict(self):
ret = super(CCompiler, self).as_invocation_environment_dict.copy()
def invocation_environment_dict(self):
ret = super(CCompiler, self).invocation_environment_dict.copy()

ret['CC'] = self.exe_filename

return ret


@_algebraic_data(_CompilerMixin)
class CppCompiler(datatype([
'path_entries',
'exe_filename',
'library_dirs',
'runtime_library_dirs',
'include_dirs',
'extra_args',
]), CompilerMixin):
])):

@property
def as_invocation_environment_dict(self):
ret = super(CppCompiler, self).as_invocation_environment_dict.copy()
def invocation_environment_dict(self):
ret = super(CppCompiler, self).invocation_environment_dict.copy()

ret['CXX'] = self.exe_filename

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ def assembler(self):
return Assembler(
path_entries=self.path_entries(),
exe_filename='as',
library_dirs=[])
runtime_library_dirs=[],
extra_args=[])

def linker(self):
return Linker(
path_entries=self.path_entries(),
exe_filename='ld',
library_dirs=[],
runtime_library_dirs=[],
linking_library_dirs=[],
extra_args=[],
extra_object_files=[],
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/native/subsystems/binaries/gcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def c_compiler(self, platform):
return CCompiler(
path_entries=self.path_entries,
exe_filename='gcc',
library_dirs=self._common_lib_dirs(platform),
runtime_library_dirs=self._common_lib_dirs(platform),
include_dirs=self._common_include_dirs,
extra_args=[])

Expand All @@ -91,7 +91,7 @@ def cpp_compiler(self, platform):
return CppCompiler(
path_entries=self.path_entries,
exe_filename='g++',
library_dirs=self._common_lib_dirs(platform),
runtime_library_dirs=self._common_lib_dirs(platform),
include_dirs=(self._common_include_dirs + self._cpp_include_dirs),
extra_args=[])

Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/native/subsystems/binaries/llvm.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def c_compiler(self):
return CCompiler(
path_entries=self.path_entries,
exe_filename='clang',
library_dirs=self._common_lib_dirs,
runtime_library_dirs=self._common_lib_dirs,
include_dirs=self._common_include_dirs,
extra_args=[])

Expand All @@ -117,7 +117,7 @@ def cpp_compiler(self):
return CppCompiler(
path_entries=self.path_entries,
exe_filename='clang++',
library_dirs=self._common_lib_dirs,
runtime_library_dirs=self._common_lib_dirs,
include_dirs=(self._cpp_include_dirs + self._common_include_dirs),
extra_args=[])

Expand Down
Loading

0 comments on commit 26b0179

Please sign in to comment.