From 916aa09192a3c0dc2806231891b6730a466530f2 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 21 Sep 2018 15:31:15 +0200 Subject: [PATCH 1/8] ci: stable lookout-sdk installation Signed-off-by: Alexander Bezzubov --- _tools/install-lookout-latest.sh | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/_tools/install-lookout-latest.sh b/_tools/install-lookout-latest.sh index 72188ac..e71d8bd 100755 --- a/_tools/install-lookout-latest.sh +++ b/_tools/install-lookout-latest.sh @@ -4,23 +4,34 @@ # Depends on GNU grep set -x -#TODO(bzz): check local cached version +#TODO(bzz): check for local cached version first -curl -s --connect-timeout 5 \ +OS="$(uname | tr '[:upper:]' '[:lower:]')" + +oIFS=$IFS IFS=' ' +curl -v ${GITHUB_TOKEN:+'-H' "Authorization: token $GITHUB_TOKEN"} \ + --connect-timeout 5 \ --max-time 10 \ --retry 5 \ --retry-delay 0 \ --retry-max-time 40\ "https://api.github.com/repos/src-d/lookout/releases/latest" \ - |& tee -a ../lookout-install.log \ + | tee -a ../lookout-install.log \ | grep -oP '"browser_download_url": "\K(.*)(?=")' \ - | grep linux \ + | grep "${OS}" \ | wget -qi - -if [[ "${PIPESTATUS[0]}" -ne 0 || "${PIPESTATUS[1]}" -ne 0 || "${PIPESTATUS[2]}" -ne 0 || "${PIPESTATUS[4]}" -ne 0 ]]; then +if [[ "${PIPESTATUS[0]}" -ne 0 || \ + "${PIPESTATUS[1]}" -ne 0 || \ + "${PIPESTATUS[2]}" -ne 0 || \ + "${PIPESTATUS[3]}" -ne 0 || \ + "${PIPESTATUS[4]}" -ne 0 ]]; +then echo "Unable download latest lookout SDK release" >&2 exit 2 fi +IFS=$oIFS; unset -v oIFS +# http://mywiki.wooledge.org/BashFAQ/050#I_only_want_to_pass_options_if_the_runtime_data_needs_them if ! tar -xvzf lookout-sdk_*.tar.gz ; then echo "Unable to extract lookout release archive" >&2 From 1b44fd249c1f8682b56c125b8940cb98cccb2fc3 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 21 Sep 2018 17:24:07 +0200 Subject: [PATCH 2/8] fix setting log-level Signed-off-by: Alexander Bezzubov --- README.md | 2 +- cmd/gometalint-analyzer/main.go | 2 ++ gometalint.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 128312c..ac74f90 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ $ lookout-sdk review -v ipv4://localhost:2001 \ | `GOMETALINT_HOST` | `0.0.0.0` | IP address to bind the gRCP serve | | `GOMETALINT_PORT` | `2001` | Port to bind the gRPC server | | `GOMETALINT_SERVER_URL` | `ipv4://localhost:10302` | gRPC URL of the [Data service](https://github.com/src-d/lookout/tree/master/docs#components) -| `GOMETALINT_LOG_LEVEL` | `info` | Logging level (info, debug, warning or error) | +| `GOMETALINT_LOG_LEVEL` | `info` | Logging level ("info", "debug", "warning" or "error") | # Licens diff --git a/cmd/gometalint-analyzer/main.go b/cmd/gometalint-analyzer/main.go index 967c8e8..061a67b 100644 --- a/cmd/gometalint-analyzer/main.go +++ b/cmd/gometalint-analyzer/main.go @@ -52,6 +52,8 @@ func main() { var conf config envconfig.MustProcess("GOMETALINT", &conf) log.Infof("Starting %s, %s", name, litter.Sdump(conf)) + log.DefaultFactory = &log.LoggerFactory{Level: conf.LogLevel} + log.DefaultLogger = log.New(nil) grpcAddr, err := grpchelper.ToGoGrpcAddress(conf.DataServiceURL) if err != nil { diff --git a/gometalint.go b/gometalint.go index ae8c754..527eff3 100644 --- a/gometalint.go +++ b/gometalint.go @@ -30,7 +30,7 @@ type Comment struct { func RunGometalinter(args []string) []Comment { dArgs := append([]string(nil), defaultArgs...) args = append(dArgs, args...) - log.Infof("Running '%s %v'\n", bin, args) + log.Debugf("Running '%s %v'\n", bin, args) out, _ := exec.Command(bin, args...).Output() // nolint: gas // ignoring err, as it's always not nil if anything found From 331e2d519b8fe6c99b34e9625cb59a5bafc50a4c Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 21 Sep 2018 17:50:12 +0200 Subject: [PATCH 3/8] fix: revert path transformation in text Signed-off-by: Alexander Bezzubov --- analyzer.go | 52 ++++++++++++++++++++++++++++++++++++++---------- analyzer_test.go | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 11 deletions(-) diff --git a/analyzer.go b/analyzer.go index fe426a0..2d4f896 100644 --- a/analyzer.go +++ b/analyzer.go @@ -102,19 +102,18 @@ func (a *Analyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent log.Debugf("no Golang files found. skip running gometalinter") return &lookout.EventResponse{AnalyzerVersion: a.Version}, nil } + log.Debugf("%d Golang files found. running gometalinter", saved) withArgs := append(append(a.Args, tmp), a.linterArguments(e.Configuration)...) comments := RunGometalinter(withArgs) var allComments []*lookout.Comment for _, comment := range comments { - //TrimLeft(, tmp) but \w rel path - file := comment.file[strings.LastIndex(comment.file, tmp)+len(tmp):] + origPathFile := revertOriginalPath(comment.file, tmp) + origPathText := revertOriginalPathIn(comment.text, tmp) newComment := lookout.Comment{ - File: strings.TrimLeft( - path.Join(strings.Split(file, artificialSep)...), - string(os.PathSeparator)), + File: origPathFile, Line: comment.lino, - Text: comment.text, + Text: origPathText, } allComments = append(allComments, &newComment) log.Debugf("Get comment %v", newComment) @@ -127,16 +126,47 @@ func (a *Analyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent }, nil } +// faltternPath flattens relative path and puts it inside tmp. +func faltternPath(file string, tmp string) string { + nFile := strings.Join(strings.Split(file, string(os.PathSeparator)), artificialSep) + nPath := path.Join(tmp, nFile) + return nPath +} + +// revertOriginalPath reverses origina path from a flat one. +func revertOriginalPath(file string, tmp string) string { + //TrimLeft(, tmp) but works for rel paths + noTmpfile := file[strings.Index(file, tmp)+len(tmp):] + origPathFile := strings.TrimLeft( + path.Join(strings.Split(noTmpfile, artificialSep)...), + string(os.PathSeparator)) + return origPathFile +} + +// revertOriginalPathIn a given text, recovers original path in words +// that have 'artificialSep'. +func revertOriginalPathIn(text string, tmp string) string { + if strings.LastIndex(text, artificialSep) < 0 { + return text + } + var words []string + for _, word := range strings.Fields(text) { + if strings.Index(word, artificialSep) >= 0 { + word = revertOriginalPath(word, tmp) + } + words = append(words, word) + } + return strings.Join(words, " ") +} + // tryToSaveTo saves a file to given dir, preserving it's original path. // It only loggs any errors and does not fail. All files saved this way will // be in the root of the same dir. func tryToSaveTo(file *lookout.File, tmp string) { - nFile := strings.Join(strings.Split(file.Path, string(os.PathSeparator)), artificialSep) - nPath := path.Join(tmp, nFile) - log.Debugf("Saving file '%s', as '%s'", file.Path, nPath) - err := ioutil.WriteFile(nPath, file.Content, 0644) + flatPath := faltternPath(file.Path, tmp) + err := ioutil.WriteFile(flatPath, file.Content, 0644) if err != nil { - log.Errorf(err, "failed to write a file %s", nPath) + log.Errorf(err, "failed to write a file %q", flatPath) } } func (a *Analyzer) NotifyPushEvent(ctx context.Context, e *lookout.PushEvent) (*lookout.EventResponse, error) { diff --git a/analyzer_test.go b/analyzer_test.go index 9cd564d..02fd7d1 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -75,3 +75,39 @@ func TestArgsCorrect(t *testing.T) { }, }))) } + +var pathTests = []struct { + in string + out string +}{ + {"a/b.go", "/tmp/a___.___b.go"}, + {"tmp/a/b.go", "/tmp/tmp___.___a___.___b.go"}, + {"a/b/c/d/e.go", "/tmp/a___.___b___.___c___.___d___.___e.go"}, +} + +func TestPathTransformations(t *testing.T) { + for _, tt := range pathTests { + t.Run(tt.in, func(t *testing.T) { + flat := faltternPath(tt.in, "/tmp") + if flat != tt.out { + t.Errorf("forward: got %q, want %q", flat, tt.out) + } + + orig := revertOriginalPath(tt.out, "/tmp") + if orig != tt.in { + t.Errorf("backward: got %q, want %q", orig, tt.in) + } + }) + } +} + +func TestPathInTextTransformations(t *testing.T) { + tmp := "/var/folders/rx/z9zyr71d70x92zwbn3rrjx4c0000gn/T/gometalint584398570" + text := "duplicate of /var/folders/rx/z9zyr71d70x92zwbn3rrjx4c0000gn/T/gometalint584398570/provider___.___github___.___poster_test.go:549-554 (dupl)" + expectedText := "duplicate of provider/github/poster_test.go:549-554 (dupl)" + + newText := revertOriginalPathIn(text, tmp) + if newText != expectedText { + t.Fatalf("got %q, want %q", newText, expectedText) + } +} From e1bf56801b31e76a7e3bb49d231ae8de618f2195 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Fri, 21 Sep 2018 17:56:57 +0200 Subject: [PATCH 4/8] ci: enable non-src-d ci Signed-off-by: Alexander Bezzubov --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index a2bbfcc..0a05018 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,8 @@ stages: - name: release if: tag IS present +go_import_path: github.com/src-d/lookout-gometalint-analyzer + jobs: include: - name: 'Unit tests' From 1d9e1aa1fc8ebe905977c04a6e7a799d726e803b Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 24 Sep 2018 19:01:53 +0200 Subject: [PATCH 5/8] test: refactor to use testify/assert Signed-off-by: Alexander Bezzubov --- Gopkg.lock | 1 + analyzer_test.go | 12 +++--------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index b77d7a3..a517c85 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -334,6 +334,7 @@ "github.com/sanity-io/litter", "github.com/src-d/lookout", "github.com/src-d/lookout/util/grpchelper", + "github.com/stretchr/testify/assert", "github.com/stretchr/testify/require", "google.golang.org/grpc", "gopkg.in/src-d/go-log.v1", diff --git a/analyzer_test.go b/analyzer_test.go index 02fd7d1..16bb246 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -5,6 +5,7 @@ import ( types "github.com/gogo/protobuf/types" "github.com/src-d/lookout/util/grpchelper" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -88,15 +89,8 @@ var pathTests = []struct { func TestPathTransformations(t *testing.T) { for _, tt := range pathTests { t.Run(tt.in, func(t *testing.T) { - flat := faltternPath(tt.in, "/tmp") - if flat != tt.out { - t.Errorf("forward: got %q, want %q", flat, tt.out) - } - - orig := revertOriginalPath(tt.out, "/tmp") - if orig != tt.in { - t.Errorf("backward: got %q, want %q", orig, tt.in) - } + assert.Equal(t, tt.out, faltternPath(tt.in, "/tmp")) + assert.Equal(t, tt.in, revertOriginalPath(tt.out, "/tmp")) }) } } From dd5418e86a3d9136154f8df651c4797d7a6c8df2 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 24 Sep 2018 19:02:05 +0200 Subject: [PATCH 6/8] ci: update job label Signed-off-by: Alexander Bezzubov --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 0a05018..51af9bc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,7 +39,7 @@ jobs: - cat analyzer.log - cat ../lookout-install.log - - name: 'Generated code' + - name: 'Check deps' install: - curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh script: From dabb38e296683a8fb7fab1f4bba0b9e4839eccd6 Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Mon, 24 Sep 2018 19:36:59 +0200 Subject: [PATCH 7/8] doc: update default port val in readme Signed-off-by: Alexander Bezzubov --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ac74f90..6c4235d 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ $ lookout-sdk review -v ipv4://localhost:2001 \ | -- | -- | -- | | `GOMETALINT_HOST` | `0.0.0.0` | IP address to bind the gRCP serve | | `GOMETALINT_PORT` | `2001` | Port to bind the gRPC server | -| `GOMETALINT_SERVER_URL` | `ipv4://localhost:10302` | gRPC URL of the [Data service](https://github.com/src-d/lookout/tree/master/docs#components) +| `GOMETALINT_DATA_SERVICE_URL` | `ipv4://localhost:10301` | gRPC URL of the [Data service](https://github.com/src-d/lookout/tree/master/docs#components) | `GOMETALINT_LOG_LEVEL` | `info` | Logging level ("info", "debug", "warning" or "error") | From 4012b42fc2a5655af5cf2163b0ea3b1def56081b Mon Sep 17 00:00:00 2001 From: Alexander Bezzubov Date: Tue, 25 Sep 2018 11:08:09 +0200 Subject: [PATCH 8/8] tixing fypo in method name Signed-off-by: Alexander Bezzubov --- analyzer.go | 6 +++--- analyzer_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/analyzer.go b/analyzer.go index 2d4f896..381b061 100644 --- a/analyzer.go +++ b/analyzer.go @@ -126,8 +126,8 @@ func (a *Analyzer) NotifyReviewEvent(ctx context.Context, e *lookout.ReviewEvent }, nil } -// faltternPath flattens relative path and puts it inside tmp. -func faltternPath(file string, tmp string) string { +// flattenPath flattens relative path and puts it inside tmp. +func flattenPath(file string, tmp string) string { nFile := strings.Join(strings.Split(file, string(os.PathSeparator)), artificialSep) nPath := path.Join(tmp, nFile) return nPath @@ -163,7 +163,7 @@ func revertOriginalPathIn(text string, tmp string) string { // It only loggs any errors and does not fail. All files saved this way will // be in the root of the same dir. func tryToSaveTo(file *lookout.File, tmp string) { - flatPath := faltternPath(file.Path, tmp) + flatPath := flattenPath(file.Path, tmp) err := ioutil.WriteFile(flatPath, file.Content, 0644) if err != nil { log.Errorf(err, "failed to write a file %q", flatPath) diff --git a/analyzer_test.go b/analyzer_test.go index 16bb246..ac45fe9 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -89,7 +89,7 @@ var pathTests = []struct { func TestPathTransformations(t *testing.T) { for _, tt := range pathTests { t.Run(tt.in, func(t *testing.T) { - assert.Equal(t, tt.out, faltternPath(tt.in, "/tmp")) + assert.Equal(t, tt.out, flattenPath(tt.in, "/tmp")) assert.Equal(t, tt.in, revertOriginalPath(tt.out, "/tmp")) }) }