refactor(eval): add get_env to Io trait

Abstracts enviornment access to the IO trait.
It also replaces the unsafe set_var in the test harness.

Change-Id: I3908f2d3cc4392ad2f5bc843c4de63382b692dfd
Reviewed-on: https://cl.snix.dev/c/snix/+/30639
Autosubmit: Bence Nemes <nemes.bence1@gmail.com>
Tested-by: besadii
Reviewed-by: Florian Klink <flokli@flokli.de>

snix/eval/src/builtins/impure.rs

@@ -1,10 +1,7 @@
 use builtin_macros::builtins;
 use genawaiter::rc::Gen;
 
-use std::{
-    env,
-    time::{SystemTime, UNIX_EPOCH},
-};
+use std::time::{SystemTime, UNIX_EPOCH};
 
 use crate::{
     self as snix_eval,
@@ -24,9 +21,10 @@ mod impure_builtins {
 
     #[builtin("getEnv")]
     async fn builtin_get_env(co: GenCo, var: Value) -> Result<Value, ErrorKind> {
-        Ok(env::var(OsStr::from_bytes(&var.to_str()?))
-            .unwrap_or_else(|_| "".into())
-            .into())
+        let key = OsStr::from_bytes(&var.to_str()?).to_os_string();
+
+        let env = generators::request_get_env(&co, key).await;
+        Ok(Value::String(NixString::from(env.as_bytes())))
     }
 
     #[builtin("hashFile")]

snix/eval/src/io.rs

@@ -16,6 +16,8 @@
 //! how store paths are opened and so on.
 
 use std::{
+    env,
+    ffi::{OsStr, OsString},
     io,
     path::{Path, PathBuf},
 };
@@ -114,6 +116,9 @@ pub trait EvalIO {
     fn store_dir(&self) -> Option<String> {
         None
     }
+
+    /// Fetches the environment variable key from the current process.
+    fn get_env(&self, key: &OsStr) -> Option<OsString>;
 }
 
 /// Implementation of [`EvalIO`] that simply uses the equivalent
@@ -178,6 +183,10 @@ impl EvalIO for StdIO {
     fn import_path(&self, path: &Path) -> io::Result<PathBuf> {
         Ok(path.to_path_buf())
     }
+
+    fn get_env(&self, key: &OsStr) -> Option<OsString> {
+        env::var_os(key)
+    }
 }
 
 /// Dummy implementation of [`EvalIO`], can be used in contexts where
@@ -219,4 +228,8 @@ impl EvalIO for DummyIO {
             "I/O methods are not implemented in DummyIO",
         ))
     }
+
+    fn get_env(&self, _: &OsStr) -> Option<OsString> {
+        None
+    }
 }

snix/eval/src/tests/nix_tests.rs

@@ -1,8 +1,13 @@
-use crate::value::Value;
+use crate::{value::Value, EvalIO, FileType};
 use builtin_macros::builtins;
 use pretty_assertions::assert_eq;
 use rstest::rstest;
-use std::path::PathBuf;
+use std::{
+    ffi::{OsStr, OsString},
+    io::{self},
+    path::{Path, PathBuf},
+    str::FromStr,
+};
 
 #[builtins]
 mod mock_builtins {
@@ -35,11 +40,58 @@ mod mock_builtins {
     }
 }
 
