Skip to content

Conversation

@StefanScherer
Copy link
Member

Signed-off-by: Stefan Scherer scherer_stefan@icloud.com

- What I did

I installed the static Windows binaries 17.06.0-ce-rc2 and found out that the docker cli is compiled with go1.8.1

PS C:\Users\stefan> docker version
Client:
 Version:      17.06.0-ce-rc2
 API version:  1.30
 Go version:   go1.8.1
 Git commit:   402dd4a
 Built:        Thu Jun  8 02:36:20 2017
 OS/Arch:      windows/amd64

Server:
 Version:      17.06.0-ce-rc2
 API version:  1.30 (minimum version 1.24)
 Go version:   go1.8.3
 Git commit:   402dd4a
 Built:        Thu Jun  8 02:47:18 2017
 OS/Arch:      windows/amd64
 Experimental: false

So I think this Dockerfile to cross-compile the docker cli should be updated as well as the release notes show us an explicit update to Go 1.8.3 moby/moby#33387

- How I did it

Just updating the Dockerfile.cross

- How to verify it

Good question, a how-to guide for the community would help me verify the whole build. To build the Windows ZIP I use docker/docker-ce repo. So how to test my pull request in docker/cli repo to build the ZIP in docker/docker-ce repo? I don't have a best practise for that right now.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@StefanScherer
Copy link
Member Author

Same applies to OSX Docker cli

$ docker/docker version
Client:
 Version:      17.06.0-ce-rc2
 API version:  1.27 (downgraded from 1.30)
 Go version:   go1.8.1
 Git commit:   402dd4a
 Built:        Thu Jun  8 02:26:19 2017
 OS/Arch:      darwin/amd64

@codecov-io
Copy link

codecov-io commented Jun 9, 2017

Codecov Report

Merging #173 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
- Coverage   45.33%   45.32%   -0.01%     
==========================================
  Files         171      171              
  Lines       11480    11480              
==========================================
- Hits         5204     5203       -1     
- Misses       5979     5980       +1     
  Partials      297      297

@tiborvass
Copy link
Collaborator

LGTM

@@ -1,5 +1,5 @@

FROM golang:1.8.1
FROM golang:1.8.3
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the Dockerfile.dev floats the golang version to latest patch of 1.8. I'd suggest floating the version in Dockerfile.cross as well so they match, .e.g. FROM golang:1.8

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure; we've had cases where a patch release brought a specific fix, using a rolling update would make it undeterministic

Copy link
Contributor

Choose a reason for hiding this comment

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

Then I'd suggest making sure all Dockerfiles are locked into a specific golang version. The 3 files in dockerfiles dir.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that would be my personal preference

Copy link
Member Author

Choose a reason for hiding this comment

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

updated the other two Dockerfiles as well to use golang 1.8.3

@andrewhsu andrewhsu mentioned this pull request Jun 9, 2017
40 tasks
@andrewhsu
Copy link
Contributor

In regards to the comment on how to verify, since the docker.Makefile cross target is used to build docker-ce 17.06.0 rc2, making sure the golang version is in the output of the build will do:

$ make -f docker.Makefile cross
$ build/docker-darwin-amd64 version # make sure Go version is 1.8.3

Signed-off-by: Stefan Scherer <scherer_stefan@icloud.com>
@StefanScherer
Copy link
Member Author

Thanks @andrewhsu for the explanation, now I can build another part of the new repos.

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

I just realized that circle CI has upgraded to Docker 17.05 so we can use ARG in FROM.

But no worries if you don't want to do that here. I can do it in a follow up PR.

Edit: actually, I'm not sure if it has. I misread the build output.

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

nevermind, it's still on 17.03.0-ce

@andrewhsu
Copy link
Contributor

LGTM

@tiborvass tiborvass merged commit 6a963c5 into docker:master Jun 9, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.07.0 milestone Jun 9, 2017
@thaJeztah thaJeztah modified the milestones: 17.06.0, 17.07.0 Jun 9, 2017
@StefanScherer StefanScherer deleted the update-golang-1.8.3 branch June 10, 2017 05:22
@StefanScherer
Copy link
Member Author

With Circle 2.0 you could update Docker Engine to 17.05 for multi-stage builds. No longer a special handcrafted Circle Docker Engine.

@dnephin
Copy link
Contributor

dnephin commented Jun 10, 2017

We're using Circle 2.0, and we use a docker:17.05 image, but that only give us the client. The "remote daemon" step spins up a 17.03 daemon.

nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
[17.07] backport Fixing issue with driver opt not passed to drivers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants