CLI bundle: add debug and kernel options. fixes #1447 (#1597)

* cli: bundle add debug option

* assembly: rename error EmptyKernel into NoExport

* assembly: check library has at least one export

* cli: bundle add kernel option

* fixup: move tests

* cli: bundle add `--output` option

* fixup: comment

* cli: changelog

* fixup: test debug

* cli: bundle include stdlib

tested with:
cargo run --features executable -- bundle ~/miden/miden-base/miden-lib/asm/kernels/transaction/lib --kernel ~/miden/miden-base/miden-lib/asm/kernels/transaction/api.masm

* cli: bundle improve checks and errors

* fixup test, add iterator for decorators in a node

CHANGELOG.md

@@ -11,6 +11,7 @@
 - [BREAKING] Use `thiserror` 2.0 to derive errors and refactor them (#1588).
 
 #### Enhancements
+- Added options `--kernel`, `--debug` and `--output` to `miden bundle` (#1447).
 - Added `miden_core::mast::MastForest::advice_map` to load it into the advice provider before the `MastForest` execution (#1574).
 
 ## 0.11.0 (2024-11-04)

assembly/src/library/error.rs

@@ -4,9 +4,9 @@ use crate::{ast::QualifiedProcedureName, diagnostics::Diagnostic};
 
 #[derive(Debug, thiserror::Error, Diagnostic)]
 pub enum LibraryError {
-    #[error("kernel library must contain at least one exported procedure")]
+    #[error("library must contain at least one exported procedure")]
     #[diagnostic()]
-    EmptyKernel,
+    NoExport,
     #[error("invalid export in kernel library: {procedure_path}")]
     InvalidKernelExport { procedure_path: QualifiedProcedureName },
     // Technically KernelError is the source error here, but since LibraryError is sometimes

assembly/src/library/mod.rs

@@ -72,12 +72,16 @@ impl Library {
     /// Constructs a new [`Library`] from the provided MAST forest and a set of exports.
     ///
     /// # Errors
+    /// Returns an error if the set of exports is empty.
     /// Returns an error if any of the specified exports do not have a corresponding procedure root
     /// in the provided MAST forest.
     pub fn new(
         mast_forest: Arc<MastForest>,
         exports: BTreeMap<QualifiedProcedureName, MastNodeId>,
     ) -> Result<Self, LibraryError> {
+        if exports.is_empty() {
+            return Err(LibraryError::NoExport);
+        }
         for (fqn, &proc_body_id) in exports.iter() {
             if !mast_forest.is_procedure_root(proc_body_id) {
                 return Err(LibraryError::NoProcedureRootForExport { procedure_path: fqn.clone() });
@@ -177,6 +181,9 @@ impl Deserializable for Library {
         let mast_forest = Arc::new(MastForest::read_from(source)?);
 
         let num_exports = source.read_usize()?;
+        if num_exports == 0 {
+            return Err(DeserializationError::InvalidValue(String::from("No exported procedures")));
+        };
         let mut exports = BTreeMap::new();
         for _ in 0..num_exports {
             let proc_module = source.read()?;
@@ -353,10 +360,6 @@ impl TryFrom<Library> for KernelLibrary {
     type Error = LibraryError;
 
     fn try_from(library: Library) -> Result<Self, Self::Error> {
-        if library.exports.is_empty() {
-            return Err(LibraryError::EmptyKernel);
-        }
-
         let kernel_path = LibraryPath::from(LibraryNamespace::Kernel);
         let mut proc_digests = Vec::with_capacity(library.exports.len());
 

core/src/mast/mod.rs

@@ -465,6 +465,10 @@ impl MastForest {
         &self.nodes
     }
 
+    pub fn decorators(&self) -> &[Decorator] {
+        &self.decorators
+    }
+
     pub fn advice_map(&self) -> &AdviceMap {
         &self.advice_map
     }

miden/src/cli/bundle.rs

@@ -2,25 +2,37 @@ use std::path::PathBuf;
 
 use assembly::{
     diagnostics::{IntoDiagnostic, Report},
-    Assembler, Library, LibraryNamespace,
+    Assembler, KernelLibrary, Library, LibraryNamespace,
 };
 use clap::Parser;
+use stdlib::StdLibrary;
 
 #[derive(Debug, Clone, Parser)]
 #[clap(
     name = "Compile Library",
-    about = "Bundles .masm files into a single .masl library"
+    about = "Bundles .masm files into a single .masl library with access to the stdlib."
 )]
 pub struct BundleCmd {
+    /// Include debug symbols.
+    #[clap(short, long, action)]
+    debug: bool,
     /// Path to a directory containing the `.masm` files which are part of the library.
     #[clap(value_parser)]
     dir: PathBuf,
-    /// Defines the top-level namespace, e.g. `mylib`, otherwise the directory name is used.
+    /// Defines the top-level namespace, e.g. `mylib`, otherwise the directory name is used. For a
+    /// kernel library the namespace defaults to `kernel`.
     #[clap(short, long)]
     namespace: Option<String>,
     /// Version of the library, defaults to `0.1.0`.
     #[clap(short, long, default_value = "0.1.0")]
     version: String,
+    /// Build a kernel library from module `kernel` and using the library `dir` as kernel
+    /// namespace. The `kernel` file should not be in the directory `dir`.
+    #[clap(short, long)]
+    kernel: Option<PathBuf>,
+    /// Path of the output `.masl` file.
+    #[clap(short, long)]
+    output: Option<PathBuf>,
 }
 
 impl BundleCmd {
@@ -29,29 +41,49 @@ impl BundleCmd {
         println!("Build library");
         println!("============================================================");
 
-        let namespace = match &self.namespace {
-            Some(namespace) => namespace.to_string(),
-            None => self
-                .dir
-                .file_name()
-                .expect("dir must be a folder")
-                .to_string_lossy()
-                .into_owned(),
-        };
+        let mut assembler = Assembler::default().with_debug_mode(self.debug);
 
-        let assembler = Assembler::default().with_debug_mode(true);
-        let library_namespace =
-            namespace.parse::<LibraryNamespace>().expect("invalid base namespace");
-        let library = Library::from_dir(&self.dir, library_namespace, assembler)?;
+        if self.dir.is_file() {
+            return Err(Report::msg("`dir` must be a directory."));
+        }
+        let dir = self.dir.file_name().ok_or("`dir` cannot end with `..`.").map_err(Report::msg)?;
 
         // write the masl output
-        let output_file = self
-            .dir
-            .join(self.namespace.as_deref().unwrap_or("out"))
-            .with_extension(Library::LIBRARY_EXTENSION);
-        library.write_to_file(output_file).into_diagnostic()?;
+        let output_file = match &self.output {
+            Some(output) => output,
+            None => {
+                let parent =
+                    &self.dir.parent().ok_or("Invalid output path").map_err(Report::msg)?;
+                &parent.join("out").with_extension(Library::LIBRARY_EXTENSION)
+            },
+        };
 
-        println!("Built library {}", namespace);
+        match &self.kernel {
+            Some(kernel) => {
+                if !kernel.is_file() {
+                    return Err(Report::msg("`kernel` must be a file"));
+                };
+                assembler.add_library(StdLibrary::default())?;
+                let library = KernelLibrary::from_dir(kernel, Some(&self.dir), assembler)?;
+                library.write_to_file(output_file).into_diagnostic()?;
+                println!(
+                    "Built kernel module {} with library {}",
+                    kernel.display(),
+                    &self.dir.display()
+                );
+            },
+            None => {
+                let namespace = match &self.namespace {
+                    Some(namespace) => namespace.to_string(),
+                    None => dir.to_string_lossy().into_owned(),
+                };
+                let library_namespace = namespace.parse::<LibraryNamespace>()?;
+                assembler.add_library(StdLibrary::default())?;
+                let library = Library::from_dir(&self.dir, library_namespace, assembler)?;
+                library.write_to_file(output_file).into_diagnostic()?;
+                println!("Built library {}", namespace);
+            },
+        }
 
         Ok(())
     }

miden/tests/integration/cli/cli_test.rs

@@ -1,12 +1,11 @@
+use std::{fs, path::Path};
+
 use assert_cmd::prelude::*;
 use predicates::prelude::*;
 extern crate escargot;
 
-#[test]
-// Tt test might be an overkill to test only that the 'run' cli command
-// outputs steps and ms.
-fn cli_run() -> Result<(), Box<dyn std::error::Error>> {
-    let bin_under_test = escargot::CargoBuild::new()
+fn bin_under_test() -> escargot::CargoRun {
+    escargot::CargoBuild::new()
         .bin("miden")
         .features("executable internal")
         .current_release()
@@ -15,9 +14,14 @@ fn cli_run() -> Result<(), Box<dyn std::error::Error>> {
         .unwrap_or_else(|err| {
             eprintln!("{err}");
             panic!("failed to build `miden`");
-        });
+        })
+}
 
-    let mut cmd = bin_under_test.command();
+#[test]
+// Tt test might be an overkill to test only that the 'run' cli command
+// outputs steps and ms.
+fn cli_run() -> Result<(), Box<dyn std::error::Error>> {
+    let mut cmd = bin_under_test().command();
 
     cmd.arg("run")
         .arg("-a")
@@ -38,3 +42,64 @@ fn cli_run() -> Result<(), Box<dyn std::error::Error>> {
 
     Ok(())
 }
+
+use assembly::Library;
+use vm_core::Decorator;
+
+#[test]
+fn cli_bundle_debug() {
+    let mut cmd = bin_under_test().command();
+    cmd.arg("bundle").arg("--debug").arg("./tests/integration/cli/data/lib");
+    cmd.assert().success();
+
+    let lib = Library::deserialize_from_file("./tests/integration/cli/data/out.masl").unwrap();
+    // If there are any AsmOp decorators in the forest, the bundle is in debug mode.
+    let found_one_asm_op =
+        lib.mast_forest().decorators().iter().any(|d| matches!(d, Decorator::AsmOp(_)));
+    assert!(found_one_asm_op);
+    fs::remove_file("./tests/integration/cli/data/out.masl").unwrap();
+}
+
+#[test]
+fn cli_bundle_no_exports() {
+    let mut cmd = bin_under_test().command();
+    cmd.arg("bundle").arg("./tests/integration/cli/data/lib_noexports");
+    cmd.assert()
+        .failure()
+        .stderr(predicate::str::contains("library must contain at least one exported procedure"));
+}
+
+#[test]
+fn cli_bundle_kernel() {
+    let mut cmd = bin_under_test().command();
+    cmd.arg("bundle")
+        .arg("./tests/integration/cli/data/lib")
+        .arg("--kernel")
+        .arg("./tests/integration/cli/data/kernel_main.masm");
+    cmd.assert().success();
+    fs::remove_file("./tests/integration/cli/data/out.masl").unwrap()
+}
+
+/// A kernel can bundle with a library w/o exports.
+#[test]
+fn cli_bundle_kernel_noexports() {
+    let mut cmd = bin_under_test().command();
+    cmd.arg("bundle")
+        .arg("./tests/integration/cli/data/lib_noexports")
+        .arg("--kernel")
+        .arg("./tests/integration/cli/data/kernel_main.masm");
+    cmd.assert().success();
+    fs::remove_file("./tests/integration/cli/data/out.masl").unwrap()
+}
+
+#[test]
+fn cli_bundle_output() {
+    let mut cmd = bin_under_test().command();
+    cmd.arg("bundle")
+        .arg("./tests/integration/cli/data/lib")
+        .arg("--output")
+        .arg("test.masl");
+    cmd.assert().success();
+    assert!(Path::new("test.masl").exists());
+    fs::remove_file("test.masl").unwrap()
+}

miden/tests/integration/cli/data/kernel_main.masm

@@ -0,0 +1,9 @@
+proc.internal_proc
+    caller
+    drop
+    exec.::kernel::lib::lib_proc
+end
+
+export.kernel_proc
+    exec.internal_proc
+end

miden/tests/integration/cli/data/lib/lib.masm

@@ -0,0 +1,3 @@
+export.lib_proc
+    swap
+end

miden/tests/integration/cli/data/lib_noexports/lib.masm

@@ -0,0 +1,3 @@
+proc.lib_proc
+    swap
+end