Fix cross compile build #78

dnephin · 2017-05-12T16:10:59Z

Fixes #26
Fixes #75

-tags pkcs11 is working for the dynbinary on linux and osx. It's not working on windows. I'm not sure if we need it on windows.

The OSX cross has some warnings "was built for newer OSX version (10.11) than being linked (10.6)". I'm using the same OSX SDK as docker/docker, so I'm guessing these have existed for a while?

linux/ppc64le is broken because of pkg/term, needs a fix in moby/moby

linux/s390x is blocked on mitchellh/gox#85. We should just run a fork of gox to support it. However we might hit the same problem as ppc64le with pkg/term

dnephin · 2017-05-12T16:16:40Z

My cleanup of Dockerfile.cross should let us use it for CI which will fix the build

Cleanup dynbinary and binary builds Signed-off-by: Daniel Nephin <dnephin@docker.com>

Signed-off-by: Daniel Nephin <dnephin@docker.com>

andrewhsu

~~can we not have parallel as a requirement?~~

andrewhsu · 2017-05-12T23:22:17Z

scripts/build/cross

-	github.com/docker/cli/cmd/docker
+export BUILDDIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
+SHELL=/bin/bash parallel ::: \


do we need parallel?

$ make -f docker.Makefile cross sha256:2860f619648726547325ceec0b8768f370a1973dae7f16b91d1b54d89c1e954e ./scripts/build/cross: line 7: parallel: command not found make: *** [Makefile:28: cross] Error 127 docker.Makefile:46: recipe for target 'cross' failed make: *** [cross] Error 2

Oops, docker.Makefile was using the wrong image!

Thanks, just pushed a fix.

Fix CI build Signed-off-by: Daniel Nephin <dnephin@docker.com>

dnephin · 2017-05-14T17:31:30Z

@andrewhsu what's wrong with parallel ? If you want to run them sequentially you can always call the 3 scripts directly.

mlaventure · 2017-05-15T14:57:01Z

