8000
Skip to content

Bump Golang to 1.9.4 - #36243

Merged
yongtang merged 2 commits into
moby:masterfrom
thaJeztah:bump-golang-1.9.4
Feb 8, 2018
Merged

Bump Golang to 1.9.4#36243
yongtang merged 2 commits into
moby:masterfrom
thaJeztah:bump-golang-1.9.4

Conversation

@thaJeztah
@thaJeztah thaJeztah commented Feb 8, 2018
Copy link
Copy Markdown
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.

 

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
thaJeztah commented Feb 8, 2018
Copy link
Copy Markdown
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
thaJeztah commented Feb 8, 2018 •
edited
Loading
Copy link
Copy Markdown
Member Author

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
Comment thread cmd/dockerd/docker_windows.go
_ "github.com/docker/docker/autogen/winresources/dockerd"
)

//go:cgo_import_dynamic main.dummy CommandLineToArgvW%2 "shell32.dll"
@thaJeztah thaJeztah Feb 8, 2018
Copy link
Copy Markdown
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
@vdemeester vdemeester left a comment
Copy link
Copy Markdown
Member

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
@yongtang yongtang left a comment
Copy link
Copy Markdown
Member

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

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes
@cpuguy83 cpuguy83 Awaiting requested review from cpuguy83
+2 more reviewers
@vdemeester vdemeester vdemeester approved these changes
@yongtang yongtang yongtang approved these changes
Reviewers whose approvals may not affect merge requirements

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
0