Skip to content

refactor: remove unnecessary asm/ tree copy from standards and agglayer build scripts #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

Open
mmagician wants to merge 5 commits into
base: next
Choose a base branch
from
Open

refactor: remove unnecessary asm/ tree copy from standards and agglayer build scripts #2538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 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
bcf21a8
Merge branch 'next' into claude/review-pr-2452-qd5DI
mmagician Mar 17, 2026
302dcf0
refactor: remove copy_directory from agglayer build.rs
claude Mar 17, 2026
ef73342
Merge branch 'next' into claude/review-pr-2452-qd5DI
mmagician Mar 17, 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
60 changes: 4 additions & 56 deletions crates/miden-agglayer/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,15 @@ fn main() -> Result<()> {
println!("cargo::rerun-if-changed={ASM_DIR}/");
println!("cargo::rerun-if-env-changed=BUILD_GENERATED_FILES_IN_SRC");

// 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);

// generate canonical zeros in `asm/agglayer/bridge/canonical_zeros.masm`
generate_canonical_zeros(&src.join(ASM_AGGLAYER_BRIDGE_DIR))?;
let crate_path = Path::new(&crate_dir);
generate_canonical_zeros(&crate_path.join(ASM_DIR).join(ASM_AGGLAYER_BRIDGE_DIR))?;
Comment on lines -47 to +46
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Mar 18, 2026

Choose a reason for hiding this comment

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

I believe the only two things left writing into src are the canonical zeros and the error constants. Can we generate the error constants in agglayer the same way as in miden-standards, by writing to OUT_DIR?

Then we'd only have the canonical zeros left. These shouldn't really change, right? If so, we could consider generating them with a script once and have the build.rs only validate their correctness without writing into src. Then we would be fully compliant with the "don't write into src policy from rust/cargo" and could remove BUILD_GENERATED_FILES_IN_SRC.

Another strategy for removing the canonical zeros from src is to keep the copy_directory approach for agglayer MASM code. We copy the MASM source to OUT_DIR and generate the canonical zeros directly in the appropriate location within the MASM source code in OUT_DIR. Similar to how the miden-protocol build.rs script creates the kernel and protocol libraries with the shared_utils and account_id.masm. Wdyt?

PR otherwise lgtm 👍


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.
let source_dir = crate_path.join(ASM_DIR);

// set target directory to {OUT_DIR}/assets
let target_dir = Path::new(&build_dir).join(ASSETS_DIR);
Expand Down Expand Up @@ -328,54 +324,6 @@ mod shared {
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>>(
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.
///
/// All non-MASM files are skipped.
Expand Down
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 @@ -31,15 +31,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 @@ -200,58 +197,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>>(
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
Loading