-
Notifications
You must be signed in to change notification settings - Fork 18.9k
Use a separate docker image for cross compiling #26389
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
Conversation
dockerfiles/Dockerfile.cross
Outdated
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 suspect a bunch of these aren't necessary, but I haven't gone through each yet.
Does anyone know offhand of any of these are runtime dependencies and aren't required for compiling?
|
microservices! |
|
/me likes it 👼 |
604670d to
a196b3d
Compare
|
I really like this. This will reduce bloat by so much for people who aren't routinely doing cross-compiles. |
|
Yes, definitely an improvement. Will try it out shortly. |
Dockerfile
Outdated
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.
Is is valid to test the existence of /.dockerenv instead of introducing this environment variable?
I'm not sure /.dockerenv will continue to exist in future releases, but many people have been already using that file for checking whether running in Docker:
https://github.com/search?utf8=%E2%9C%93&q=if+-f+%2F.dockerenv&type=Code&ref=searchresults
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 am inclined to remove .dockerenv when we get introspection, so I wouldnt use it here.
|
Looks like janky and experimental are failing with this: |
|
I think to make this work with CI we are going to need a separate jenkins build for cross+tgz. |
Signed-off-by: Daniel Nephin <dnephin@docker.com>
a196b3d to
da68b42
Compare
|
good idea! needs a rebase @dnephin |
|
It's blocked on #27359 , but it doesn't replace it |
|
@dnephin we're looking at this one, but it looks like we should be able to merge this without the other PR, correct? |
|
Not really. It breaks janky because we run builds in a single image. We need a way to tell it to use different images for different builds. Making that change now is difficult because it would break any branches that don't include this change. The Jenkinsfile makes this a much easier operation, because the build config is stored in the branch. |
| -t ${DOCKER_CROSS_IMAGE} \ | ||
| -f dockerfiles/Dockerfile.cross dockerfiles/ | ||
|
|
||
| cross: bundles build-cross ## cross build the binaries for darwin, freebsd and\nwindows |
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.
stale \n
|
|
||
| tgz: build ## build the archives (.zip on windows and .tgz\notherwise) containing the binaries | ||
| $(DOCKER_RUN_DOCKER) hack/make.sh dynbinary binary cross tgz | ||
| tgz: build cross ## build the archives (.zip on windows and .tgz\notherwise) containing the binaries |
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.
why there are \n in comments?
|
I opened #28952 to replace this |
Instead of attempting to use one monolithic docker image for all the build tasks we should aim to have images for each logical group of tasks. This PR creates a new Dockerfile for running the cross build.
cc @justincormack - we talked about this earlier this week