[internal] Switch from Rust-Cpython to PyO3

src/python/pants/bin/daemon_pants_runner.py

@@ -142,7 +142,6 @@ def __call__(
         command: str,
         args: Tuple[str, ...],
         env: Dict[str, str],
-        working_directory: bytes,
         cancellation_latch: PySessionCancellationLatch,
         stdin_fileno: int,
         stdout_fileno: int,

src/python/pants/engine/fs_test.py

@@ -40,8 +40,6 @@
     Workspace,
 )
 from pants.engine.goal import Goal, GoalSubsystem
-from pants.engine.internals.native_engine_pyo3 import PyDigest as DigestPyO3
-from pants.engine.internals.native_engine_pyo3 import PySnapshot as SnapshotPyO3
 from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.rules import Get, goal_rule, rule
 from pants.testutil.rule_runner import QueryRule, RuleRunner
@@ -1093,60 +1091,50 @@ def is_changed_snapshot() -> bool:
 # -----------------------------------------------------------------------------------------------
 
 
-@pytest.mark.parametrize("cls", [Digest, DigestPyO3])
-def test_digest_properties(cls) -> None:
-    digest = cls("a" * 64, 1000)
+def test_digest_properties() -> None:
+    digest = Digest("a" * 64, 1000)
     assert digest.fingerprint == "a" * 64
     assert digest.serialized_bytes_length == 1000
 
 
-@pytest.mark.parametrize("cls", [Digest, DigestPyO3])
-def test_digest_repr(cls) -> None:
-    assert str(cls("a" * 64, 1)) == f"Digest({repr('a' * 64)}, 1)"
+def test_digest_repr() -> None:
+    assert str(Digest("a" * 64, 1)) == f"Digest({repr('a' * 64)}, 1)"
 
 
-@pytest.mark.parametrize("cls", [Digest, DigestPyO3])
-def test_digest_hash(cls) -> None:
-    assert hash(cls("a" * 64, 1)) == -6148914691236517206
-    assert hash(cls("b" * 64, 1)) == -4919131752989213765
+def test_digest_hash() -> None:
+    assert hash(Digest("a" * 64, 1)) == -6148914691236517206
+    assert hash(Digest("b" * 64, 1)) == -4919131752989213765
     # Note that the size bytes is not considered in the hash.
-    assert hash(cls("a" * 64, 1000)) == -6148914691236517206
+    assert hash(Digest("a" * 64, 1000)) == -6148914691236517206
 
 
-@pytest.mark.parametrize("cls", [Digest, DigestPyO3])
-def test_digest_equality(cls) -> None:
-    digest = cls("a" * 64, 1)
-    assert digest == cls("a" * 64, 1)
-    assert digest != cls("a" * 64, 1000)
-    assert digest != cls("0" * 64, 1)
+def test_digest_equality() -> None:
+    digest = Digest("a" * 64, 1)
+    assert digest == Digest("a" * 64, 1)
+    assert digest != Digest("a" * 64, 1000)
+    assert digest != Digest("0" * 64, 1)
     with pytest.raises(TypeError):
-        digest < digest
+        digest < digest  # type: ignore[operator]
 
 
-@pytest.mark.parametrize(
-    "snapshot_cls,digest_cls", [(Snapshot, Digest), (SnapshotPyO3, DigestPyO3)]
-)
-def test_snapshot_properties(snapshot_cls, digest_cls) -> None:
-    digest = digest_cls("a" * 64, 1000)
-    snapshot = snapshot_cls._create_for_testing(digest, ["f.ext", "dir/f.ext"], ["dir"])
+def test_snapshot_properties() -> None:
+    digest = Digest("a" * 64, 1000)
+    snapshot = Snapshot._create_for_testing(digest, ["f.ext", "dir/f.ext"], ["dir"])
     assert snapshot.digest == digest
     assert snapshot.files == ("f.ext", "dir/f.ext")
     assert snapshot.dirs == ("dir",)
 
 
-@pytest.mark.parametrize(
-    "snapshot_cls,digest_cls", [(Snapshot, Digest), (SnapshotPyO3, DigestPyO3)]
-)
-def test_snapshot_hash(snapshot_cls, digest_cls) -> None:
+def test_snapshot_hash() -> None:
     def assert_hash(
         expected: int,
         *,
         digest_char: str = "a",
         files: Optional[List[str]] = None,
         dirs: Optional[List[str]] = None,
     ) -> None:
