10BC0 hack/ci/windows.ps1: fix Go version check (due to trailing .0) by thaJeztah · Pull Request #39879 · moby/moby · GitHub
[go: up one dir, main page]
Skip to content

hack/ci/windows.ps1: fix Go version check (due to trailing .0) #39879

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 1 commit into from
Sep 12, 2019
Merged

hack/ci/windows.ps1: fix Go version check (due to trailing .0) #39879

merged 1 commit into from
Sep 12, 2019

Conversation

thaJeztah
Copy link
Member

(taken from #39549)

Similar to the fix I did in #39582, but where I made the wrong assumption that the GO_VERSION variable on Linux and Windows would be the same. However, the Windows Dockerfile downloads the Go binaries, which (unlike the Golang images) do not have a trailing .0 in their version.

@thaJeztah thaJeztah requested a review from tianon as a code owner September 7, 2019 15:58
@thaJeztah
Copy link
Member Author

@StefanScherer @vikramhh ptal

@vikramhh
Copy link
vikramhh commented Sep 7, 2019

The proposed change below will convert 1.10.0 to 1.1 which is probably unintended.

$goVersionDockerfileWindows=(Select-String -Path ".\Dockerfile.windows" -Pattern "^ARG[\s]+GO_VERSION=(.*)$").Matches.groups[1].Value.TrimEnd(".0")

Replacing TrimEnd with -replace should help in removing the trailing .0 without any side effects like above:

$goVersionDockerfileWindows=(Select-String -Path ".\Dockerfile.windows" -Pattern "^ARG[\s]+GO_VERSION=(.*)$").Matches.groups[1].Value -replace '\.0$',''

The following takes care of converting x.0.0 to x as well [if you would rather account for 2.0.0 too]:

$goVersionDockerfileWindows=(Select-String -Path ".\Dockerfile.windows" -Pattern "^ARG[\s]+GO_VERSION=(.*)$").Matches.groups[1].Value -replace '\.0$','' -replace '\.0$',''

@StefanScherer
Copy link
Contributor

Agreed, the -replace '\.0$','' is better to remove the trailing .0. 👍

@thaJeztah
Copy link
Member Author

Ah, dang, yes .TrimEnd() uses a cut-set, not a "string to remove"; I'll update

@thaJeztah
hack/ci/windows.ps1: fix Go version check (due to trailing .0)
61450a6 
The Windows Dockerfile downloads the Go binaries, which (unlike
the Golang images) do not have a trailing `.0` in their version.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the fix_go_version_check_again branch from d90fb92 to 61450a6 Compare September 10, 2019 09:41
@thaJeztah
Copy link
Member Author

🤦‍♂ I actually forgot we modified the Dockerfile to remove the trailing .0 when downloading, so my "bump Go 1.13" PR should use the same version as Linux (with the trailing .0); I slightly modified the scripts and Dockerfile to use the -replace; PTAL

@vikramhh
Copy link

LGTM

@thaJeztah
Copy link
Member Author

Windows RS1 failure is unrelated https://ci.docker.com/public/blue/rest/organizations/jenkins/pipelines/moby/branches/PR-39879/runs/2/nodes/50/log/?start=0

tracked through #39857, and currently skipped on Windows on master (#39887)

[2019-09-10T10:01:18.173Z] time="2019-09-10T10:01:17Z" level=info msg="Log stream already exists" errorCode=ResourceAlreadyExistsException logGroupName= logStreamName= message= origError="<nil>"
[2019-09-10T10:01:18.173Z] --- FAIL: TestLogBlocking (0.04s)
[2019-09-10T10:01:18.173Z]     cloudwatchlogs_test.go:313: Expected to be able to read from stream.messages but was unable to
[2019-09-10T10:01:18.173Z] time="2019-09-10T10:01:17Z" level=error msg=Error
[2019-09-10T10:01:18.173Z] time="2019-09-10T10:01:17Z" level=error msg="Failed to put log events" errorCode=InvalidSequenceTokenException logGroupName=groupName logStreamName=streamName message="use token token" origError="<nil>"
[2019-09-10T10:01:18.173Z] time="2019-09-10T10:01:17Z" level=error msg="Failed to put log events" errorCode=DataAlreadyAcceptedException logGroupName=groupName logStreamName=streamName message="use token token" origError="<nil>"
[2019-09-10T10:01:18.173Z] time="2019-09-10T10:01:17Z" level=info msg="Data already accepted, ignoring error" errorCode=DataAlreadyAcceptedException logGroupName=groupName logStreamName=streamName message="use token token"
[2019-09-10T10:01:18.173Z] FAIL

everything else is green

StefanScherer
StefanScherer approved these changes Sep 11, 2019
View reviewed changes
Copy link
Contributor
@StefanScherer StefanScherer 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 tiborvass merged commit 2900a91 into moby:master Sep 12, 2019
@thaJeztah thaJeztah deleted the fix_go_version_check_again branch September 12, 2019 06:20
@thaJeztah thaJeztah mentioned this pull request Sep 25, 2019
Merged
@thaJeztah thaJeztah mentioned this pull request Oct 8, 2019
Merged
@thaJeztah thaJeztah mentioned this pull request Oct 22, 2019
Merged
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiborvass tiborvass tiborvass approved these changes

@tianon tianon Awaiting requested review from tianon tianon is a code owner

+1 more reviewer

@StefanScherer StefanScherer StefanScherer approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
area/testing platform/windows process/cherry-picked status/2-code-review
Projects
None yet
Milestone
20.10.0
Development

Successfully merging this pull request may close these issues.
5 participants
@thaJeztah @vikramhh @StefanScherer @tiborvass @GordonTheTurtle
0