fix(*): Use go env GOPATH for install golangci-lint #6

Meoop · 2019-09-10T11:56:39Z

What this PR does / why we need it:

在新版本的 Golang 中，GOPATH 环境变量可以不设置，当系统未设置 GOPATH 环境变量时，golangci-lint 将安装到 /bin 目录下，安装错位置。使用 go env GOPATH 命令获取 GOPATH 路径可以应对所有的场景。

参考 golangci-lint 将 GOPATH 换成 go env GOPATH： golangci/golangci-lint#319

Which issue(s) this PR is related to (optional, link to 3rd issue(s)):

Fixes #

Reference to #

Special notes for your reviewer:

/cc @bbbmj

Release note:

NONE

Meoop · 2019-09-10T11:58:36Z

/cc @bbbmj

supereagle · 2019-09-11T01:41:33Z

Makefile

 DOCKER_LABELS ?= git-describe="$(shell date -u +v%Y%m%d)-$(shell git describe --tags --always --dirty)"

 # Golang standard bin directory.
+GOPATH ?= $(shell go env GOPATH)


What about including this in #5? @Meoop

在流水线执行的时候，代码构建阶段的环境没有装 go，而是通过启 go 容器运行的，会有问题。
之前将 PKGS 变量去掉，就是基于这个原因

@zhujian7 确认一下

没有装 go 的话，GOPATH 也是另外设置的好像？

或许安装到项目的 bin 目录也是个不错的选择？

# install it into ./bin/ curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s vX.Y.Z

Makefile 不兼容本地环境么？新安装的 golang 本地是不一定会设置 GOPATH 环境变量的。

使用 GOPATH:

基础镜像不需要装 go

流水线需要额外设置 GOPATH

每个研发自己的电脑，需要装 go 并设置 GOPATH

使用 go env GOPATH:

基础镜像需要装 go

流水线不需要额外设置 GOPATH

每个研发自己的电脑，只需要装 go，不需要设置 GOPATH

综合看，我觉得设置成 go env GOPATH 可能好一点？

不用纠结吧，加个判断，存在 go 就 go env GOPATH，不存在就按照之前的

@Meoop 乐乐说的这种方式可以吗？

en，和上面写法比较类似吧，GOPATH ?= $(shell go env GOPATH)，如果有 GOPATH 这个环境变量，就不会执行 shell 了，也就不检测 go 存不存在，只有不存在 GOPATH 的时候才去调用 go 命令。

zhujian7 · 2019-09-11T02:50:52Z

/hold

supereagle · 2019-09-11T03:03:07Z

流水线上肯定不存在这个问题，本地开发环境的话，建议手动设置 GOPATH（一次性操作）。因此，没有必要通过 go env GOPATH 自动获取，建议 close 这个 PR。@Meoop WDYT?

Meoop · 2019-09-11T03:18:33Z

说实话，我并不认为 “本地开发环境的话，建议手动设置 GOPATH（一次性操作）” 是一个好的方案...

如果远程是通过启动一个 Golang 镜像去跑 lint ，那还不如直接用 golangci-lint 镜像呢，还少一次 curl。

lsytj0413 · 2019-09-11T03:51:49Z

我们不是准备切换到 go module 了么，到时候 GOPATH 直接基本就没用了；那么这段过渡时间是否值得改这个？
然后基础镜像为啥要装 go 啊......

Meoop · 2019-09-11T07:32:24Z

我们不是准备切换到 go module 了么，到时候 GOPATH 直接基本就没用了；那么这段过渡时间是否值得改这个？

@lsytj0413 这个改动就是由于系统不设置 GOPATH 这个环境变量导致的，切换到 go module，GOPATH 这个环境变量估计也没有人会去设置了，到时候根据 $GOPATH 环境变量去保存 golangci-lint 二进制就会出现问题（$GOPATH 为空，下载到 /bin 下，普通用户是没有权限的）。

zhujian7 · 2019-09-11T09:12:36Z

验证完了，对 build-linux 没有影响，可以加 go env GOPATH
/hold cancel

/cc @Meoop @supereagle @bbbmj

caicloud-bot · 2019-09-11T09:12:42Z

@zhujian7: GitHub didn't allow me to request PR reviews from the following users: Meoop.

Note that only caicloud members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

验证完了，对 build-linux 没有影响，可以加 go env GOPATH
/hold cancel

/cc @Meoop @supereagle @bbbmj

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the caicloud/tools repository.

bbbmj · 2019-09-11T09:16:00Z

/lgtm

GOPATH environment variable may be empty, go env GOPATH will work for all cases.

Meoop · 2019-09-11T09:42:55Z

/reopen

supereagle · 2019-09-11T09:45:11Z

/lgtm

caicloud-bot · 2019-09-11T09:45:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bbbmj, supereagle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [bbbmj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bbbmj · 2019-09-11T09:47:47Z

@Meoop please cherry-pick to https://github.com/caicloud/nirvana-template-project

supereagle · 2019-09-11T09:49:09Z

@Meoop please cherry-pick to https://github.com/caicloud/nirvana-template-project

I will update Makefile for this project including my changes.

supereagle · 2019-09-11T10:01:09Z

Cherry pick to caicloud/nirvana-template-project by caicloud/nirvana-template-project#6

caicloud-bot added release-note-none Denotes a PR that doesn't merit a release note. caicloud-cla: yes Indicates the PR's author has not signed the Caicloud CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 10, 2019

caicloud-bot requested a review from bbbmj September 10, 2019 11:58

supereagle reviewed Sep 11, 2019

View reviewed changes

caicloud-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2019

caicloud-bot requested a review from supereagle September 11, 2019 09:12

caicloud-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2019

caicloud-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2019

caicloud-bot assigned bbbmj Sep 11, 2019

caicloud-bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 11, 2019

caicloud-bot closed this Sep 11, 2019

caicloud-bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2019

fix(*): Use go env GOPATH for install golangci-lint

be9431a

GOPATH environment variable may be empty, go env GOPATH will work for all cases.

caicloud-bot reopened this Sep 11, 2019

caicloud-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 11, 2019

caicloud-bot assigned supereagle Sep 11, 2019

caicloud-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 11, 2019

caicloud-bot merged commit ea379e5 into caicloud:master Sep 11, 2019

fix(*): Use go env GOPATH for install golangci-lint #6

fix(*): Use go env GOPATH for install golangci-lint #6

Uh oh!

Conversation

Meoop commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Meoop commented Sep 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhujian7 commented Sep 11, 2019

Uh oh!

supereagle commented Sep 11, 2019

Uh oh!

Meoop commented Sep 11, 2019

Uh oh!

lsytj0413 commented Sep 11, 2019

Uh oh!

Meoop commented Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhujian7 commented Sep 11, 2019

Uh oh!

caicloud-bot commented Sep 11, 2019

Uh oh!

bbbmj commented Sep 11, 2019

Uh oh!

Meoop commented Sep 11, 2019

Uh oh!

supereagle commented Sep 11, 2019

Uh oh!

caicloud-bot commented Sep 11, 2019

Uh oh!

bbbmj commented Sep 11, 2019

Uh oh!

supereagle commented Sep 11, 2019

Uh oh!

supereagle commented Sep 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Meoop commented Sep 10, 2019 •

edited

Loading

Meoop commented Sep 11, 2019 •

edited

Loading