Refactor host env lookup to return an hashmap

Author: ranfdev

src/backends/distrobox/distrobox.rs

@@ -3,7 +3,7 @@ use crate::fakers::{Child, Command, CommandRunner, FdMode, NullCommandRunnerBuil
 use serde::{Deserialize, Deserializer};
 use std::{
     cell::LazyCell,
-    collections::BTreeMap,
+    collections::{BTreeMap, HashMap},
     ffi::OsString,
     io,
     os::unix::ffi::OsStringExt,
@@ -658,46 +658,40 @@ impl Distrobox {
         Ok(s.to_string())
     }
 
-    async fn host_applications_path(&self) -> Result<PathBuf, Error> {
-        // Resolve XDG_DATA_HOME via runner (works in Flatpak via map_flatpak_spawn_host)
-        let xdg_data_home_opt =
-            match crate::fakers::resolve_host_env_via_runner(&self.cmd_runner, "XDG_DATA_HOME")
-                .await
-            {
-                Ok(Some(s)) if !s.trim().is_empty() => Some(Path::new(s.trim()).to_path_buf()),
-                Ok(_) => None,
-                Err(e) => {
-                    tracing::warn!("failed to resolve XDG_DATA_HOME via CommandRunner: {e:?}");
-                    None
-                }
-            };
+    async fn host_applications_path(
+        &self,
+        host_env: &HashMap<String, String>,
+    ) -> Result<PathBuf, Error> {
+        let xdg_data_home_opt = host_env
+            .get("XDG_DATA_HOME")
+            .filter(|s| !s.trim().is_empty())
+            .map(|s| Path::new(s.trim()).to_path_buf());
 
         let apps_base = if let Some(p) = xdg_data_home_opt {
             p
         } else {
             // Fallback to HOME
-            match crate::fakers::resolve_host_env_via_runner(&self.cmd_runner, "HOME").await {
-                Ok(Some(s)) if !s.trim().is_empty() => Path::new(s.trim()).join(".local/share"),
-                Ok(_) => {
+            match host_env.get("HOME").filter(|s| !s.trim().is_empty()) {
+                Some(s) => Path::new(s.trim()).join(".local/share"),
+                None => {
                     return Err(Error::ResolveHostPath(
                         "XDG_DATA_HOME and HOME are not set on the host".into(),
                     ));
                 }
-                Err(e) => {
-                    tracing::warn!("failed to resolve HOME via CommandRunner: {e:?}");
-                    return Err(Error::ResolveHostPath("failed to resolve host HOME".into()));
-                }
             }
         };
 
         let apps_path = apps_base.join("applications");
         Ok(apps_path)
     }
-    async fn get_exported_desktop_files(&self) -> Result<Vec<String>, Error> {
+    async fn get_exported_desktop_files(
+        &self,
+        host_env: &HashMap<String, String>,
+    ) -> Result<Vec<String>, Error> {
         // We do everything with the command line to ensure we can access the files and environment variables
         // even when inside a flatpak sandbox, with only the permissions to run `flatpak-spawn`
         let mut cmd = Command::new("ls");
-        cmd.arg(self.host_applications_path().await?);
+        cmd.arg(self.host_applications_path(host_env).await?);
         let ls_out = self.cmd_output_string(cmd).await?;
         let apps = ls_out
             .trim()
@@ -707,7 +701,11 @@ impl Distrobox {
         Ok(apps)
     }
 
-    async fn get_desktop_files(&self, box_name: &str) -> Result<Vec<(String, String)>, Error> {
+    async fn get_desktop_files(
+        &self,
+        box_name: &str,
+        host_env: &HashMap<String, String>,
+    ) -> Result<Vec<(String, String)>, Error> {
         let mut cmd = self.dbcmd();
         cmd.args([
             "enter",
@@ -721,16 +719,7 @@ impl Distrobox {
             .map_err(|e| Error::ParseOutput(format!("{e:?}")))?;
         debug!(desktop_files = format_args!("{desktop_files:#?}"));
 
-        // Resolve host HOME via CommandRunner so this works inside Flatpak as well
-        let host_home_opt =
-            match crate::fakers::resolve_host_env_via_runner(&self.cmd_runner, "HOME").await {
-                Ok(Some(s)) => Some(PathBuf::from(s)),
-                Ok(None) => None,
-                Err(e) => {
-                    tracing::warn!("failed to resolve host HOME via CommandRunner: {e:?}");
-                    None
-                }
-            };
+        let host_home_opt = host_env.get("HOME").cloned().map(PathBuf::from);
 
         Ok(desktop_files
             .into_map(host_home_opt)
@@ -740,9 +729,17 @@ impl Distrobox {
     }
 
     pub async fn list_apps(&self, box_name: &str) -> Result<Vec<ExportableApp>, Error> {
-        let files = self.get_desktop_files(box_name).await?;
+        let host_env = match crate::fakers::resolve_host_env(&self.cmd_runner).await {
+            Ok(env) => env,
+            Err(e) => {
+                tracing::warn!("failed to resolve host env via CommandRunner: {e:?}");
+                HashMap::new()
+            }
+        };
+
+        let files = self.get_desktop_files(box_name, &host_env).await?;
         debug!(desktop_files=?files);
-        let exported = self.get_exported_desktop_files().await?;
+        let exported = self.get_exported_desktop_files(&host_env).await?;
         debug!(exported_files=?exported);
         let res: Vec<ExportableApp> = files
             .into_iter()

src/fakers/host_env.rs

@@ -1,47 +1,9 @@
 use crate::fakers::{Command, CommandRunner};
+use std::collections::HashMap;
 use std::io;
 
-/// Resolve an environment variable on the host via a `CommandRunner`.
-///
-/// Returns `Ok(Some(value))` if present, `Ok(None)` if unset/empty, or `Err(e)` on I/O/command errors.
-pub async fn resolve_host_env_via_runner(
-    runner: &CommandRunner,
-    key: &str,
-) -> io::Result<Option<String>> {
-    // Try `printenv KEY` first — simpler when available
-    let cmd = Command::new_with_args("printenv", [key]);
-    match runner.output_string(cmd.clone()).await {
-        Ok(out) => {
-            let trimmed = out.trim().to_string();
-            if !trimmed.is_empty() {
-                return Ok(Some(trimmed));
-            }
-            // fallthrough to sh -c printf if empty
-        }
-        Err(_) => {
-            // fallback below
-        }
-    }
-
-    // Fallback: sh -c 'printf "%s" "$KEY"'
-    let mut cmd = Command::new("sh");
-    cmd.arg("-c");
-    // Only interpolate the variable name to avoid injection
-    cmd.arg(format!("printf '%s' \"${}\"", key));
-
-    let out = runner.output_string(cmd).await?;
-    let trimmed = out.trim().to_string();
-    if trimmed.is_empty() {
-        Ok(None)
-    } else {
-        Ok(Some(trimmed))
-    }
-}
-
-/// Resolve all host environment variables via a `CommandRunner` as `KEY=VALUE` entries.
-///
-/// This is useful when spawning processes that need the host `PATH` and other vars, e.g. VTE.
-pub async fn resolve_host_env_list_via_runner(runner: &CommandRunner) -> io::Result<Vec<String>> {
+/// Resolve all host environment variables via a `CommandRunner`.
+pub async fn resolve_host_env(runner: &CommandRunner) -> io::Result<HashMap<String, String>> {
     // Prefer NUL-separated output to preserve values containing newlines.
     let mut cmd = Command::new("env");
     cmd.arg("-0");
@@ -56,9 +18,9 @@ pub async fn resolve_host_env_list_via_runner(runner: &CommandRunner) -> io::Res
                         return None;
                     }
                     let s = String::from_utf8_lossy(entry).to_string();
-                    if s.contains('=') { Some(s) } else { None }
+                    s.split_once('=').map(|(k, v)| (k.to_string(), v.to_string()))
                 })
-                .collect::<Vec<_>>();
+                .collect::<HashMap<_, _>>();
             if !vars.is_empty() {
                 return Ok(vars);
             }
@@ -73,43 +35,55 @@ pub async fn resolve_host_env_list_via_runner(runner: &CommandRunner) -> io::Res
 
     Ok(String::from_utf8_lossy(&output.stdout)
         .lines()
-        .filter(|line| line.contains('='))
-        .map(ToString::to_string)
+        .filter_map(|line| {
+            line.split_once('=')
+                .map(|(k, v)| (k.to_string(), v.to_string()))
+        })
         .collect())
 }
 
+pub fn host_env_to_list(env: &HashMap<String, String>) -> Vec<String> {
+    let mut list = env
+        .iter()
+        .map(|(key, value)| format!("{key}={value}"))
+        .collect::<Vec<_>>();
+    list.sort_unstable();
+    list
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::fakers::NullCommandRunnerBuilder;
     use smol::block_on;
 
     #[test]
-    fn test_resolve_host_env_with_null_runner() {
-        let runner = NullCommandRunnerBuilder::new()
-            .cmd(&["printenv", "HOME"], "/fake/home")
-            .build();
-
-        let res = block_on(resolve_host_env_via_runner(&runner, "HOME")).unwrap();
-        assert_eq!(res, Some("/fake/home".to_string()));
-    }
-
-    #[test]
-    fn test_resolve_host_env_unset_with_null_runner() {
+    fn test_resolve_host_env_empty_with_null_runner() {
         let runner = NullCommandRunnerBuilder::new().build();
-        let res = block_on(resolve_host_env_via_runner(&runner, "HOME")).unwrap();
-        assert_eq!(res, None);
+        let res = block_on(resolve_host_env(&runner)).unwrap();
+        assert!(res.is_empty());
     }
 
     #[test]
-    fn test_resolve_host_env_list_with_null_runner_nul_separated() {
+    fn test_resolve_host_env_with_null_runner_nul_separated() {
         let mut builder = NullCommandRunnerBuilder::new();
         let mut cmd = Command::new("env");
         cmd.arg("-0");
         builder.cmd_full(cmd, || Ok("HOME=/fake/home\0PATH=/nix/store/bin\0".to_string()));
         let runner = builder.build();
 
-        let res = block_on(resolve_host_env_list_via_runner(&runner)).unwrap();
-        assert_eq!(res, vec!["HOME=/fake/home", "PATH=/nix/store/bin"]);
+        let res = block_on(resolve_host_env(&runner)).unwrap();
+        assert_eq!(res.get("HOME"), Some(&"/fake/home".to_string()));
+        assert_eq!(res.get("PATH"), Some(&"/nix/store/bin".to_string()));
+    }
+
+    #[test]
+    fn test_host_env_to_list() {
+        let env = HashMap::from([
+            ("PATH".to_string(), "/nix/store/bin".to_string()),
+            ("HOME".to_string(), "/fake/home".to_string()),
+        ]);
+        let list = host_env_to_list(&env);
+        assert_eq!(list, vec!["HOME=/fake/home", "PATH=/nix/store/bin"]);
     }
 }

src/fakers/mod.rs

@@ -5,5 +5,5 @@ mod output_tracker;
 
 pub use command::{Command, FdMode};
 pub use command_runner::{Child, CommandRunner, CommandRunnerEvent, NullCommandRunnerBuilder};
-pub use host_env::{resolve_host_env_list_via_runner, resolve_host_env_via_runner};
+pub use host_env::{host_env_to_list, resolve_host_env};
 pub use output_tracker::OutputTracker;

src/widgets/integrated_terminal.rs

@@ -6,7 +6,7 @@ use gtk::{
 };
 use vte4::prelude::*;
 
-use crate::fakers::resolve_host_env_list_via_runner;
+use crate::fakers::{host_env_to_list, resolve_host_env};
 use crate::i18n::gettext;
 use crate::gtk_utils::ColorPalette;
 use crate::models::Container;
@@ -176,16 +176,19 @@ impl IntegratedTerminal {
                     .filter_map(|s| s.to_str())
                     .collect::<Vec<_>>();
 
-                let host_env = match resolve_host_env_list_via_runner(&command_runner).await {
+                let host_env = match resolve_host_env(&command_runner).await {
                     Ok(env) => env,
                     Err(err) => {
                         eprintln!("Failed to resolve host env for terminal: {}", err);
-                        Vec::new()
+                        std::collections::HashMap::new()
                     }
                 };
-                let host_env_refs = host_env.iter().map(String::as_str).collect::<Vec<_>>();
+                let host_env_list = host_env_to_list(&host_env);
+                let host_env_refs = host_env_list
+                    .iter()
+                    .map(String::as_str)
+                    .collect::<Vec<_>>();
 
-                dbg!(&host_env_refs);
                 let fut = this.imp().terminal.spawn_future(
                     vte4::PtyFlags::DEFAULT,
                     None,