Remove project_ methods from the externs module (#10955)

This commit removes all the `project_` functions in `externs` in favor of `externs::getattr` with type specialization at each callsite. `project_frozendict` and `project_str` both have a little bit of custom logic, so it makes sense to leave these two functions alone, although they have been renamed to `getattr_from_frozendict` and `getattr_as_string` to make it clear that these functions are variations on the fundamental `getattr` operation.

src/rust/engine/src/core.rs

@@ -157,8 +157,8 @@ impl Function {
   pub fn name(&self) -> String {
     let Function(key) = self;
     let val = externs::val_for(&key);
-    let module = externs::project_str(&val, "__module__");
-    let name = externs::project_str(&val, "__name__");
+    let module = externs::getattr_as_string(&val, "__module__");
+    let name = externs::getattr_as_string(&val, "__name__");
     // NB: this is a custom dunder method that Python code should populate before sending the
     // function (e.g. an `@rule`) through FFI.
     let line_number: u64 = externs::getattr(&val, "__line_number__").unwrap();

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

@@ -694,9 +694,10 @@ fn nailgun_server_create(
         ];
         match externs::call_function(&runner, &runner_args) {
           Ok(exit_code) => {
-            // TODO: We don't currently expose a "project_i32", but it will not be necessary with
-            // https://github.com/pantsbuild/pants/pull/9593.
-            nailgun::ExitCode(externs::val_to_str(&exit_code).parse().unwrap())
+            let gil = Python::acquire_gil();
+            let py = gil.python();
+            let code: i32 = exit_code.extract(py).unwrap();
+            nailgun::ExitCode(code)
           }
           Err(e) => {
             error!("Uncaught exception in nailgun handler: {:#?}", e);
@@ -1367,20 +1368,23 @@ fn capture_snapshots(
       session.workunit_store().init_thread_state(None);
       let core = scheduler.core.clone();
 
-      let values = externs::project_iterable(&path_globs_and_root_tuple_wrapper.into());
+      let values = externs::collect_iterable(&path_globs_and_root_tuple_wrapper).unwrap();
       let path_globs_and_roots = values
         .iter()
         .map(|value| {
-          let root = PathBuf::from(externs::project_str(&value, "root"));
+          let root = PathBuf::from(externs::getattr_as_string(&value, "root"));
           let path_globs = nodes::Snapshot::lift_prepared_path_globs(
-            &externs::project_ignoring_type(&value, "path_globs"),
+            &externs::getattr(&value, "path_globs").unwrap(),
           );
           let digest_hint = {
-            let maybe_digest = externs::project_ignoring_type(&value, "digest_hint");
-            if maybe_digest == Value::from(externs::none()) {
+            let maybe_digest: PyObject = externs::getattr(&value, "digest_hint").unwrap();
+            if maybe_digest == externs::none() {
               None
             } else {
-              Some(nodes::lift_directory_digest(&core.types, &maybe_digest)?)
+              Some(nodes::lift_directory_digest(
+                &core.types,
+                &Value::new(maybe_digest),
+              )?)
             }
           };
           path_globs.map(|path_globs| (path_globs, root, digest_hint))
@@ -1508,7 +1512,7 @@ fn run_local_interactive_process(
 
           let value: Value = request.into();
 
-          let argv: Vec<String> = externs::project_multi_strs(&value, "argv");
+          let argv: Vec<String> = externs::getattr(&value, "argv").unwrap();
           if argv.is_empty() {
             return Err("Empty argv list not permitted".to_string());
           }
@@ -1520,7 +1524,7 @@ fn run_local_interactive_process(
             Some(TempDir::new().map_err(|err| format!("Error creating tempdir: {}", err))?)
           };
 
-          let input_digest_value = externs::project_ignoring_type(&value, "input_digest");
+          let input_digest_value: Value = externs::getattr(&value, "input_digest").unwrap();
           let digest: Digest = nodes::lift_directory_digest(types, &input_digest_value)?;
           if digest != EMPTY_DIGEST {
             if run_in_workspace {
@@ -1562,7 +1566,7 @@ fn run_local_interactive_process(
           if hermetic_env {
             command.env_clear();
           }
-          let env = externs::project_frozendict(&value, "env");
+          let env = externs::getattr_from_frozendict(&value, "env");
           command.envs(env);
 
           let mut subprocess = command.spawn().map_err(|e| format!("Error executing interactive process: {}", e.to_string()))?;

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

@@ -197,16 +197,16 @@ where
 }
 
 ///
-/// Collect the Values contained within an outer Python Iterable Value.
+/// Collect the Values contained within an outer Python Iterable PyObject.
 ///
-fn collect_iterable(value: &Value) -> Result<Vec<Value>, String> {
+pub fn collect_iterable(value: &PyObject) -> Result<Vec<PyObject>, String> {
   let gil = Python::acquire_gil();
   let py = gil.python();
   match value.iter(py) {
     Ok(py_iter) => py_iter
       .enumerate()
       .map(|(i, py_res)| {
-        py_res.map(Value::from).map_err(|py_err| {
+        py_res.map_err(|py_err| {
           format!(
             "Could not iterate {}, failed to extract {}th item: {:?}",
             val_to_str(value),
@@ -224,26 +224,7 @@ fn collect_iterable(value: &Value) -> Result<Vec<Value>, String> {
   }
 }
 
-///
-/// Pulls out the value specified by the field name from a given Value
-///
-pub fn project_ignoring_type(value: &PyObject, field: &str) -> Value {
-  getattr(value, field).unwrap()
-}
-
-pub fn project_iterable(value: &Value) -> Vec<Value> {
-  collect_iterable(value).unwrap()
-}
-
-pub fn project_multi(value: &PyObject, field: &str) -> Vec<Value> {
-  getattr(value, field).unwrap()
-}
-
-pub fn project_multi_strs(value: &PyObject, field: &str) -> Vec<String> {
-  getattr(value, field).unwrap()
-}
-
-pub fn project_frozendict(value: &PyObject, field: &str) -> BTreeMap<String, String> {
+pub fn getattr_from_frozendict(value: &PyObject, field: &str) -> BTreeMap<String, String> {
   let frozendict = getattr(value, field).unwrap();
   let pydict: PyDict = getattr(&frozendict, "_data").unwrap();
   let gil = Python::acquire_gil();
@@ -255,7 +236,7 @@ pub fn project_frozendict(value: &PyObject, field: &str) -> BTreeMap<String, Str
     .collect()
 }
 
-pub fn project_str(value: &PyObject, field: &str) -> String {
+pub fn getattr_as_string(value: &PyObject, field: &str) -> String {
   // TODO: It's possible to view a python string as a `Cow<str>`, so we could avoid actually
   // cloning in some cases.
   // TODO: We can't directly extract as a string here, because val_to_str defaults to empty string
@@ -268,7 +249,7 @@ pub fn key_to_str(key: &Key) -> String {
 }
 
 pub fn type_to_str(type_id: TypeId) -> String {
-  project_str(&type_for_type_id(type_id).into_object(), "__name__")
+  getattr_as_string(&type_for_type_id(type_id).into_object(), "__name__")
 }
 
 pub fn val_to_str(obj: &PyObject) -> String {

src/rust/engine/src/intrinsics.rs

@@ -227,12 +227,10 @@ fn remove_prefix_request_to_digest(
   let store = core.store();
 
   async move {
-    let input_digest = lift_directory_digest(
-      &core.types,
-      &externs::project_ignoring_type(&args[0], "digest"),
-    )
-    .map_err(|e| throw(&e))?;
-    let prefix = externs::project_str(&args[0], "prefix");
+    let input_digest =
+      lift_directory_digest(&core.types, &externs::getattr(&args[0], "digest").unwrap())
+        .map_err(|e| throw(&e))?;
+    let prefix = externs::getattr_as_string(&args[0], "prefix");
     let prefix = RelativePath::new(PathBuf::from(prefix))
       .map_err(|e| throw(&format!("The `prefix` must be relative: {:?}", e)))?;
     let digest = store
@@ -251,12 +249,10 @@ fn add_prefix_request_to_digest(
   let core = context.core;
   let store = core.store();
   async move {
-    let input_digest = lift_directory_digest(
-      &core.types,
-      &externs::project_ignoring_type(&args[0], "digest"),
-    )
-    .map_err(|e| throw(&e))?;
-    let prefix = externs::project_str(&args[0], "prefix");
+    let input_digest =
+      lift_directory_digest(&core.types, &externs::getattr(&args[0], "digest").unwrap())
+        .map_err(|e| throw(&e))?;
+    let prefix = externs::getattr_as_string(&args[0], "prefix");
     let prefix = RelativePath::new(PathBuf::from(prefix))
       .map_err(|e| throw(&format!("The `prefix` must be relative: {:?}", e)))?;
     let digest = store
@@ -286,10 +282,12 @@ fn merge_digests_request_to_digest(
 ) -> BoxFuture<'static, NodeResult<Value>> {
   let core = context.core;
   let store = core.store();
-  let digests: Result<Vec<hashing::Digest>, String> = externs::project_multi(&args[0], "digests")
-    .into_iter()
-    .map(|val| lift_directory_digest(&core.types, &val))
-    .collect();
+  let digests: Result<Vec<hashing::Digest>, String> =
+    externs::getattr::<Vec<Value>>(&args[0], "digests")
+      .unwrap()
+      .into_iter()
+      .map(|val: Value| lift_directory_digest(&core.types, &val))
+      .collect();
   async move {
     let digest = store
       .merge(digests.map_err(|e| throw(&e))?)
@@ -345,17 +343,17 @@ fn create_digest_to_digest(
   context: Context,
   args: Vec<Value>,
 ) -> BoxFuture<'static, NodeResult<Value>> {
-  let file_contents_and_directories = externs::project_iterable(&args[0]);
+  let file_contents_and_directories = externs::collect_iterable(&args[0]).unwrap();
   let digests: Vec<_> = file_contents_and_directories
     .into_iter()
     .map(|file_content_or_directory| {
-      let path = externs::project_str(&file_content_or_directory, "path");
+      let path = externs::getattr_as_string(&file_content_or_directory, "path");
       let store = context.core.store();
       async move {
         let path = RelativePath::new(PathBuf::from(path))
           .map_err(|e| format!("The `path` must be relative: {:?}", e))?;
 
-        if externs::hasattr(file_content_or_directory.as_ref(), "content") {
+        if externs::hasattr(&file_content_or_directory, "content") {
           let bytes = bytes::Bytes::from(
             externs::getattr::<Vec<u8>>(&file_content_or_directory, "content").unwrap(),
           );
@@ -394,14 +392,14 @@ fn digest_subset_to_digest(
   context: Context,
   args: Vec<Value>,
 ) -> BoxFuture<'static, NodeResult<Value>> {
-  let globs = externs::project_ignoring_type(&args[0], "globs");
+  let globs = externs::getattr(&args[0], "globs").unwrap();
   let store = context.core.store();
 
   async move {
     let path_globs = Snapshot::lift_prepared_path_globs(&globs).map_err(|e| throw(&e))?;
     let original_digest = lift_directory_digest(
       &context.core.types,
-      &externs::project_ignoring_type(&args[0], "digest"),
+      &externs::getattr(&args[0], "digest").unwrap(),
     )
     .map_err(|e| throw(&e))?;
     let subset_params = SubsetParams { globs: path_globs };