From 5745753b5207ef91bb6e8c151edd96a6bb71f0c4 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 9 Jun 2021 09:40:17 -0700 Subject: [PATCH] Revert "Start incremental migration from rust-cpython to PyO3 (#12110)" This reverts commit 7a36876573f0dec0b5001687af02c7364ec834c6. --- .github/workflows/test-cron.yaml | 8 - .github/workflows/test.yaml | 8 - .../bin/generate_github_workflows.py | 1 - build-support/bin/rust/bootstrap_code.sh | 36 ++--- cargo | 1 - src/python/pants/engine/internals/.gitignore | 1 - src/python/pants/engine/internals/BUILD | 4 +- .../pants/engine/internals/native_engine.pyi | 9 ++ .../engine/internals/native_engine_pyo3.pyi | 19 --- src/rust/engine/Cargo.lock | 143 +----------------- src/rust/engine/Cargo.toml | 2 - src/rust/engine/build.rs | 6 +- src/rust/engine/engine_pyo3/Cargo.toml | 29 ---- src/rust/engine/engine_pyo3/build.rs | 19 --- .../engine_pyo3/src/externs/interface.rs | 33 ---- .../src/externs/interface/testutil.rs | 62 -------- .../engine/engine_pyo3/src/externs/mod.rs | 4 - src/rust/engine/engine_pyo3/src/lib.rs | 35 ----- src/rust/engine/src/externs/interface.rs | 5 + .../engine/src/externs/interface/testutil.rs | 48 ++++++ .../remote_cache_integration_test.py | 8 +- 21 files changed, 83 insertions(+), 398 deletions(-) delete mode 100644 src/python/pants/engine/internals/native_engine_pyo3.pyi delete mode 100644 src/rust/engine/engine_pyo3/Cargo.toml delete mode 100644 src/rust/engine/engine_pyo3/build.rs delete mode 100644 src/rust/engine/engine_pyo3/src/externs/interface.rs delete mode 100644 src/rust/engine/engine_pyo3/src/externs/interface/testutil.rs delete mode 100644 src/rust/engine/engine_pyo3/src/externs/mod.rs delete mode 100644 src/rust/engine/engine_pyo3/src/lib.rs create mode 100644 src/rust/engine/src/externs/interface/testutil.rs diff --git a/.github/workflows/test-cron.yaml b/.github/workflows/test-cron.yaml index ed1ea5addcc..c2b2a2e36f9 100644 --- a/.github/workflows/test-cron.yaml +++ b/.github/workflows/test-cron.yaml @@ -102,8 +102,6 @@ jobs: ' path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - name: Bootstrap Pants run: './pants --version @@ -133,8 +131,6 @@ jobs: name: native_engine.so.${{ matrix.python-version }}.${{ runner.os }} path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - if: '!contains(env.COMMIT_MESSAGE, ''[ci skip-rust]'')' name: Test and Lint Rust @@ -246,8 +242,6 @@ jobs: ' path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - name: Bootstrap Pants run: './pants --version @@ -259,8 +253,6 @@ jobs: name: native_engine.so.${{ matrix.python-version }}.${{ runner.os }} path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - env: TMPDIR: ${{ runner.temp }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 61c2eb25d44..1398d8044f3 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -102,8 +102,6 @@ jobs: ' path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - name: Bootstrap Pants run: './pants --version @@ -133,8 +131,6 @@ jobs: name: native_engine.so.${{ matrix.python-version }}.${{ runner.os }} path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - if: '!contains(env.COMMIT_MESSAGE, ''[ci skip-rust]'')' name: Test and Lint Rust @@ -245,8 +241,6 @@ jobs: ' path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - name: Bootstrap Pants run: './pants --version @@ -258,8 +252,6 @@ jobs: name: native_engine.so.${{ matrix.python-version }}.${{ runner.os }} path: 'src/python/pants/engine/internals/native_engine.so - src/python/pants/engine/internals/native_engine_pyo3.so - src/python/pants/engine/internals/native_engine.so.metadata' - env: TMPDIR: ${{ runner.temp }} diff --git a/build-support/bin/generate_github_workflows.py b/build-support/bin/generate_github_workflows.py index 83d81de6b22..e83d6eb6715 100644 --- a/build-support/bin/generate_github_workflows.py +++ b/build-support/bin/generate_github_workflows.py @@ -34,7 +34,6 @@ NATIVE_ENGINE_SO_FILES = [ "src/python/pants/engine/internals/native_engine.so", - "src/python/pants/engine/internals/native_engine_pyo3.so", "src/python/pants/engine/internals/native_engine.so.metadata", ] diff --git a/build-support/bin/rust/bootstrap_code.sh b/build-support/bin/rust/bootstrap_code.sh index e0ade29545d..4d020f63a62 100644 --- a/build-support/bin/rust/bootstrap_code.sh +++ b/build-support/bin/rust/bootstrap_code.sh @@ -5,9 +5,6 @@ REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && cd ../../.. && pwd -P)" -# shellcheck source=build-support/common.sh -source "${REPO_ROOT}/build-support/common.sh" - # Defines: # + NATIVE_ROOT: The Rust code directory, ie: src/rust/engine. # + MODE: Whether to run in debug or release mode. @@ -31,9 +28,7 @@ case "${KERNEL}" in esac readonly NATIVE_ENGINE_BINARY="native_engine.so" -readonly NATIVE_ENGINE_BINARY_PYO3="native_engine_pyo3.so" readonly NATIVE_ENGINE_RESOURCE="${REPO_ROOT}/src/python/pants/engine/internals/${NATIVE_ENGINE_BINARY}" -readonly NATIVE_ENGINE_RESOURCE_PYO3="${REPO_ROOT}/src/python/pants/engine/internals/${NATIVE_ENGINE_BINARY_PYO3}" readonly NATIVE_ENGINE_RESOURCE_METADATA="${NATIVE_ENGINE_RESOURCE}.metadata" function _build_native_code() { @@ -42,21 +37,15 @@ function _build_native_code() { echo "${NATIVE_ROOT}/target/${MODE}/libengine.${LIB_EXTENSION}" } -function _build_native_code_pyo3() { - # NB: See engine_pyo3/Cargo.toml with regard to the `extension-module` feature. - "${REPO_ROOT}/cargo" build --features=engine_pyo3/extension-module ${MODE_FLAG} -p engine_pyo3 || die - echo "${NATIVE_ROOT}/target/${MODE}/libengine_pyo3.${LIB_EXTENSION}" -} - function bootstrap_native_code() { # We expose a safety valve to skip compilation iff the user already has `native_engine.so`. This # can result in using a stale `native_engine.so`, but we trust that the user knows what # they're doing. if [[ "${SKIP_NATIVE_ENGINE_SO_BOOTSTRAP}" == "true" ]]; then - if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" || ! -f "${NATIVE_ENGINE_RESOURCE_PYO3}" ]]; then - die "You requested to override bootstrapping native_engine.so and native_engine_pyo3.so via the env var" \ - "SKIP_NATIVE_ENGINE_SO_BOOTSTRAP, but the files do not exist at" \ - "${NATIVE_ENGINE_RESOURCE} and ${NATIVE_ENGINE_BINARY_PYO3}. This is not safe to do." + if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" ]]; then + die "You requested to override bootstrapping native_engine.so via the env var" \ + "SKIP_NATIVE_ENGINE_SO_BOOTSTRAP, but the file does not exist at" \ + "${NATIVE_ENGINE_RESOURCE}. This is not safe to do." fi return fi @@ -67,20 +56,14 @@ function bootstrap_native_code() { if [[ -f "${NATIVE_ENGINE_RESOURCE_METADATA}" ]]; then engine_version_in_metadata="$(sed -n 's/^engine_version: //p' "${NATIVE_ENGINE_RESOURCE_METADATA}")" fi - if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" || ! -f "${NATIVE_ENGINE_RESOURCE_PYO3}" || "${engine_version_calculated}" != "${engine_version_in_metadata}" ]]; then - banner "Building native engine..." - banner "Building native_engine.so" - local -r native_binary="$(_build_native_code)" || die - banner "Building native_engine_pyo3.so" - local -r native_binary_pyo3="$(_build_native_code_pyo3)" || die + if [[ ! -f "${NATIVE_ENGINE_RESOURCE}" || "${engine_version_calculated}" != "${engine_version_in_metadata}" ]]; then + echo "Building native engine" + local -r native_binary="$(_build_native_code)" # If bootstrapping the native engine fails, don't attempt to run pants # afterwards. if [[ ! -f "${native_binary}" ]]; then - die "Failed to build native engine, file missing at ${native_binary}." - fi - if [[ ! -f "${native_binary_pyo3}" ]]; then - die "Failed to build native engine, file missing at ${native_binary_pyo3}." + die "Failed to build native engine." fi # Pick up Cargo.lock changes if any caused by the `cargo build`. @@ -89,9 +72,8 @@ function bootstrap_native_code() { # Create the native engine resource. # NB: On Mac Silicon, for some reason, first removing the old native_engine.so is necessary to avoid the Pants # process from being killed when recompiling. - rm -f "${NATIVE_ENGINE_RESOURCE}" "${NATIVE_ENGINE_RESOURCE_PYO3}" + rm -f "${NATIVE_ENGINE_RESOURCE}" cp "${native_binary}" "${NATIVE_ENGINE_RESOURCE}" - cp "${native_binary_pyo3}" "${NATIVE_ENGINE_RESOURCE_PYO3}" # Create the accompanying metadata file. local -r metadata_file=$(mktemp -t pants.native_engine.metadata.XXXXXX) diff --git a/cargo b/cargo index fe3b4307772..3f5ed4e8f1d 100755 --- a/cargo +++ b/cargo @@ -9,7 +9,6 @@ source "${REPO_ROOT}/build-support/common.sh" PY="$(determine_python)" export PY -export PYO3_PYTHON="${PY}" export PYTHON_SYS_EXECUTABLE="${PY}" # Consumed by the cpython crate. if ! command -v rustup &> /dev/null; then diff --git a/src/python/pants/engine/internals/.gitignore b/src/python/pants/engine/internals/.gitignore index b6b9e91a378..346513bbf77 100644 --- a/src/python/pants/engine/internals/.gitignore +++ b/src/python/pants/engine/internals/.gitignore @@ -1,3 +1,2 @@ /native_engine.so -/native_engine_pyo3.so /native_engine.so.metadata diff --git a/src/python/pants/engine/internals/BUILD b/src/python/pants/engine/internals/BUILD index c606abbe5d2..ebd2525ece5 100644 --- a/src/python/pants/engine/internals/BUILD +++ b/src/python/pants/engine/internals/BUILD @@ -23,13 +23,13 @@ python_tests( python_library( name='native', - sources=['native_engine.pyi', 'native_engine_pyo3.pyi'], + sources=['native_engine.pyi'], dependencies=[':native_engine'], ) resources( name='native_engine', - sources=['native_engine.so', 'native_engine_pyo3.so', 'native_engine.so.metadata'], + sources=['native_engine.so', 'native_engine.so.metadata'], ) resources( diff --git a/src/python/pants/engine/internals/native_engine.pyi b/src/python/pants/engine/internals/native_engine.pyi index 465c1913c0f..6fff01bf2cd 100644 --- a/src/python/pants/engine/internals/native_engine.pyi +++ b/src/python/pants/engine/internals/native_engine.pyi @@ -212,6 +212,15 @@ class PyTasks: class PyTypes: def __init__(self, **kwargs: Any) -> None: ... +class PyStubCASBuilder: + def always_errors(self) -> None: ... + def build(self, executor: PyExecutor) -> PyStubCAS: ... + +class PyStubCAS: + @classmethod + def builder(cls) -> PyStubCASBuilder: ... + def address(self) -> str: ... + class PyStdioDestination: pass diff --git a/src/python/pants/engine/internals/native_engine_pyo3.pyi b/src/python/pants/engine/internals/native_engine_pyo3.pyi deleted file mode 100644 index e72b546f578..00000000000 --- a/src/python/pants/engine/internals/native_engine_pyo3.pyi +++ /dev/null @@ -1,19 +0,0 @@ -# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -# Licensed under the Apache License, Version 2.0 (see LICENSE). - -# TODO: black and flake8 disagree about the content of this file: -# see https://github.com/psf/black/issues/1548 -# flake8: noqa: E302 - -class PyExecutor: - def __init__(self, core_threads: int, max_threads: int) -> None: ... - -class PyStubCASBuilder: - def always_errors(self) -> PyStubCASBuilder: ... - def build(self, executor: PyExecutor) -> PyStubCAS: ... - -class PyStubCAS: - @classmethod - def builder(cls) -> PyStubCASBuilder: ... - @property - def address(self) -> str: ... diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index ff247ee6ab7..a42f6f08671 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -407,7 +407,7 @@ source = "git+https://github.com/pantsbuild/rust-cpython?rev=46d7eff26a705384e41 dependencies = [ "libc", "num-traits", - "paste 1.0.5", + "paste", "python3-sys", ] @@ -545,16 +545,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "ctor" -version = "0.1.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7fbaabec2c953050352311293be5c6aba8e141ba19d6811862b232d6fd020484" -dependencies = [ - "quote 1.0.8", - "syn 1.0.55", -] - [[package]] name = "derivative" version = "2.1.1" @@ -688,17 +678,6 @@ dependencies = [ "workunit_store", ] -[[package]] -name = "engine_pyo3" -version = "0.0.1" -dependencies = [ - "hashing", - "mock", - "parking_lot", - "pyo3", - "task_executor", -] - [[package]] name = "env_logger" version = "0.5.13" @@ -1019,17 +998,6 @@ dependencies = [ "wasi 0.10.0+wasi-snapshot-preview1", ] -[[package]] -name = "ghost" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a5bcf1bbeab73aa4cf2fde60a846858dc036163c7c33bec309f8d17de785479" -dependencies = [ - "proc-macro2 1.0.24", - "quote 1.0.8", - "syn 1.0.55", -] - [[package]] name = "glob" version = "0.2.11" @@ -1296,29 +1264,6 @@ dependencies = [ "regex", ] -[[package]] -name = "indoc" -version = "0.3.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47741a8bc60fb26eb8d6e0238bbb26d8575ff623fdc97b1a2c00c050b9684ed8" -dependencies = [ - "indoc-impl", - "proc-macro-hack", -] - -[[package]] -name = "indoc-impl" -version = "0.3.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce046d161f000fffde5f432a0d034d0341dc152643b2598ed5bfce44c4f3a8f0" -dependencies = [ - "proc-macro-hack", - "proc-macro2 1.0.24", - "quote 1.0.8", - "syn 1.0.55", - "unindent", -] - [[package]] name = "inotify" version = "0.8.3" @@ -1348,28 +1293,6 @@ dependencies = [ "cfg-if 1.0.0", ] -[[package]] -name = "inventory" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f0f7efb804ec95e33db9ad49e4252f049e37e8b0a4652e3cd61f7999f2eff7f" -dependencies = [ - "ctor", - "ghost", - "inventory-impl", -] - -[[package]] -name = "inventory-impl" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75c094e94816723ab936484666968f5b58060492e880f3c8d00489a1e244fa51" -dependencies = [ - "proc-macro2 1.0.24", - "quote 1.0.8", - "syn 1.0.55", -] - [[package]] name = "iovec" version = "0.1.4" @@ -1970,31 +1893,12 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "paste" -version = "0.1.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45ca20c77d80be666aef2b45486da86238fabe33e38306bd3118fe4af33fa880" -dependencies = [ - "paste-impl", - "proc-macro-hack", -] - [[package]] name = "paste" version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "acbf547ad0c65e31259204bd90935776d1c693cec2f4ff7abb7a1bbbd40dfe58" -[[package]] -name = "paste-impl" -version = "0.1.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d95a7db200b97ef370c8e6de0088252f7e0dfff7d047a28528e47456c0fc98b6" -dependencies = [ - "proc-macro-hack", -] - [[package]] name = "percent-encoding" version = "2.1.0" @@ -2292,45 +2196,6 @@ dependencies = [ "bytes 0.5.6", ] -[[package]] -name = "pyo3" -version = "0.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4837b8e8e18a102c23f79d1e9a110b597ea3b684c95e874eb1ad88f8683109c3" -dependencies = [ - "cfg-if 1.0.0", - "ctor", - "indoc", - "inventory", - "libc", - "parking_lot", - "paste 0.1.18", - "pyo3-macros", - "unindent", -] - -[[package]] -name = "pyo3-macros" -version = "0.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a47f2c300ceec3e58064fd5f8f5b61230f2ffd64bde4970c81fdd0563a2db1bb" -dependencies = [ - "pyo3-macros-backend", - "quote 1.0.8", - "syn 1.0.55", -] - -[[package]] -name = "pyo3-macros-backend" -version = "0.13.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87b097e5d84fcbe3e167f400fbedd657820a375b034c78bd852050749a575d66" -dependencies = [ - "proc-macro2 1.0.24", - "quote 1.0.8", - "syn 1.0.55", -] - [[package]] name = "python3-sys" version = "0.5.2" @@ -3542,12 +3407,6 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f7fe0bb3479651439c9112f72b6c505038574c9fbb575ed1bf3b797fa39dd564" -[[package]] -name = "unindent" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f14ee04d9415b52b3aeab06258a3f07093182b88ba0f9b8d203f211a7a7d41c7" - [[package]] name = "unreachable" version = "1.0.0" diff --git a/src/rust/engine/Cargo.toml b/src/rust/engine/Cargo.toml index 7620ec1b70e..452f781d19b 100644 --- a/src/rust/engine/Cargo.toml +++ b/src/rust/engine/Cargo.toml @@ -23,7 +23,6 @@ members = [ "async_semaphore", "async_value", "concrete_time", - "engine_pyo3", "fs", "fs/brfs", "fs/fs_util", @@ -62,7 +61,6 @@ default-members = [ "async_semaphore", "async_value", "concrete_time", - "engine_pyo3", "fs", "fs/fs_util", "fs/store", diff --git a/src/rust/engine/build.rs b/src/rust/engine/build.rs index 9e620c57515..111fbe643b1 100644 --- a/src/rust/engine/build.rs +++ b/src/rust/engine/build.rs @@ -28,8 +28,10 @@ #![allow(clippy::mutex_atomic)] fn main() { - // NB: The native extension only works with the Python interpreter version it was built with - // (e.g. Python 3.7 vs 3.8). + // NB: When built with Python 3, `native_engine.so` only works with a Python 3 interpreter. + // When built with Python 2, it works with both Python 2 and Python 3. + // So, we check to see if the under-the-hood interpreter has changed and rebuild the native engine + // when needed. println!("cargo:rerun-if-env-changed=PY"); if cfg!(target_os = "macos") { diff --git a/src/rust/engine/engine_pyo3/Cargo.toml b/src/rust/engine/engine_pyo3/Cargo.toml deleted file mode 100644 index 62303ae7720..00000000000 --- a/src/rust/engine/engine_pyo3/Cargo.toml +++ /dev/null @@ -1,29 +0,0 @@ -[package] -version = "0.0.1" -edition = "2018" -name = "engine_pyo3" -authors = [ "Pants Build " ] -publish = false - -[lib] -crate-type = ["cdylib"] - -[features] -# NB: To actually load this crate from python, the `extension-module` feature must be enabled. But -# unfortunately, enabling `extension-module` causes tests linked against `pyo3` to fail. We -# define a feature to enable that, but we do not enable it by default: someone building this module -# in order to extract `libengine_pyo3.so` should pass `cargo build .. --features=engine_pyo3/extension-module`. -# see https://github.com/PyO3/pyo3/issues/340 -extension-module = ["pyo3/extension-module"] -default = [] - -[dependencies] -hashing = { path = "../hashing" } -parking_lot = "0.11" -# We must disable the `auto-initialize` feature because we do not enable `extension-module` normally -# (see above comment in `features`), so `auto-initialize` would try to link to a static Python interpreter during -# tests, which may fail. However, we need to then re-activate the `macros` feature. See -# https://pyo3.rs/v0.13.2/features.html -pyo3 = { version = "0.13", default-features = false, features = ["macros"] } -task_executor = { path = "../task_executor" } -testutil_mock = { package = "mock", path = "../testutil/mock" } diff --git a/src/rust/engine/engine_pyo3/build.rs b/src/rust/engine/engine_pyo3/build.rs deleted file mode 100644 index b38f1a27191..00000000000 --- a/src/rust/engine/engine_pyo3/build.rs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -// Licensed under the Apache License, Version 2.0 (see LICENSE). - -fn main() { - // NB: The native extension only works with the Python interpreter version it was built with - // (e.g. Python 3.7 vs 3.8). - println!("cargo:rerun-if-env-changed=PY"); - - if cfg!(target_os = "macos") { - // N.B. On OSX, we force weak linking by passing the param `-undefined dynamic_lookup` to - // the underlying linker. This avoids "missing symbol" errors for Python symbols - // (e.g. `_PyImport_ImportModule`) at build time when bundling the PyO3 sources. - // The missing symbols will instead by dynamically resolved in the address space of the parent - // binary (e.g. `python`) at runtime. We do this to avoid needing to link to libpython - // (which would constrain us to specific versions of Python). - println!("cargo:rustc-cdylib-link-arg=-undefined"); - println!("cargo:rustc-cdylib-link-arg=dynamic_lookup"); - } -} diff --git a/src/rust/engine/engine_pyo3/src/externs/interface.rs b/src/rust/engine/engine_pyo3/src/externs/interface.rs deleted file mode 100644 index fcc116f2d9a..00000000000 --- a/src/rust/engine/engine_pyo3/src/externs/interface.rs +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -// Licensed under the Apache License, Version 2.0 (see LICENSE). - -use pyo3::exceptions::PyException; -use pyo3::prelude::*; - -mod testutil; - -#[pymodule] -fn native_engine_pyo3(_py: Python, m: &PyModule) -> PyResult<()> { - m.add_class::()?; - - m.add_class::()?; - m.add_class::()?; - - Ok(()) -} - -#[pyclass] -#[derive(Debug, Clone)] -struct PyExecutor { - executor: task_executor::Executor, -} - -#[pymethods] -impl PyExecutor { - #[new] - fn __new__(core_threads: usize, max_threads: usize) -> PyResult { - task_executor::Executor::global(core_threads, max_threads) - .map(|executor| PyExecutor { executor }) - .map_err(PyException::new_err) - } -} diff --git a/src/rust/engine/engine_pyo3/src/externs/interface/testutil.rs b/src/rust/engine/engine_pyo3/src/externs/interface/testutil.rs deleted file mode 100644 index 5f4c1a8d345..00000000000 --- a/src/rust/engine/engine_pyo3/src/externs/interface/testutil.rs +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -// Licensed under the Apache License, Version 2.0 (see LICENSE). - -use std::sync::Arc; - -use super::PyExecutor; -use parking_lot::Mutex; -use pyo3::exceptions::PyAssertionError; -use pyo3::prelude::*; -use pyo3::types::PyType; -use testutil_mock::{StubCAS, StubCASBuilder}; - -#[pyclass] -pub struct PyStubCASBuilder { - builder: Arc>>, -} - -#[pymethods] -impl PyStubCASBuilder { - fn always_errors(&mut self) -> PyResult { - let mut builder_opt = self.builder.lock(); - let builder = builder_opt - .take() - .ok_or_else(|| PyAssertionError::new_err("Unable to unwrap StubCASBuilder"))?; - *builder_opt = Some(builder.always_errors()); - Ok(PyStubCASBuilder { - builder: self.builder.clone(), - }) - } - - fn build(&mut self, py_executor: PyExecutor) -> PyResult { - let mut builder_opt = self.builder.lock(); - let builder = builder_opt - .take() - .ok_or_else(|| PyAssertionError::new_err("Unable to unwrap StubCASBuilder"))?; - // NB: A Tokio runtime must be used when building StubCAS. - py_executor.executor.enter(|| { - Ok(PyStubCAS { - stub_cas: builder.build(), - }) - }) - } -} - -#[pyclass] -pub struct PyStubCAS { - stub_cas: StubCAS, -} - -#[pymethods] -impl PyStubCAS { - #[classmethod] - fn builder(_cls: &PyType) -> PyStubCASBuilder { - let builder = Arc::new(Mutex::new(Some(StubCAS::builder()))); - PyStubCASBuilder { builder } - } - - #[getter] - fn address(&self) -> String { - self.stub_cas.address() - } -} diff --git a/src/rust/engine/engine_pyo3/src/externs/mod.rs b/src/rust/engine/engine_pyo3/src/externs/mod.rs deleted file mode 100644 index 770301dcaa7..00000000000 --- a/src/rust/engine/engine_pyo3/src/externs/mod.rs +++ /dev/null @@ -1,4 +0,0 @@ -// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -// Licensed under the Apache License, Version 2.0 (see LICENSE). - -mod interface; diff --git a/src/rust/engine/engine_pyo3/src/lib.rs b/src/rust/engine/engine_pyo3/src/lib.rs deleted file mode 100644 index 5d6c11b4051..00000000000 --- a/src/rust/engine/engine_pyo3/src/lib.rs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). -// Licensed under the Apache License, Version 2.0 (see LICENSE). - -#![deny(warnings)] -// Enable all clippy lints except for many of the pedantic ones. It's a shame this needs to be copied and pasted across crates, but there doesn't appear to be a way to include inner attributes from a common source. -#![deny( - clippy::all, - clippy::default_trait_access, - clippy::expl_impl_clone_on_copy, - clippy::if_not_else, - clippy::needless_continue, - clippy::unseparated_literal_suffix, - // TODO: Falsely triggers for async/await: - // see https://github.com/rust-lang/rust-clippy/issues/5360 - // clippy::used_underscore_binding -)] -// It is often more clear to show that nothing is being moved. -#![allow(clippy::match_ref_pats)] -// Subjective style. -#![allow( - clippy::len_without_is_empty, - clippy::redundant_field_names, - clippy::too_many_arguments -)] -// Default isn't as big a deal as people seem to think it is. -#![allow(clippy::new_without_default, clippy::new_ret_no_self)] -// Arc can be more clear than needing to grok Orderings: -#![allow(clippy::mutex_atomic)] -// We only use unsafe pointer dereferences in our no_mangle exposed API, but it is nicer to list -// just the one minor call as unsafe, than to mark the whole function as unsafe which may hide -// other unsafeness. -#![allow(clippy::not_unsafe_ptr_arg_deref)] -#![type_length_limit = "43757804"] - -mod externs; diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 8dcda00ccd5..d76a516fafc 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -81,6 +81,8 @@ use crate::{ Tasks, Types, Value, }; +mod testutil; + py_exception!(native_engine, PollTimeout); py_exception!(native_engine, PantsdConnectionException); py_exception!(native_engine, PantsdClientException); @@ -437,6 +439,9 @@ py_module_initializer!(native_engine, |py, m| { m.add_class::(py)?; + m.add_class::(py)?; + m.add_class::(py)?; + Ok(()) }); diff --git a/src/rust/engine/src/externs/interface/testutil.rs b/src/rust/engine/src/externs/interface/testutil.rs new file mode 100644 index 00000000000..cbd3360f517 --- /dev/null +++ b/src/rust/engine/src/externs/interface/testutil.rs @@ -0,0 +1,48 @@ +use std::sync::Arc; + +use super::PyExecutor; +use cpython::{exc, py_class, PyErr, PyObject, PyResult, PyString}; +use parking_lot::Mutex; +use testutil_mock::{StubCAS, StubCASBuilder}; + +py_class!(pub class PyStubCASBuilder |py| { + data builder: Arc>>; + + def always_errors(&self) -> PyResult { + let mut builder_opt = self.builder(py).lock(); + let builder = builder_opt + .take() + .ok_or_else(|| PyErr::new::(py, (PyString::new(py, "unable to unwrap StubCASBuilder"),)))? + .always_errors(); + *builder_opt = Some(builder); + Ok(py.None()) + } + + def build(&self, executor: PyExecutor) -> PyResult { + let executor = executor.executor(py); + executor.enter(|| { + let mut builder_opt = self.builder(py).lock(); + let builder = builder_opt + .take() + .ok_or_else(|| PyErr::new::(py, (PyString::new(py, "unable to unwrap StubCASBuilder"),)))?; + let cas = builder.build(); + PyStubCAS::create_instance(py, cas) + }) + } +}); + +py_class!(pub class PyStubCAS |py| { + data server: StubCAS; + + @classmethod + def builder(_cls) -> PyResult { + let builder = StubCAS::builder(); + PyStubCASBuilder::create_instance(py, Arc::new(Mutex::new(Some(builder)))) + } + + def address(&self) -> PyResult { + let server = self.server(py); + let address = server.address(); + Ok(PyString::new(py, &address)) + } +}); diff --git a/tests/python/pants_test/integration/remote_cache_integration_test.py b/tests/python/pants_test/integration/remote_cache_integration_test.py index 9cdb62940cc..3c3d480e8eb 100644 --- a/tests/python/pants_test/integration/remote_cache_integration_test.py +++ b/tests/python/pants_test/integration/remote_cache_integration_test.py @@ -1,7 +1,7 @@ # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from pants.engine.internals.native_engine_pyo3 import PyExecutor, PyStubCAS +from pants.engine.internals.native_engine import PyExecutor, PyStubCAS from pants.option.global_options import RemoteCacheWarningsBehavior from pants.option.scope import GLOBAL_SCOPE_CONFIG_SECTION from pants.testutil.pants_integration_test import run_pants @@ -9,7 +9,9 @@ def test_warns_on_remote_cache_errors(): executor = PyExecutor(core_threads=2, max_threads=4) - cas = PyStubCAS.builder().always_errors().build(executor) + builder = PyStubCAS.builder() + builder.always_errors() + cas = builder.build(executor) def run(behavior: RemoteCacheWarningsBehavior) -> str: pants_run = run_pants( @@ -27,7 +29,7 @@ def run(behavior: RemoteCacheWarningsBehavior) -> str: "remote_cache_warnings": behavior.value, # NB: Our options code expects `grpc://`, which it will then convert back to # `http://` before sending over FFI. - "remote_store_address": cas.address.replace("http://", "grpc://"), + "remote_store_address": cas.address().replace("http://", "grpc://"), } }, )