address review comments

Author: jbtrystram

Cargo.lock

@@ -128,22 +128,17 @@ pub(crate) fn install_via_bootupd(
         let mut bwrap_args = vec!["bootupctl"];
         bwrap_args.extend(bootupd_args);
 
-        let mut cmd = BwrapCmd::new(target_root.as_str())
+        let mut cmd = BwrapCmd::new(&target_root)
             // Bind mount /boot from the physical target root so bootupctl can find
             // the boot partition and install the bootloader there
-            .bind(boot_path.as_str(), "/boot")
+            .bind(&boot_path, &"/boot")
             // Bind the target block device inside the bwrap container so bootupctl can access it
             .bind_device(device.path().as_str());
 
         // Also bind all partitions of the tafet block device
         for partition in &device.partitions {
             cmd = cmd.bind_device(&partition.node);
         }
-        // // TODO : is it needed ?
-        // cmd.setenv(
-        //     "PATH",
-        //     "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin",
-        // )
         cmd.run(bwrap_args)
     } else {
         // Running directly without chroot

crates/lib/src/bootloader.rs

@@ -10,6 +10,7 @@ repository = "https://github.com/bootc-dev/bootc"
 # Workspace dependencies
 anstream = { workspace = true }
 anyhow = { workspace = true }
+cap-std-ext = {workspace = true }
 chrono = { workspace = true, features = ["std"] }
 owo-colors = { workspace = true }
 rustix = { workspace = true }

crates/utils/Cargo.toml

@@ -1,16 +1,21 @@
 /// Builder for running commands inside a target os tree using bubblewrap (bwrap).
+use std::borrow::Cow;
+use std::env;
 use std::ffi::OsStr;
+use std::os::fd::AsRawFd;
 use std::process::Command;
 
 use anyhow::Result;
+use cap_std_ext::camino::{Utf8Path, Utf8PathBuf};
+use cap_std_ext::cap_std::fs::Dir;
 
 use crate::CommandRunExt;
 
 /// Builder for running commands inside a target directory using bwrap.
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub struct BwrapCmd<'a> {
     /// The target directory to use as root for the container
-    chroot_path: &'a str,
+    chroot_path: Cow<'a, Utf8Path>,
     /// Bind mounts in format (source, target)
     bind_mounts: Vec<(&'a str, &'a str)>,
     /// Device nodes to bind into the container
@@ -20,17 +25,36 @@ pub struct BwrapCmd<'a> {
 }
 
 impl<'a> BwrapCmd<'a> {
-    /// Create a new BwrapCmd builder with the given root directory.
-    pub fn new(path: &'a str) -> Self {
+    /// Create a new BwrapCmd builder with a root directory as a File Descriptor.
+    #[allow(dead_code)]
+    pub fn new_with_dir(path: &'a Dir) -> Self {
+        let fd_path: String = format!("/proc/self/fd/{}", path.as_raw_fd());
         Self {
-            chroot_path: path,
-            ..Default::default()
+            chroot_path: Cow::Owned(Utf8PathBuf::from(&fd_path)),
+            bind_mounts: Vec::new(),
+            devices: Vec::new(),
+            env_vars: Vec::new(),
+        }
+    }
+
+    /// Create a new BwrapCmd builder with a root directory
+    pub fn new(path: &'a Utf8Path) -> Self {
+        Self {
+            chroot_path: Cow::Borrowed(path),
+            bind_mounts: Vec::new(),
+            devices: Vec::new(),
+            env_vars: Vec::new(),
         }
     }
 
     /// Add a bind mount from source to target inside the container.
-    pub fn bind(mut self, source: &'a str, target: &'a str) -> Self {
-        self.bind_mounts.push((source, target));
+    pub fn bind(
+        mut self,
+        source: &'a impl AsRef<Utf8Path>,
+        target: &'a impl AsRef<Utf8Path>,
+    ) -> Self {
+        self.bind_mounts
+            .push((source.as_ref().as_str(), target.as_ref().as_str()));
         self
     }
 
@@ -51,14 +75,22 @@ impl<'a> BwrapCmd<'a> {
         let mut cmd = Command::new("bwrap");
 
         // Bind the root filesystem
-        cmd.args(["--bind", self.chroot_path, "/"]);
+        cmd.args(["--bind", self.chroot_path.as_str(), "/"]);
 
         // Setup API filesystems
         // See https://systemd.io/API_FILE_SYSTEMS/
         cmd.args(["--proc", "/proc"]);
         cmd.args(["--dev", "/dev"]);
         cmd.args(["--ro-bind", "/sys", "/sys"]);
 
+        // the $PATH available in the bwrap is a default one so let's forward the
+        // existing $PATH of inject a reasonnable default.
+        let inject_path = env::var("PATH").unwrap_or({
+            String::from("bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin")
+        });
+
+        cmd.args(["--setenv", "PATH", inject_path.as_str()]);
+
         // Add bind mounts
         for (source, target) in &self.bind_mounts {
             cmd.args(["--bind", source, target]);