README.md


 ```
-$ make clean
+$ make -f docker.Makefile binary


If building with docker is the default as specified in the beginning of that section, then I would make docker.Makefile the default Makefile and the in container one the secondary one.

i.e.: as an user I just want to do make, make binary or make cross directly. This is the most common behavior and doesn't force someone to have to read the README if they just want the binary.

Yes, I understand that.

My concern is that docker.Makefile is not really a good use of make. Every target is effectively .PHONY. It is not really taking advantage of any of what Make provides. It could just as easily be a bash script, or ideally a tool that is actually designed for this use case.

We should be looking to remove docker.Makefile and replace it with something else, which is why it's named this way. We could very easily drop in a dobi.yaml for this purpose and it would provide a lot of additional benefits.

It should also (eventually) be merged with what is now circle.yml so we don't have a separate copy for CI tasks. This is not really possible with circleci, just yet.

We could, for now, create a script that wraps this make -f docker.Makefile so the instructions would be something like ./task binary ./task lint, etc. This would give us a shim to transition to new build tools and would probably be a lot more concise when what we have in docker.Makefile right now.

Since this PR provides some necessary support for building cross, I'd like to get this merged first and address that in a separate PR.

WDTY?

Since this is blocking quite a few things, I'm fine with merging it as it is.

But I'm afraid we'll end up breaking people relying on the cli downstream if we don't update quickly afterwards.

make is the standard, but I agree it is ill fitted to deal with output depending on docker images (depending on the Dockerfiles themselves would be an improvement though).

Thanks.

I agree make is pretty standard for some languages, but I don't see it widely used by many golang projects. If it is a standard it's not very widely adopted. I think there are a lot more issues with make beyond just handling docker images, but Ill leave that for another thread.

I would be fine with having a build.sh script instead. Not for this PR, just my 2¢.

mlaventure · 2017-05-15T15:05:34Z

dockerfiles/Dockerfile.cross

+            libltdl-dev \
+            gcc-mingw-w64 \
+            parallel \
+            ;


Is this ; necessary?

It's necessary because of the \ on the previous line. I like this style because it lets you add items to the list, and keep it sorted, without worrying about the special case of no \ on the last line.

It's a minor thing though, and I'm fine to change it if you prefer I drop the \ and ;

🤷‍♂️ , I'm more used to the other way around, I was just wondering if this was adding any side effect I wasn't aware off.

mlaventure · 2017-05-15T15:08:48Z

docker.Makefile


 # build the CLI for multiple architectures using a container
 .PHONY: cross
 cross: build_docker_image


should be build_cross_image

mlaventure · 2017-05-15T15:19:12Z

scripts/build/linux-cross

+# Not yet supported by gox
+# linux/s390x
+
+gox -output build/docker-{{.OS}}-{{.Arch}} \


Can we print something here (and the others) telling what we're building? The compilation can take quite a time sometimes, and all we get is the sha output of the container being used (and this one is there without context). It's kind of weird to see a blank terminal and not knowing if the program is stuck or doing something.

A simple Building {x}... would be nice :)

dnephin · 2017-05-15T16:01:50Z

Added echo about building and fixed the cross dependency. Let me know what you think about the other two

mlaventure

one nit but LGTM

mlaventure · 2017-05-15T17:35:32Z

docker.Makefile


+.PHONY: build_cross_image
+build_cross_image:
+	@docker build -t $(CROSS_IMAGE_NAME) -f ./dockerfiles/Dockerfile.cross .


missing -q

I actually left this out on purpose. I prefer to see the build output. If we get to a point where the build output is too long, it's probably a sign that we should fix our dockerfile.

What do you think? I would like to make these consistent. Should I remove -q from the others?

One way of the other, as long as it's consistent :).

I actually prefer having the output too.

Remove referenced to developing on the host, we shouldn't support it. Move script/validate to scripts/validate to be consistent. Set the default target to be binary instead of clean. Signed-off-by: Daniel Nephin <dnephin@docker.com>

mlaventure

LGTM

/cc @tiborvass @andrewhsu

tiborvass · 2017-05-15T18:52:56Z

The OSX cross has some warnings "was built for newer OSX version (10.11) than being linked (10.6)". I'm using the same OSX SDK as docker/docker, so I'm guessing these have existed for a while?

Yes those are normal.

andrewhsu · 2017-05-15T18:55:32Z

LGTM

stuff works:

$ make -f docker.Makefile cross
$ ls -al build
total 70936
-rwxr-xr-x  1 root   root   20487656 May 15 18:49 docker-darwin-amd64
-rwxr-xr-x  1 root   root   18359704 May 15 18:49 docker-linux-amd64
-rwxr-xr-x  1 root   root   15234812 May 15 18:49 docker-linux-arm
-rwxr-xr-x  1 root   root   18540544 May 15 18:49 docker-windows-amd64

tiborvass · 2017-05-15T19:31:22Z

LGTM I checked the osx binaries' dependencies they match with what's released.

Uncomment TasksMax=unlimited for recent distros Upstream-commit: 33325c8a2598c94bc5136a9fe7213e3ebe152dd7 Component: packaging

GordonTheTurtle added the status/0-triage label May 12, 2017

vdemeester added status/2-code-review and removed status/0-triage labels May 12, 2017

vdemeester requested review from andrewhsu, mlaventure and tiborvass May 12, 2017 16:13

dnephin mentioned this pull request May 12, 2017

add cross compile for linux/ppc64le #33

Closed

dnephin force-pushed the fix-cross-build branch from b996502 to db4dd3b Compare May 12, 2017 17:14

dnephin mentioned this pull request May 12, 2017

Support building a dynbinary #76

Closed

dnephin force-pushed the fix-cross-build branch from db4dd3b to 07f2564 Compare May 12, 2017 17:33

dnephin added 2 commits May 12, 2017 13:34

Support building a dynbinary

a787cbc

Cleanup dynbinary and binary builds Signed-off-by: Daniel Nephin <dnephin@docker.com>

Add windows and linux cross.

84a7788

Signed-off-by: Daniel Nephin <dnephin@docker.com>

dnephin force-pushed the fix-cross-build branch 2 times, most recently from 7f3e5f0 to 3ae1064 Compare May 12, 2017 17:55

andrewhsu suggested changes May 12, 2017

View reviewed changes

dnephin force-pushed the fix-cross-build branch from 3ae1064 to 5d513f8 Compare May 13, 2017 00:00

osx cross

a408fb0

Fix CI build Signed-off-by: Daniel Nephin <dnephin@docker.com>

dnephin force-pushed the fix-cross-build branch from 5d513f8 to a408fb0 Compare May 13, 2017 17:48

dnephin mentioned this pull request May 14, 2017

Fix build #81

Closed

dnephin force-pushed the fix-cross-build branch from e5e835b to 09da11e Compare May 15, 2017 14:51

mlaventure suggested changes May 15, 2017

View reviewed changes

dnephin force-pushed the fix-cross-build branch from 09da11e to 48e294b Compare May 15, 2017 16:01

mlaventure approved these changes May 15, 2017

View reviewed changes

Update the README

60b2dd3

Remove referenced to developing on the host, we shouldn't support it. Move script/validate to scripts/validate to be consistent. Set the default target to be binary instead of clean. Signed-off-by: Daniel Nephin <dnephin@docker.com>

dnephin force-pushed the fix-cross-build branch from 48e294b to 60b2dd3 Compare May 15, 2017 18:08

mlaventure approved these changes May 15, 2017

View reviewed changes

tiborvass merged commit a222527 into docker:master May 15, 2017

GordonTheTurtle added this to the 17.06.0 milestone May 15, 2017

dnephin deleted the fix-cross-build branch May 15, 2017 19:31

This was referenced May 16, 2017

Make fails with make: *** [binary] Error 1 #94

Closed

Improve makefile situation #99

Closed

thaJeztah mentioned this pull request Jul 26, 2022

missing DWARF symbol table #3717

Merged

nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025

Merge pull request docker#78 from kolyshkin/tasks-max

3fe329a

Uncomment TasksMax=unlimited for recent distros Upstream-commit: 33325c8a2598c94bc5136a9fe7213e3ebe152dd7 Component: packaging

Fix cross compile build #78

Fix cross compile build #78

Uh oh!

Conversation

dnephin commented May 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnephin commented May 12, 2017

Uh oh!

andrewhsu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnephin commented May 14, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnephin commented May 15, 2017

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

tiborvass commented May 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewhsu commented May 15, 2017

Uh oh!

tiborvass commented May 15, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dnephin commented May 12, 2017 •

edited

Loading

andrewhsu left a comment •

edited

Loading

tiborvass commented May 15, 2017 •

edited

Loading