Skip to content

Commit

Permalink
Remove yielding @rules (pantsbuild#8652)
Browse files Browse the repository at this point in the history
### Problem

We believe the new `async`/`await` syntax for coroutine `@rule`s is superior to `yield`ing generators, in part because it allows mypy to successfully be run on files defining `@rule`s.

### Solution

- Convert `yield`ing `@rule`s into `async` methods, and use `await Get(...)` and `await MultiGet(...)` instead of `yield Get(...)` and `yield [Get(...) ...]`.
- Remove the `asttokens` 3rdparty library and a *ridiculous* amount of code in `rules.py`!!!
  • Loading branch information
cosmicexplorer authored Nov 24, 2019
1 parent ecaf644 commit fbe99a1
Show file tree
Hide file tree
Showing 37 changed files with 249 additions and 508 deletions.
1 change: 0 additions & 1 deletion 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
ansicolors==1.0.2
asttokens==1.1.13
beautifulsoup4>=4.6.0,<4.7
cffi==1.13.2
contextlib2==0.5.5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def fast_list_and_die_for_testing(
) -> ListAndDieForTesting:
for address in addresses.dependencies:
console.print_stdout(address.spec)
yield ListAndDieForTesting(exit_code=42)
return ListAndDieForTesting(exit_code=42)


def rules():
Expand Down
96 changes: 48 additions & 48 deletions src/python/pants/backend/native/subsystems/native_toolchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,22 @@ class LLVMCppToolchain:


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


@rule
def select_assembler(platform: Platform, native_toolchain: NativeToolchain) -> Assembler:
async def select_assembler(platform: Platform, native_toolchain: NativeToolchain) -> Assembler:
if platform == Platform.darwin:
assembler = yield Get(Assembler, XCodeCLITools, native_toolchain._xcode_cli_tools)
assembler = await Get(Assembler, XCodeCLITools, native_toolchain._xcode_cli_tools)
else:
assembler = yield Get(Assembler, Binutils, native_toolchain._binutils)
yield assembler
assembler = await Get(Assembler, Binutils, native_toolchain._binutils)
return assembler


@dataclass(frozen=True)
Expand All @@ -146,23 +146,23 @@ class BaseLinker:

# TODO: select the appropriate `Platform` in the `@rule` decl using variants!
@rule
def select_base_linker(platform: Platform, native_toolchain: NativeToolchain) -> BaseLinker:
async def select_base_linker(platform: Platform, native_toolchain: NativeToolchain) -> BaseLinker:
if platform == Platform.darwin:
# TODO(#5663): turn this into LLVM when lld works.
linker = yield Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools)
linker = await Get(Linker, XCodeCLITools, native_toolchain._xcode_cli_tools)
else:
linker = yield Get(Linker, Binutils, native_toolchain._binutils)
linker = await Get(Linker, Binutils, native_toolchain._binutils)
base_linker = BaseLinker(linker=linker)
yield base_linker
return base_linker


@rule
def select_gcc_linker(native_toolchain: NativeToolchain) -> GCCLinker:
base_linker = yield Get(BaseLinker, NativeToolchain, native_toolchain)
async def select_gcc_linker(native_toolchain: NativeToolchain) -> GCCLinker:
base_linker = await Get(BaseLinker, NativeToolchain, native_toolchain)
linker = base_linker.linker
libc_objects = yield Get(LibcObjects, NativeToolchain, native_toolchain)
libc_objects = await Get(LibcObjects, NativeToolchain, native_toolchain)
linker_with_libc = linker.append_field('extra_object_files', libc_objects.crti_object_paths)
yield GCCLinker(linker_with_libc)
return GCCLinker(linker_with_libc)


@rule
Expand Down Expand Up @@ -191,15 +191,15 @@ def select_gcc_install_location(gcc: GCC) -> GCCInstallLocationForLLVM:


@rule
def select_llvm_c_toolchain(platform: Platform, native_toolchain: NativeToolchain) -> LLVMCToolchain:
provided_clang = yield Get(CCompiler, LLVM, native_toolchain._llvm)
async def select_llvm_c_toolchain(platform: Platform, native_toolchain: NativeToolchain) -> LLVMCToolchain:
provided_clang = await Get(CCompiler, LLVM, native_toolchain._llvm)

if platform == Platform.darwin:
xcode_clang = yield Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
xcode_clang = await Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
joined_c_compiler = provided_clang.sequence(xcode_clang)
else:
gcc_install = yield Get(GCCInstallLocationForLLVM, GCC, native_toolchain._gcc)
provided_gcc = yield Get(CCompiler, GCC, native_toolchain._gcc)
gcc_install = await Get(GCCInstallLocationForLLVM, GCC, native_toolchain._gcc)
provided_gcc = await Get(CCompiler, GCC, native_toolchain._gcc)
joined_c_compiler = (provided_clang
.sequence(provided_gcc)
.append_field('extra_args', gcc_install.as_clang_argv)
Expand All @@ -210,28 +210,28 @@ def select_llvm_c_toolchain(platform: Platform, native_toolchain: NativeToolchai
'-x', 'c', '-std=c11',
])

llvm_linker_wrapper = yield Get(LLVMLinker, NativeToolchain, native_toolchain)
llvm_linker_wrapper = await Get(LLVMLinker, NativeToolchain, native_toolchain)
working_linker = llvm_linker_wrapper.for_compiler(working_c_compiler, platform)

yield LLVMCToolchain(CToolchain(working_c_compiler, working_linker))
return LLVMCToolchain(CToolchain(working_c_compiler, working_linker))