-        digest = digest_cls(digest_char * 64, 1000)
-        snapshot = snapshot_cls._create_for_testing(
+        digest = Digest(digest_char * 64, 1000)
+        snapshot = Snapshot._create_for_testing(
             digest, files or ["f.ext", "dir/f.ext"], dirs or ["dir"]
         )
         assert hash(snapshot) == expected
@@ -1159,28 +1147,21 @@ def assert_hash(
     assert_hash(-4919131752989213765, digest_char="b")
 
 
-@pytest.mark.parametrize(
-    "snapshot_cls,digest_cls", [(Snapshot, Digest), (SnapshotPyO3, DigestPyO3)]
-)
-def test_snapshot_equality(snapshot_cls, digest_cls) -> None:
+def test_snapshot_equality() -> None:
     # Only the digest is used for equality.
-    snapshot = snapshot_cls._create_for_testing(
-        digest_cls("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"]
-    )
-    assert snapshot == snapshot_cls._create_for_testing(
-        digest_cls("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"]
-    )
-    assert snapshot == snapshot_cls._create_for_testing(
-        digest_cls("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["foo"]
+    snapshot = Snapshot._create_for_testing(Digest("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"])
+    assert snapshot == Snapshot._create_for_testing(
+        Digest("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"]
     )
-    assert snapshot == snapshot_cls._create_for_testing(
-        digest_cls("a" * 64, 1000), ["f.ext"], ["dir"]
+    assert snapshot == Snapshot._create_for_testing(
+        Digest("a" * 64, 1000), ["f.ext", "dir/f.ext"], ["foo"]
     )
-    assert snapshot != snapshot_cls._create_for_testing(
-        digest_cls("a" * 64, 0), ["f.ext", "dir/f.ext"], ["dir"]
+    assert snapshot == Snapshot._create_for_testing(Digest("a" * 64, 1000), ["f.ext"], ["dir"])
+    assert snapshot != Snapshot._create_for_testing(
+        Digest("a" * 64, 0), ["f.ext", "dir/f.ext"], ["dir"]
     )
-    assert snapshot != snapshot_cls._create_for_testing(
-        digest_cls("b" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"]
+    assert snapshot != Snapshot._create_for_testing(
+        Digest("b" * 64, 1000), ["f.ext", "dir/f.ext"], ["dir"]
     )
     with pytest.raises(TypeError):
-        snapshot < snapshot
+        snapshot < snapshot  # type: ignore[operator]

src/python/pants/engine/internals/native_engine.pyi

@@ -22,7 +22,6 @@ class RawFdRunner(Protocol):
         command: str,
         args: tuple[str, ...],
         env: dict[str, str],
-        working_directory: bytes,
         cancellation_latch: PySessionCancellationLatch,
         stdin_fileno: int,
         stdout_fileno: int,
@@ -149,6 +148,9 @@ class PyDigest:
     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: ...
@@ -162,6 +164,9 @@ class PySnapshot:
     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__(
@@ -184,7 +189,7 @@ class PyGeneratorResponseGetMulti:
     def __init__(self, gets: tuple[PyGeneratorResponseGet, ...]) -> None: ...
 
 class PyNailgunServer:
-    pass
+    def port(self) -> int: ...
 
 class PyRemotingOptions:
     def __init__(self, **kwargs: Any) -> None: ...

src/python/pants/engine/internals/native_engine_pyo3.pyi

@@ -3,8 +3,6 @@
 
 from __future__ import annotations
 
-from typing import Any, Sequence
-
 from pants.engine.fs import PathGlobs
 
 # TODO: black and flake8 disagree about the content of this file:
@@ -28,30 +26,6 @@ def default_cache_path() -> str: ...
 #  cast to `tuple()` when not necessary.
 def match_path_globs(path_globs: PathGlobs, paths: tuple[str, ...]) -> str: ...
 
-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: ...
-
-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: ...
-
 # ------------------------------------------------------------------------------
 # Workunits
 # ------------------------------------------------------------------------------

src/python/pants/engine/internals/scheduler.py

@@ -469,12 +469,12 @@ def execute(
 
         states = [
             Throw(
-                raw_root.result(),
-                python_traceback=raw_root.python_traceback(),
-                engine_traceback=raw_root.engine_traceback(),
+                raw_root.result,
+                python_traceback=raw_root.python_traceback,
+                engine_traceback=raw_root.engine_traceback,
             )
-            if raw_root.is_throw()
-            else Return(raw_root.result())
+            if raw_root.is_throw
+            else Return(raw_root.result)
             for raw_root in raw_roots
         ]
 

src/rust/engine/Cargo.toml

@@ -98,11 +98,11 @@ 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 `cpython` to fail. We
+# 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.so` should pass `cargo build .. --features=extension-module`.
 #  see https://github.com/PyO3/pyo3/issues/340
-extension-module = ["cpython/extension-module"]
+extension-module = ["pyo3/extension-module"]
 default = []
 
 [dependencies]
@@ -114,8 +114,6 @@ protos = { path = "protos" }
 bytes = "1.0"
 cache = { path = "cache" }
 concrete_time = { path = "concrete_time" }
-# TODO: Go back to upstream once https://github.com/dgrunwald/rust-cpython/pull/261 lands.
-cpython = { git = "https://github.com/pantsbuild/rust-cpython", rev = "46d7eff26a705384e41eb6f7b870cd3f5f14b3bc" }
 crossbeam-channel = "0.4"
 derivative = "2.2"
 double-checked-cell-async = "2.0"
@@ -138,6 +136,7 @@ num_enum = "0.4"
 parking_lot = "0.11"
 petgraph = "0.5"
 process_execution = { path = "process_execution" }
+pyo3 = "0.15"
 rand = "0.8"
 regex = "1"
 reqwest = { version = "0.11", default_features = false, features = ["stream", "rustls-tls"] }
@@ -163,6 +162,9 @@ testutil = { path = "./testutil" }
 fs = { path = "./fs" }
 env_logger = "0.5.4"
 
+[build-dependencies]
+pyo3-build-config = "0.15"
+
 [patch.crates-io]
 # TODO: Waiting for release after https://github.com/mitsuhiko/console/pull/93.
 console = { git = "https://github.com/mitsuhiko/console", rev = "3232775295149e6f0aace26d4cfe61013ed3e2d8" }

src/rust/engine/build.rs

@@ -26,18 +26,9 @@
 #![allow(clippy::mutex_atomic)]
 
 fn main() {
+  pyo3_build_config::add_extension_module_link_args();
+
   // 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 cpython 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");
-  }
 }