Set go version to 1.14, to enable vendor builds by default. #2749
Conversation
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
Signed-off-by: Peter Štibraný <[email protected]>
|
Minimum version in Thanos is still 1.13 (even if built with 1.14). Could this change be a problem? @bwplotka |
This should not be a problem for dependencies. This sets expected language version for the module, but go 1.14 doesn’t change the language. It changes how vendoring works, but that is no concern when used as a dependency. Usage of some go new methods in 1.14 (eg. testing.T.Cleanup) is also not a problem, unless used in integration tests. |
tools/test
Outdated
| @@ -104,6 +104,7 @@ run_test() { | |||
| local dir=$1 | |||
| if [ -z "$NO_GO_GET" ]; then | |||
| go get -t -tags "${TAGS[@]}" "$dir" | |||
| go mod vendor # re-vendor everything | |||
There was a problem hiding this comment.
Could you explain why we need to run go mod vendor in this file?
There was a problem hiding this comment.
Reason is to avoid error when go get fetches additional depedendency, but doesn’t put it into vendor. In go 1.14, if you have vendor directory, it is used by default, but vendor/modules.txt is verified against go.mod. Without calling go mod vendor here, that verification fails.
There was a problem hiding this comment.
I would prefer if our test didn’t do additional go get, but I’m not sure why it’s here, so left it for now.
In case of cover tool later in the file, I think that should be part of the build image instead.
There was a problem hiding this comment.
What if we remove the -go-get and -no-go-get support at all, always enforcing the behaviour of NO_GO_GET=true (current default)? -go-get is not used by any script we have, so the only way to use it is to manually call tools/test -go-get.
There was a problem hiding this comment.
What if we remove the
-go-getand-no-go-getsupport at all, always enforcing the behaviour ofNO_GO_GET=true(current default)?-go-getis not used by any script we have, so the only way to use it is to manually calltools/test -go-get.
I will remove it. This doesn't fix problem with cover tool, used when run under CIRCLECI.
Signed-off-by: Peter Štibraný <[email protected]>
|
Thanos is basing on 1.14 as minimum, we will fix the docs. So all good ! (: |
|
If we merge it soon we'll get it into next Cortex release. |
| @@ -147,6 +135,8 @@ fi | |||
|
|
|||
| if [ -n "$SLOW" ] && [ -z "$COVERDIR" ]; then | |||
| go get github.com/weaveworks/tools/cover | |||
| go mod vendor # re-vendor everything | |||
There was a problem hiding this comment.
What about putting the cover tool into the build image, like we do with various other tools?
Lines 19 to 23 in b64dabe
There was a problem hiding this comment.
I will send PR to include cover tool in build image.
|
Yes, I think that makes sense, and would like to do that.
… On 22 Jun 2020, at 18.12, Bryan Boreham ***@***.***> wrote:
@bboreham commented on this pull request.
In tools/test:
> @@ -147,6 +135,8 @@ fi
if [ -n "$SLOW" ] && [ -z "$COVERDIR" ]; then
go get github.com/weaveworks/tools/cover
+ go mod vendor # re-vendor everything
What about putting the cover tool into the build image, like we do with various other tools?
https://github.com/cortexproject/cortex/blob/b64dabe749023da882c9739b216de683194b9bfb/build-image/Dockerfile#L19-L23
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
What this PR does: After #2480, we can enable go 1.14 in go.mod. That enables -mod=vendor by default for all builds.
Requiring go 1.14 for builds also allows us to use new features from this version.
Pinging Loki team: @slim-bean @cyriltovena @owen-d.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]