+struct MockIo<T> {
+    // Actual underlying [EvalIO] implementation.
+    actual: T,
+}
+
+impl<T> MockIo<T> {
+    pub fn new(actual: T) -> Self {
+        Self { actual }
+    }
+}
+
+impl<T> EvalIO for MockIo<T>
+where
+    T: AsRef<dyn EvalIO>,
+{
+    fn store_dir(&self) -> Option<String> {
+        self.actual.as_ref().store_dir()
+    }
+
+    fn import_path(&self, path: &Path) -> io::Result<PathBuf> {
+        self.actual.as_ref().import_path(path)
+    }
+
+    fn path_exists(&self, path: &Path) -> io::Result<bool> {
+        self.actual.as_ref().path_exists(path)
+    }
+
+    fn open(&self, path: &Path) -> io::Result<Box<dyn io::Read>> {
+        self.actual.as_ref().open(path)
+    }
+
+    fn file_type(&self, path: &Path) -> io::Result<FileType> {
+        self.actual.as_ref().file_type(path)
+    }
+
+    fn read_dir(&self, path: &Path) -> io::Result<Vec<(bytes::Bytes, FileType)>> {
+        self.actual.as_ref().read_dir(path)
+    }
+
+    fn get_env(&self, key: &OsStr) -> Option<OsString> {
+        // for eval-okay-getenv.nix
+        if key == "TEST_VAR" {
+            return Some(OsString::from_str("foo").expect("This conversion is infallible."));
+        }
+
+        self.actual.as_ref().get_env(key)
+    }
+}
+
 #[cfg(feature = "impure")]
 fn eval_test(code_path: PathBuf, expect_success: bool) {
-    use crate::vm::EvalMode;
-
-    std::env::set_var("TEST_VAR", "foo"); // for eval-okay-getenv.nix
+    use crate::{vm::EvalMode, StdIO};
 
     eprintln!("path: {}", code_path.display());
     assert_eq!(
@@ -52,6 +104,7 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
 
     let eval = crate::Evaluation::builder_impure()
         .mode(EvalMode::Strict)
+        .io_handle(Box::new(MockIo::new(Box::new(StdIO) as Box<dyn EvalIO>)) as Box<dyn EvalIO>)
         .add_builtins(mock_builtins::builtins())
         .build();
 

snix/eval/src/vm/generators.rs

@@ -10,6 +10,7 @@
 use core::pin::Pin;
 use genawaiter::rc::Co;
 pub use genawaiter::rc::Gen;
+use std::ffi::OsString;
 use std::fmt::Display;
 use std::future::Future;
 
@@ -119,6 +120,9 @@ pub enum VMRequest {
 
     /// Request the VM for the file type of the given path.
     ReadFileType(PathBuf),
+
+    // Request that the VM reads the given environment variable.
+    GetEnv(OsString),
 }
 
 /// Human-readable representation of a generator message, used by observers.
@@ -176,6 +180,7 @@ impl Display for VMRequest {
             VMRequest::Span => write!(f, "span"),
             VMRequest::TryForce(v) => write!(f, "try_force({})", v.type_of()),
             VMRequest::ReadFileType(p) => write!(f, "read_file_type({})", p.to_string_lossy()),
+            VMRequest::GetEnv(p) => write!(f, "get_env({})", p.to_string_lossy()),
         }
     }
 }
@@ -202,6 +207,9 @@ pub enum VMResponse {
     Reader(Box<dyn std::io::Read>),
 
     FileType(FileType),
+
+    /// environment variable
+    Env(OsString),
 }
 
 impl Display for VMResponse {
@@ -214,6 +222,7 @@ impl Display for VMResponse {
             VMResponse::Span(_) => write!(f, "span"),
             VMResponse::Reader(_) => write!(f, "reader"),
             VMResponse::FileType(t) => write!(f, "file_type({t})"),
+            VMResponse::Env(t) => write!(f, "env({})", t.to_string_lossy()),
         }
     }
 }
@@ -504,6 +513,11 @@ where
 
                             message = VMResponse::FileType(file_type);
                         }
+                        VMRequest::GetEnv(key) => {
+                            let env = self.io_handle.as_ref().get_env(&key).unwrap_or_default();
+
+                            message = VMResponse::Env(env);
+                        }
                     }
                 }
 
@@ -738,6 +752,14 @@ pub(crate) async fn request_read_file_type(co: &GenCo, path: PathBuf) -> FileTyp
     }
 }
 
+#[cfg_attr(not(feature = "impure"), allow(unused))]
+pub(crate) async fn request_get_env(co: &GenCo, key: OsString) -> OsString {
+    match co.yield_(VMRequest::GetEnv(key)).await {
+        VMResponse::Env(env) => env,
+        msg => panic!("Snix bug: VM responded with incorrect generator message: {msg}"),
+    }
+}
+
 /// Call the given value as if it was an attribute set containing a functor. The
 /// arguments must already be prepared on the stack when a generator frame from
 /// this function is invoked.

snix/glue/src/snix_io.rs

@@ -7,6 +7,7 @@
 //! otherwise nixpkgs bootstrapping will not work.
 
 use snix_eval::{EvalIO, FileType};
+use std::ffi::{OsStr, OsString};
 use std::io::{self, Cursor};
 use std::path::{Path, PathBuf};
 
@@ -65,4 +66,8 @@ where
     fn read_dir(&self, path: &Path) -> io::Result<Vec<(bytes::Bytes, FileType)>> {
         self.actual.as_ref().read_dir(path)
     }
+
+    fn get_env(&self, key: &OsStr) -> Option<OsString> {
+        self.actual.as_ref().get_env(key)
+    }
 }

snix/glue/src/snix_store_io.rs

@@ -6,6 +6,8 @@ use snix_eval::{EvalIO, FileType, StdIO};
 use snix_store::nar::NarCalculationService;
 use std::{
     cell::RefCell,
+    env,
+    ffi::{OsStr, OsString},
     io,
     path::{Path, PathBuf},
     sync::Arc,
@@ -490,6 +492,10 @@ impl EvalIO for SnixStoreIO {
     fn store_dir(&self) -> Option<String> {
         Some("/nix/store".to_string())
     }
+
+    fn get_env(&self, key: &OsStr) -> Option<OsString> {
+        env::var_os(key)
+    }
 }
 
 #[cfg(test)]