From 6080228e686e65eba8658090168abf7c52fc6e8b Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Sun, 24 Dec 2023 12:43:09 -0800 Subject: [PATCH 1/2] Lint setup Signed-off-by: Chris Koch --- .github/workflows/golangci-lint.yml | 29 +++++++++++++ .golangci.yml | 66 +++++++++++++++++++++++++++++ .revive.toml | 28 ++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 .golangci.yml create mode 100644 .revive.toml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 00000000..8dd7c17f --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,29 @@ +name: golangci-lint +on: + push: + tags: + - v* + branches: + - main + pull_request: + branches: + - main + +permissions: + contents: read + # Optional: allow read access to pull request. Use with `only-new-issues` option. + pull-requests: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/setup-go@v3 + with: + go-version: '1.20' + - uses: actions/checkout@v3 + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: latest diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000..333e8fed --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,66 @@ +linters: + enable: + - containedctx + - gocritic + - godot + - nilerr + - revive + - unconvert + +issues: + include: + - EXC0012 + - EXC0013 + - EXC0014 + - EXC0015 + +linters-settings: + gocritic: + disabledChecks: + - ifElseChain + revive: + # Maximum number of open files at the same time. + # See https://github.com/mgechev/revive#command-line-flags + # Defaults to unlimited. + max-open-files: 2048 + # When set to false, ignores files with "GENERATED" header, similar to golint. + # See https://github.com/mgechev/revive#available-rules for details. + # Default: false + ignore-generated-header: true + # Sets the default severity. + # See https://github.com/mgechev/revive#configuration + # Default: warning + severity: error + # Default: false + # Sets the default failure confidence. + # This means that linting errors with less than 0.8 confidence will be ignored. + # Default: 0.8 + confidence: 0.8 + rules: + - name: blank-imports + - name: context-as-argument + arguments: + - allowTypesBefore: "*testing.T,*github.com/user/repo/testing.Harness" + - name: context-keys-type + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + arguments: + - "checkPrivateReceivers" + - "sayRepetitiveInsteadOfStutters" + - name: if-return + - name: increment-decrement + - name: var-naming + - name: var-declaration + - name: package-comments + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: early-return + - name: file-header + arguments: + - "Copyright 20[1-2][0-9](-20[1-2][0-9])? the u-root Authors. All rights reserved Use of this source code is governed by a BSD-style license that can be found in the LICENSE file." diff --git a/.revive.toml b/.revive.toml new file mode 100644 index 00000000..c403b4d7 --- /dev/null +++ b/.revive.toml @@ -0,0 +1,28 @@ +ignoreGeneratedHeader = false +severity = "warning" +confidence = 0.8 +errorCode = 0 +warningCode = 0 + +[rule.blank-imports] +[rule.context-as-argument] +[rule.context-keys-type] +[rule.dot-imports] +[rule.error-return] +[rule.error-strings] +[rule.error-naming] +[rule.exported] +[rule.if-return] +[rule.increment-decrement] +[rule.var-naming] +[rule.var-declaration] +[rule.package-comments] +[rule.range] +[rule.receiver-naming] +[rule.time-naming] +[rule.unexported-return] +[rule.indent-error-flow] +[rule.errorf] +[rule.early-return] +[rule.file-header] + arguments=["Copyright 20[1-2][0-9](-20[1-2][0-9])? the u-root Authors. All rights reserved Use of this source code is governed by a BSD-style license that can be found in the LICENSE file."] From 0bf4cbe221a917fa38564c9d1a6b404dce790a57 Mon Sep 17 00:00:00 2001 From: Chris Koch Date: Sun, 24 Dec 2023 13:03:14 -0800 Subject: [PATCH 2/2] Lint fixes Signed-off-by: Chris Koch --- src/cmd/makebbmain/main.go | 1 - src/gobb2.bzl | 1 - src/pkg/bb/bb.go | 32 +++++++++-------------------- src/pkg/bb/findpkg/bb_test.go | 38 +++++++++++++++++++++-------------- src/pkg/golang/build.go | 4 +++- 5 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/cmd/makebbmain/main.go b/src/cmd/makebbmain/main.go index cbaf0ed6..35926906 100644 --- a/src/cmd/makebbmain/main.go +++ b/src/cmd/makebbmain/main.go @@ -16,7 +16,6 @@ import ( ) var ( - pkg = flag.String("template_pkg", "", "Go import package path") destDir = flag.String("dest_dir", "", "Destination directory") pkgFiles uflag.Strings commands uflag.Strings diff --git a/src/gobb2.bzl b/src/gobb2.bzl index 13995d81..c4d02793 100644 --- a/src/gobb2.bzl +++ b/src/gobb2.bzl @@ -182,7 +182,6 @@ def _go_busybox_impl(ctx): args = ctx.actions.args() template = ctx.attr._template[0] if type(ctx.attr._template) == "list" else ctx.attr._template - args.add("--template_pkg", "%s/main" % template.label.package) outputs = [] inputs = [] diff --git a/src/pkg/bb/bb.go b/src/pkg/bb/bb.go index 224595bc..d129b08d 100644 --- a/src/pkg/bb/bb.go +++ b/src/pkg/bb/bb.go @@ -30,7 +30,6 @@ import ( "os" "path" "path/filepath" - "regexp" "strings" "github.com/google/goterm/term" @@ -255,11 +254,10 @@ func BuildBusybox(l ulog.Logger, opts *Opts) (nerr error) { GOPATH: tmpDir, Err: err, } - } else { - return &ErrModuleBuild{ - CmdDir: bbDir, - Err: err, - } + } + return &ErrModuleBuild{ + CmdDir: bbDir, + Err: err, } } return nil @@ -504,7 +502,7 @@ func findLocalModules(l ulog.Logger, mainPkgs []*bbinternal.Package) (map[string if lm, ok := localModules[p.Module.Path]; ok && lm.m.Dir != p.Module.Dir { gbbstrict, set := os.LookupEnv("GBB_STRICT") - if set == false { + if !set { l.Printf("GBB_STRICT is not set.") } if gbbstrict != "1" { @@ -647,7 +645,9 @@ func copyLocalDeps(l ulog.Logger, env *golang.Environ, bbDir, tmpDir, pkgDir str // decides to go on the internet. var mod modfile.File - mod.AddModuleStmt("bb.u-root.com/bb") + if err := mod.AddModuleStmt("bb.u-root.com/bb"); err != nil { + return fmt.Errorf("failed to add bb.u-root.com/bb to generated go.mod: %w", err) + } for mpath, module := range localModules { v := module.Version if len(v) == 0 { @@ -679,25 +679,11 @@ func copyLocalDeps(l ulog.Logger, env *golang.Environ, bbDir, tmpDir, pkgDir str // directives. // // Warn the user if they are potentially incompatible. - if err := ioutil.WriteFile(filepath.Join(bbDir, "go.mod"), gomod, 0755); err != nil { - return err - } - return nil + return ioutil.WriteFile(filepath.Join(bbDir, "go.mod"), gomod, 0755) } return nil } -func versionNum(mpath string) string { - last := path.Base(mpath) - if len(last) == 0 { - return "v0" - } - if matched, _ := regexp.Match("v[0-9]+", []byte(last)); matched { - return last - } - return "v0" -} - // deps recursively iterates through imports and returns the set of packages // for which filter returns true. func deps(p *packages.Package, filter func(p *packages.Package) bool) []*packages.Package { diff --git a/src/pkg/bb/findpkg/bb_test.go b/src/pkg/bb/findpkg/bb_test.go index d3a621b7..94662745 100644 --- a/src/pkg/bb/findpkg/bb_test.go +++ b/src/pkg/bb/findpkg/bb_test.go @@ -23,24 +23,32 @@ var ( urootSource = flag.String("uroot-source", "", "Directory path to u-root source location") ) -func TestModules(t *testing.T) { - dir, err := ioutil.TempDir("", "test-modules-") - if err != nil { +func mustMkdir(t *testing.T, path string, mode os.FileMode) { + if err := os.MkdirAll(path, mode); err != nil { t.Fatal(err) } - defer os.RemoveAll(dir) +} + +func mustWrite(t *testing.T, path string, contents []byte, mode os.FileMode) { + if err := os.WriteFile(path, contents, mode); err != nil { + t.Fatal(err) + } +} + +func TestModules(t *testing.T) { + dir := t.TempDir() - os.MkdirAll(filepath.Join(dir, "mod1/cmd/cmd1"), 0755) - os.MkdirAll(filepath.Join(dir, "mod1/cmd/cmd2"), 0755) - os.MkdirAll(filepath.Join(dir, "mod1/nestedmod1/cmd/cmd5"), 0755) - os.MkdirAll(filepath.Join(dir, "mod1/nestedmod2/cmd/cmd6"), 0755) - os.MkdirAll(filepath.Join(dir, "mod2/cmd/cmd3"), 0755) - os.MkdirAll(filepath.Join(dir, "mod2/cmd/cmd4"), 0755) - os.MkdirAll(filepath.Join(dir, "nomod/cmd/cmd7"), 0755) - ioutil.WriteFile(filepath.Join(dir, "mod1/go.mod"), nil, 0644) - ioutil.WriteFile(filepath.Join(dir, "mod1/nestedmod1/go.mod"), nil, 0644) - ioutil.WriteFile(filepath.Join(dir, "mod1/nestedmod2/go.mod"), nil, 0644) - ioutil.WriteFile(filepath.Join(dir, "mod2/go.mod"), nil, 0644) + mustMkdir(t, filepath.Join(dir, "mod1/cmd/cmd1"), 0755) + mustMkdir(t, filepath.Join(dir, "mod1/cmd/cmd2"), 0755) + mustMkdir(t, filepath.Join(dir, "mod1/nestedmod1/cmd/cmd5"), 0755) + mustMkdir(t, filepath.Join(dir, "mod1/nestedmod2/cmd/cmd6"), 0755) + mustMkdir(t, filepath.Join(dir, "mod2/cmd/cmd3"), 0755) + mustMkdir(t, filepath.Join(dir, "mod2/cmd/cmd4"), 0755) + mustMkdir(t, filepath.Join(dir, "nomod/cmd/cmd7"), 0755) + mustWrite(t, filepath.Join(dir, "mod1/go.mod"), nil, 0644) + mustWrite(t, filepath.Join(dir, "mod1/nestedmod1/go.mod"), nil, 0644) + mustWrite(t, filepath.Join(dir, "mod1/nestedmod2/go.mod"), nil, 0644) + mustWrite(t, filepath.Join(dir, "mod2/go.mod"), nil, 0644) paths := []string{ filepath.Join(dir, "mod1/cmd/cmd1"), diff --git a/src/pkg/golang/build.go b/src/pkg/golang/build.go index 9bcf6fd2..dab4cadc 100644 --- a/src/pkg/golang/build.go +++ b/src/pkg/golang/build.go @@ -175,6 +175,8 @@ func (c Environ) envCommon() []string { return env } +// EnvHuman returns all env vars used by c, with an abbreviated +// PATH=$PATH: rather than displaying the full PATH. func (c Environ) EnvHuman() []string { env := c.envCommon() if c.GOROOT != "" { @@ -200,7 +202,7 @@ func (c Environ) String() string { return strings.Join(c.EnvHuman(), " ") } -// Optional arguments to Environ.BuildDir. +// BuildOpts are `go build` arguments. type BuildOpts struct { // NoStrip builds an unstripped binary. //