Skip to content

refactor: avoid unnecessary asm/ tree copy in standards build script #2538

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mmagician wants to merge 2 commits into
base: next
Choose a base branch
from
Draft

refactor: avoid unnecessary asm/ tree copy in standards build script #2538

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0c1cc38
refactor: avoid unnecessary asm/ tree copy in standards build script
claude Mar 2, 2026
d958376
Merge branch 'next' into claude/review-pr-2452-qd5DI
mmagician Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 4 additions & 55 deletions crates/miden-standards/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,12 @@ fn main() -> Result<()> {
// re-build when the MASM code changes
println!("cargo::rerun-if-changed={ASM_DIR}/");

// Copies the MASM code to the build directory
let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
let build_dir = env::var("OUT_DIR").unwrap();
let src = Path::new(&crate_dir).join(ASM_DIR);
let dst = Path::new(&build_dir).to_path_buf();
shared::copy_directory(src, &dst, ASM_DIR)?;

// set source directory to {OUT_DIR}/asm
let source_dir = dst.join(ASM_DIR);
// Read MASM sources directly from the crate's asm/ directory.
// No copy to OUT_DIR is needed because this crate doesn't mutate the source tree.
let source_dir = Path::new(&crate_dir).join(ASM_DIR);

// set target directory to {OUT_DIR}/assets
let target_dir = Path::new(&build_dir).join(ASSETS_DIR);
Expand Down Expand Up @@ -188,58 +185,10 @@ mod shared {

use fs_err as fs;
use miden_assembly::Report;
use miden_assembly::diagnostics::{IntoDiagnostic, Result, WrapErr};
use miden_assembly::diagnostics::{IntoDiagnostic, Result};
use regex::Regex;
use walkdir::WalkDir;

/// Recursively copies `src` into `dst`.
///
/// This function will overwrite the existing files if re-executed.
pub fn copy_directory<T: AsRef<Path>, R: AsRef<Path>>(
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do the same removal in the other build.rs scripts in protocol and agglayer?

Copy link
Collaborator Author

@mmagician mmagician Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think it can be applied similarly to the agglayer crate.

Re: protocol, we cannot use the same pattern. The reason is the copy_shared_modules function, which copies asm/shared_modules/ into both asm/kernels/transaction/lib/ and asm/protocol/ before compilation. This mutates the directory layout so the assembler can resolve imports like $kernel::account_id.

I'd put this PR on hold until we re-enable the agglayer crate though, because otherwise we can't really know if the build process works well or not.

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: protocol, we cannot use the same pattern. The reason is the copy_shared_modules function, which copies asm/shared_modules/ into both asm/kernels/transaction/lib/ and asm/protocol/ before compilation. This mutates the directory layout so the assembler can resolve imports like $kernel::account_id.

Makes sense! Then we can remove it once we can set up a proper project for this shared library (0xMiden/miden-vm#2510).

src: T,
dst: R,
asm_dir: &str,
) -> Result<()> {
let mut prefix = src.as_ref().canonicalize().unwrap();
// keep all the files inside the `asm` folder
prefix.pop();

let target_dir = dst.as_ref().join(asm_dir);
if target_dir.exists() {
// Clear existing asm files that were copied earlier which may no longer exist.
fs::remove_dir_all(&target_dir)
.into_diagnostic()
.wrap_err("failed to remove ASM directory")?;
}

// Recreate the directory structure.
fs::create_dir_all(&target_dir)
.into_diagnostic()
.wrap_err("failed to create ASM directory")?;

let dst = dst.as_ref();
let mut todo = vec![src.as_ref().to_path_buf()];

while let Some(goal) = todo.pop() {
for entry in fs::read_dir(goal).unwrap() {
let path = entry.unwrap().path();
if path.is_dir() {
let src_dir = path.canonicalize().unwrap();
let dst_dir = dst.join(src_dir.strip_prefix(&prefix).unwrap());
if !dst_dir.exists() {
fs::create_dir_all(&dst_dir).unwrap();
}
todo.push(src_dir);
} else {
let dst_file = dst.join(path.strip_prefix(&prefix).unwrap());
fs::copy(&path, dst_file).unwrap();
}
}
}

Ok(())
}

/// Returns a vector with paths to all MASM files in the specified directory and its
/// subdirectories.
///
Expand Down