Merge pull request #320 from jasoncouture/simplified_disk_builder

Simplified disk builder

Author: phil-opp

build.rs

@@ -266,6 +266,7 @@ async fn build_bios_stage_4(out_dir: &Path) -> PathBuf {
     convert_elf_to_bin(elf_path).await
 }
 
+#[cfg(not(docsrs_dummy_build))]
 #[cfg(feature = "bios")]
 async fn convert_elf_to_bin(elf_path: PathBuf) -> PathBuf {
     let flat_binary_path = elf_path.with_extension("bin");

src/bios/mod.rs

@@ -1,98 +1,36 @@
-use crate::fat;
-use anyhow::Context;
-use bootloader_boot_config::BootConfig;
-use std::io::Write;
-use std::{
-    collections::BTreeMap,
-    path::{Path, PathBuf},
-};
-use tempfile::NamedTempFile;
+use std::path::Path;
 
-mod mbr;
+use bootloader_boot_config::BootConfig;
 
-const BIOS_STAGE_3: &str = "boot-stage-3";
-const BIOS_STAGE_4: &str = "boot-stage-4";
+use crate::DiskImageBuilder;
 
 /// Create disk images for booting on legacy BIOS systems.
 pub struct BiosBoot {
-    kernel: PathBuf,
-    ramdisk: Option<PathBuf>,
-    config: Option<String>,
+    image_builder: DiskImageBuilder,
 }
 
 impl BiosBoot {
     /// Start creating a disk image for the given bootloader ELF executable.
     pub fn new(kernel_path: &Path) -> Self {
         Self {
-            kernel: kernel_path.to_owned(),
-            ramdisk: None,
-            config: None,
+            image_builder: DiskImageBuilder::new(kernel_path.to_owned()),
         }
     }
 
     /// Add a ramdisk file to the image.
     pub fn set_ramdisk(&mut self, ramdisk_path: &Path) -> &mut Self {
-        self.ramdisk = Some(ramdisk_path.to_owned());
+        self.image_builder.set_ramdisk(ramdisk_path.to_owned());
         self
     }
 
-    /// Configures the runtime behavior of the bootloader.
+    /// Creates a configuration file (boot.json) that configures the runtime behavior of the bootloader.
     pub fn set_boot_config(&mut self, config: &BootConfig) -> &mut Self {
-        self.config = Some(serde_json::to_string(&config).expect("failed to serialize BootConfig"));
+        self.image_builder.set_boot_config(config);
         self
     }
 
     /// Create a bootable BIOS disk image at the given path.
     pub fn create_disk_image(&self, out_path: &Path) -> anyhow::Result<()> {
-        let bootsector_path = Path::new(env!("BIOS_BOOT_SECTOR_PATH"));
-        let stage_2_path = Path::new(env!("BIOS_STAGE_2_PATH"));
-
-        let fat_partition = self
-            .create_fat_partition()
-            .context("failed to create FAT partition")?;
-
-        mbr::create_mbr_disk(
-            bootsector_path,
-            stage_2_path,
-            fat_partition.path(),
-            out_path,
-        )
-        .context("failed to create BIOS MBR disk image")?;
-
-        fat_partition
-            .close()
-            .context("failed to delete FAT partition after disk image creation")?;
-
-        Ok(())
-    }
-
-    /// Creates an BIOS-bootable FAT partition with the kernel.
-    fn create_fat_partition(&self) -> anyhow::Result<NamedTempFile> {
-        let stage_3_path = Path::new(env!("BIOS_STAGE_3_PATH"));
-        let stage_4_path = Path::new(env!("BIOS_STAGE_4_PATH"));
-
-        let mut files = BTreeMap::new();
-        files.insert(crate::KERNEL_FILE_NAME, self.kernel.as_path());
-        files.insert(BIOS_STAGE_3, stage_3_path);
-        files.insert(BIOS_STAGE_4, stage_4_path);
-        if let Some(ramdisk_path) = &self.ramdisk {
-            files.insert(crate::RAMDISK_FILE_NAME, ramdisk_path);
-        }
-
-        let mut config_file: NamedTempFile;
-
-        if let Some(config_ser) = &self.config {
-            config_file = NamedTempFile::new()
-                .context("failed to create temp file")
-                .unwrap();
-            writeln!(config_file, "{config_ser}")?;
-            files.insert(crate::CONFIG_FILE_NAME, config_file.path());
-        }
-
-        let out_file = NamedTempFile::new().context("failed to create temp file")?;
-        fat::create_fat_filesystem(files, out_file.path())
-            .context("failed to create BIOS FAT filesystem")?;
-
-        Ok(out_file)
+        self.image_builder.create_bios_image(out_path)
     }
 }

src/fat.rs

@@ -1,21 +1,21 @@
+use crate::file_data_source::FileDataSource;
 use anyhow::Context;
-use std::{collections::BTreeMap, fs, io, path::Path};
+use fatfs::Dir;
+use std::fs::File;
+use std::{collections::BTreeMap, fs, path::Path};
 
 use crate::KERNEL_FILE_NAME;
 
 pub fn create_fat_filesystem(
-    files: BTreeMap<&str, &Path>,
+    files: BTreeMap<&str, &FileDataSource>,
     out_fat_path: &Path,
 ) -> anyhow::Result<()> {
     const MB: u64 = 1024 * 1024;
 
     // calculate needed size
     let mut needed_size = 0;
-    for path in files.values() {
-        let file_size = fs::metadata(path)
-            .with_context(|| format!("failed to read metadata of file `{}`", path.display()))?
-            .len();
-        needed_size += file_size;
+    for source in files.values() {
+        needed_size += source.len()?;
     }
 
     // create new filesystem image file at the given path and set its length
@@ -31,7 +31,9 @@ pub fn create_fat_filesystem(
 
     // choose a file system label
     let mut label = *b"MY_RUST_OS!";
-    if let Some(path) = files.get(KERNEL_FILE_NAME) {
+
+    // This __should__ always be a file, but maybe not. Should we allow the caller to set the volume label instead?
+    if let Some(FileDataSource::File(path)) = files.get(KERNEL_FILE_NAME) {
         if let Some(name) = path.file_stem() {
             let converted = name.to_string_lossy();
             let name = converted.as_bytes();
@@ -48,10 +50,17 @@ pub fn create_fat_filesystem(
     fatfs::format_volume(&fat_file, format_options).context("Failed to format FAT file")?;
     let filesystem = fatfs::FileSystem::new(&fat_file, fatfs::FsOptions::new())
         .context("Failed to open FAT file system of UEFI FAT file")?;
+    let root_dir = filesystem.root_dir();
 
     // copy files to file system
-    let root_dir = filesystem.root_dir();
-    for (target_path_raw, file_path) in files {
+    add_files_to_image(&root_dir, files)
+}
+
+pub fn add_files_to_image(
+    root_dir: &Dir<&File>,
+    files: BTreeMap<&str, &FileDataSource>,
+) -> anyhow::Result<()> {
+    for (target_path_raw, source) in files {
         let target_path = Path::new(target_path_raw);
         // create parent directories
         let ancestors: Vec<_> = target_path.ancestors().skip(1).collect();
@@ -70,12 +79,14 @@ pub fn create_fat_filesystem(
             .create_file(target_path_raw)
             .with_context(|| format!("failed to create file at `{}`", target_path.display()))?;
         new_file.truncate().unwrap();
-        io::copy(
-            &mut fs::File::open(file_path)
-                .with_context(|| format!("failed to open `{}` for copying", file_path.display()))?,
-            &mut new_file,
-        )
-        .with_context(|| format!("failed to copy `{}` to FAT filesystem", file_path.display()))?;
+
+        source.copy_to(&mut new_file).with_context(|| {
+            format!(
+                "failed to copy source data `{:?}` to file at `{}`",
+                source,
+                target_path.display()
+            )
+        })?;
     }
 
     Ok(())

src/file_data_source.rs

@@ -0,0 +1,58 @@
+use alloc::vec::Vec;
+use anyhow::Context;
+use core::fmt::{Debug, Formatter};
+
+use std::io::Cursor;
+use std::path::PathBuf;
+use std::{fs, io};
+
+#[derive(Clone)]
+/// Defines a data source, either a source `std::path::PathBuf`, or a vector of bytes.
+pub enum FileDataSource {
+    File(PathBuf),
+    Data(Vec<u8>),
+}
+
+impl Debug for FileDataSource {
+    fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result {
+        match self {
+            FileDataSource::File(file) => {
+                f.write_fmt(format_args!("data source: File {}", file.display()))
+            }
+            FileDataSource::Data(d) => {
+                f.write_fmt(format_args!("data source: {} raw bytes ", d.len()))
+            }
+        }
+    }
+}
+
+impl FileDataSource {
+    /// Get the length of the inner data source
+    pub fn len(&self) -> anyhow::Result<u64> {
+        Ok(match self {
+            FileDataSource::File(path) => fs::metadata(path)
+                .with_context(|| format!("failed to read metadata of file `{}`", path.display()))?
+                .len(),
+            FileDataSource::Data(v) => v.len() as u64,
+        })
+    }
+    /// Copy this data source to the specified target that implements io::Write
+    pub fn copy_to(&self, target: &mut dyn io::Write) -> anyhow::Result<()> {
+        match self {
+            FileDataSource::File(file_path) => {
+                io::copy(
+                    &mut fs::File::open(file_path).with_context(|| {
+                        format!("failed to open `{}` for copying", file_path.display())
+                    })?,
+                    target,
+                )?;
+            }
+            FileDataSource::Data(contents) => {
+                let mut cursor = Cursor::new(contents);
+                io::copy(&mut cursor, target)?;
+            }
+        };
+
+        Ok(())
+    }
+}