apollo_compile_to_casm,apollo_compile_to_native: switch to script-based compiler installation#13675
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
Artifacts upload workflows: |
5f2553f to
7c87ee5
Compare
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 14 files and all commit messages, and made 7 comments.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on avi-starkware).
crates/blockifier_reexecution/replay/Dockerfile line 91 at r1 (raw file):
# ============================================================================= # Stage 4: Final runtime image
Suggestion:
COPY rust-toolchain.toml rust-toolchain.toml
COPY scripts/install_compiler_binaries.sh scripts/install_compiler_binaries.sh
# .. copy other files required... version.txt etc.
COPY --from=planner /app/recipe.json recipe.json
RUN cargo chef cook --release -p blockifier_reexecution --features cairo_native --recipe-path recipe.json
COPY . .
RUN cargo build --release -p blockifier_reexecution --features cairo_native
# Install compiler binaries used for Sierra compilation at runtime.
RUN scripts/install_compiler_binaries.sh
# =============================================================================
# Stage 4: Final runtime imagecrates/blockifier_reexecution/replay/Dockerfile line 109 at r1 (raw file):
COPY --from=builder /app/target/release/blockifier_reexecution ./target/release/blockifier_reexecution COPY --from=builder /var/tmp/rust/bin/starknet-sierra-compile /usr/local/bin/starknet-sierra-compile COPY --from=builder /var/tmp/rust/bin/starknet-native-compile /usr/local/bin/starknet-native-compile
how do you know this is where the binaries were installed to...?
maybe make the install_compiler_binaries.sh script return the absolute paths to the installed packages? then you'd never have to guess
Code quote:
COPY --from=builder /var/tmp/rust/bin/starknet-sierra-compile /usr/local/bin/starknet-sierra-compile
COPY --from=builder /var/tmp/rust/bin/starknet-native-compile /usr/local/bin/starknet-native-compilecrates/starknet_transaction_prover/Dockerfile line 140 at r1 (raw file):
# Install compiler binary used for Sierra to CASM compilation at runtime. # Version is read from a plain text file (the single source of truth). RUN cargo install --locked starknet-sierra-compile --version "$(cat crates/apollo_infra_utils/src/cairo_compiler_version.txt)"
see above: you are duplicating logic from you installation script.
here specifically you only need the sierra compiler, but I'm sure you can add an optional argument to your bash script to skip native installation
Code quote:
# Install compiler binary used for Sierra to CASM compilation at runtime.
# Version is read from a plain text file (the single source of truth).
RUN cargo install --locked starknet-sierra-compile --version "$(cat crates/apollo_infra_utils/src/cairo_compiler_version.txt)"crates/starknet_transaction_prover/Dockerfile line 165 at r1 (raw file):
# Copy starknet-sierra-compile binary required for Sierra to Casm compilation. COPY --from=builder /var/tmp/rust/bin/starknet-sierra-compile /usr/local/bin/starknet-sierra-compile
ditto - consider taking the output path from the script output
Code quote:
COPY --from=builder /var/tmp/rust/bin/starknet-sierra-compile /usr/local/bin/starknet-sierra-compiledeployments/images/base/Dockerfile line 50 at r1 (raw file):
# Cleanup. RUN rm -f scripts/install_build_tools.sh scripts/install_cargo_tools.sh scripts/install_compiler_binaries.sh scripts/compiler_versions.sh scripts/cargo_tool_utils.sh scripts/dependencies.sh scripts/apt_utils.sh scripts/requirements.txt && \ rm -rf crates/apollo_infra_utils crates/apollo_compile_to_native
I get that you save ink using -rf but please keep on being explicit here :P
Code quote:
RUN rm -f scripts/install_build_tools.sh scripts/install_cargo_tools.sh scripts/install_compiler_binaries.sh scripts/compiler_versions.sh scripts/cargo_tool_utils.sh scripts/dependencies.sh scripts/apt_utils.sh scripts/requirements.txt && \
rm -rf crates/apollo_infra_utils crates/apollo_compile_to_nativedeployments/images/sequencer/Dockerfile line 0 at r1 (raw file):
same remarks as above (reuse script + get output paths as output)
scripts/build_native_blockifier.sh line 20 at r1 (raw file):
# Install starknet-native-compile for artifact upload. source "$(dirname "${BASH_SOURCE[0]}")/compiler_versions.sh" cargo install --locked starknet-native-compile --version "$NATIVE_COMPILE_VERSION" || ret=$?
I prefer you pass an argument to the original script indicating if you want to install sierra, native or both, and use it..
Code quote:
# Install starknet-native-compile for artifact upload.
source "$(dirname "${BASH_SOURCE[0]}")/compiler_versions.sh"
cargo install --locked starknet-native-compile --version "$NATIVE_COMPILE_VERSION" || ret=$?92e7bf5 to
9c686ef
Compare
7c87ee5 to
5814201
Compare
9c686ef to
ac92fec
Compare
5814201 to
0f636e6
Compare
ac92fec to
36d086b
Compare
0f636e6 to
1c8c980
Compare
36d086b to
e332679
Compare
1c8c980 to
6b06cdf
Compare
e332679 to
7cc069b
Compare
6b06cdf to
a529b9e
Compare
7cc069b to
1efda68
Compare
a529b9e to
c698fc8
Compare
avi-starkware
left a comment
There was a problem hiding this comment.
@avi-starkware made 7 comments and resolved 1 discussion.
Reviewable status: 8 of 16 files reviewed, 6 unresolved discussions (waiting on dorimedini-starkware).
crates/blockifier_reexecution/replay/Dockerfile line 109 at r1 (raw file):
Previously, dorimedini-starkware wrote…
how do you know this is where the binaries were installed to...?
maybe make theinstall_compiler_binaries.shscript return the absolute paths to the installed packages? then you'd never have to guess
Done. Added a --dest flag to install the binaries to a given directory.
crates/starknet_transaction_prover/Dockerfile line 140 at r1 (raw file):
Previously, dorimedini-starkware wrote…
see above: you are duplicating logic from you installation script.
here specifically you only need the sierra compiler, but I'm sure you can add an optional argument to your bash script to skip native installation
Done.
crates/starknet_transaction_prover/Dockerfile line 165 at r1 (raw file):
Previously, dorimedini-starkware wrote…
ditto - consider taking the output path from the script output
Done. (Same approach as above)
deployments/images/base/Dockerfile line 50 at r1 (raw file):
Previously, dorimedini-starkware wrote…
I get that you save ink using
-rfbut please keep on being explicit here :P
Done.
deployments/images/sequencer/Dockerfile line at r1 (raw file):
Previously, dorimedini-starkware wrote…
same remarks as above (reuse script + get output paths as output)
Done.
scripts/build_native_blockifier.sh line 20 at r1 (raw file):
Previously, dorimedini-starkware wrote…
I prefer you pass an argument to the original script indicating if you want to install sierra, native or both, and use it..
Done.
crates/blockifier_reexecution/replay/Dockerfile line 91 at r1 (raw file):
# ============================================================================= # Stage 4: Final runtime image
Done. All three Dockerfiles now call scripts/install_compiler_binaries.sh
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.
Reviewed by Cursor Bugbot for commit c698fc8. Configure here.
dorimedini-starkware
left a comment
There was a problem hiding this comment.
@dorimedini-starkware reviewed 8 files and all commit messages, made 2 comments, and resolved 6 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on avi-starkware).
deployments/images/base/Dockerfile line 50 at r5 (raw file):
# Cleanup. RUN rm -f scripts/install_build_tools.sh scripts/install_cargo_tools.sh scripts/install_compiler_binaries.sh scripts/compiler_versions.sh scripts/cargo_tool_utils.sh scripts/dependencies.sh scripts/apt_utils.sh scripts/requirements.txt && \ rm -f crates/apollo_infra_utils/src/cairo_compiler_version.txt crates/apollo_compile_to_native/src/native_compiler_version.txt && \
non-blocking
Suggestion:
RUN rm -f scripts/install_build_tools.sh scripts/install_cargo_tools.sh scripts/install_compiler_binaries.sh scripts/compiler_versions.sh scripts/cargo_tool_utils.sh scripts/dependencies.sh scripts/apt_utils.sh scripts/requirements.txt crates/apollo_infra_utils/src/cairo_compiler_version.txt crates/apollo_compile_to_native/src/native_compiler_version.txt && \|
There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. |
1efda68 to
4af9d22
Compare
c698fc8 to
fd0d72f
Compare
PR SummaryMedium Risk Overview Switches runtime resolution to a Updates build tooling and images to install/copy compiler binaries via Reviewed by Cursor Bugbot for commit 7a58f78. Bugbot is set up for automated code reviews on this repo. Configure here. |
4af9d22 to
7e79823
Compare
fd0d72f to
5ee741b
Compare
7e79823 to
5b12ec8
Compare
5ee741b to
c0b3e12
Compare
…ed compiler installation Remove build.rs from both compiler crates — compiler binaries are now installed by scripts/install_compiler_binaries.sh (called from install_cargo_tools.sh) instead of as a cargo build side effect. - Delete build.rs and [build-dependencies] from both crates - Compiler constructors use verify_compiler_binary + PATH-based lookup (skip version check for custom paths in SierraToNativeCompiler) - install_cargo_tools.sh calls install_compiler_binaries.sh - Dockerfiles use cargo install with versions from .txt files - Base Dockerfile copies new scripts and version files Old install_compiler_binary() and legacy path functions are left for removal in a follow-up cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5b12ec8 to
eb8a28f
Compare
c0b3e12 to
7a58f78
Compare
avi-starkware
left a comment
There was a problem hiding this comment.
@avi-starkware made 1 comment and resolved 1 discussion.
Reviewable status: 7 of 16 files reviewed, all discussions resolved (waiting on dorimedini-starkware).
deployments/images/base/Dockerfile line 50 at r5 (raw file):
Previously, dorimedini-starkware wrote…
non-blocking
Done.


Remove build.rs from both compiler crates — compiler binaries are now
installed by scripts/install_compiler_binaries.sh (called from
install_cargo_tools.sh) instead of as a cargo build side effect.
custom paths in SierraToNativeCompiler)
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
Note
Medium Risk
Touches build/packaging and runtime compiler-binary resolution across CI and multiple Docker images; misconfigured PATH or missing installs could break compilation at runtime or artifact publishing.
Overview
Switches Sierra compiler binary provisioning from
build.rsside effects (writing intotarget/.../shared_executablesviaOUT_DIR) to an explicitscripts/install_compiler_binaries.shflow that installs versionedstarknet-sierra-compile/starknet-native-compileand exposes them via PATH, with runtime constructors now callingverify_compiler_binaryto enforce required versions.Updates Dockerfiles and CI/artifact upload to install/copy these binaries into
/usr/local/bin(and to uploadstarknet-native-compileby resolved PATH), adjusts BazelBUILDexports accordingly, and tweaksinstall_cargo_tools.shversion detection to handle prerelease versions before invoking the new installer.Reviewed by Cursor Bugbot for commit c698fc8. Bugbot is set up for automated code reviews on this repo. Configure here.