Skip to content

Bump Golang to 1.9.4 #36243

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 8, 2018
Merged

Bump Golang to 1.9.4 #36243

merged 2 commits into from
Feb 8, 2018

Conversation

thaJeztah
Copy link
Member

This fixes a vulnerability in go get (CVE-2018-6574, http://golang.org/issue/23672),
but shouldn't really affect our code, but it's good to keep in sync.

@thaJeztah
Bump Golang to 1.9.4
caeab26 
This fixes a vulnerability in `go get` (CVE-2018-6574, http://golang.org/issue/23672),
but shouldn't really affect our code, but it's good to keep in sync.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

ping @jhowardmsft @johnstep looks like the fix in Golang actually broke our Windows build (see the patch: golang/go@867fb18)

06:37:59 # github.com/docker/docker/cmd/dockerd
06:37:59 .\docker_windows.go:9:3: //go:cgo_import_dynamic main.dummy CommandLineToArgvW%2 "shell32.dll" only allowed in cgo-generated code
06:37:59 
06:37:59 ERROR: make.ps1 failed:
06:37:59 Failed to compile daemon
06:37:59 
06:37:59 ___________      .__.__             .___
06:37:59 \_   _____/____  |__|  |   ____   __| _/
06:37:59  |    __) \__  \ |  |  | _/ __ \ / __ | 
06:37:59  |     \   / __ \|  |  |_\  ___// /_/ | 
06:37:59  \___  /  (____  /__|____/\___  >____ | 
06:37:59      \/        \/             \/     \/ 
06:37:59 
06:37:59 INFO: make.ps1 ended at 02/08/2018 06:37:59
06:37:59 Failed to compile daemon
06:37:59 At C:\go\src\github.com\docker\docker\hack\make.ps1:189 char:32
06:37:59 +     if ($LASTEXITCODE -ne 0) { Throw "Failed to compile $type" }
06:37:59 +                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
06:37:59     + CategoryInfo          : OperationStopped: (Failed to compile daemon:Stri 
06:37:59    ng) [], RuntimeException
06:37:59     + FullyQualifiedErrorId : Failed to compile daemon
06:37:59  
06:38:00 
06:38:00 
06:38:00 ERROR: Failed 'ERROR: Failed to build binary' at 02/08/2018 06:38:00

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Feb 8, 2018
@thaJeztah thaJeztah mentioned this pull request Feb 8, 2018
Merged
@thaJeztah
Copy link
Member Author

thaJeztah commented Feb 8, 2018 •
edited
Loading

Looks like this was added in fa82c0a (#22840) to work around an issue on Nano Server TP5 (see golang/go#15286)

I see that fix was removed in Docker CLI (docker/cli#8), and looks like it's no longer needed, so I'll remove that code

@thaJeztah
Remove workaround for Nano server TP5
2501933 
This workaround for golang/go#15286 was added for Nano server TP5 in
fa82c0a, and should no longer be
needed

Due to a security fix in Go 1.9.4/1.8.7, loading the .dll is no longer
allowed, and produces an error:

   .\docker_windows.go:9:3: //go:cgo_import_dynamic main.dummy CommandLineToArgvW%2 "shell32.dll" only allowed in cgo-generated code

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah
thaJeztah commented Feb 8, 2018
View reviewed changes
cmd/dockerd/docker_windows.go
_ "github.com/docker/docker/autogen/winresources/dockerd"
)

//go:cgo_import_dynamic main.dummy CommandLineToArgvW%2 "shell32.dll"
Copy link
Member Author

Choose a reason for hiding this comment

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

I assume this is no longer needed @jstarks ?

@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Feb 8, 2018
vdemeester
vdemeester approved these changes Feb 8, 2018
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM if green 🐸

yongtang
yongtang approved these changes Feb 8, 2018
View reviewed changes
Copy link
Member

@yongtang yongtang left a comment

Choose a reason for hiding this comment

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

LGTM

@yongtang yongtang merged commit 6f33b5a into moby:master Feb 8, 2018
@thaJeztah thaJeztah deleted the bump-golang-1.9.4 branch February 8, 2018 15:42
@thaJeztah thaJeztah mentioned this pull request Feb 8, 2018
Merged
@thaJeztah thaJeztah mentioned this pull request Mar 21, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdemeester vdemeester vdemeester approved these changes

@yongtang yongtang yongtang approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@cpuguy83 cpuguy83 Awaiting requested review from cpuguy83

Assignees
No one assigned
Labels
status/2-code-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@thaJeztah @vdemeester @yongtang @AkihiroSuda @GordonTheTurtle