apollo_compile_to_casm,apollo_compile_to_native,scripts: simplify compiler binary installation#13628
Open
avi-starkware wants to merge 10 commits intomain-v0.14.2from
Open
apollo_compile_to_casm,apollo_compile_to_native,scripts: simplify compiler binary installation#13628avi-starkware wants to merge 10 commits intomain-v0.14.2from
avi-starkware wants to merge 10 commits intomain-v0.14.2from
Conversation
…piler binary installation Move compiler binary installation (starknet-sierra-compile, starknet-native-compile) from build.rs side effects into scripts/install_cargo_tools.sh. At runtime, verify the installed version is correct and panic with installation instructions if not. This removes the OUT_DIR/RUNTIME_ACCESSIBLE_OUT_DIR env var hack and the shared_executables directory, replacing them with standard PATH-based binary lookup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This change is |
cargo install runs outside the workspace, so .cargo/config.toml env vars (LLVM_SYS_191_PREFIX, MLIR_SYS_190_PREFIX, TABLEGEN_190_PREFIX) are not picked up. Pass them explicitly in the subshell. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminate hardcoded compiler binary versions from scripts and Dockerfiles. Rust constants (CAIRO1_COMPILER_VERSION, REQUIRED_CAIRO_NATIVE_VERSION) are already the single source of truth with tests that verify they match Cargo.toml. Scripts and Dockerfiles now read from these files directly. Also parse LLVM env vars from .cargo/config.toml instead of hardcoding them in install_cargo_tools.sh. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
LLVM 19 is only available on CI runners with cairo_native support. Skip the starknet-native-compile installation gracefully on runners without it. The runtime version check will catch missing binaries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bootstrap action ran install_cargo_tools.sh before dependencies.sh, so LLVM 19 was not yet available when starknet-native-compile was built. Swap the step order so setup_native_deps (LLVM) runs first. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Same ordering fix as bootstrap: dependencies.sh must run before install_cargo_tools.sh so LLVM 19 is available for starknet-native-compile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ase Dockerfile - Copy compiler_versions.sh and the two Rust source files it reads into the base Docker image so install_cargo_tools.sh can extract versions. - Move LLVM env vars before install_build_tools.sh so they are available when cargo install starknet-native-compile runs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…s and dedicated install script - Move compiler versions into plain text files (.txt) as the single source of truth. Rust code uses include_str!(), shell scripts use cat. - Extract compiler installation into scripts/install_compiler_binaries.sh, called from install_cargo_tools.sh. - verify_compiler_binary now takes a Path and is called in both arms of SierraToNativeCompiler::new() (custom path and default). - Panic messages point to install_compiler_binaries.sh. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Print clear instructions instead of failing with a cryptic tblgen build error when a dev runs install_compiler_binaries.sh without LLVM. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| None => binary_path(&out_dir(), CAIRO_NATIVE_BINARY_NAME), | ||
| None => binary_path(CAIRO_NATIVE_BINARY_NAME), | ||
| }; | ||
| verify_compiler_binary(&path_to_binary, REQUIRED_CAIRO_NATIVE_VERSION); |
There was a problem hiding this comment.
Version check enforced on user-provided custom binary path
Medium Severity
verify_compiler_binary is called unconditionally after resolving path_to_binary, which means it also runs when the user provides a custom compiler_binary_path via config. The old code performed no version check for custom paths. Now, if a user configures a custom binary (e.g., for testing or using a patched compiler), the application panics if that binary's version doesn't exactly match REQUIRED_CAIRO_NATIVE_VERSION. The verification call likely belongs inside the None branch of the match, not after it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
starknet-sierra-compile,starknet-native-compile) frombuild.rsside effects intoscripts/install_cargo_tools.shOUT_DIR/RUNTIME_ACCESSIBLE_OUT_DIRenv var hack andshared_executables/directory — binaries are found via standard PATH lookup/usr/local/bin/Test plan
main-v0.14.2CI run)cargo build -p apollo_compile_to_casmsucceeds without installing compiler binaries (no build.rs)cargo test -p apollo_compile_to_casmpasses after runningscripts/install_cargo_tools.shSierraToCasmCompiler::new()without binaries installed panics with clear instructions🤖 Generated with Claude Code
Note
Medium Risk
Changes how
starknet-sierra-compile/starknet-native-compileare provisioned (CI, Docker images, local scripts) and removes build-time auto-installation, so misconfigured environments may now fail at runtime/build with panics or missing artifacts.Overview
Stops installing Sierra compiler binaries as
build.rsside effects and removesOUT_DIR/shared_executables-based pathing; compilers are now resolved fromPATHand validated at runtime viaverify_compiler_binary(panic with install instructions on missing/wrong versions).Introduces version files (
cairo_compiler_version.txt,native_compiler_version.txt) as the single source of truth, plus new scripts (compiler_versions.sh,install_compiler_binaries.sh) and updatesinstall_cargo_tools.sh/install_build_tools.shto install the required compiler binaries.Updates CI/Docker packaging to match the new locations: Dockerfiles now
cargo installthe compilers and copy them to/usr/local/bin, and the artifacts workflow/build scripts copy/uploadstarknet-native-compilefrom the cargo bin rather thantarget/.../shared_executables.Written by Cursor Bugbot for commit 30da8c6. This will update automatically on new commits. Configure here.