-
Notifications
You must be signed in to change notification settings - Fork 38
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.
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
refactor!: docker builds #357
Conversation
👌 for the github release image. We will use it as discussed and agreed. |
# Disable the lld linker for now, as it's causing issues with the linkme package. | ||
# https://github.com/rust-lang/rust/pull/124129 | ||
# https://github.com/dtolnay/linkme/pull/88 | ||
export RUSTFLAGS='-C target-cpu=native -Zlinker-features=-lld' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you moved the disabling of lld linker in config.toml
but why removing -C target-cpu=native
. This should be disabled when building cross-platform images, but it doesn't hurt keeping it in the scripts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take your lead on this
env::set_var( | ||
EVM_ARITH_VER_KEY, | ||
// see build.rs | ||
env!("EVM_ARITHMETIZATION_PACKAGE_VERSION"), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this bit sets the entire version number, i.e. X.Y.Z
while we're not interested in the patch section. Any increment of Z
should not yield circuits
regeneration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know the code that reads this, but I thought it would be safer (and rare enough) to include. I'll take your lead on this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM on the versioning part.
This blocks #309
Breaking changes
rpc
,worker
,verifier
, andleader
, rather than separateworker
andleader
containers. Users may have to select anENTRYPOINT
or pass additional parameters to their run command.env
file from the repo in our docker buildChange summary:
zk_evm/zero_bin/leader/src/utils.rs
Lines 45 to 72 in 653b7e4
evm_arithmetization/src/lib.rs
: can anyone educate me?Dockerfile
, with:cargo build --dev
.dockerignore
,apt install
...sed
iting ourCargo.toml
with a 200-character long commandRUSTFLAGS