efi: update the ESP by creating a tmpdir and RENAME_EXCHANGE

See Timothée's comment coreos#454 (comment)
Reuse `TMP_PREFIX`, logic is like this:
- `cp -a fedora .btmp.fedora`
  - We start with a copy to make sure to keep all other files
that we do not explicitly track in bootupd
- Update the content of `.btmp.fedora` with the new binaries
- Exchange `.btmp.fedora` -> `fedora`
- Remove now "old" `.btmp.fedora`

If we have a file not in a directory in `EFI`, then we can copy
it to `.btmp.foo` and then act on it and finally rename it. No
need to copy the entire `EFI`.

And use `insert()` instead of `push()` to match `starts_with()`
when scanning temp files & dirs.
Fixes coreos#454

Cargo.toml

@@ -21,6 +21,7 @@ path = "src/main.rs"
 [dependencies]
 anyhow = "1.0"
 bincode = "1.3.2"
+camino = "1.1.7"
 chrono = { version = "0.4.38", features = ["serde"] }
 clap = { version = "3.2", default-features = false, features = ["cargo", "derive", "std", "suggestions"] }
 env_logger = "0.10"

src/filetree.rs

@@ -7,17 +7,20 @@
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 use anyhow::{bail, Context, Result};
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
+use camino::{Utf8Path, Utf8PathBuf};
+#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 use openat_ext::OpenatDirExt;
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 use openssl::hash::{Hasher, MessageDigest};
+use rustix::fd::BorrowedFd;
 use serde::{Deserialize, Serialize};
 #[allow(unused_imports)]
 use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt::Display;
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 use std::os::unix::io::AsRawFd;
-#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
-use std::path::Path;
+use std::os::unix::process::CommandExt;
+use std::process::Command;
 
 /// The prefix we apply to our temporary files.
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
@@ -231,7 +234,7 @@ impl FileTree {
     }
 }
 
-// Recursively remove all files in the directory that start with our TMP_PREFIX
+// Recursively remove all files/dirs in the directory that start with our TMP_PREFIX
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 fn cleanup_tmp(dir: &openat::Dir) -> Result<()> {
     for entry in dir.list_dir(".")? {
@@ -245,8 +248,13 @@ fn cleanup_tmp(dir: &openat::Dir) -> Result<()> {
 
         match dir.get_file_type(&entry)? {
             openat::SimpleType::Dir => {
-                let child = dir.sub_dir(name)?;
-                cleanup_tmp(&child)?;
+                if name.starts_with(TMP_PREFIX) {
+                    dir.remove_all(name)?;
+                    continue;
+                } else {
+                    let child = dir.sub_dir(name)?;
+                    cleanup_tmp(&child)?;
+                }
             }
             openat::SimpleType::File => {
                 if name.starts_with(TMP_PREFIX) {
@@ -272,7 +280,6 @@ pub(crate) struct ApplyUpdateOptions {
 // Let's just fork off a helper process for now.
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> {
-    use rustix::fd::BorrowedFd;
     use rustix::fs::{Mode, OFlags};
     let d = unsafe { BorrowedFd::borrow_raw(d.as_raw_fd()) };
     let oflags = OFlags::RDONLY | OFlags::CLOEXEC | OFlags::DIRECTORY;
@@ -281,13 +288,43 @@ pub(crate) fn syncfs(d: &openat::Dir) -> Result<()> {
 }
 
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
-fn tmpname_for_path<P: AsRef<Path>>(path: P) -> std::path::PathBuf {
+fn tmpname_for_path<P: AsRef<Utf8Path>>(path: P) -> Utf8PathBuf {
     let path = path.as_ref();
-    let mut buf = path.file_name().expect("filename").to_os_string();
-    buf.push(TMP_PREFIX);
+    let mut buf = path.file_name().expect("filename").to_string();
+    buf.insert_str(0, TMP_PREFIX);
     path.with_file_name(buf)
 }
 
+/// Copy from src to dst at root dir
+#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
+fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> {
+    let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) };
+    let r = unsafe {
+        Command::new("cp")
+            .args(["-a"])
+            .arg(src)
+            .arg(dst)
+            .pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into))
+            .status()?
+    };
+    if !r.success() {
+        anyhow::bail!("Failed to copy {src} to {dst}");
+    }
+    log::debug!("Copy {src} to {dst}");
+    Ok(())
+}
+
+/// Get first sub dir and tmp sub dir for the path
+/// "fedora/foo/bar" -> ("fedora", ".btmp.fedora")
+/// "foo" -> ("foo", ".btmp.foo")
+#[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
+fn get_subdir(path: &Utf8Path) -> Result<(String, String)> {
+    let buf = path.iter().next().unwrap().to_owned();
+    let mut buf_tmp = buf.clone();
+    buf_tmp.insert_str(0, TMP_PREFIX);
+    Ok((buf, buf_tmp))
+}
+
 /// Given two directories, apply a diff generated from srcdir to destdir
 #[cfg(any(target_arch = "x86_64", target_arch = "aarch64"))]
 pub(crate) fn apply_diff(
@@ -302,41 +339,101 @@ pub(crate) fn apply_diff(
     let opts = opts.unwrap_or(&default_opts);
     cleanup_tmp(destdir).context("cleaning up temporary files")?;
 
-    // Write new and changed files
-    for pathstr in diff.additions.iter().chain(diff.changes.iter()) {
-        let path = Path::new(pathstr);
-        if let Some(parent) = path.parent() {
-            destdir.ensure_dir_all(parent, DEFAULT_FILE_MODE)?;
+    let mut updates = HashMap::new();
+    // Handle removals in temp dir
+    if !opts.skip_removals {
+        for pathstr in diff.removals.iter() {
+            let path = Utf8Path::new(pathstr);
+            let (subdir, subdir_tmp) = get_subdir(path)?;
+            let path_tmp;
+            if subdir != *pathstr {
+                // need to copy to temp subdir and remember
+                if !destdir.exists(&subdir_tmp)? {
+                    copy_dir(destdir, &subdir, &subdir_tmp)?;
+                    updates.insert(subdir.clone(), subdir_tmp.clone());
+                }
+                path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
+            } else {
+                // remove the file directly that is not in dir
+                path_tmp = path.to_path_buf();
+            }
+            destdir
+                .remove_file(path_tmp.as_std_path())
+                .with_context(|| format!("removing {:?}", path_tmp))?;
         }
-        let destp = tmpname_for_path(path);
-        srcdir
-            .copy_file_at(path, destdir, destp.as_path())
-            .with_context(|| format!("writing {}", &pathstr))?;
     }
-    // Ensure all of the new files are written persistently to disk
-    if !opts.skip_sync {
-        syncfs(destdir)?;
+    // Write changed files to temp dir and exchange finally
+    for pathstr in diff.changes.iter() {
+        let path = Utf8Path::new(pathstr);
+        let (subdir, subdir_tmp) = get_subdir(path)?;
+        let path_tmp;
+        if subdir != *pathstr {
+            if !destdir.exists(&subdir_tmp)? {
+                // need to copy to temp subdir and remember
+                copy_dir(destdir, &subdir, &subdir_tmp)?;
+                updates.insert(subdir.clone(), subdir_tmp.clone());
+            }
+            path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
+            destdir
+                .remove_file_optional(path_tmp.as_std_path())
+                .with_context(|| format!("removing {path_tmp}"))?;
+        } else {
+            // file that is not in dir, copy file to foo.tmp
+            updates.insert(subdir.clone(), subdir_tmp.clone());
+            path_tmp = Utf8PathBuf::from(&subdir_tmp);
+        }
+        srcdir
+            .copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
+            .with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
     }
-    // Now move them all into place (TODO track interruption)
-    for path in diff.additions.iter().chain(diff.changes.iter()) {
-        let pathtmp = tmpname_for_path(path);
-        destdir
-            .local_rename(&pathtmp, path)
-            .with_context(|| format!("renaming {path}"))?;
+    // Write new files to temp dir if exists, else write to tmp
+    for pathstr in diff.additions.iter() {
+        let path = Utf8Path::new(pathstr);
+        let (subdir, subdir_tmp) = get_subdir(path)?;
+        let path_tmp;
+        if destdir.exists(&subdir_tmp)? {
+            path_tmp = Utf8Path::new(&subdir_tmp).join(path.strip_prefix(&subdir)?);
+        } else {
+            path_tmp = tmpname_for_path(path);
+            updates.insert(pathstr.to_owned(), path_tmp.to_string());
+        }
+        // ensure new additions dir exists
+        if let Some(parent) = path_tmp.parent().filter(|&v| !v.as_os_str().is_empty()) {
+            destdir.ensure_dir_all(parent.as_std_path(), DEFAULT_FILE_MODE)?;
+        }
+        srcdir
+            .copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path())
+            .with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?;
     }
-    if !opts.skip_removals {
-        for path in diff.removals.iter() {
+
+    // do local exchange
+    for (src, tmp) in updates.iter() {
+        log::trace!("doing local exchange/rename for {} and {}", tmp, src);
+        if destdir.exists(src)? {
+            destdir
+                .local_exchange(tmp, src)
+                .with_context(|| format!("exchange for {} and {}", tmp, src))?;
+        } else {
             destdir
-                .remove_file_optional(path)
-                .with_context(|| format!("removing {path}"))?;
+                .local_rename(tmp, src)
+                .with_context(|| format!("rename for {} and {}", tmp, src))?;
         }
     }
+    // Ensure all of the updates & changes are written persistently to disk
+    if !opts.skip_sync {
+        syncfs(destdir)?;
+    }
+
+    // finally remove the temp dir
+    for (_, tmp) in updates.iter() {
+        log::trace!("cleanup: {}", tmp);
+        destdir.remove_all(tmp).context("clean up temp")?;
+    }
     // A second full filesystem sync to narrow any races rather than
     // waiting for writeback to kick in.
     if !opts.skip_sync {
         syncfs(destdir)?;
     }
-
     Ok(())
 }
 
@@ -345,6 +442,7 @@ mod tests {
     use super::*;
     use std::fs;
     use std::io::Write;
+    use std::path::Path;
 
     fn run_diff(a: &openat::Dir, b: &openat::Dir) -> Result<FileTreeDiff> {
         let ta = FileTree::new_from_dir(a)?;
@@ -508,4 +606,127 @@ mod tests {
         assert!(!a.join(relp).join("shim.x64").exists());
         Ok(())
     }
+    #[test]
+    fn test_get_subdir() -> Result<()> {
+        // test path
+        let path = Utf8Path::new("foo/subdir/bar");
+        let (tp, tp_tmp) = get_subdir(path)?;
+        assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
+        // test file
+        let path = Utf8Path::new("foo");
+        let (tp, tp_tmp) = get_subdir(path)?;
+        assert_eq!(tp.eq("foo"), tp_tmp.eq(".btmp.foo"));
+        Ok(())
+    }
+    #[test]
+    fn test_cleanup_tmp() -> Result<()> {
+        let tmpd = tempfile::tempdir()?;
+        let p = tmpd.path();
+        let pa = p.join("a/.btmp.a");
+        let pb = p.join(".btmp.b/b");
+        std::fs::create_dir_all(&pa)?;
+        std::fs::create_dir_all(&pb)?;
+        let dp = openat::Dir::open(p)?;
+        {
+            let mut buf = dp.write_file("a/foo", 0o644)?;
+            buf.write_all("foocontents".as_bytes())?;
+            let mut buf = dp.write_file("a/.btmp.foo", 0o644)?;
+            buf.write_all("foocontents".as_bytes())?;
+            let mut buf = dp.write_file(".btmp.b/foo", 0o644)?;
+            buf.write_all("foocontents".as_bytes())?;
+        }
+        assert!(dp.exists("a/.btmp.a")?);
+        assert!(dp.exists("a/foo")?);
+        assert!(dp.exists("a/.btmp.foo")?);
+        assert!(dp.exists("a/.btmp.a")?);
+        assert!(dp.exists(".btmp.b/b")?);
+        assert!(dp.exists(".btmp.b/foo")?);
+        cleanup_tmp(&dp)?;
+        assert!(!dp.exists("a/.btmp.a")?);
+        assert!(dp.exists("a/foo")?);
+        assert!(!dp.exists("a/.btmp.foo")?);
+        assert!(!dp.exists(".btmp.b")?);
+        Ok(())
+    }
+    #[test]
+    fn test_apply_with_file() -> Result<()> {
+        let tmpd = tempfile::tempdir()?;
+        let p = tmpd.path();
+        let pa = p.join("a");
+        let pb = p.join("b");
+        std::fs::create_dir(&pa)?;
+        std::fs::create_dir(&pb)?;
+        let a = openat::Dir::open(&pa)?;
+        let b = openat::Dir::open(&pb)?;
+        a.create_dir("foo", 0o755)?;
+        a.create_dir("bar", 0o755)?;
+        let foo = Path::new("foo/bar");
+        let bar = Path::new("bar/foo");
+        let testfile = "testfile";
+        {
+            let mut buf = a.write_file(foo, 0o644)?;
+            buf.write_all("foocontents".as_bytes())?;
+            let mut buf = a.write_file(bar, 0o644)?;
+            buf.write_all("barcontents".as_bytes())?;
+            let mut buf = a.write_file(testfile, 0o644)?;
+            buf.write_all("testfilecontents".as_bytes())?;
+        }
+
+        let diff = run_diff(&a, &b)?;
+        assert_eq!(diff.count(), 3);
+        b.create_dir("foo", 0o755)?;
+        {
+            let mut buf = b.write_file(foo, 0o644)?;
+            buf.write_all("foocontents".as_bytes())?;
+        }
+        let b_btime_foo = fs::metadata(pb.join(foo))?.created()?;
+
+        {
+            let diff = run_diff(&b, &a)?;
+            assert_eq!(diff.count(), 2);
+            apply_diff(&a, &b, &diff, None).context("test additional files")?;
+            assert_eq!(
+                String::from_utf8(std::fs::read(pb.join(testfile))?)?,
+                "testfilecontents"
+            );
+            assert_eq!(
+                String::from_utf8(std::fs::read(pb.join(bar))?)?,
+                "barcontents"
+            );
+            // 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);
+        }
+        {
+            fs::write(pa.join(testfile), "newtestfile")?;
+            fs::write(pa.join(bar), "newbar")?;
+            let diff = run_diff(&b, &a)?;
+            assert_eq!(diff.count(), 2);
+            apply_diff(&a, &b, &diff, None).context("test changed files")?;
+            assert_eq!(
+                String::from_utf8(std::fs::read(pb.join(testfile))?)?,
+                "newtestfile"
+            );
+            assert_eq!(String::from_utf8(std::fs::read(pb.join(bar))?)?, "newbar");
+            // 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(testfile)?;
+            a.remove_file(bar)?;
+            let diff = run_diff(&b, &a)?;
+            assert_eq!(diff.count(), 2);
+            apply_diff(&a, &b, &diff, None).context("test removed files")?;
+            assert_eq!(b.exists(testfile)?, false);
+            assert_eq!(b.exists(bar)?, false);
+            let diff = run_diff(&b, &a)?;
+            assert_eq!(diff.count(), 0);
+            // 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);
+        }
+
+        Ok(())
+    }
 }