Skip to content
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

dockerfile based binary building #2993

Merged
merged 7 commits into from
Apr 7, 2021
Merged

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Mar 3, 2021

Using cross-compilation toolchains that work from any platform
Adds darwin/arm64 support and bake targets. Static and dynamic
binary targets are available, both with glibc and musl.

Build static binary for the current architecture.

VERSION=20.10.6 docker buildx bake

Equivalent:

docker buildx build --platform=local --output ./build

Out of tree:

docker buildx bake "git://github.com/tonistiigi/docker-cli#xx-build"

docker buildx bake "git://github.com/tonistiigi/docker-cli#xx-build" binary

Any other platform

docker buildx bake --set binary.platform=darwin/arm64
docker buildx build --platform=darwin/arm64 --output ./build

Build dynamic binaries

docker buildx bake dynbinary
USE_GLIBC=1 docker buildx bake dynbinary # debian based build

All supported platforms

docker buildx bake cross
docker buildx bake dynbinary-cross

Here is the output: https://gist.github.com/tonistiigi/7f16d059ae97d790bdde314f23f2c94d

Toolchain build from tonistiigi/xx#14 (some early docs: https://gist.github.com/tonistiigi/ebcac239dd3baef6747fc54f3f73cbba)

TODO:

  • Windows resources with windres not currently added
  • Update the callers
  • Remove more old targets
  • Examples

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

@thaJeztah
Copy link
Member

I guess this means we can remove (a couple of) the dockerfiles in https://github.com/docker/cli/tree/master/dockerfiles, correct?

@tonistiigi
Copy link
Member Author

@thaJeztah Yes, mentioned in 61ae3bb . Only did the parts that were not optional to the binary building.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(apparently I had some review comments, in draft but not submitted; let me post them 😂)

/work/build
- run: apk add make curl
- run: mkdir -vp ~/.docker/cli-plugins/
- run: curl --silent -L --output ~/.docker/cli-plugins/docker-buildx https://github.com/docker/buildx/releases/download/v0.5.1/buildx-v0.5.1.linux-amd64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- run: curl --silent -L --output ~/.docker/cli-plugins/docker-buildx https://github.com/docker/buildx/releases/download/v0.5.1/buildx-v0.5.1.linux-amd64
- run: curl -fsSL --output ~/.docker/cli-plugins/docker-buildx https://github.com/docker/buildx/releases/download/v0.5.1/buildx-v0.5.1.linux-amd64

Perhaps we could use a BUILDX_VERSION env-var for easier grepping and updating?

Suggested change
- run: curl --silent -L --output ~/.docker/cli-plugins/docker-buildx https://github.com/docker/buildx/releases/download/v0.5.1/buildx-v0.5.1.linux-amd64
- run: curl -fsSL --output ~/.docker/cli-plugins/docker-buildx https://github.com/docker/buildx/releases/download/"${BUILDX_VERSION}"/buildx-"${BUILDX_VERSION}".linux-amd64

Using cross compilation toolchains that work from any platform
Adds darwin/arm64 support and bake targets. Static and dynamic
binary targets are available, both with glibc and musl.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
New solution is not hardcoded to amd64 but integrates
with the cross toolchain and support creating arm binaries.

Go has been updated so that ASLR works

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
More can be removed/refactored but avoiding a huge change.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #2993 (c60419f) into master (59fd6f0) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head c60419f differs from pull request most recent head 26b633d. Consider uploading reports for the commit 26b633d to get more accurate results

@@            Coverage Diff             @@
##           master    #2993      +/-   ##
==========================================
- Coverage   57.08%   57.06%   -0.02%     
==========================================
  Files         299      299              
  Lines       18689    18683       -6     
==========================================
- Hits        10668    10662       -6     
  Misses       7155     7155              
  Partials      866      866              

@@ -1,14 +1,80 @@
#!/usr/bin/env bash
#!/usr/bin/env sh
#
# Build a static binary for the host OS/ARCH
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

Copy link
Collaborator

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can revert some of the Makefile changes for 20.10 branch to make less big changes there.

tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 6, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 6, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 6, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 6, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 7, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
@thaJeztah thaJeztah merged commit a32cd16 into docker:master Apr 7, 2021
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 7, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 8, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 8, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 8, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 8, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 8, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
PLATFORM=${PLATFORM:-}
VERSION=${VERSION:-"unknown-version"}
VERSION=${VERSION:-$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags | sed 's/^v//' 2>/dev/null || echo "unknown-version" )}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the 2>/dev/null was intended to be on git describe here, not sed, ala:

Suggested change
VERSION=${VERSION:-$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags | sed 's/^v//' 2>/dev/null || echo "unknown-version" )}
VERSION=${VERSION:-$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags 2>/dev/null | sed 's/^v//' || echo "unknown-version" )}

Otherwise we get (the otherwise harmless) fatal: not a git repository (or any of the parent directories): .git output on non-Git builds.

(I found this PR because I've got a job that's building CLI in a container, and it broke today with make: docker: Command not found so I got to go digging to figure out why, and found I now have to use ./scripts/build/binary directly with the variables that are defined in that file but documented in the Dockerfile -- I'm sure this is going to affect others who try to build the CLI in restricted environments too.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, and worse, the || only takes effect if the sed fails, so I'd suggest something like this instead:

Suggested change
VERSION=${VERSION:-$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags | sed 's/^v//' 2>/dev/null || echo "unknown-version" )}
VERSION=${VERSION:-$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags 2>/dev/null || echo "unknown-version" | sed 's/^v//' )}

tianon added a commit to tianon/dockerfiles that referenced this pull request Apr 8, 2021
Account for changes in docker/cli#2993
cpuguy83 added a commit to cpuguy83/docker-testing that referenced this pull request Apr 9, 2021
docker/cli builds changed with docker/cli#2993
Also minor fix to not clone by branch in order to make sure CLI_REPO
works.
Will follow-up to do the same for the other projects.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 9, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 9, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tiborvass pushed a commit to tiborvass/docker-ce-packaging that referenced this pull request Apr 9, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
docker-jenkins pushed a commit to docker-archive/docker-ce that referenced this pull request Apr 10, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
Upstream-commit: 5d52107
Component: packaging
@thaJeztah thaJeztah added this to the 21.xx milestone Apr 30, 2021
tianon added a commit to tianon/debian-moby that referenced this pull request Sep 22, 2021
This preemptively works around the changes in docker/cli#2993.
HeroCC pushed a commit to HeroCC/docker-ce-packaging that referenced this pull request Oct 6, 2021
In docker/cli#2993, Makefile target dynbinary
changed from a host script not depending on docker to a dockerized script.
Instead call the underlying script directly for deb/rpm.

I still think we should build deb/rpms using docker.

Signed-off-by: Tibor Vass <tibor@docker.com>
tianon added a commit to tianon/debian-moby that referenced this pull request Oct 15, 2021
This preemptively works around the changes in docker/cli#2993.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants