Skip to content

Commit

Permalink
[internal] Consolidate PyO3 native extensions (#13614)
Browse files Browse the repository at this point in the history
No need for two distinct extensions anymore post #13526.
  • Loading branch information
Eric-Arellano authored Nov 14, 2021
1 parent 021c3c0 commit ccd9b83
Show file tree
Hide file tree
Showing 28 changed files with 217 additions and 422 deletions.
8 changes: 0 additions & 8 deletions .github/workflows/test-cron.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ jobs:
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: github.event_name != 'pull_request'
name: Setup toolchain auth
Expand Down Expand Up @@ -142,8 +140,6 @@ jobs:
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
Expand Down Expand Up @@ -255,8 +251,6 @@ jobs:
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: github.event_name != 'pull_request'
name: Setup toolchain auth
Expand All @@ -275,8 +269,6 @@ jobs:
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 }}
Expand Down
8 changes: 0 additions & 8 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,6 @@ jobs:
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: github.event_name != 'pull_request'
name: Setup toolchain auth
Expand Down Expand Up @@ -142,8 +140,6 @@ jobs:
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
Expand Down Expand Up @@ -254,8 +250,6 @@ jobs:
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: github.event_name != 'pull_request'
name: Setup toolchain auth
Expand All @@ -274,8 +268,6 @@ jobs:
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 }}
Expand Down
1 change: 0 additions & 1 deletion build-support/bin/generate_github_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
NATIVE_FILES = [
".pants",
"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",
]

Expand Down
12 changes: 1 addition & 11 deletions build-support/bin/rust/bootstrap_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,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"
readonly NATIVE_CLIENT_PATH="${REPO_ROOT}/.pants"
readonly NATIVE_CLIENT_TARGET="${NATIVE_ROOT}/target/${MODE}/pants"
Expand All @@ -43,10 +41,8 @@ function _build_native_code() {
# NB: See Cargo.toml with regard to the `extension-module` features.
"${REPO_ROOT}/cargo" build \
--features=extension-module \
--features=engine_pyo3/extension-module \
${MODE_FLAG} \
-p engine \
-p engine_pyo3 \
-p client || die
}

Expand All @@ -60,7 +56,6 @@ function bootstrap_native_code() {
fi

if [[ -f "${NATIVE_ENGINE_RESOURCE}" && -f \
"${NATIVE_ENGINE_RESOURCE_PYO3}" && -f \
"${NATIVE_CLIENT_PATH}" && \
"${engine_version_calculated}" == "${engine_version_in_metadata}" ]]; then
return 0
Expand All @@ -70,13 +65,9 @@ function bootstrap_native_code() {

# If bootstrapping the native engine fails, don't attempt to run Pants afterwards.
local -r native_binary="${NATIVE_ROOT}/target/${MODE}/libengine.${LIB_EXTENSION}"
local -r native_binary_pyo3="${NATIVE_ROOT}/target/${MODE}/libengine_pyo3.${LIB_EXTENSION}"
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}."
fi

# If bootstrapping the native client fails, don't attempt to run Pants afterwards.
if [[ ! -f "${NATIVE_CLIENT_TARGET}" ]]; then
Expand All @@ -89,9 +80,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}" "${NATIVE_CLIENT_PATH}"
rm -f "${NATIVE_ENGINE_RESOURCE}" "${NATIVE_CLIENT_PATH}"
cp "${native_binary}" "${NATIVE_ENGINE_RESOURCE}"
cp "${native_binary_pyo3}" "${NATIVE_ENGINE_RESOURCE_PYO3}"
cp "${NATIVE_CLIENT_TARGET}" "${NATIVE_CLIENT_PATH}"

# Create the accompanying metadata file.
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/base/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Optional

from pants.base.build_root import BuildRoot
from pants.engine.internals import native_engine_pyo3
from pants.engine.internals import native_engine
from pants.vcs.git import Git, GitException
from pants.version import VERSION

Expand All @@ -29,7 +29,7 @@ def get_buildroot() -> str:

def get_pants_cachedir() -> str:
"""Return the Pants global cache directory."""
return native_engine_pyo3.default_cache_path()
return native_engine.default_cache_path()


def get_default_pants_config_file() -> str:
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/bin/remote_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from typing import List, Mapping

from pants.base.exiter import ExitCode
from pants.engine.internals.native_engine_pyo3 import PantsdConnectionException, PyNailgunClient
from pants.engine.internals.native_engine import PantsdConnectionException, PyNailgunClient
from pants.option.global_options import GlobalOptions
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.pantsd.pants_daemon_client import PantsDaemonClient
Expand Down Expand Up @@ -119,7 +119,7 @@ def run(self) -> ExitCode:

def _connect_and_execute(self, pantsd_handle: PantsDaemonClient.Handle) -> ExitCode:
global_options = self._bootstrap_options.for_global_scope()
executor = GlobalOptions.create_py_executor_pyo3(global_options)
executor = GlobalOptions.create_py_executor(global_options)

