Skip to content

Remove need for build.rs in every cargo-pvm-contract contract crate#17

Open
charlesHetterich wants to merge 2 commits intomainfrom
ch/handle-build-rs
Open

Remove need for build.rs in every cargo-pvm-contract contract crate#17
charlesHetterich wants to merge 2 commits intomainfrom
ch/handle-build-rs

Conversation

@charlesHetterich
Copy link

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.

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.
@github-actions
Copy link

github-actions bot commented Mar 13, 2026

Benchmark Size Comparison (vs main)

Artifact Baseline Current Delta Status
fibonacci_builder-dsl.debug.polkavm 36472 36480 +0.02% OK
fibonacci_builder-dsl.release.polkavm 1194 1194 +0.00% OK
fibonacci_no-alloc.debug.polkavm 35712 35712 +0.00% OK
fibonacci_no-alloc.release.polkavm 469 469 +0.00% OK
fibonacci_with-alloc.debug.polkavm 44946 44946 +0.00% OK
fibonacci_with-alloc.release.polkavm 448 448 +0.00% OK
multi_builder-dsl.debug.polkavm 53579 53579 +0.00% OK
multi_builder-dsl.release.polkavm 3568 3568 +0.00% OK
multi_no-alloc.debug.polkavm 55438 55438 +0.00% OK
multi_no-alloc.release.polkavm 3419 3419 +0.00% OK
multi_with-alloc.debug.polkavm 64479 64479 +0.00% OK
multi_with-alloc.release.polkavm 3864 3864 +0.00% OK
mytoken_builder-dsl.debug.polkavm 49847 49855 +0.02% OK
mytoken_builder-dsl.release.polkavm 4097 4097 +0.00% OK
mytoken_no-alloc.debug.polkavm 51855 51855 +0.00% OK
mytoken_no-alloc.release.polkavm 3751 3751 +0.00% OK
mytoken_with-alloc.debug.polkavm 62706 62706 +0.00% OK
mytoken_with-alloc.release.polkavm 4301 4301 +0.00% OK
---------------------------------------------------- ------------ ------------ ---------- --------
Total 480145 480161 +0.00%

OK: All artifacts within 5% regression threshold

Run 23059078364 | b672f43 | 2026-03-13 16:19 UTC

@pgherveou
Copy link
Collaborator

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

@charlesHetterich
Copy link
Author

What's the benefits of requiring an external cli to build?

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?

@pgherveou
Copy link
Collaborator

You need the cli to generate the project, but then anyone else cloning or forking your project doesn't

@smiasojed
Copy link
Contributor

@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?

@pgherveou
Copy link
Collaborator

pgherveou commented Mar 16, 2026

@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:

  • not having to invoke another build tool but just do a cargo build (same as what you do when you build a parachain runtime to wasm)
  • not asking external contributors to install an additional cli
  • Hopefully there are no breaking changes, but version management is defined in Cargo.toml
  • the DX is more practical too, if you run cargo watch or similar tool, you know the build is always up to date.

)
})?;

for _pkg in &packages {
Copy link
Contributor

Choose a reason for hiding this comment

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

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(
Copy link
Contributor

Choose a reason for hiding this comment

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

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"));
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of unwrap() should use ? or .context(...)

);

let status = Command::new("cargo")
let bin_path = workspace_root().join("target/debug/cargo-pvm-contract");
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we assume that we have debug version compiled?

Copy link
Contributor

Choose a reason for hiding this comment

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

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()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why mytoken.release.polkavm instead of target/release/mytoken.polkavm?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants