efi: Extend install() & update() to support usr/lib/efi

src/efi.rs

@@ -55,6 +55,9 @@ const EFIVARFS: &str = "/sys/firmware/efi/efivars";
 const LOADER_INFO_VAR_STR: &str = "LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f";
 const STUB_INFO_VAR_STR: &str = "StubInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f";
 
+/// The options of cp command for installation
+const OPTIONS: &[&str] = &["-rp", "--reflink=auto"];
+
 /// Return `true` if the system is booted via EFI
 pub(crate) fn is_efi_booted() -> Result<bool> {
     Path::new("/sys/firmware/efi")
@@ -319,10 +322,19 @@ impl Component for Efi {
             return Ok(None);
         };
 
+        let updated_path = {
+            let efilib_path = rootcxt.path.join(EFILIB);
+            if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some()
+            {
+                PathBuf::from(EFILIB)
+            } else {
+                component_updatedirname(self)
+            }
+        };
         let updated = rootcxt
             .sysroot
-            .sub_dir(&component_updatedirname(self))
-            .context("opening update dir")?;
+            .sub_dir(&updated_path)
+            .with_context(|| format!("opening update dir {}", updated_path.display()))?;
         let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?;
 
         let esp_devices = esp_devices.unwrap_or_default();
@@ -376,8 +388,6 @@ impl Component for Efi {
             anyhow::bail!("No update metadata for component {} found", self.name());
         };
         log::debug!("Found metadata {}", meta.version);
-        let srcdir_name = component_updatedirname(self);
-        let ft = crate::filetree::FileTree::new_from_dir(&src_dir.sub_dir(&srcdir_name)?)?;
 
         // Let's attempt to use an already mounted ESP at the target
         // dest_root if one is already mounted there in a known ESP location.
@@ -398,16 +408,37 @@ impl Component for Efi {
             .with_context(|| format!("opening dest dir {}", destpath.display()))?;
         validate_esp_fstype(destd)?;
 
-        // TODO - add some sort of API that allows directly setting the working
-        // directory to a file descriptor.
-        std::process::Command::new("cp")
-            .args(["-rp", "--reflink=auto"])
-            .arg(&srcdir_name)
-            .arg(destpath)
-            .current_dir(format!("/proc/self/fd/{}", src_dir.as_raw_fd()))
-            .run()?;
+        let src_path = Utf8Path::new(src_root);
+        let efi_comps = if src_path.join(EFILIB).exists() {
+            get_efi_component_from_usr(&src_path, EFILIB)?
+        } else {
+            None
+        };
+        let dest = destpath.to_str().with_context(|| {
+            format!(
+                "Include invalid UTF-8 characters in dest {}",
+                destpath.display()
+            )
+        })?;
+
+        let efi_path = if let Some(efi_components) = efi_comps {
+            for efi in efi_components {
+                filetree::copy_dir_with_args(&src_dir, efi.path.as_str(), dest, OPTIONS)?;
+            }
+            EFILIB
+        } else {
+            let updates = component_updatedirname(self);
+            let src = updates
+                .to_str()
+                .context("Include invalid UTF-8 characters in path")?;
+            filetree::copy_dir_with_args(&src_dir, src, dest, OPTIONS)?;
+            &src.to_owned()
+        };
+
+        // Get filetree from efi path
+        let ft = crate::filetree::FileTree::new_from_dir(&src_dir.sub_dir(efi_path)?)?;
         if update_firmware {
-            if let Some(vendordir) = self.get_efi_vendor(&Path::new(src_root))? {
+            if let Some(vendordir) = self.get_efi_vendor(src_path.join(efi_path).as_std_path())? {
                 self.update_firmware(device, destd, &vendordir)?
             }
         }
@@ -429,9 +460,20 @@ impl Component for Efi {
             .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?;
         let sysroot_dir = &rootcxt.sysroot;
         let updatemeta = self.query_update(sysroot_dir)?.expect("update available");
-        let updated = sysroot_dir
-            .sub_dir(&component_updatedirname(self))
-            .context("opening update dir")?;
+        let updated_path = {
+            let efilib_path = rootcxt.path.join(EFILIB);
+            if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some()
+            {
+                PathBuf::from(EFILIB)
+            } else {
+                component_updatedirname(self)
+            }
+        };
+
+        let updated = rootcxt
+            .sysroot
+            .sub_dir(&updated_path)
+            .with_context(|| format!("opening update dir {}", updated_path.display()))?;
         let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?;
         let diff = currentf.diff(&updatef)?;
 
@@ -480,7 +522,6 @@ impl Component for Efi {
             ostreeboot.remove_all_optional("efi/EFI")?;
         }
 
-        // copy EFI files from usr/lib/efi to updates dir
         if let Some(efi_components) =
             get_efi_component_from_usr(Utf8Path::from_path(sysroot_path).unwrap(), EFILIB)?
         {
@@ -491,12 +532,6 @@ impl Component for Efi {
             let mut packages = Vec::new();
             let mut modules_vec: Vec<Module> = vec![];
             for efi in efi_components {
-                Command::new("cp")
-                    .args(["-rp", "--reflink=auto"])
-                    .arg(&efi.path)
-                    .arg(crate::model::BOOTUPD_UPDATES_DIR)
-                    .current_dir(format!("/proc/self/fd/{}", sysroot_dir.as_raw_fd()))
-                    .run()?;
                 packages.push(format!("{}-{}", efi.name, efi.version));
                 modules_vec.push(Module {
                     name: efi.name,

src/filetree.rs

@@ -68,16 +68,25 @@ const DEFAULT_FILE_MODE: u32 = 0o700;
 use crate::sha512string::SHA512String;
 
 /// Metadata for a single file
-#[derive(Clone, Serialize, Deserialize, Debug, Hash, PartialEq)]
+#[derive(Clone, Serialize, Deserialize, Debug)]
 #[serde(rename_all = "kebab-case")]
 pub(crate) struct FileMetadata {
+    /// File source path
+    pub(crate) source: Option<String>,
     /// File size in bytes
     pub(crate) size: u64,
     /// Content checksum; chose SHA-512 because there are not a lot of files here
     /// and it's ok if the checksum is large.
     pub(crate) sha512: SHA512String,
 }
 
+impl PartialEq for FileMetadata {
+    fn eq(&self, other: &Self) -> bool {
+        // Skip the source
+        self.size == other.size && self.sha512 == other.sha512
+    }
+}
+
 #[derive(Clone, Serialize, Deserialize, Debug, PartialEq)]
 #[serde(rename_all = "kebab-case")]
 pub(crate) struct FileTree {
@@ -127,6 +136,7 @@ impl FileMetadata {
         let _ = std::io::copy(&mut r, &mut hasher)?;
         let digest = SHA512String::from_hasher(&mut hasher);
         Ok(FileMetadata {
+            source: None,
             size: meta.len(),
             sha512: digest,
         })
@@ -183,8 +193,10 @@ impl FileTree {
     ))]
     pub(crate) fn new_from_dir(dir: &openat::Dir) -> Result<Self> {
         let mut children = BTreeMap::new();
-        for (k, v) in Self::unsorted_from_dir(dir)?.drain() {
-            children.insert(k, v);
+        for (k, mut v) in Self::unsorted_from_dir(dir)?.drain() {
+            let k_path = get_dest_efi_path(Utf8Path::new(&k)).to_string();
+            v.source = Some(k);
+            children.insert(k_path, v);
         }
 
         Ok(Self { children })
@@ -228,18 +240,20 @@ impl FileTree {
         for (k, v1) in self.children.iter() {
             if let Some(v2) = updated.children.get(k) {
                 if v1 != v2 {
-                    changes.insert(k.clone());
+                    // Save the source path for changes
+                    changes.insert(v2.source.as_ref().unwrap_or(k).clone());
                 }
             } else {
                 removals.insert(k.clone());
             }
         }
         if check_additions {
-            for k in updated.children.keys() {
+            for (k, v) in updated.children.iter() {
                 if self.children.contains_key(k) {
                     continue;
                 }
-                additions.insert(k.clone());
+                // Save the source path for additions
+                additions.insert(v.source.as_ref().unwrap_or(k).clone());
             }
         }
         Ok(FileTreeDiff {
@@ -268,12 +282,13 @@ impl FileTree {
                     openat::SimpleType::File => {
                         let target_info = FileMetadata::new_from_path(dir, path)?;
                         if info != &target_info {
-                            changes.insert(path.clone());
+                            // Save the source path for changes
+                            changes.insert(info.source.as_ref().unwrap_or(path).clone());
                         }
                     }
                     _ => {
                         // If a file became a directory
-                        changes.insert(path.clone());
+                        changes.insert(info.source.as_ref().unwrap_or(path).clone());
                     }
                 }
             } else {
@@ -334,21 +349,40 @@ pub(crate) struct ApplyUpdateOptions {
     pub(crate) skip_sync: bool,
 }
 
-/// Copy from src to dst at root dir
+/// Copy from src to dst at root dir with default option '-a'
+#[cfg(any(
+    target_arch = "x86_64",
+    target_arch = "aarch64",
+    target_arch = "riscv64"
+))]
+pub(crate) fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
+    copy_dir_with_args(root, src, dst, ["-a"])
+}
+
+/// Copy from src to dst at root dir with args
 #[cfg(any(
     target_arch = "x86_64",
     target_arch = "aarch64",
     target_arch = "riscv64"
 ))]
-fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
+pub(crate) fn copy_dir_with_args<I, S>(
+    root: &openat::Dir,
+    src: &str,
+    dst: &str,
+    args: I,
+) -> Result<()>
+where
+    I: IntoIterator<Item = S>,
+    S: AsRef<std::ffi::OsStr>,
+{
     use bootc_internal_utils::CommandRunExt;
     use std::os::unix::process::CommandExt;
     use std::process::Command;
 
     let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
     unsafe {
         Command::new("cp")
-            .args(["-a"])
+            .args(args)
             .arg(src)
             .arg(dst)
             .pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
@@ -366,7 +400,7 @@ fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
     target_arch = "aarch64",
     target_arch = "riscv64"
 ))]
-fn get_first_dir(path: &Utf8Path) -> Result<(&Utf8Path, String)> {
+fn get_first_dir(path: &Utf8Path) -> Result<(Utf8PathBuf, String)> {
     let first = path
         .iter()
         .next()
@@ -376,6 +410,20 @@ fn get_first_dir(path: &Utf8Path) -> Result<(&Utf8Path, String)> {
     Ok((first.into(), tmp))
 }
 
+/// Get dest efi path "shim/<ver>/EFI/fedora/shim.efi" -> "fedora/shim.efi"
+#[cfg(any(
+    target_arch = "x86_64",
+    target_arch = "aarch64",
+    target_arch = "riscv64"
+))]
+fn get_dest_efi_path(path: &Utf8Path) -> Utf8PathBuf {
+    let parts: Vec<_> = path.iter().collect();
+    if parts.get(2).map(|c| *c == "EFI").unwrap_or(false) {
+        return parts.iter().skip(3).collect();
+    }
+    path.to_path_buf()
+}
+
 /// Given two directories, apply a diff generated from srcdir to destdir
 #[cfg(any(
     target_arch = "x86_64",
@@ -421,8 +469,9 @@ pub(crate) fn apply_diff(
     }
     // Write changed or new files to temp dir or temp file
     for pathstr in diff.changes.iter().chain(diff.additions.iter()) {
-        let path = Utf8Path::new(pathstr);
-        let (first_dir, first_dir_tmp) = get_first_dir(path)?;
+        let src_path = Utf8Path::new(pathstr);
+        let path = get_dest_efi_path(src_path);
+        let (first_dir, first_dir_tmp) = get_first_dir(&path)?;
         let mut path_tmp = Utf8PathBuf::from(&first_dir_tmp);
         if first_dir != path {
             if !destdir.exists(&first_dir_tmp)? && destdir.exists(first_dir.as_std_path())? {
@@ -443,8 +492,8 @@ pub(crate) fn apply_diff(
         }
         updates.insert(first_dir, first_dir_tmp);
         srcdir
-            .copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
-            .with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
+            .copy_file_at(src_path.as_std_path(), destdir, path_tmp.as_std_path())
+            .with_context(|| format!("copying {:?} to {:?}", src_path, path_tmp))?;
     }
 
     // do local exchange or rename
@@ -647,6 +696,37 @@ mod tests {
             "newgrub data"
         );
         assert!(!a.join(relp).join("shim.x64").exists());
+        // test apply from foo/1.0 and bar/2.0
+        {
+            let c = tmpdp.join("c");
+            let foo = "foo/1.0/EFI/fedora";
+            let bar = "bar/2.0/EFI/new";
+            for p in [foo, bar] {
+                fs::create_dir_all(c.join(p))?;
+            }
+            // change: "foo/1.0/EFI/fedora/grub.x64"
+            fs::write(c.join(foo).join("grub.x64"), "grub data 3")?;
+            // addition: "bar/2.0/EFI/new/newfile"
+            fs::write(c.join(bar).join("newfile"), "filedata")?;
+            let a = openat::Dir::open(&a.join("EFI"))?;
+            let c = openat::Dir::open(&c)?;
+            let ta = FileTree::new_from_dir(&a)?;
+            let tc = FileTree::new_from_dir(&c)?;
+            let diff = ta.diff(&tc)?;
+            assert_eq!(diff.changes.len(), 1);
+            assert_eq!(diff.additions.len(), 1);
+            assert_eq!(diff.count(), 3);
+            super::apply_diff(&c, &a, &diff, None)?;
+        }
+        assert_eq!(
+            String::from_utf8(std::fs::read(a.join(relp).join("grub.x64"))?)?,
+            "grub data 3"
+        );
+        assert_eq!(
+            String::from_utf8(std::fs::read(a.join("EFI/new").join("newfile"))?)?,
+            "filedata"
+        );
+        assert!(!a.join(newsubp).join("newgrub.x64").exists());
         Ok(())
     }
     #[test]
@@ -664,6 +744,24 @@ mod tests {
         Ok(())
     }
     #[test]
+    fn test_get_dest_efi_path() -> Result<()> {
+        let test_cases = [
+            ("foo/1.0/EFI/vendor/test.efi", "vendor/test.efi"),
+            ("vendor/test.efi", "vendor/test.efi"),
+            ("EFI/vendor/test.efi", "EFI/vendor/test.efi"),
+            (
+                "bar/foo/1.0/EFI/vendor/test.efi",
+                "bar/foo/1.0/EFI/vendor/test.efi",
+            ),
+        ];
+
+        for (input, expected) in test_cases {
+            let path = Utf8Path::new(input);
+            assert_eq!(get_dest_efi_path(path), Utf8Path::new(expected));
+        }
+        Ok(())
+    }
+    #[test]
     fn test_cleanup_tmp() -> Result<()> {
         let tmpd = tempfile::tempdir()?;
         let p = tmpd.path();
@@ -763,9 +861,13 @@ mod tests {
             b.remove_file(testfile)?;
             let ta = FileTree::new_from_dir(&a)?;
             let diff = ta.relative_diff_to(&b)?;
+            assert_eq!(diff.count(), 1);
             assert_eq!(diff.removals.len(), 1);
             apply_diff(&a, &b, &diff, None).context("test removed files with relative_diff")?;
             assert_eq!(b.exists(testfile)?, false);
+            // creation time is not changed for unchanged file
+            let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?;
+            assert_eq!(b_btime_foo_new, b_btime_foo);
         }
         {
             a.remove_file(bar)?;