Skip to content
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.

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

hack/make.ps1: don't rely on GO_VERSION #37592

Merged
merged 1 commit into from Aug 16, 2018

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Aug 5, 2018

This removes the last use of ENV GO_VERSION which happened to be in hack/make.ps. Once this is merged, we can remove ENV GO_VERSION from the Dockerfile.

Apparently one can't modify both Dockerfile and hack/make.ps1 in a single commit (although I fail to understand why), so Dockerfile remains intact for now.

@codecov
Copy link

codecov bot commented Aug 6, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@678d4b3). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37592   +/-   ##
=========================================
  Coverage          ?   35.61%           
=========================================
  Files             ?      611           
  Lines             ?    44967           
  Branches          ?        0           
=========================================
  Hits              ?    16015           
  Misses            ?    26742           
  Partials          ?     2210

@kolyshkin kolyshkin force-pushed the no-go-version-env branch 2 times, most recently from 55df9e0 to 2e601b3 Compare August 6, 2018 09:03
@kolyshkin kolyshkin force-pushed the no-go-version-env branch 3 times, most recently from 77d1fd3 to 5d0616c Compare August 15, 2018 08:59
@kolyshkin
Copy link
Contributor Author

OK, looks like it didn't work only because this PR modified both Dockerfile and hack/make.ps1 in a single commit, which created some sort of access problem for Dockerfile (don't ask why). Split into two commits to make Windows CI happy again.

@kolyshkin
Copy link
Contributor Author

OK, it looks like one can not patch both Dockerfile and hack/make.ps1 in a single PR. Removing the Dockerfile patch.

@kolyshkin kolyshkin changed the title [DO NOT MERGE] Dockerfile: rm GO_VERSION hack/make.ps1: don't rely on GO_VERSION Aug 15, 2018
Modify hack/make.ps1 to use the version value used in
"FROM golang" statement.

While at it:
 1. Make search expression a bit more strict (use ^ to match at BOL only).
 2. Simplify by removing Get-Contents as Select-String can read files.

After this, ENV GO_VERSION can be removed from Dockerfile.
Unfortunately it can't be done in one commit as Windows CI
fails (presumably because Dockerfile is being modified in
place).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor Author

@johnstep @jhowardmsft PTAL; should be trivial to merge

Copy link
Member

@lowenna lowenna left a comment

Choose a reason for hiding this comment

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

LGTM

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 🐯

@vdemeester vdemeester merged commit b1ba744 into moby:master Aug 16, 2018
kolyshkin added a commit to kolyshkin/moby that referenced this pull request Sep 6, 2018
It's that time of year again! Go 1.11 is released, time to use it.

This commit also

* removes our archive/tar fork, since upstream archive/tar
  is fixed for static builds, and osusergo build tag is set.

* removes ENV GO_VERSION from Dockerfile as it's not needed
  anymore since PR moby#37592 is merged.

[v2: switch to beta2]
[v3: switch to beta3]
[v4: rc1]
[v5: remove ENV GO_VERSION as PR moby#37592 is now merged]
[v6: rc2]
[v7: final!]
[v8: use 1.11.0]
[v9: back to 1.11]
[v8: use 1.11.0]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
thaJeztah pushed a commit to thaJeztah/docker that referenced this pull request Jun 20, 2019
It's that time of year again! Go 1.11 is released, time to use it.

This commit also

* removes our archive/tar fork, since upstream archive/tar
  is fixed for static builds, and osusergo build tag is set.

* removes ENV GO_VERSION from Dockerfile as it's not needed
  anymore since PR moby#37592 is merged.

[v2: switch to beta2]
[v3: switch to beta3]
[v4: rc1]
[v5: remove ENV GO_VERSION as PR moby#37592 is now merged]
[v6: rc2]
[v7: final!]
[v8: use 1.11.0]
[v9: back to 1.11]
[v8: use 1.11.0]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 10fd051)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
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.

None yet

5 participants