@rule
def select_llvm_cpp_toolchain(
async def select_llvm_cpp_toolchain(
platform: Platform, native_toolchain: NativeToolchain
) -> LLVMCppToolchain:
provided_clangpp = yield Get(CppCompiler, LLVM, native_toolchain._llvm)
provided_clangpp = await Get(CppCompiler, LLVM, native_toolchain._llvm)

# On OSX, we use the libc++ (LLVM) C++ standard library implementation. This is feature-complete
# for OSX, but not for Linux (see https://libcxx.llvm.org/ for more info).
if platform == Platform.darwin:
xcode_clangpp = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
xcode_clangpp = await Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
joined_cpp_compiler = provided_clangpp.sequence(xcode_clangpp)
extra_llvm_linking_library_dirs = []
linker_extra_args = []
else:
gcc_install = yield Get(GCCInstallLocationForLLVM, GCC, native_toolchain._gcc)
provided_gpp = yield Get(CppCompiler, GCC, native_toolchain._gcc)
gcc_install = await Get(GCCInstallLocationForLLVM, GCC, native_toolchain._gcc)
provided_gpp = await Get(CppCompiler, GCC, native_toolchain._gcc)
joined_cpp_compiler = (provided_clangpp
.sequence(provided_gpp)
# NB: we use g++'s headers on Linux, and therefore their C++ standard
Expand All @@ -253,59 +253,59 @@ def select_llvm_cpp_toolchain(
'-nostdinc++',
])

llvm_linker_wrapper = yield Get(LLVMLinker, NativeToolchain, native_toolchain)
llvm_linker_wrapper = await Get(LLVMLinker, NativeToolchain, native_toolchain)
working_linker = (llvm_linker_wrapper
.for_compiler(working_cpp_compiler, platform)
.append_field('linking_library_dirs', extra_llvm_linking_library_dirs)
.prepend_field('extra_args', linker_extra_args))

yield LLVMCppToolchain(CppToolchain(working_cpp_compiler, working_linker))
return LLVMCppToolchain(CppToolchain(working_cpp_compiler, working_linker))


@rule
def select_gcc_c_toolchain(platform: Platform, native_toolchain: NativeToolchain) -> GCCCToolchain:
provided_gcc = yield Get(CCompiler, GCC, native_toolchain._gcc)
async def select_gcc_c_toolchain(platform: Platform, native_toolchain: NativeToolchain) -> GCCCToolchain:
provided_gcc = await Get(CCompiler, GCC, native_toolchain._gcc)

if platform == Platform.darwin:
# GCC needs access to some headers that are only provided by the XCode toolchain
# currently (e.g. "_stdio.h"). These headers are unlikely to change across versions, so this is
# probably safe.
xcode_clang = yield Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
xcode_clang = await Get(CCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
joined_c_compiler = provided_gcc.sequence(xcode_clang)
else:
joined_c_compiler = provided_gcc

# GCC needs an assembler, so we provide that (platform-specific) tool here.
assembler = yield Get(Assembler, NativeToolchain, native_toolchain)
assembler = await Get(Assembler, NativeToolchain, native_toolchain)
working_c_compiler = joined_c_compiler.sequence(assembler).prepend_field('extra_args', [
'-x', 'c', '-std=c11',
])

gcc_linker_wrapper = yield Get(GCCLinker, NativeToolchain, native_toolchain)
gcc_linker_wrapper = await Get(GCCLinker, NativeToolchain, native_toolchain)
working_linker = gcc_linker_wrapper.for_compiler(working_c_compiler, platform)

yield GCCCToolchain(CToolchain(working_c_compiler, working_linker))
return GCCCToolchain(CToolchain(working_c_compiler, working_linker))


@rule
def select_gcc_cpp_toolchain(
async def select_gcc_cpp_toolchain(
platform: Platform, native_toolchain: NativeToolchain
) -> GCCCppToolchain:
provided_gpp = yield Get(CppCompiler, GCC, native_toolchain._gcc)
provided_gpp = await Get(CppCompiler, GCC, native_toolchain._gcc)

if platform == Platform.darwin:
# GCC needs access to some headers that are only provided by the XCode toolchain
# currently (e.g. "_stdio.h"). These headers are unlikely to change across versions, so this is
# probably safe.
# TODO: we should be providing all of these (so we can eventually phase out XCodeCLITools
# entirely).
xcode_clangpp = yield Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
xcode_clangpp = await Get(CppCompiler, XCodeCLITools, native_toolchain._xcode_cli_tools)
joined_cpp_compiler = provided_gpp.sequence(xcode_clangpp)
else:
joined_cpp_compiler = provided_gpp

# GCC needs an assembler, so we provide that (platform-specific) tool here.
assembler = yield Get(Assembler, NativeToolchain, native_toolchain)
assembler = await Get(Assembler, NativeToolchain, native_toolchain)
working_cpp_compiler = joined_cpp_compiler.sequence(assembler).prepend_field('extra_args', [
'-x', 'c++', '-std=c++11',
# This flag is intended to avoid using any of the headers from our LLVM distribution's C++
Expand All @@ -315,10 +315,10 @@ def select_gcc_cpp_toolchain(
'-nostdinc++',
])

gcc_linker_wrapper = yield Get(GCCLinker, NativeToolchain, native_toolchain)
gcc_linker_wrapper = await Get(GCCLinker, NativeToolchain, native_toolchain)
working_linker = gcc_linker_wrapper.for_compiler(working_cpp_compiler, platform)

yield GCCCppToolchain(CppToolchain(working_cpp_compiler, working_linker))
return GCCCppToolchain(CppToolchain(working_cpp_compiler, working_linker))


@dataclass(frozen=True)
Expand All @@ -328,27 +328,27 @@ class ToolchainVariantRequest:


@rule
def select_c_toolchain(toolchain_variant_request: ToolchainVariantRequest) -> CToolchain:
async def select_c_toolchain(toolchain_variant_request: ToolchainVariantRequest) -> CToolchain:
use_gcc = toolchain_variant_request.variant == ToolchainVariant.gnu
if use_gcc:
toolchain_resolved = yield Get(GCCCToolchain, NativeToolchain, toolchain_variant_request.toolchain)
toolchain_resolved = await Get(GCCCToolchain, NativeToolchain, toolchain_variant_request.toolchain)
else:
toolchain_resolved = yield Get(LLVMCToolchain, NativeToolchain, toolchain_variant_request.toolchain)
yield toolchain_resolved.c_toolchain
toolchain_resolved = await Get(LLVMCToolchain, NativeToolchain, toolchain_variant_request.toolchain)
return toolchain_resolved.c_toolchain


@rule
def select_cpp_toolchain(toolchain_variant_request: ToolchainVariantRequest) -> CppToolchain:
async def select_cpp_toolchain(toolchain_variant_request: ToolchainVariantRequest) -> CppToolchain:
use_gcc = toolchain_variant_request.variant == ToolchainVariant.gnu
if use_gcc:
toolchain_resolved = yield Get(
toolchain_resolved = await Get(
GCCCppToolchain, NativeToolchain, toolchain_variant_request.toolchain
)
else:
toolchain_resolved = yield Get(
toolchain_resolved = await Get(
LLVMCppToolchain, NativeToolchain, toolchain_variant_request.toolchain
)
yield toolchain_resolved.cpp_toolchain
return toolchain_resolved.cpp_toolchain


def create_native_toolchain_rules():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
from pants.engine.legacy.graph import HydratedTarget, HydratedTargets
from pants.engine.objects import Collection
from pants.engine.rules import console_rule, optionable_rule, rule
from pants.engine.selectors import Get
from pants.engine.selectors import Get, MultiGet
from pants.subsystem.subsystem import Subsystem
from pants.util.collections import Enum
from pants.util.memo import memoized_method
Expand Down Expand Up @@ -117,7 +117,7 @@ def matches(self, s):

class PathMatcher(Matcher):
"""A matcher for matching file paths."""

def __init__(self, pattern, inverted=False, content_encoding='utf8'):
super().__init__(pattern, inverted)
# The expected encoding of the content of files whose paths match this pattern.
Expand Down Expand Up @@ -214,10 +214,10 @@ def get_applicable_content_pattern_names(self, path):
# TODO: Switch this to `lint` once we figure out a good way for v1 tasks and v2 rules
# to share goal names.
@console_rule
def validate(
async def validate(
console: Console, hydrated_targets: HydratedTargets, validate_options: Validate.Options
) -> Validate:
per_tgt_rmrs = yield [Get(RegexMatchResults, HydratedTarget, ht) for ht in hydrated_targets]
per_tgt_rmrs = await MultiGet(Get(RegexMatchResults, HydratedTarget, ht) for ht in hydrated_targets)
regex_match_results = list(itertools.chain(*per_tgt_rmrs))

detail_level = validate_options.values.detail_level
Expand Down Expand Up @@ -250,21 +250,21 @@ def validate(
exit_code = PANTS_FAILED_EXIT_CODE
else:
exit_code = PANTS_SUCCEEDED_EXIT_CODE
yield Validate(exit_code)
return Validate(exit_code)


@rule
def match_regexes_for_one_target(
async def match_regexes_for_one_target(
hydrated_target: HydratedTarget, source_file_validation: SourceFileValidation
) -> RegexMatchResults:
multi_matcher = source_file_validation.get_multi_matcher()
rmrs = []
if hasattr(hydrated_target.adaptor, 'sources'):
files_content = yield Get(FilesContent,
files_content = await Get(FilesContent,
Digest, hydrated_target.adaptor.sources.snapshot.directory_digest)
for file_content in files_content:
rmrs.append(multi_matcher.check_source_file(file_content.path, file_content.content))
yield RegexMatchResults(rmrs)
return RegexMatchResults(rmrs)


def rules():
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/python/rules/download_pex_bin.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ def create_execute_request(self,


@rule
def download_pex_bin() -> DownloadedPexBin:
async def download_pex_bin() -> DownloadedPexBin:
# TODO: Inject versions and digests here through some option, rather than hard-coding it.
url = 'https://github.com/pantsbuild/pex/releases/download/v1.6.12/pex'
digest = Digest('ce64cb72cd23d2123dd48126af54ccf2b718d9ecb98c2ed3045ed1802e89e7e1', 1842359)
snapshot = yield Get(Snapshot, UrlToFetch(url, digest))
yield DownloadedPexBin(executable=snapshot.files[0], directory_digest=snapshot.directory_digest)
snapshot = await Get(Snapshot, UrlToFetch(url, digest))
return DownloadedPexBin(executable=snapshot.files[0], directory_digest=snapshot.directory_digest)


def rules():
Expand Down
6 changes: 3 additions & 3 deletions src/python/pants/backend/python/rules/inject_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class InjectedInitDigest:


@rule
def inject_init(snapshot: Snapshot) -> InjectedInitDigest:
async def inject_init(snapshot: Snapshot) -> InjectedInitDigest:
"""Ensure that every package has an __init__.py file in it."""
missing_init_files = tuple(sorted(identify_missing_init_files(snapshot.files)))
if not missing_init_files:
Expand All @@ -31,11 +31,11 @@ def inject_init(snapshot: Snapshot) -> InjectedInitDigest:
description="Inject missing __init__.py files: {}".format(", ".join(missing_init_files)),
input_files=snapshot.directory_digest,
)
touch_init_result = yield Get(ExecuteProcessResult, ExecuteProcessRequest, touch_init_request)
touch_init_result = await Get(ExecuteProcessResult, ExecuteProcessRequest, touch_init_request)
new_init_files_digest = touch_init_result.output_directory_digest
# TODO(#7710): Once this gets fixed, merge the original source digest and the new init digest
# into one unified digest.
yield InjectedInitDigest(directory_digest=new_init_files_digest)
return InjectedInitDigest(directory_digest=new_init_files_digest)


def rules():
Expand Down
10 changes: 5 additions & 5 deletions src/python/pants/backend/python/rules/pex.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class Pex(HermeticPex):
# TODO: This is non-hermetic because the requirements will be resolved on the fly by
# pex, where it should be hermetically provided in some way.
@rule
def create_pex(
async def create_pex(
request: CreatePex,
pex_bin: DownloadedPexBin,
python_setup: PythonSetup,
Expand All @@ -106,9 +106,9 @@ def create_pex(

argv.append(f'--sources-directory={source_dir_name}')
sources_digest = request.input_files_digest if request.input_files_digest else EMPTY_DIRECTORY_DIGEST
sources_digest_as_subdir = yield Get(Digest, DirectoryWithPrefixToAdd(sources_digest, source_dir_name))
sources_digest_as_subdir = await Get(Digest, DirectoryWithPrefixToAdd(sources_digest, source_dir_name))
all_inputs = (pex_bin.directory_digest, sources_digest_as_subdir,)
merged_digest = yield Get(Digest, DirectoriesToMerge(directories=all_inputs))
merged_digest = await Get(Digest, DirectoriesToMerge(directories=all_inputs))

# NB: PEX outputs are platform dependent so in order to get a PEX that we can use locally, without
# cross-building, we specify that out PEX command be run on the current local platform. When we
Expand All @@ -134,12 +134,12 @@ def create_pex(
}
)

result = yield Get(
result = await Get(
ExecuteProcessResult,
MultiPlatformExecuteProcessRequest,
execute_process_request
)
yield Pex(directory_digest=result.output_directory_digest, output_filename=request.output_filename)
return Pex(directory_digest=result.output_directory_digest, output_filename=request.output_filename)


def rules():
Expand Down
Loading

0 comments on commit fbe99a1

Please sign in to comment.