xtask: Fail packaging if the vendor archive has PCRE2 C sources

crates/lib/src/install.rs

@@ -2448,19 +2448,27 @@ fn remove_all_in_dir_no_xdev(d: &Dir, mount_err: bool) -> Result<()> {
         let name = entry.file_name();
         let etype = entry.file_type()?;
         if etype == FileType::dir() {
-            if let Some(subdir) = d.open_dir_noxdev(&name)? {
-                remove_all_in_dir_no_xdev(&subdir, mount_err)?;
-                d.remove_dir(&name)?;
-            } else if mount_err {
-                anyhow::bail!("Found unexpected mount point {name:?}");
-            }
+            remove_dir_no_xdev(d, &name, mount_err)?;
         } else {
             d.remove_file_optional(&name)?;
         }
     }
     anyhow::Ok(())
 }
 
+/// Recursively remove the directory `name` in `d`, without crossing devices.
+/// A mount point is left alone, or is an error if `mount_err` is true.
+fn remove_dir_no_xdev(d: &Dir, name: impl AsRef<Path>, mount_err: bool) -> Result<()> {
+    let name = name.as_ref();
+    if let Some(subdir) = d.open_dir_noxdev(name)? {
+        remove_all_in_dir_no_xdev(&subdir, mount_err)?;
+        d.remove_dir(name)?;
+    } else if mount_err {
+        anyhow::bail!("Found unexpected mount point {name:?}");
+    }
+    Ok(())
+}
+
 #[context("Removing boot directory content except loader dir on ostree")]
 fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
     let entries = bootdir
@@ -2485,12 +2493,8 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
 
         let etype = entry.file_type()?;
         if etype == FileType::dir() {
-            // Open the directory and remove its contents
-            if let Some(subdir) = bootdir.open_dir_noxdev(&file_name)? {
-                remove_all_in_dir_no_xdev(&subdir, false)
-                    .with_context(|| format!("Removing directory contents: {}", file_name))?;
-                bootdir.remove_dir(&file_name)?;
-            }
+            remove_dir_no_xdev(bootdir, file_name, false)
+                .with_context(|| format!("Removing directory: {file_name}"))?;
         } else {
             bootdir
                 .remove_file_optional(&file_name)
@@ -2500,6 +2504,22 @@ fn remove_all_except_loader_dirs(bootdir: &Dir, is_ostree: bool) -> Result<()> {
     Ok(())
 }
 
+/// Remove the bootloader dirs (bootupd/grub, systemd-boot) from the ESP.
+/// Other content, e.g. Asahi's `m1n1/` and `vendorfw/`, may be firmware
+/// or earlier boot stages we cannot recreate, so it is preserved.
+// TODO: be more selective, e.g. keep other OSes' `EFI/<vendor>` and
+// non-bootc `loader/` entries, and drop old Type #1 kernels (see #2243).
+#[context("Removing bootloader content from EFI system partition")]
+fn clean_esp_bootloader_dirs(efidir: &Dir) -> Result<()> {
+    for name in ["EFI", "loader"] {
+        if efidir.try_exists(name)? {
+            remove_dir_no_xdev(efidir, name, false)
+                .with_context(|| format!("Removing directory: {name}"))?;
+        }
+    }
+    Ok(())
+}
+
 #[context("Removing boot directory content")]
 fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool) -> Result<()> {
     let bootdir =
@@ -2514,13 +2534,12 @@ fn clean_boot_directories(rootfs: &Dir, rootfs_path: &Utf8Path, is_ostree: bool)
     // This should not remove /boot/efi note.
     remove_all_except_loader_dirs(&bootdir, is_ostree).context("Emptying /boot")?;
 
-    // TODO: we should also support not wiping the ESP.
     if ARCH_USES_EFI {
         if let Some(efidir) = bootdir
             .open_dir_optional(crate::bootloader::EFI_DIR)
             .context("Opening /boot/efi")?
         {
-            remove_all_in_dir_no_xdev(&efidir, false).context("Emptying EFI system partition")?;
+            clean_esp_bootloader_dirs(&efidir)?;
         }
     }
 
@@ -3249,6 +3268,32 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn test_clean_esp_bootloader_dirs() -> Result<()> {
+        let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
+
+        td.create_dir_all("EFI/BOOT")?;
+        td.write("EFI/BOOT/BOOTAA64.EFI", b"shim")?;
+        td.create_dir_all("loader/entries")?;
+        td.write("loader/entries/foo.conf", b"entry")?;
+        // Asahi content which must survive
+        td.create_dir_all("m1n1")?;
+        td.write("m1n1/boot.bin", b"m1n1")?;
+        td.write("ubootefi.var", b"efivars")?;
+
+        clean_esp_bootloader_dirs(&td)?;
+        assert!(!td.exists("EFI"));
+        assert!(!td.exists("loader"));
+        assert_eq!(td.read("m1n1/boot.bin")?, b"m1n1");
+        assert_eq!(td.read("ubootefi.var")?, b"efivars");
+
+        // Idempotent
+        clean_esp_bootloader_dirs(&td)?;
+        assert_eq!(td.entries()?.count(), 2);
+
+        Ok(())
+    }
+
     #[test]
     fn test_read_boot_fstab_entry() -> Result<()> {
         let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;

crates/xtask/src/xtask.rs

@@ -8,7 +8,7 @@ use std::borrow::Cow;
 use std::fmt::Display;
 use std::fs::File;
 use std::io::{BufRead, BufReader, BufWriter, Write};
-use std::process::Command;
+use std::process::{Command, Stdio};
 
 use anyhow::{Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
@@ -32,6 +32,15 @@ const JSON_SCHEMAS: &[(&str, &str)] = &[
 ];
 /// File used to identify the bootc source tree toplevel.
 const TOPLEVEL_MARKER: &str = "ADOPTERS.md";
+/// Directory prefix of the vendored crates in the vendor archive.
+const VENDOR_DIR: &str = "vendor";
+/// Files that must not appear in the vendor archive, as (crate, file extensions).
+/// These crates link against system libraries, so their bundled C sources are
+/// excluded via `exclude-crate-paths` in the toplevel Cargo.toml; this catches
+/// a crate update that moves them somewhere the exclusion no longer covers.
+const VENDOR_DENYLIST: &[(&str, &[&str])] = &[("pcre2-sys", &["c", "h"])];
+/// Maximum number of denied files to list in the error message.
+const VENDOR_DENIED_SHOWN: usize = 10;
 const TAR_REPRODUCIBLE_OPTS: &[&str] = &[
     "--sort=name",
     "--owner=0",
@@ -500,6 +509,72 @@ fn edit_vendor_config(config: &str) -> Result<String> {
     Ok(config.to_string())
 }
 
+/// If `path` (relative to the vendor directory) matches [`VENDOR_DENYLIST`],
+/// return the name of the crate it belongs to.
+fn vendor_denied_crate(path: &Utf8Path) -> Option<&'static str> {
+    let crate_dir = path.components().next()?.as_str();
+    let ext = path.extension()?;
+    VENDOR_DENYLIST.iter().find_map(|&(name, exts)| {
+        // cargo vendor uses `<name>-<version>` when several versions are vendored
+        let is_crate = crate_dir == name
+            || crate_dir
+                .strip_prefix(name)
+                .and_then(|v| v.strip_prefix('-'))
+                .is_some_and(|v| v.starts_with(|c: char| c.is_ascii_digit()));
+        (is_crate && exts.contains(&ext)).then_some(name)
+    })
+}
+
+/// List the entries of the (uncompressed) vendor tar stream that match
+/// [`VENDOR_DENYLIST`].
+fn vendor_denied_entries(archive: impl std::io::Read) -> Result<Vec<String>> {
+    let mut denied = Vec::new();
+    for entry in tar::Archive::new(archive).entries()? {
+        let entry = entry?;
+        let path = entry.path()?;
+        let path = path.to_string_lossy();
+        let path = Utf8Path::new(&*path);
+        let relpath = path.strip_prefix(VENDOR_DIR).unwrap_or(path);
+        if let Some(name) = vendor_denied_crate(relpath) {
+            denied.push(format!("{path} (from {name})"));
+        }
+    }
+    Ok(denied)
+}
+
+/// Verify that the vendor archive contains no files from [`VENDOR_DENYLIST`].
+#[context("Checking vendor archive {vendorpath}")]
+fn check_vendor_archive(vendorpath: &Utf8Path) -> Result<()> {
+    let mut child = Command::new("zstd")
+        .args(["-dc", "--"])
+        .arg(vendorpath)
+        .stdout(Stdio::piped())
+        .spawn()
+        .context("Spawning zstd")?;
+    let stdout = child.stdout.take().expect("stdout is piped");
+    let denied = vendor_denied_entries(stdout);
+    // Wait for zstd even if reading failed, so its error is not lost
+    let status = child.wait().context("Waiting for zstd")?;
+    let denied = denied.context("Reading tar entries")?;
+    if !status.success() {
+        anyhow::bail!("zstd failed: {status}");
+    }
+    if !denied.is_empty() {
+        let n = denied.len();
+        let shown = denied[..n.min(VENDOR_DENIED_SHOWN)].join("\n  ");
+        let more = n.saturating_sub(VENDOR_DENIED_SHOWN);
+        let more = if more > 0 {
+            format!("\n  ... and {more} more")
+        } else {
+            String::new()
+        };
+        anyhow::bail!(
+            "Found {n} denylisted files; update exclude-crate-paths in Cargo.toml:\n  {shown}{more}"
+        );
+    }
+    Ok(())
+}
+
 #[context("Packaging")]
 fn impl_package(sh: &Shell) -> Result<Package> {
     let source_date_epoch = git_source_date_epoch(".".into())?;
@@ -514,9 +589,10 @@ fn impl_package(sh: &Shell) -> Result<Package> {
     let vendorpath = Utf8Path::new("target").join(format!("{namev}-vendor.tar.zstd"));
     let vendor_config = cmd!(
         sh,
-        "cargo vendor-filterer --prefix=vendor --format=tar.zstd {vendorpath}"
+        "cargo vendor-filterer --prefix={VENDOR_DIR} --format=tar.zstd {vendorpath}"
     )
     .read()?;
+    check_vendor_archive(&vendorpath)?;
     let vendor_config = edit_vendor_config(&vendor_config)?;
     // Append .cargo/vendor-config.toml (a made up filename) into the tar archive.
     {
@@ -840,4 +916,47 @@ mod tests {
         assert!(parse_cli_bool("").is_err());
         assert!(parse_cli_bool("maybe").is_err());
     }
+
+    #[test]
+    fn test_vendor_denied_crate() {
+        let cases = [
+            ("pcre2-sys/upstream/src/pcre2_compile.c", Some("pcre2-sys")),
+            ("pcre2-sys/upstream/src/pcre2_internal.h", Some("pcre2-sys")),
+            ("pcre2-sys/pcre2.h", Some("pcre2-sys")),
+            (
+                "pcre2-sys-0.2.9/upstream/src/pcre2_compile.c",
+                Some("pcre2-sys"),
+            ),
+            ("pcre2-sys/src/lib.rs", None),
+            ("pcre2-sys/upstream/src/pcre2.h.generic", None),
+            ("pcre2-sys", None),
+            ("pcre2/src/ffi.c", None),
+            ("pcre2-sysx/foo.c", None),
+            ("pcre2-sys-extra/foo.c", None),
+            ("libz-sys/src/zlib/adler32.c", None),
+        ];
+        for (path, expected) in cases {
+            assert_eq!(vendor_denied_crate(Utf8Path::new(path)), expected, "{path}");
+        }
+    }
+
+    #[test]
+    fn test_vendor_denied_entries() -> Result<()> {
+        let mut builder = tar::Builder::new(Vec::new());
+        for path in [
+            "vendor/pcre2-sys/src/lib.rs",
+            "vendor/pcre2-sys/upstream/src/pcre2_compile.c",
+            "vendor/libz-sys/src/zlib/adler32.c",
+        ] {
+            let mut header = tar::Header::new_gnu();
+            header.set_size(0);
+            builder.append_data(&mut header, path, std::io::empty())?;
+        }
+        let archive = builder.into_inner()?;
+        assert_eq!(
+            vendor_denied_entries(archive.as_slice())?,
+            ["vendor/pcre2-sys/upstream/src/pcre2_compile.c (from pcre2-sys)"]
+        );
+        Ok(())
+    }
 }

docs/src/bootc-installation.7.md

@@ -233,8 +233,9 @@ Set the environment variable `BOOTC_DIRECT_IO=on` to create the loopback device
 
 This is a variant of `install to-filesystem`, which maximizes convenience for using
 an existing Linux system, converting it into the target container image.  Note that
-the `/boot` (and `/boot/efi`) partitions *will be reinitialized* - so this is a
-somewhat destructive operation for the existing Linux installation.
+the `/boot` partition and the `EFI/` and `loader/` directories of the ESP
+*will be reinitialized* - so this is a somewhat destructive operation for the
+existing Linux installation. Other ESP content (e.g. Asahi's `m1n1/`) is preserved.
 
 Also, because the filesystem is reused, it's required that the target system kernel
 support the root storage setup already initialized.

tmt/tests/booted/test-multi-device-esp.nu

@@ -16,7 +16,8 @@
 #   3. Three devices, partial ESP: Three disks, ESP on disk1+disk3 only
 #
 # Reboot 2:
-#   4. Single device (no LVM): ESP + root partition on a single disk
+#   4. Single device (no LVM): ESP + root partition on a single disk;
+#      also checks that non-bootloader ESP content is preserved
 #   5. No ESP anywhere: Two disks with no ESP; install should fail gracefully
 #
 # This validates the fix for https://github.com/bootc-dev/bootc/issues/481
@@ -157,6 +158,17 @@ def validate_esp [esp_partition: string] {
     }
 }
 
+# Mount an ESP partition, run a closure on the mountpoint, and unmount it
+def with_esp [esp_partition: string, f: closure] {
+    let esp_mount = "/var/mnt/esp_seed"
+    mkdir $esp_mount
+    mount $esp_partition $esp_mount
+    let r = (do $f $esp_mount)
+    umount $esp_mount
+    rmdir $esp_mount
+    $r
+}
+
 # Run bootc install to-existing-root from within the container image under test
 def run_install [mountpoint: string] {
     (podman run
@@ -356,8 +368,15 @@ def test_single_device_no_lvm [] {
         mkdir $mountpoint
         mount $"($loop1)p2" $mountpoint
 
-        # Create boot directory
-        mkdir $"($mountpoint)/boot"
+        # Create /boot/efi so the ESP gets mounted and cleaned
+        mkdir $"($mountpoint)/boot/efi"
+
+        # Seed non-bootloader content (as on Asahi) and a stale bootloader dir
+        with_esp $"($loop1)p1" {|esp|
+            mkdir $"($esp)/m1n1" $"($esp)/EFI/stale"
+            "m1n1" | save $"($esp)/m1n1/boot.bin"
+            "stale" | save $"($esp)/EFI/stale/x.efi"
+        }
 
         # Show block device hierarchy
         lsblk --pairs --paths --inverse --output NAME,TYPE $"($loop1)p2"
@@ -366,6 +385,12 @@ def test_single_device_no_lvm [] {
 
         # Validate ESP was installed correctly
         validate_esp $"($loop1)p1"
+        let r = (with_esp $"($loop1)p1" {|esp| {
+            m1n1: ($"($esp)/m1n1/boot.bin" | path exists)
+            stale: ($"($esp)/EFI/stale" | path exists)
+        }})
+        assert $r.m1n1 "m1n1/boot.bin was removed from the ESP"
+        assert (not $r.stale) "EFI/stale was not removed from the ESP"
     } catch {|e|
         cleanup_simple $loop1 $mountpoint
         rm -f $disk1