Remove need for build.rs in every cargo-pvm-contract contract crate#17
Remove need for build.rs in every cargo-pvm-contract contract crate#17charlesHetterich wants to merge 2 commits intomainfrom
build.rs in every cargo-pvm-contract contract crate#17Conversation
Move contract building from build.rs into a CLI subcommand so scaffolded projects no longer need a build.rs or cargo-pvm-contract-builder as a build-dependency. CLI is restructured from flat args to nested subcommands: cargo pvm-contract init --init-type new ... cargo pvm-contract build [--profile dev] [--manifest-path ...] The build subcommand defaults to release profile and handles ELF compilation, PolkaVM linking, and ABI generation externally. Builder library changes: - Make Profile, get_bin_targets public; add Profile::from_name - Add get_package_name, build_contract, build_elf_cli entry points Scaffolding and examples no longer emit build.rs or [build-dependencies]. The PvmBuilder API is preserved for the benchmarks tool.
Benchmark Size Comparison (vs main)
OK: All artifacts within 5% regression thresholdRun 23059078364 | b672f43 | 2026-03-13 16:19 UTC |
|
What's the benefits of requiring an external cli to build? I kind of like that you can just clone and cargo build and not constraint user or CI to install an extra binary |
The main benefit is not requiring every contract crate to have the same build.rs file (& include the builder crate as a build dependency). It should be setup such that this isn't a requirement though— it should still be possible for a project to create its own build.rs file & use cargo build directly Also this is the same cli they already install for setting up their project. Though I should be able to take it out of CI if you'd like? |
|
You need the cli to generate the project, but then anyone else cloning or forking your project doesn't |
|
@pgherveou I think that moving the build to a CLI subcommand will be needed anyway for verifiable builds (--verify) in the future. Or do you see here other solution? |
Don't you achieve that by dockersing the build? I don't mind removing the build.rs and requiring users to run cargo pvm build instead, but I would weight the pros / cons of both approach before making the call. Personal opinion:
|
| ) | ||
| })?; | ||
|
|
||
| for _pkg in &packages { |
There was a problem hiding this comment.
Is --package intended to work here? _pkg is unused in the loop
| } | ||
|
|
||
| /// Build the ELF binary using cargo (CLI context — no build.rs env vars). | ||
| fn build_elf_cli( |
There was a problem hiding this comment.
it is very similar to build_elf, could we extract common logic?
| pub fn build_contracts(args: BuildArgs) -> Result<()> { | ||
| let manifest_path = args | ||
| .manifest_path | ||
| .unwrap_or_else(|| std::env::current_dir().unwrap().join("Cargo.toml")); |
There was a problem hiding this comment.
instead of unwrap() should use ? or .context(...)
| ); | ||
|
|
||
| let status = Command::new("cargo") | ||
| let bin_path = workspace_root().join("target/debug/cargo-pvm-contract"); |
There was a problem hiding this comment.
why do we assume that we have debug version compiled?
There was a problem hiding this comment.
maybe let status = Command::new(assert_cmd::cargo::cargo_bin!("cargo-pvm-contract"))
| anyhow::bail!("ELF binary not found at: {}", elf_path.display()); | ||
| } | ||
|
|
||
| let output_path = output_dir.join(format!("{}.{}.polkavm", bin, profile.directory())); |
There was a problem hiding this comment.
Why mytoken.release.polkavm instead of target/release/mytoken.polkavm?
Move contract building from build.rs into a CLI subcommand so scaffolded projects no longer need a build.rs or cargo-pvm-contract-builder as a build-dependency.
CLI is restructured from flat args to nested subcommands:
cargo pvm-contract init --init-type new ...
cargo pvm-contract build [--profile dev] [--manifest-path ...]
The build subcommand defaults to release profile and handles ELF compilation, PolkaVM linking, and ABI generation externally.
Builder library changes:
Scaffolding and examples no longer emit build.rs or [build-dependencies]. The PvmBuilder API is preserved for the benchmarks tool.