-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
feat: alpine based docker image #981
Conversation
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.
You can further shrink the image if you run strip /usr/src/foundry/target/release/cast
and strip /usr/src/foundry/target/release/forge
Also if this builds off of #914 let's merge into that branch instead :)
This commit creates a docker image that includes both `cast` and `forge`. It builds off of foundry-rs#914. The image comes out to 26.4MB. ```bash / # du -h /usr/local/bin/forge 12.8M /usr/local/bin/forge / # du -h /usr/local/bin/cast 7.1M /usr/local/bin/cast ``` Example usage: ```bash $ docker run --rm --entrypoint cast foundry:latest block --rpc-url https://mainnet.optimism.io latest $ docker run --rm foundry:latest 'cast block --rpc-url https://mainnet.optimism.io latest' ``` Co-authored-by: Abdul Rabbani
Good looks on using I opened a PR that doesn't merge into #914 because the author said they don't have time to contribute more, see https://github.com/gakonst/foundry/pull/914/files#r830395267. If you are strongly opinionated on me opening this PR against their repo, I can. |
Dockerfile
Outdated
WORKDIR /usr/src/foundry | ||
COPY . . | ||
|
||
ENV RUSTFLAGS="-C target-cpu=native" |
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.
Will targeting native cpu instruction set require a multi-architecture docker build when this image becomes part of a CI/CD pipeline?
@tynes running into the classic alpine solc issues when trying to compile with forge in the image. Are you seeing this? Could just be an issue on my local.
|
Thanks for pointing this out. It turns out that
After installing that package, a new error happens
Perhaps we need a
|
@tynes take a look at https://github.com/abdulrabbani00/foundry/pull/3/files. Been taking a look at this for a couple nights. I opened this PR in order to build off of #914. I think it gets the job done but def needs more eyes and testing. |
If we are migrating to this PR happy to open a PR against your repo as well :) |
Awesome, thanks for looking into this problem! I'm gonna be mostly afk this weekend so feel free to coordinate with the other PR or open your own if you are around this weekend. Happy to help test/do whatever to get this over the line :) |
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.
@tynes opened a PR into your branch. tynes#1. This dockerfile should resolve the issues we saw above with solc (tested on a number of platforms). If you rebase or merge that in than we can continue using this PR (or I can open a new one if you'd prefer that and mark you as co-author if that's less of a burden). |
Dockerfile
Outdated
ENV RUSTFLAGS="-C target-cpu=native" | ||
RUN cargo build --release | ||
|
||
RUN strip /usr/src/foundry/target/release/cast \ |
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.
Rust has built-in stripping since 1.59, could that be used instead?
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.
TIL,
perhaps we add a stripped
profile that inherits
from release
and sets strip
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.
Defer to whatever @tynes wants here, I think they're equivalent?
Dockerfile
Outdated
WORKDIR /usr/src/foundry | ||
COPY . . | ||
|
||
ENV RUSTFLAGS="-C target-cpu=native" |
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.
not sure about this one, this may be fine when built for your own PC but probably not when push to ghcr or dockerhub?
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.
Good Q. I am not sure. I think let's remove.
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.
Yeah mentioned concerns with this above. I think it should only be used for local installs.
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.
Dockerfile
Outdated
WORKDIR /usr/src/foundry | ||
COPY . . | ||
|
||
ENV RUSTFLAGS="-C target-cpu=native" |
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.
Good Q. I am not sure. I think let's remove.
Dockerfile
Outdated
WORKDIR /usr/src/foundry | ||
COPY . . | ||
|
||
ENV RUSTFLAGS="-C target-cpu=native" |
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.
ENV RUSTFLAGS="-C target-cpu=native" |
Dockerfile
Outdated
ENV RUSTFLAGS="-C target-cpu=native" | ||
RUN cargo build --release | ||
|
||
RUN strip /usr/src/foundry/target/release/cast \ |
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.
Defer to whatever @tynes wants here, I think they're equivalent?
I think what might be best is to merge a Dockerfile first (but don't merge this PR yet, as there are outstanding changes needed here tynes#1), and then open a separate PR for CI integration. We have 2 PRs (three if I were to open one rather than PRing into @tynes branch) for Docker right now. I say we get them closed / merged and we can start from a clean slate for CI integration. |
Yeah, I have an example workflow here, and a successful run and publish [here](: https://github.com/dmfxyz/foundry/runs/5615229053?check_suite_focus=true). But it needs some more refinement and won't be ready for a bit. |
re-do dockerfile to build from scratch alpine, fix solc
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! let's ship this and we can do the GHA release separately
Motivation
I have tasks that need to run in the cloud where bringing up a K8s pod
that I can attach to with
cast
would be nice. A docker image is requiredfor this and I'm sure other people would want a docker image so putting
the dockerfile here makes sense.
Solution
This commit creates a docker image that includes
both
cast
andforge
.It builds off of #914.
The image comes out to 26.4MB.
Example usage:
$ docker run --rm --entrypoint cast foundry:latest block --rpc-url https://mainnet.optimism.io latest $ docker run --rm foundry:latest 'cast block --rpc-url https://mainnet.optimism.io latest'
Co-authored-by: Abdul Rabbani
One question is whether or not the container should include
bash
- some people might want to mount in bash scripts to run. I personally think yes but did not includebash
in the image as to keep it minimal, will update if there is consensus around this.