From bcd30ceaec725e035c93cead15d7ee10176dc634 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 21 May 2025 08:32:49 +0200 Subject: [PATCH] chore: move golangci-lint & meta to separate PR-only workflow (#10119) For now. Existing code is not golangci-lint clean, but new PRs should be, ideally. --- .github/workflows/build-syncthing.yaml | 41 --------- .github/workflows/pr-linters.yaml | 49 ++++++++++ meta/metalint_test.go | 118 ------------------------- 3 files changed, 49 insertions(+), 159 deletions(-) create mode 100644 .github/workflows/pr-linters.yaml delete mode 100644 meta/metalint_test.go diff --git a/.github/workflows/build-syncthing.yaml b/.github/workflows/build-syncthing.yaml index 4daeffb08..78aab4d37 100644 --- a/.github/workflows/build-syncthing.yaml +++ b/.github/workflows/build-syncthing.yaml @@ -86,26 +86,6 @@ jobs: LOKI_PASSWORD: ${{ secrets.LOKI_PASSWORD }} LOKI_LABELS: "go=${{ matrix.go }},runner=${{ matrix.runner }},repo=${{ github.repository }},ref=${{ github.ref }}" - # - # Meta checks for formatting, copyright, etc - # - - correctness: - name: Check correctness - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - cache: false - check-latest: true - - - name: Check correctness - run: | - go test -v ./meta - # # The basic checks job is a virtual one that depends on the matrix tests, # the correctness checks, and various builds that we always do. This makes @@ -120,14 +100,12 @@ jobs: runs-on: ubuntu-latest needs: - build-test - - correctness - package-linux - package-cross - package-source - package-debian - package-windows - govulncheck - - golangci steps: - uses: actions/checkout@v4 @@ -1021,22 +999,3 @@ jobs: go run build.go assets go install golang.org/x/vuln/cmd/govulncheck@latest govulncheck ./... - - golangci: - runs-on: ubuntu-latest - name: Run golangci-lint - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version: ${{ env.GO_VERSION }} - cache: false - check-latest: true - - - name: ensure asset generation - run: go run build.go assets - - - name: golangci-lint - uses: golangci/golangci-lint-action@v8 - with: - only-new-issues: true diff --git a/.github/workflows/pr-linters.yaml b/.github/workflows/pr-linters.yaml new file mode 100644 index 000000000..6eda16c17 --- /dev/null +++ b/.github/workflows/pr-linters.yaml @@ -0,0 +1,49 @@ +name: Run PR linters + +on: + pull_request: + workflow_dispatch: + +permissions: + contents: read + pull-requests: read + +jobs: + + # + # golangci-lint runs a suite of static analysis checks on the code + # + + golangci: + runs-on: ubuntu-latest + name: Golangci-lint + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: 'stable' + + - name: ensure asset generation + run: go run build.go assets + + - name: golangci-lint + uses: golangci/golangci-lint-action@v8 + with: + only-new-issues: true + + # + # Meta checks for formatting, copyright, etc + # + + meta: + name: Meta checks + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: 'stable' + + - run: | + go run build.go assets + go test -v ./meta diff --git a/meta/metalint_test.go b/meta/metalint_test.go deleted file mode 100644 index 0c0407373..000000000 --- a/meta/metalint_test.go +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright (C) 2017 The Syncthing Authors. -// -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this file, -// You can obtain one at https://mozilla.org/MPL/2.0/. - -package meta - -import ( - "bytes" - "log" - "os/exec" - "strings" - "testing" -) - -var ( - // fast linters complete in a fraction of a second and might as well be - // run always as part of the build - fastLinters = []string{ - "deadcode", - "golint", - "ineffassign", - "vet", - } - - // slow linters take several seconds and are run only as part of the - // "metalint" command. - slowLinters = []string{ - "gosimple", - "staticcheck", - "structcheck", - "unused", - "varcheck", - } - - // Which parts of the tree to lint - lintDirs = []string{ - ".", - "../cmd/...", - "../lib/...", - "../script/...", - } - - // Messages to ignore - lintExcludes = []string{ - ".pb.go", - "should have comment", - "protocol.Vector composite literal uses unkeyed fields", - "cli.Requires composite literal uses unkeyed fields", - "Use DialContext instead", // Go 1.7 - "os.SEEK_SET is deprecated", // Go 1.7 - "SA4017", // staticcheck "is a pure function but its return value is ignored" - } -) - -func TestCheckMetalint(t *testing.T) { - if !isGometalinterInstalled() { - return - } - - gometalinter(t, lintDirs, lintExcludes...) -} - -func isGometalinterInstalled() bool { - if _, err := runError("gometalinter", "--disable-all"); err != nil { - log.Println("gometalinter is not installed") - return false - } - return true -} - -func gometalinter(_ *testing.T, dirs []string, excludes ...string) bool { - params := []string{"--disable-all", "--concurrency=2", "--deadline=300s"} - - for _, linter := range fastLinters { - params = append(params, "--enable="+linter) - } - - if !testing.Short() { - for _, linter := range slowLinters { - params = append(params, "--enable="+linter) - } - } - - for _, exclude := range excludes { - params = append(params, "--exclude="+exclude) - } - - params = append(params, dirs...) - - bs, _ := runError("gometalinter", params...) - - nerr := 0 - lines := make(map[string]struct{}) - for _, line := range strings.Split(string(bs), "\n") { - if line == "" { - continue - } - if _, ok := lines[line]; ok { - continue - } - log.Println(line) - if strings.Contains(line, "executable file not found") { - log.Println(` - Try "go run build.go setup" to install missing tools`) - } - lines[line] = struct{}{} - nerr++ - } - - return nerr == 0 -} - -func runError(cmd string, args ...string) ([]byte, error) { - ecmd := exec.Command(cmd, args...) - bs, err := ecmd.CombinedOutput() - return bytes.TrimSpace(bs), err -}