# Merge the nailgun TTY capability environment variables with the passed environment dict.
ng_env = ttynames_to_env(sys.stdin, sys.stdout, sys.stderr)
Expand Down
12 changes: 4 additions & 8 deletions src/python/pants/engine/internals/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_sources(sources=["*.py", "!*_test.py"])
python_sources(
overrides={"native_engine.pyi": {"dependencies": [":native_engine"]}},
)

python_tests(name="tests", sources=["*_test.py", "!scheduler_integration_test.py"], timeout=90)

Expand All @@ -15,15 +17,9 @@ python_tests(
timeout=180,
)

python_sources(
name="native",
sources=["native_engine.pyi", "native_engine_pyo3.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(name="fs_test_data", sources=["fs_test_data/fs_test.tar", "fs_test_data/tls/rsa/*"])
107 changes: 78 additions & 29 deletions src/python/pants/engine/internals/native_engine.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ from typing import Any, Sequence, TextIO, Tuple

from typing_extensions import Protocol

from pants.engine.fs import PathGlobs
from pants.engine.internals.scheduler import Workunit, _PathGlobsAndRootCollection
from pants.engine.internals.session import SessionValues
from pants.engine.process import InteractiveProcessResult
Expand All @@ -16,6 +17,83 @@ from pants.engine.process import InteractiveProcessResult
# see https://github.com/psf/black/issues/1548
# flake8: noqa: E302

# ------------------------------------------------------------------------------
# Scheduler
# ------------------------------------------------------------------------------

class PyExecutor:
def __init__(self, core_threads: int, max_threads: int) -> None: ...

# ------------------------------------------------------------------------------
# FS
# ------------------------------------------------------------------------------

class PyDigest:
def __init__(self, fingerprint: str, serialized_bytes_length: int) -> None: ...
@property
def fingerprint(self) -> str: ...
@property
def serialized_bytes_length(self) -> int: ...
def __eq__(self, other: PyDigest | Any) -> bool: ...
def __hash__(self) -> int: ...
def __repr__(self) -> str: ...

class PySnapshot:
def __init__(self) -> None: ...
@classmethod
def _create_for_testing(
cls, digest: PyDigest, files: Sequence[str], dirs: Sequence[str]
) -> PySnapshot: ...
@property
def digest(self) -> PyDigest: ...
@property
def dirs(self) -> tuple[str, ...]: ...
@property
def files(self) -> tuple[str, ...]: ...
def __eq__(self, other: PySnapshot | Any) -> bool: ...
def __hash__(self) -> int: ...
def __repr__(self) -> str: ...

def default_cache_path() -> str: ...

# TODO: Really, `paths` should be `Sequence[str]`. Fix and update call sites so that we don't
# cast to `tuple()` when not necessary.
def match_path_globs(path_globs: PathGlobs, paths: tuple[str, ...]) -> str: ...

# ------------------------------------------------------------------------------
# Workunits
# ------------------------------------------------------------------------------

def all_counter_names() -> list[str]: ...

# ------------------------------------------------------------------------------
# Nailgun
# ------------------------------------------------------------------------------

class PyNailgunClient:
def __init__(self, port: int, executor: PyExecutor) -> None: ...
def execute(self, command: str, args: list[str], env: dict[str, str]) -> int: ...

class PantsdConnectionException(Exception):
pass

class PantsdClientException(Exception):
pass

# ------------------------------------------------------------------------------
# Testutil
# ------------------------------------------------------------------------------

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: ...

class RawFdRunner(Protocol):
def __call__(
self,
Expand Down Expand Up @@ -142,32 +220,6 @@ def strongly_connected_components(
adjacency_lists: Sequence[Tuple[Any, Sequence[Any]]]
) -> Sequence[Sequence[Any]]: ...

class PyDigest:
def __init__(self, fingerprint: str, serialized_bytes_length: int) -> None: ...
@property
def fingerprint(self) -> str: ...
@property
def serialized_bytes_length(self) -> int: ...
def __eq__(self, other: PyDigest | Any) -> bool: ...
def __hash__(self) -> int: ...
def __repr__(self) -> str: ...

class PySnapshot:
def __init__(self) -> None: ...
@classmethod
def _create_for_testing(
cls, digest: PyDigest, files: Sequence[str], dirs: Sequence[str]
) -> PySnapshot: ...
@property
def digest(self) -> PyDigest: ...
@property
def dirs(self) -> tuple[str, ...]: ...
@property
def files(self) -> tuple[str, ...]: ...
def __eq__(self, other: PySnapshot | Any) -> bool: ...
def __hash__(self) -> int: ...
def __repr__(self) -> str: ...

class PyExecutionRequest:
def __init__(
self, *, poll: bool, poll_delay_in_ms: int | None, timeout_in_ms: int | None
Expand All @@ -176,9 +228,6 @@ class PyExecutionRequest:
class PyExecutionStrategyOptions:
def __init__(self, **kwargs: Any) -> None: ...

class PyExecutor:
def __init__(self, *, core_threads: int, max_threads: int) -> None: ...

class PyGeneratorResponseBreak:
def __init__(self, val: Any) -> None: ...

Expand Down
61 changes: 0 additions & 61 deletions src/python/pants/engine/internals/native_engine_pyo3.pyi

This file was deleted.

Loading

0 comments on commit ccd9b83

Please sign in to comment.