PyO3: complete Bound migration and disable gil-refs feature on `p…

…yo3` crate (pantsbuild#21499)

Complete the migration to the `pyo3::Bound` smart pointer and disable the `gil-refs` feature on the `pyo3` crate so any use of the "GIL refs" API will be flagged as a deprecation (which our source will error for).

src/rust/engine/Cargo.toml

@@ -294,7 +294,7 @@ prodash = { git = "https://github.com/stuhood/prodash", rev = "stuhood/raw-messa
 prost = "0.13"
 prost-build = "0.13"
 prost-types = "0.13"
-pyo3 = { version = "0.21", features = ["gil-refs"] }
+pyo3 = { version = "0.21" }
 pyo3-build-config = "0.21"
 rand = "0.8"
 regex = "1"

src/rust/engine/src/externs/address.rs

@@ -30,7 +30,10 @@ pub fn register(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> {
         "AddressParseException",
         py.get_type_bound::<AddressParseException>(),
     )?;
-    m.add("InvalidAddressError", py.get_type::<InvalidAddressError>())?;
+    m.add(
+        "InvalidAddressError",
+        py.get_type_bound::<InvalidAddressError>(),
+    )?;
     m.add(
         "InvalidSpecPathError",
         py.get_type_bound::<InvalidSpecPathError>(),
@@ -153,12 +156,19 @@ impl AddressInput {
         _cls: &Bound<'_, PyType>,
         spec: &str,
         description_of_origin: &str,
-        relative_to: Option<&str>,
-        subproject_roots: Option<Vec<&str>>,
+        relative_to: Option<String>,
+        subproject_roots: Option<Vec<String>>,
     ) -> PyResult<Self> {
-        let subproject_info = subproject_roots
-            .zip(relative_to)
-            .and_then(|(roots, relative_to)| split_on_longest_dir_prefix(relative_to, &roots));
+        let roots_as_strs = subproject_roots
+            .as_deref()
+            .map(|roots| roots.iter().map(|s| s.as_str()).collect::<Vec<_>>());
+        let relative_to2 = relative_to.as_deref();
+        let subproject_info = match (roots_as_strs, relative_to2) {
+            (Some(roots), Some(relative_to)) => {
+                split_on_longest_dir_prefix(relative_to, &roots[..])
+            }
+            _ => None,
+        };
 
         let parsed_spec =
             address::parse_address_spec(spec).map_err(AddressParseException::new_err)?;
@@ -173,7 +183,7 @@ impl AddressInput {
         let normalized_relative_to = if let Some((_, normalized_relative_to)) = subproject_info {
             Some(normalized_relative_to)
         } else {
-            relative_to
+            relative_to.as_deref()
         };
 
         let mut path_component: Cow<str> = address.path.into();

src/rust/engine/src/externs/fs.rs

@@ -11,6 +11,7 @@ use itertools::Itertools;
 use pyo3::basic::CompareOp;
 use pyo3::exceptions::{PyException, PyTypeError, PyValueError};
 use pyo3::prelude::*;
+use pyo3::pybacked::PyBackedStr;
 use pyo3::types::{PyIterator, PyString, PyTuple, PyType};
 
 use fs::{
@@ -269,13 +270,12 @@ pub struct PyMergeDigests(pub Vec<DirectoryDigest>);
 impl PyMergeDigests {
     #[new]
     fn __new__(digests: &Bound<'_, PyAny>, _py: Python) -> PyResult<Self> {
-        let digests: PyResult<Vec<DirectoryDigest>> =
-            PyIterator::from_object(digests.as_gil_ref())?
-                .map(|v| {
-                    let py_digest = v?.extract::<PyDigest>()?;
-                    Ok(py_digest.0)
-                })
-                .collect();
+        let digests: PyResult<Vec<DirectoryDigest>> = PyIterator::from_bound_object(digests)?
+            .map(|v| {
+                let py_digest = v?.extract::<PyDigest>()?;
+                Ok(py_digest.0)
+            })
+            .collect();
         Ok(Self(digests?))
     }
 
@@ -415,16 +415,18 @@ impl<'py> FromPyObject<'py> for PyPathGlobs {
             Some(description_of_origin_field.extract()?)
         };
 
-        let match_behavior_str: &str = obj
+        let match_behavior_str: PyBackedStr = obj
             .getattr("glob_match_error_behavior")?
             .getattr("value")?
             .extract()?;
-        let match_behavior = StrictGlobMatching::create(match_behavior_str, description_of_origin)
-            .map_err(PyValueError::new_err)?;
+        let match_behavior =
+            StrictGlobMatching::create(match_behavior_str.as_ref(), description_of_origin)
+                .map_err(PyValueError::new_err)?;
 
-        let conjunction_str: &str = obj.getattr("conjunction")?.getattr("value")?.extract()?;
-        let conjunction =
-            GlobExpansionConjunction::create(conjunction_str).map_err(PyValueError::new_err)?;
+        let conjunction_str: PyBackedStr =
+            obj.getattr("conjunction")?.getattr("value")?.extract()?;
+        let conjunction = GlobExpansionConjunction::create(conjunction_str.as_ref())
+            .map_err(PyValueError::new_err)?;
 
         Ok(PyPathGlobs(PathGlobs::new(
             globs,

src/rust/engine/src/externs/interface.rs

@@ -74,7 +74,7 @@ fn native_engine(py: Python, m: &Bound<'_, PyModule>) -> PyO3Result<()> {
     externs::workunits::register(m)?;
     externs::dep_inference::register(m)?;
 
-    m.add("PollTimeout", py.get_type::<PollTimeout>())?;
+    m.add("PollTimeout", py.get_type_bound::<PollTimeout>())?;
 
     m.add_class::<PyExecutionRequest>()?;
     m.add_class::<PyExecutionStrategyOptions>()?;
@@ -597,7 +597,7 @@ fn nailgun_server_create(
         let executor = py_executor.0.clone();
         nailgun::Server::new(executor, port, move |exe: nailgun::RawFdExecution| {
             Python::with_gil(|py| {
-                let result = runner.as_ref(py).call1((
+                let result = runner.bind(py).call1((
                     exe.cmd.command,
                     PyTuple::new_bound(py, exe.cmd.args),
                     exe.cmd.env.into_iter().collect::<HashMap<String, String>>(),

src/rust/engine/src/externs/mod.rs

@@ -48,11 +48,11 @@ pub fn register(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_class::<PyGeneratorResponseCall>()?;
     m.add_class::<PyGeneratorResponseGet>()?;
 
-    m.add("EngineError", py.get_type::<EngineError>())?;
-    m.add("IntrinsicError", py.get_type::<IntrinsicError>())?;
+    m.add("EngineError", py.get_type_bound::<EngineError>())?;
+    m.add("IntrinsicError", py.get_type_bound::<IntrinsicError>())?;
     m.add(
         "IncorrectProductError",
-        py.get_type::<IncorrectProductError>(),
+        py.get_type_bound::<IncorrectProductError>(),
     )?;
 
     Ok(())
@@ -144,7 +144,7 @@ pub fn store_dict(
 
 /// Store an opaque buffer of bytes to pass to Python. This will end up as a Python `bytes`.
 pub fn store_bytes(py: Python, bytes: &[u8]) -> Value {
-    Value::from(PyBytes::new(py, bytes).to_object(py))
+    Value::from(PyBytes::new_bound(py, bytes).to_object(py))
 }
 
 /// Store a buffer of utf8 bytes to pass to Python. This will end up as a Python `str`.
@@ -269,7 +269,7 @@ pub fn val_to_log_level_bound(obj: &Bound<'_, PyAny>) -> Result<log::Level, Stri
 
 /// Link to the Pants docs using the current version of Pants.
 pub fn doc_url(py: Python, slug: &str) -> String {
-    let docutil_module = py.import("pants.util.docutil").unwrap();
+    let docutil_module = py.import_bound("pants.util.docutil").unwrap();
     let doc_url_func = docutil_module.getattr("doc_url").unwrap();
     doc_url_func.call1((slug,)).unwrap().extract().unwrap()
 }
@@ -311,7 +311,7 @@ pub(crate) fn generator_send(
             let throw_method = generator.bind(py).getattr(intern!(py, "throw"))?;
             if err.is_instance_of::<NativeEngineFailure>(py) {
                 let throw = err
-                    .value(py)
+                    .value_bound(py)
                     .getattr(intern!(py, "failure"))?
                     .extract::<PyRef<PyFailure>>()?
                     .get_error(py);
@@ -334,12 +334,14 @@ pub(crate) fn generator_send(
     let response = match response_unhandled {
         Err(e) if e.is_instance_of::<PyStopIteration>(py) => {
             let value = e.into_value(py).getattr(py, intern!(py, "value"))?;
-            let type_id = TypeId::new(&value.as_ref(py).get_type().as_borrowed());
+            let type_id = TypeId::new(&value.bind(py).get_type());
             return Ok(GeneratorResponse::Break(Value::new(value), type_id));
         }
         Err(e) => {
             match (maybe_thrown, e.cause(py)) {
-                (Some((thrown, err)), Some(cause)) if thrown.value(py).is(cause.value(py)) => {
+                (Some((thrown, err)), Some(cause))
+                    if thrown.value_bound(py).is(cause.value_bound(py)) =>
+                {
                     // Preserve the engine traceback by using the wrapped failure error as cause. The cause
                     // will be swapped back again in `Failure::from_py_err_with_gil()` to preserve the python
                     // traceback.

src/rust/engine/src/externs/nailgun.rs

@@ -12,11 +12,11 @@ use task_executor::Executor;
 pub fn register(py: Python, m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add(
         "PantsdConnectionException",
-        py.get_type::<PantsdConnectionException>(),
+        py.get_type_bound::<PantsdConnectionException>(),
     )?;
     m.add(
         "PantsdClientException",
-        py.get_type::<PantsdClientException>(),
+        py.get_type_bound::<PantsdClientException>(),
     )?;
     m.add_class::<PyNailgunClient>()?;
     Ok(())

src/rust/engine/src/externs/options.rs

@@ -27,7 +27,7 @@ fn val_to_py_object(py: Python, val: &Val) -> PyResult<PyObject> {
         Val::Float(f) => f.into_py(py),
         Val::String(s) => s.into_py(py),
         Val::List(list) => {
-            let pylist = PyList::empty(py);
+            let pylist = PyList::empty_bound(py);
             for m in list {
                 pylist.append(val_to_py_object(py, m)?)?;
             }

src/rust/engine/src/externs/scheduler.rs

@@ -38,7 +38,7 @@ impl PyExecutor {
             let _ =
                 unsafe { ffi::PyThreadState_New(Python::with_gil(|_| PyInterpreterState_Main())) };
             Python::with_gil(|py| {
-                let _ = py.eval("__import__('debugpy').debug_this_thread()", None, None);
+                let _ = py.eval_bound("__import__('debugpy').debug_this_thread()", None, None);
             });
         })
         .map(PyExecutor)

src/rust/engine/src/externs/target.rs

@@ -141,7 +141,7 @@ impl Field {
     }
 
     fn __hash__(self_: &Bound<'_, Self>, py: Python) -> PyResult<isize> {
-        Ok(self_.get_type().hash()? & self_.borrow().value.as_ref(py).hash()?)
+        Ok(self_.get_type().hash()? & self_.borrow().value.bind(py).hash()?)
     }
 
     fn __repr__(self_: &Bound<'_, Self>) -> PyResult<String> {
@@ -180,7 +180,7 @@ impl Field {
             && self_
                 .borrow()
                 .value
-                .as_ref(py)
+                .bind(py)
                 .eq(&other.extract::<PyRef<Field>>()?.value)?;
         match op {
             CompareOp::Eq => Ok(is_eq.into_py(py)),

src/rust/engine/src/intrinsics/interactive_process.rs

@@ -12,6 +12,7 @@ use process_execution::local::{
 };
 use process_execution::{ManagedChild, ProcessExecutionStrategy};
 use pyo3::prelude::{pyfunction, wrap_pyfunction, PyAny, PyModule, PyResult, Python};
+use pyo3::pybacked::PyBackedStr;
 use pyo3::types::{PyAnyMethods, PyModuleMethods};
 use pyo3::Bound;
 use stdio::TryCloneAsFile;
@@ -87,7 +88,9 @@ pub async fn interactive_process_inner(
         let keep_sandboxes_value: Bound<'_, PyAny> =
             externs::getattr_bound(py_interactive_process, "keep_sandboxes").unwrap();
         let keep_sandboxes = KeepSandboxes::from_str(
-            externs::getattr_bound(&keep_sandboxes_value, "value").unwrap(),
+            externs::getattr_bound::<PyBackedStr>(&keep_sandboxes_value, "value")
+                .unwrap()
+                .as_ref(),
         )
         .unwrap();
         (run_in_workspace, keep_sandboxes)

src/rust/engine/src/nodes/execute_process.rs

@@ -13,6 +13,7 @@ use process_execution::{
     ProcessResultSource,
 };
 use pyo3::prelude::{PyAny, Python};
+use pyo3::pybacked::PyBackedStr;
 use pyo3::Bound;
 use store::{self, Store, StoreError};
 use workunit_store::{
@@ -113,9 +114,12 @@ impl ExecuteProcess {
         let level = externs::val_to_log_level_bound(&py_level)?;
 
         let append_only_caches =
-            externs::getattr_from_str_frozendict_bound::<&str>(value, "append_only_caches")
+            externs::getattr_from_str_frozendict_bound::<PyBackedStr>(value, "append_only_caches")
                 .into_iter()
-                .map(|(name, dest)| Ok((CacheName::new(name)?, RelativePath::new(dest)?)))
+                .map(|(name, dest)| {
+                    let path: &str = dest.as_ref();
+                    Ok((CacheName::new(name)?, RelativePath::new(path)?))
+                })
                 .collect::<Result<_, String>>()?;
 
         let jdk_home = externs::getattr_as_optional_string_bound(value, "jdk_home")

src/rust/engine/src/python.rs

@@ -137,7 +137,11 @@ impl TypeId {
         // SAFETY: Dereferencing a pointer to a PyTypeObject is safe as long as the module defining the
         // type is not unloaded. That is true today, but would not be if we implemented support for hot
         // reloading of plugins.
-        unsafe { PyType::from_type_ptr(py, self.0).as_borrowed().to_owned() }
+        unsafe {
+            PyType::from_borrowed_type_ptr(py, self.0)
+                .as_borrowed()
+                .to_owned()
+        }
     }
 
     pub fn is_union(&self) -> bool {
@@ -378,7 +382,7 @@ impl<'py, T> From<&Bound<'py, T>> for Value {
 
 impl IntoPy<PyObject> for &Value {
     fn into_py(self, py: Python) -> PyObject {
-        (*self.0).as_ref(py).into_py(py)
+        (*self.0).bind(py).into_py(py)
     }
 }
 
@@ -473,13 +477,13 @@ impl Failure {
         };
 
         let maybe_ptraceback = py_err
-            .traceback(py)
+            .traceback_bound(py)
             .map(|traceback| traceback.to_object(py));
         let val = Value::from(py_err.into_py(py));
         let python_traceback = if let Some(tb) = maybe_ptraceback {
             let locals = PyDict::new_bound(py);
             locals
-                .set_item("traceback", py.import("traceback").unwrap())
+                .set_item("traceback", py.import_bound("traceback").unwrap())
                 .unwrap();
             locals.set_item("tb", tb).unwrap();
             locals.set_item("val", &val).unwrap();