diff --git a/src/rust/engine/Cargo.lock b/src/rust/engine/Cargo.lock index d22e5d488971..8577417998dd 100644 --- a/src/rust/engine/Cargo.lock +++ b/src/rust/engine/Cargo.lock @@ -672,6 +672,7 @@ dependencies = [ "petgraph 0.5.1", "process_execution", "protos", + "pyo3", "rand 0.8.2", "regex", "reqwest", diff --git a/src/rust/engine/Cargo.toml b/src/rust/engine/Cargo.toml index 83e8eecd05d3..4239063ccc67 100644 --- a/src/rust/engine/Cargo.toml +++ b/src/rust/engine/Cargo.toml @@ -138,6 +138,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"] } diff --git a/src/rust/engine/src/externs/mod.rs b/src/rust/engine/src/externs/mod.rs index 8016aa6de2b5..d335506d7dbc 100644 --- a/src/rust/engine/src/externs/mod.rs +++ b/src/rust/engine/src/externs/mod.rs @@ -23,25 +23,26 @@ use std::fmt; use crate::interning::Interns; use crate::python::{Failure, Key, TypeId, Value}; -use cpython::{ - py_class, CompareOp, FromPyObject, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, - PyObject, PyResult as CPyResult, PyTuple, PyType, Python, PythonObject, ToPyObject, -}; +use cpython::ObjectProtocol; use lazy_static::lazy_static; +use pyo3::basic::CompareOp; +use pyo3::prelude::*; +use pyo3::types::{PyBool, PyBytes, PyDict, PyTuple}; +use pyo3::ToPyObject; use logging::PythonLogLevel; -pub fn equals(py: Python, h1: &PyObject, h2: &PyObject) -> bool { +pub fn equals(h1: &PyAny, h2: &PyAny) -> bool { // NB: Although it does not precisely align with Python's definition of equality, we ban matches // between non-equal types to avoid legacy behavior like `assert True == 1`, which is very // surprising in interning, and would likely be surprising anywhere else in the engine where we // compare things. - if h1.get_type(py) != h2.get_type(py) { + if h1.get_type() != h2.get_type() { return false; } - h1.rich_compare(py, h2, CompareOp::Eq) + h1.rich_compare(h2, CompareOp::Eq) .unwrap() - .cast_as::(py) + .cast_as::() .unwrap() .is_true() } @@ -55,14 +56,10 @@ pub fn store_tuple(py: Python, values: Vec) -> Value { } /// Store a slice containing 2-tuples of (key, value) as a Python dictionary. -pub fn store_dict(py: Python, keys_and_values: Vec<(Value, Value)>) -> Result { +pub fn store_dict(py: Python, keys_and_values: Vec<(Value, Value)>) -> PyResult { let dict = PyDict::new(py); for (k, v) in keys_and_values { - dict.set_item( - py, - k.consume_into_py_object(py), - v.consume_into_py_object(py), - )?; + dict.set_item(k.consume_into_py_object(py), v.consume_into_py_object(py))?; } Ok(Value::from(dict.into_object())) } @@ -72,31 +69,31 @@ pub fn store_bytes(py: Python, bytes: &[u8]) -> Value { Value::from(PyBytes::new(py, bytes).into_object()) } -/// Store an buffer of utf8 bytes to pass to Python. This will end up as a Python `str`. +/// Store a buffer of utf8 bytes to pass to Python. This will end up as a Python `str`. pub fn store_utf8(py: Python, utf8: &str) -> Value { - Value::from(utf8.to_py_object(py).into_object()) + Value::from(utf8.to_object(py).into_object()) } pub fn store_u64(py: Python, val: u64) -> Value { - Value::from(val.to_py_object(py).into_object()) + Value::from(val.to_object(py).into_object()) } pub fn store_i64(py: Python, val: i64) -> Value { - Value::from(val.to_py_object(py).into_object()) + Value::from(val.to_object(py).into_object()) } pub fn store_bool(py: Python, val: bool) -> Value { - Value::from(val.to_py_object(py).into_object()) + Value::from(val.to_object(py).into_object()) } /// /// Gets an attribute of the given value as the given type. /// -pub fn getattr(value: &PyObject, field: &str) -> Result +pub fn getattr(value: &cpython::PyObject, field: &str) -> Result where - for<'a> T: FromPyObject<'a>, + for<'a> T: cpython::FromPyObject<'a>, { - let gil = Python::acquire_gil(); + let gil = cpython::Python::acquire_gil(); let py = gil.python(); value .getattr(py, field) @@ -115,8 +112,8 @@ where /// /// Collect the Values contained within an outer Python Iterable PyObject. /// -pub fn collect_iterable(value: &PyObject) -> Result, String> { - let gil = Python::acquire_gil(); +pub fn collect_iterable(value: &cpython::PyObject) -> Result, String> { + let gil = cpython::Python::acquire_gil(); let py = gil.python(); match value.iter(py) { Ok(py_iter) => py_iter @@ -140,10 +137,10 @@ pub fn collect_iterable(value: &PyObject) -> Result, String> { } } -pub fn getattr_from_frozendict(value: &PyObject, field: &str) -> BTreeMap { +pub fn getattr_from_frozendict(value: &cpython::PyObject, field: &str) -> BTreeMap { let frozendict = getattr(value, field).unwrap(); - let pydict: PyDict = getattr(&frozendict, "_data").unwrap(); - let gil = Python::acquire_gil(); + let pydict: cpython::PyDict = getattr(&frozendict, "_data").unwrap(); + let gil = cpython::Python::acquire_gil(); let py = gil.python(); pydict .items(py) @@ -152,7 +149,11 @@ pub fn getattr_from_frozendict(value: &PyObject, field: &str) -> BTreeMap Option { +pub fn getattr_as_optional_string( + py: cpython::Python, + value: &cpython::PyObject, + field: &str, +) -> Option { let v = value.getattr(py, field).unwrap(); if v.is_none(py) { return None; @@ -162,8 +163,8 @@ pub fn getattr_as_optional_string(py: Python, value: &PyObject, field: &str) -> Some(v.extract(py).unwrap()) } -pub fn val_to_str(obj: &PyObject) -> String { - let gil = Python::acquire_gil(); +pub fn val_to_str(obj: &cpython::PyObject) -> String { + let gil = cpython::Python::acquire_gil(); let py = gil.python(); if *obj == py.None() { @@ -174,7 +175,7 @@ pub fn val_to_str(obj: &PyObject) -> String { pystring.to_string(py).unwrap().into_owned() } -pub fn val_to_log_level(obj: &PyObject) -> Result { +pub fn val_to_log_level(obj: &cpython::PyObject) -> Result { let res: Result = getattr(obj, "_level").and_then(|n: u64| { n.try_into() .map_err(|e: num_enum::TryFromPrimitiveError<_>| { @@ -186,39 +187,43 @@ pub fn val_to_log_level(obj: &PyObject) -> Result { /// Link to the Pants docs using the current version of Pants. pub fn doc_url(py: Python, slug: &str) -> String { - let docutil = py.import("pants.util.docutil").unwrap(); - docutil - .call(py, "doc_url", (slug,), None) - .unwrap() - .extract(py) - .unwrap() + let docutil_module = py.import("pants.util.docutil").unwrap(); + let doc_url_func = docutil_module.getattr("doc_url").unwrap(); + doc_url_func.call1((slug,)).unwrap().extract().unwrap() } -pub fn create_exception(py: Python, msg: &str) -> Value { - Value::from(PyErr::new::(py, msg).instance(py)) +pub fn create_exception(py: cpython::Python, msg: &str) -> Value { + Value::from(cpython::PyErr::new::(py, msg).instance(py)) } -pub fn call_method0(py: Python, value: &PyObject, method: &str) -> Result { - value.call_method(py, method, PyTuple::new(py, &[]), None) +pub fn call_method0( + py: cpython::Python, + value: &cpython::PyObject, + method: &str, +) -> Result { + value.call_method(py, method, cpython::PyTuple::new(py, &[]), None) } -pub fn call_function>(func: T, args: &[Value]) -> Result { - let func: &PyObject = func.as_ref(); - let arg_handles: Vec = args.iter().map(|v| v.clone().into()).collect(); - let gil = Python::acquire_gil(); - let args_tuple = PyTuple::new(gil.python(), &arg_handles); +pub fn call_function>( + func: T, + args: &[Value], +) -> Result { + let func: &cpython::PyObject = func.as_ref(); + let arg_handles: Vec = args.iter().map(|v| v.clone().into()).collect(); + let gil = cpython::Python::acquire_gil(); + let args_tuple = cpython::PyTuple::new(gil.python(), &arg_handles); func.call(gil.python(), args_tuple, None) } pub fn generator_send(generator: &Value, arg: &Value) -> Result { - let gil = Python::acquire_gil(); + let gil = cpython::Python::acquire_gil(); let py = gil.python(); let selectors = py.import("pants.engine.internals.selectors").unwrap(); let response = selectors .call( py, "native_engine_generator_send", - (generator as &PyObject, arg as &PyObject), + (generator as &cpython::PyObject, arg as &cpython::PyObject), None, ) .map_err(|py_err| Failure::from_py_err_with_gil(py, py_err))?; @@ -253,15 +258,15 @@ pub fn generator_send(generator: &Value, arg: &Value) -> Result Value { let py_type = type_id.as_py_type(py); - let arg_handles: Vec = args.iter().map(|v| v.clone().into()).collect(); + let arg_handles: Vec = args.iter().map(|v| v.clone().into()).collect(); let args_tuple = PyTuple::new(py, &arg_handles); py_type - .call(py, args_tuple, None) + .call(args_tuple, None) .map(Value::from) .unwrap_or_else(|e| { panic!( "Core type constructor `{}` failed: {:?}", - py_type.name(py), + py_type.name().unwrap(), e ); }) diff --git a/src/rust/engine/src/interning.rs b/src/rust/engine/src/interning.rs index c0a61a32c043..92bb8a9e29e7 100644 --- a/src/rust/engine/src/interning.rs +++ b/src/rust/engine/src/interning.rs @@ -5,8 +5,9 @@ use std::collections::HashMap; use std::hash; use std::sync::atomic; -use cpython::{ObjectProtocol, PyErr, PyType, Python, ToPyObject}; use parking_lot::{Mutex, RwLock}; +use pyo3::prelude::*; +use pyo3::types::PyType; use crate::externs; use crate::python::{Fnv, Key, Value}; @@ -51,10 +52,10 @@ impl Interns { Interns::default() } - pub fn key_insert(&self, py: Python, v: Value) -> Result { + pub fn key_insert(&self, py: Python, v: Value) -> PyResult { let (intern_key, type_id) = { - let obj = v.to_py_object(py).into(); - (InternKey(v.hash(py)?, obj), (&v.get_type(py)).into()) + let obj = v.to_object(py).into(); + (InternKey(v.hash()?, obj), (&v.get_type()).into()) }; py.allow_threads(|| { @@ -99,8 +100,7 @@ impl Eq for InternKey {} impl PartialEq for InternKey { fn eq(&self, other: &InternKey) -> bool { - let gil = Python::acquire_gil(); - externs::equals(gil.python(), &self.1, &other.1) + Python::with_gil(|py| externs::equals(self.1.into_ref(py), other.1.into_ref(py))) } } diff --git a/src/rust/engine/src/intrinsics.rs b/src/rust/engine/src/intrinsics.rs index ec9753f20e5f..0dc99710bbe3 100644 --- a/src/rust/engine/src/intrinsics.rs +++ b/src/rust/engine/src/intrinsics.rs @@ -12,11 +12,12 @@ use crate::tasks::Intrinsic; use crate::types::Types; use crate::Failure; -use cpython::{ObjectProtocol, Python}; +use cpython::ObjectProtocol; use fs::RelativePath; use futures::future::{self, BoxFuture, FutureExt, TryFutureExt}; use hashing::{Digest, EMPTY_DIGEST}; use indexmap::IndexMap; +use pyo3::Python; use stdio::TryCloneAsFile; use store::{SnapshotOps, SubsetParams}; use tempfile::TempDir; @@ -197,25 +198,26 @@ fn multi_platform_process_request_to_process_result( })?; let platform_name: String = result.platform.into(); - let gil = Python::acquire_gil(); - let py = gil.python(); - Ok(externs::unsafe_call( - py, - context.core.types.process_result, - &[ - externs::store_bytes(py, &stdout_bytes), - Snapshot::store_file_digest(py, &context.core.types, &result.stdout_digest), - externs::store_bytes(py, &stderr_bytes), - Snapshot::store_file_digest(py, &context.core.types, &result.stderr_digest), - externs::store_i64(py, result.exit_code.into()), - Snapshot::store_directory_digest(py, &result.output_directory).map_err(|s| throw(&s))?, - externs::unsafe_call( - py, - context.core.types.platform, - &[externs::store_utf8(py, &platform_name)], - ), - ], - )) + let res = Python::with_gil(|py| { + externs::unsafe_call( + py, + context.core.types.process_result, + &[ + externs::store_bytes(py, &stdout_bytes), + Snapshot::store_file_digest(py, &context.core.types, &result.stdout_digest), + externs::store_bytes(py, &stderr_bytes), + Snapshot::store_file_digest(py, &context.core.types, &result.stderr_digest), + externs::store_i64(py, result.exit_code.into()), + Snapshot::store_directory_digest(py, &result.output_directory).map_err(|s| throw(&s))?, + externs::unsafe_call( + py, + context.core.types.platform, + &[externs::store_utf8(py, &platform_name)], + ), + ], + ) + }); + Ok(res) } .boxed() } @@ -232,8 +234,7 @@ fn directory_digest_to_digest_contents( .contents_for_directory(digest) .await .and_then(move |digest_contents| { - let gil = Python::acquire_gil(); - Snapshot::store_digest_contents(gil.python(), &context, &digest_contents) + Python::with_gil(|py| Snapshot::store_digest_contents(py, &context, &digest_contents)) }) .map_err(|s| throw(&s))?; Ok(snapshot) @@ -253,8 +254,7 @@ fn directory_digest_to_digest_entries( .entries_for_directory(digest) .await .and_then(move |digest_entries| { - let gil = Python::acquire_gil(); - Snapshot::store_digest_entries(gil.python(), &context, &digest_entries) + Python::with_gil(|py| Snapshot::store_digest_entries(py, &context, &digest_entries)) }) .map_err(|s| throw(&s))?; Ok(snapshot) @@ -279,8 +279,7 @@ fn remove_prefix_request_to_digest( .strip_prefix(input_digest, prefix) .await .map_err(|e| throw(&format!("{:?}", e)))?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -301,8 +300,7 @@ fn add_prefix_request_to_digest( .add_prefix(input_digest, prefix) .await .map_err(|e| throw(&format!("{:?}", e)))?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -312,8 +310,7 @@ fn digest_to_snapshot(context: Context, args: Vec) -> BoxFuture<'static, async move { let digest = lift_directory_digest(&args[0])?; let snapshot = store::Snapshot::from_digest(store, digest).await?; - let gil = Python::acquire_gil(); - Snapshot::store_snapshot(gil.python(), snapshot) + Python::with_gil(|py| Snapshot::store_snapshot(py, snapshot)) } .map_err(|e: String| throw(&e)) .boxed() @@ -336,8 +333,7 @@ fn merge_digests_request_to_digest( .merge(digests.map_err(|e| throw(&e))?) .await .map_err(|e| throw(&format!("{:?}", e)))?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -349,8 +345,7 @@ fn download_file_to_digest( async move { let key = Key::from_value(args.pop().unwrap()).map_err(Failure::from_py_err)?; let digest = context.get(DownloadedFile(key)).await?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -364,8 +359,7 @@ fn path_globs_to_digest( let path_globs = Snapshot::lift_path_globs(&val) .map_err(|e| throw(&format!("Failed to parse PathGlobs: {}", e)))?; let digest = context.get(Snapshot::from_path_globs(path_globs)).await?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -380,8 +374,7 @@ fn path_globs_to_paths( let path_globs = Snapshot::lift_path_globs(&val) .map_err(|e| throw(&format!("Failed to parse PathGlobs: {}", e)))?; let paths = context.get(Paths::from_path_globs(path_globs)).await?; - let gil = Python::acquire_gil(); - Paths::store_paths(gil.python(), &core, &paths).map_err(|e: String| throw(&e)) + Python::with_gil(|py| Paths::store_paths(py, &core, &paths).map_err(|e: String| throw(&e))) } .boxed() } @@ -401,7 +394,7 @@ fn create_digest_to_digest( .map_err(|e| format!("The `path` must be relative: {:?}", e))?; let (is_file_content, is_file_entry) = { - let gil = Python::acquire_gil(); + let gil = cpython::Python::acquire_gil(); let py = gil.python(); ( file_item.hasattr(py, "content").unwrap(), @@ -446,8 +439,7 @@ fn create_digest_to_digest( .merge(digests) .await .map_err(|e| throw(&format!("{:?}", e)))?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -468,8 +460,7 @@ fn digest_subset_to_digest( .subset(original_digest, subset_params) .await .map_err(|e| throw(&format!("{:?}", e)))?; - let gil = Python::acquire_gil(); - Snapshot::store_directory_digest(gil.python(), &digest).map_err(|s| throw(&s)) + Python::with_gil(|py| Snapshot::store_directory_digest(py, &digest).map_err(|s| throw(&s))) } .boxed() } @@ -599,15 +590,13 @@ fn interactive_process( .await?; let code = exit_status.code().unwrap_or(-1); - let result = { - let gil = Python::acquire_gil(); - let py = gil.python(); + let result = Python::with_gil(|py| { externs::unsafe_call( py, interactive_process_result, &[externs::store_i64(py, i64::from(code))], ) - }; + }); Ok(result) }.boxed() } diff --git a/src/rust/engine/src/nodes.rs b/src/rust/engine/src/nodes.rs index ba1fc5dd8d82..1449ddab9edd 100644 --- a/src/rust/engine/src/nodes.rs +++ b/src/rust/engine/src/nodes.rs @@ -15,6 +15,7 @@ use bytes::Bytes; use futures::future::{self, BoxFuture, FutureExt, TryFutureExt}; use grpc_util::prost::MessageExt; use protos::gen::pants::cache::{CacheKey, CacheKeyType, ObservedUrl}; +use pyo3::prelude::Python; use url::Url; use crate::context::{Context, Core}; @@ -24,7 +25,7 @@ use crate::python::{display_sorted_in_parens, throw, Failure, Key, Params, TypeI use crate::selectors; use crate::tasks::{self, Rule}; use crate::Types; -use cpython::{PyObject, Python, PythonObject}; +use cpython::PythonObject; use fs::{ self, DigestEntry, Dir, DirectoryListing, File, FileContent, FileEntry, GlobExpansionConjunction, GlobMatching, Link, PathGlobs, PathStat, PreparedPathGlobs, RelativePath, StrictGlobMatching, @@ -264,12 +265,15 @@ impl From