From dc260916c7deb9412a1b0618856b8cb7bada22e8 Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Fri, 23 Feb 2024 11:50:12 -0600 Subject: [PATCH] lint: make runnable under `bazel test` This test was never directly runnable under `bazel test`, and instead required using `bazel run` with the wrapper target `//build/bazelutil:lint` that set up some dependencies and some environment variables appropriately. Instead, we simply pass in those environment variables from the CI script or from `dev`, and add extra dependencies to the `lint_test` target (`crlfmt`, `optfmt`) so the test can use them. It's necessary to pass `GO_SDK` as a path to the Bazel-provided Go SDK and `COCKROACH_WORKSPACE` as the path to the checkout as this stuff will not be found in the sandbox. Epic: CRDB-8308 Release note: None --- build/bazelutil/BUILD.bazel | 6 -- build/bazelutil/lint.bzl | 56 ------------- build/bazelutil/lint.sh.in | 46 ----------- .../teamcity/cockroach/ci/tests/lint_impl.sh | 19 +++-- .../cockroach/nightlies/lint_urls_impl.sh | 21 ++--- dev | 2 +- pkg/cmd/dev/datadriven_test.go | 2 +- pkg/cmd/dev/io/os/os.go | 16 +++- pkg/cmd/dev/lint.go | 71 ++++++++++------- pkg/cmd/dev/testdata/datadriven/bench | 12 +-- pkg/cmd/dev/testdata/datadriven/lint | 57 -------------- pkg/cmd/dev/testdata/recorderdriven/lint | 11 +++ pkg/cmd/dev/testdata/recorderdriven/lint.rec | 23 ++++++ pkg/testutils/lint/BUILD.bazel | 7 +- pkg/testutils/lint/lint_test.go | 78 ++++++++++++++----- 15 files changed, 189 insertions(+), 238 deletions(-) delete mode 100644 build/bazelutil/lint.bzl delete mode 100644 build/bazelutil/lint.sh.in delete mode 100644 pkg/cmd/dev/testdata/datadriven/lint create mode 100644 pkg/cmd/dev/testdata/recorderdriven/lint create mode 100644 pkg/cmd/dev/testdata/recorderdriven/lint.rec diff --git a/build/bazelutil/BUILD.bazel b/build/bazelutil/BUILD.bazel index c54e0f0b14df..a04b0dafee40 100644 --- a/build/bazelutil/BUILD.bazel +++ b/build/bazelutil/BUILD.bazel @@ -2,7 +2,6 @@ exports_files(["nogo_config.json"]) load("@bazel_skylib//rules:analysis_test.bzl", "analysis_test") load("@io_bazel_rules_go//go:def.bzl", "go_library") -load(":lint.bzl", "lint_binary") analysis_test( name = "test_nogo_configured", @@ -25,11 +24,6 @@ genrule( }), ) -lint_binary( - name = "lint", - test = "//pkg/testutils/lint:lint_test", -) - # This noop target is a workaround for https://github.com/bazelbuild/bazel-gazelle/issues/1078. # We use it in //pkg/kv/kvclient/{kvcoord,rangefeed}. go_library( diff --git a/build/bazelutil/lint.bzl b/build/bazelutil/lint.bzl deleted file mode 100644 index ce57886ea873..000000000000 --- a/build/bazelutil/lint.bzl +++ /dev/null @@ -1,56 +0,0 @@ -load("@bazel_skylib//lib:shell.bzl", "shell") - -# lint_binary works as follows: -# 1. For each test, we generate a script, which uses lint.sh.in as a -# template. It simply bootstraps the environment by locating the go SDK, -# setting an appropriate `PATH` and `GOROOT`, and cd-ing to the right -# directory in the workspace. This roughly replicates what `go test` would -# do. -# 2. Using that script, we create a `sh_binary` using that script as an entry -# point with the appropriate dependencies. - -def _gen_script_impl(ctx): - subs = { - "@@PACKAGE@@": shell.quote(ctx.attr.test.label.package), - "@@NAME@@": shell.quote(ctx.attr.test.label.name), - } - out_file = ctx.actions.declare_file(ctx.label.name) - ctx.actions.expand_template( - template = ctx.file._template, - output = out_file, - substitutions = subs, - ) - return [ - DefaultInfo(files = depset([out_file])), - ] - -_gen_script = rule( - implementation = _gen_script_impl, - attrs = { - "test": attr.label(mandatory = True), - "_template": attr.label( - default = "//build/bazelutil:lint.sh.in", - allow_single_file = True, - ), - }, -) - -def lint_binary(name, test): - script_name = name + ".sh" - _gen_script( - name = script_name, - test = test, - testonly = 1, - ) - native.sh_binary( - name = name, - srcs = [script_name], - data = [ - test, - "//pkg/sql/opt/optgen/cmd/optfmt", - "@com_github_cockroachdb_crlfmt//:crlfmt", - "@go_sdk//:bin/go", - ], - deps = ["@bazel_tools//tools/bash/runfiles"], - testonly = 1, - ) diff --git a/build/bazelutil/lint.sh.in b/build/bazelutil/lint.sh.in deleted file mode 100644 index 9e0ee7fd331b..000000000000 --- a/build/bazelutil/lint.sh.in +++ /dev/null @@ -1,46 +0,0 @@ -# This is boilerplate taken directly from -# https://github.com/bazelbuild/bazel/blob/master/tools/bash/runfiles/runfiles.bash -# See that page for an explanation of what this is and why it's necessary. -# --- begin runfiles.bash initialization v2 --- -# Copy-pasted from the Bazel Bash runfiles library v2. -set -uo pipefail; f=bazel_tools/tools/bash/runfiles/runfiles.bash -source "${RUNFILES_DIR:-/dev/null}/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "${RUNFILES_MANIFEST_FILE:-/dev/null}" | cut -f2- -d' ')" 2>/dev/null || \ - source "$0.runfiles/$f" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - source "$(grep -sm1 "^$f " "$0.exe.runfiles_manifest" | cut -f2- -d' ')" 2>/dev/null || \ - { echo>&2 "ERROR: cannot find $f"; exit 1; }; f=; set -e -# --- end runfiles.bash initialization v2 --- - -PACKAGE=@@PACKAGE@@ -NAME=@@NAME@@ - -# Wrap rlocation such that we immediately fail if a dep is not found. -rlocation_ck () { - loc="$(rlocation $1)" - if [ -z "${loc-}" ]; then - echo "error: could not find the location of $1" >&2 - exit 1 - fi - echo $loc -} - -test_bin="$(rlocation_ck com_github_cockroachdb_cockroach/$PACKAGE/${NAME}_/$NAME)" -go_bin="$(rlocation_ck go_sdk/bin/go)" -crlfmt_bin="$(rlocation_ck com_github_cockroachdb_crlfmt/crlfmt_/crlfmt)" -optfmt_bin="$(rlocation_ck com_github_cockroachdb_cockroach/pkg/sql/opt/optgen/cmd/optfmt/optfmt_/optfmt)" - -# Need to run this so that Go can find the runfiles. -runfiles_export_envvars - -if [ -z "${BUILD_WORKSPACE_DIRECTORY-}" ]; then - echo "error: BUILD_WORKSPACE_DIRECTORY not set" >&2 - exit 1 -fi - -cd "$BUILD_WORKSPACE_DIRECTORY/$PACKAGE" - -TEST_WORKSPACE=com_github_cockroachdb_cockroach \ - PATH="$(dirname $go_bin):$(dirname $crlfmt_bin):$(dirname $optfmt_bin):$PATH" \ - GOROOT="$(dirname $(dirname $go_bin))" \ - "$test_bin" "$@" diff --git a/build/teamcity/cockroach/ci/tests/lint_impl.sh b/build/teamcity/cockroach/ci/tests/lint_impl.sh index 7851c97b74d8..748272e82177 100755 --- a/build/teamcity/cockroach/ci/tests/lint_impl.sh +++ b/build/teamcity/cockroach/ci/tests/lint_impl.sh @@ -2,15 +2,22 @@ set -xeuo pipefail +dir="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))" + +source "$dir/teamcity-support.sh" # For $root + # GCAssert and unused need generated files in the workspace to work properly. # generated files requirements -- begin bazel run //pkg/gen:code -bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $(bazel info workspace) && " +bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $root && " # generated files requirements -- end bazel build //pkg/cmd/bazci --config=ci -XML_OUTPUT_FILE=/artifacts/test.xml GO_TEST_WRAP_TESTV=1 GO_TEST_WRAP=1 CC=$(which gcc) CXX=$(which gcc) bazel \ - run --config=ci --config=test //build/bazelutil:lint -# The schema of the output test.xml will be slightly wrong -- ask `bazci` to fix -# it up. -$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci munge-test-xml /artifacts/test.xml +$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci -- test \ + //pkg/testutils/lint:lint_test --config=ci \ + --test_env=CC=$(which gcc) \ + --test_env=CXX=$(which gcc) \ + --test_env=HOME \ + --sandbox_writable_path=$HOME \ + --test_env=GO_SDK=$(dirname $(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath))) \ + --test_env=COCKROACH_WORKSPACE=$root diff --git a/build/teamcity/cockroach/nightlies/lint_urls_impl.sh b/build/teamcity/cockroach/nightlies/lint_urls_impl.sh index a2be4fa66dc9..1936617c036a 100755 --- a/build/teamcity/cockroach/nightlies/lint_urls_impl.sh +++ b/build/teamcity/cockroach/nightlies/lint_urls_impl.sh @@ -4,19 +4,20 @@ set -xeuo pipefail dir="$(dirname $(dirname $(dirname $(dirname "${0}"))))" +source "$dir/teamcity-support.sh" # For $root + # GCAssert and unused need generated files in the workspace to work properly. # generated files requirements -- begin bazel run //pkg/gen:code -bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $(bazel info workspace) && " +bazel run //pkg/cmd/generate-cgo:generate-cgo --run_under="cd $root && " # generated files requirements -- end bazel build //pkg/cmd/bazci --config=ci -BAZEL_BIN=$(bazel info bazel-bin --config=ci) -exit_status=0 -XML_OUTPUT_FILE=/artifacts/test.xml GO_TEST_WRAP_TESTV=1 GO_TEST_WRAP=1 bazel \ - run --config=ci --config=test --define gotags=bazel,gss,nightly,lint \ - //build/bazelutil:lint || exit_status=$? -# The schema of the output test.xml will be slightly wrong -- ask `bazci` to fix -# it up. -$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci munge-test-xml /artifacts/test.xml -exit $exit_status +$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci -- \ + test --config=ci --define gotags=bazel,gss,nightly,lint \ + --test_env=CC=$(which gcc) \ + --test_env=CXX=$(which gcc) \ + --test_env=HOME \ + --sandbox_writable_path=$HOME \ + --test_env=GO_SDK=$(dirname $(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath))) \ + --test_env=COCKROACH_WORKSPACE=$root diff --git a/dev b/dev index f0577586971e..b9ee3fb5ed4d 100755 --- a/dev +++ b/dev @@ -8,7 +8,7 @@ fi set -euo pipefail # Bump this counter to force rebuilding `dev` on all machines. -DEV_VERSION=91 +DEV_VERSION=92 THIS_DIR=$(cd "$(dirname "$0")" && pwd) BINARY_DIR=$THIS_DIR/bin/dev-versions diff --git a/pkg/cmd/dev/datadriven_test.go b/pkg/cmd/dev/datadriven_test.go index 68b4587e4066..9b3f3d2c3342 100644 --- a/pkg/cmd/dev/datadriven_test.go +++ b/pkg/cmd/dev/datadriven_test.go @@ -63,7 +63,7 @@ func TestDataDriven(t *testing.T) { osOpts := []os.Option{ os.WithLogger(log.New(logger, "", 0)), os.WithDryrun(), - os.WithIntercept("echo $HOME", testFixturesDirPlaceholder), + os.WithIntercept("echo $HOME/.cache", testFixturesDirPlaceholder), } if !verbose { // suppress all internal output unless told otherwise diff --git a/pkg/cmd/dev/io/os/os.go b/pkg/cmd/dev/io/os/os.go index 96842691b66c..c9194304563f 100644 --- a/pkg/cmd/dev/io/os/os.go +++ b/pkg/cmd/dev/io/os/os.go @@ -537,7 +537,7 @@ func (o *OS) CurrentUserAndGroup() (uid string, gid string, err error) { // UserCacheDir returns the cache directory for the current user if possible. func (o *OS) UserCacheDir() (dir string, err error) { - command := "echo $HOME" + command := "echo $HOME/.cache" if !o.knobs.silent { o.logger.Print(command) } @@ -548,6 +548,20 @@ func (o *OS) UserCacheDir() (dir string, err error) { } +// UserCacheDir returns the cache directory for the current user if possible. +func (o *OS) HomeDir() (dir string, err error) { + command := "echo $HOME" + if !o.knobs.silent { + o.logger.Print(command) + } + + dir, err = o.Next(command, func() (dir string, err error) { + return os.UserHomeDir() + }) + + return strings.TrimSpace(dir), err +} + // Next is a thin interceptor for all os activity, running them through // testing knobs first. func (o *OS) Next(command string, f func() (output string, err error)) (string, error) { diff --git a/pkg/cmd/dev/lint.go b/pkg/cmd/dev/lint.go index aa6037a4c8aa..27cef8dbc8cb 100644 --- a/pkg/cmd/dev/lint.go +++ b/pkg/cmd/dev/lint.go @@ -13,7 +13,7 @@ package main import ( "fmt" "log" - "os" + "path/filepath" "strings" "github.com/spf13/cobra" @@ -53,8 +53,42 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error { } } - lintEnv := os.Environ() - + var args []string + args = append(args, "test", "//pkg/testutils/lint:lint_test") + if numCPUs != 0 { + args = append(args, fmt.Sprintf("--local_cpu_resources=%d", numCPUs)) + } + args = append(args, additionalBazelArgs...) + args = append(args, "--test_arg", "-test.v") + if short { + args = append(args, "--test_arg", "-test.short") + } + if timeout > 0 { + args = append(args, fmt.Sprintf("--test_timeout=%d", int(timeout.Seconds()))) + } + if filter != "" { + args = append(args, fmt.Sprintf("--test_filter=Lint/%s", filter)) + } + workspace, err := d.getWorkspace(ctx) + if err != nil { + return err + } + args = append(args, fmt.Sprintf("--test_env=COCKROACH_WORKSPACE=%s", workspace)) + home, err := d.os.HomeDir() + if err != nil { + return err + } + args = append(args, + fmt.Sprintf("--test_env=HOME=%s", home), + fmt.Sprintf("--sandbox_writable_path=%s", home)) + args = append(args, "--test_output", "streamed") + // We need the location of the Go SDK for staticcheck and gcassert. + out, err := d.exec.CommandContextSilent(ctx, "bazel", "run", "@go_sdk//:bin/go", "--run_under=//build/bazelutil/whereis") + if err != nil { + return fmt.Errorf("could not find location of Go SDK (%w)", err) + } + goBin := strings.TrimSpace(string(out)) + args = append(args, fmt.Sprintf("--test_env=GO_SDK=%s", filepath.Dir(filepath.Dir(goBin)))) if !short { // First, generate code to make sure GCAssert and any other // tests that depend on generated code still work. @@ -67,30 +101,11 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error { return fmt.Errorf("`cc` is not installed; needed for `TestGCAssert` (%w)", err) } cc = strings.TrimSpace(cc) - d.log.Printf("export CC=%s", cc) - d.log.Printf("export CXX=%s", cc) - envWithCc := []string{"CC=" + cc, "CXX=" + cc} - lintEnv = append(envWithCc, lintEnv...) + args = append(args, + fmt.Sprintf("--test_env=CC=%s", cc), + fmt.Sprintf("--test_env=CXX=%s", cc)) } - var args []string - // NOTE the --config=test here. It's very important we compile the test binary with the - // appropriate stuff (gotags, etc.) - args = append(args, "run", "--config=test", "//build/bazelutil:lint") - if numCPUs != 0 { - args = append(args, fmt.Sprintf("--local_cpu_resources=%d", numCPUs)) - } - args = append(args, additionalBazelArgs...) - args = append(args, "--", "-test.v") - if short { - args = append(args, "-test.short") - } - if timeout > 0 { - args = append(args, "-test.timeout", timeout.String()) - } - if filter != "" { - args = append(args, "-test.run", fmt.Sprintf("Lint/%s", filter)) - } logCommand("bazel", args...) if len(pkgs) > 1 { return fmt.Errorf("can only lint a single package (found %s)", strings.Join(pkgs, ", ")) @@ -100,11 +115,9 @@ func (d *dev) lint(cmd *cobra.Command, commandLine []string) error { if !strings.HasPrefix(pkg, "./") { pkg = "./" + pkg } - envvar := fmt.Sprintf("PKG=%s", pkg) - d.log.Printf("export %s", envvar) - lintEnv = append(lintEnv, envvar) + args = append(args, fmt.Sprintf("--test_env=PKG=%s", pkg)) } - err := d.exec.CommandContextWithEnv(ctx, lintEnv, "bazel", args...) + err = d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...) if err != nil { return err } diff --git a/pkg/cmd/dev/testdata/datadriven/bench b/pkg/cmd/dev/testdata/datadriven/bench index d1ad2ff8fc2d..a9b43ca5c37a 100644 --- a/pkg/cmd/dev/testdata/datadriven/bench +++ b/pkg/cmd/dev/testdata/datadriven/bench @@ -1,36 +1,36 @@ exec dev bench pkg/spanconfig/... ---- -echo $HOME +echo $HOME/.cache bazel test pkg/spanconfig/...:all --test_arg -test.run=- --test_arg -test.bench=. --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors exec dev bench pkg/sql/parser --filter=BenchmarkParse ---- -echo $HOME +echo $HOME/.cache bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors exec dev bench pkg/sql/parser --filter=BenchmarkParse --stream-output ---- -echo $HOME +echo $HOME/.cache bazel test pkg/sql/parser:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkParse --test_sharding_strategy=disabled --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output streamed exec dev bench pkg/bench -f=BenchmarkTracing/1node/scan/trace=off --count=2 --bench-time=10x --bench-mem ---- -echo $HOME +echo $HOME/.cache bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --test_arg -test.count=2 --test_arg -test.benchtime=10x --test_arg -test.benchmem --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors exec dev bench pkg/spanconfig/spanconfigkvsubscriber -f=BenchmarkSpanConfigDecoder --cpus=10 --ignore-cache -v --timeout=50s ---- -echo $HOME +echo $HOME/.cache bazel test --local_cpu_resources=10 --test_timeout=50 pkg/spanconfig/spanconfigkvsubscriber:all --nocache_test_results --test_arg -test.run=- --test_arg -test.bench=BenchmarkSpanConfigDecoder --test_sharding_strategy=disabled --test_arg -test.v --crdb_test_off --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output all exec dev bench pkg/bench -f='BenchmarkTracing/1node/scan/trace=off' --test-args '-test.memprofile=mem.out -test.cpuprofile=cpu.out' ---- bazel info workspace --color=no -echo $HOME +echo $HOME/.cache bazel test pkg/bench:all --test_arg -test.run=- --test_arg -test.bench=BenchmarkTracing/1node/scan/trace=off --test_sharding_strategy=disabled --crdb_test_off --test_arg -test.outputdir=crdb-checkout --sandbox_writable_path=crdb-checkout --test_arg -test.memprofile=mem.out --test_arg -test.cpuprofile=cpu.out --test_env COCKROACH_TEST_FIXTURES_DIR=crdb-mock-test-fixtures/crdb-test-fixtures --sandbox_writable_path=crdb-mock-test-fixtures/crdb-test-fixtures --test_output errors diff --git a/pkg/cmd/dev/testdata/datadriven/lint b/pkg/cmd/dev/testdata/datadriven/lint deleted file mode 100644 index 30f15c4575ad..000000000000 --- a/pkg/cmd/dev/testdata/datadriven/lint +++ /dev/null @@ -1,57 +0,0 @@ -exec -dev lint ----- -bazel run //pkg/gen:code -bazel info workspace --color=no -bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' -which cc -export CC= -export CXX= -bazel run --config=test //build/bazelutil:lint -- -test.v -bazel build //pkg/cmd/cockroach-short //pkg/cmd/dev //pkg/obsservice/cmd/obsservice //pkg/cmd/roachprod //pkg/cmd/roachtest --//build/toolchains:nogo_flag - -exec -dev lint --short --timeout=5m ----- -bazel run --config=test //build/bazelutil:lint -- -test.v -test.short -test.timeout 5m0s - -exec -dev lint pkg/cmd/dev ----- -bazel run //pkg/gen:code -bazel info workspace --color=no -bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' -which cc -export CC= -export CXX= -export PKG=./pkg/cmd/dev -bazel run --config=test //build/bazelutil:lint -- -test.v -bazel build //pkg/cmd/cockroach-short //pkg/cmd/dev //pkg/obsservice/cmd/obsservice //pkg/cmd/roachprod //pkg/cmd/roachtest --//build/toolchains:nogo_flag - -exec -dev lint -f TestLowercaseFunctionNames --cpus 4 ----- -bazel run //pkg/gen:code -bazel info workspace --color=no -bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' -which cc -export CC= -export CXX= -bazel run --config=test //build/bazelutil:lint --local_cpu_resources=4 -- -test.v -test.run Lint/TestLowercaseFunctionNames - -exec -dev lint --cpus 4 ----- -bazel run //pkg/gen:code -bazel info workspace --color=no -bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' -which cc -export CC= -export CXX= -bazel run --config=test //build/bazelutil:lint --local_cpu_resources=4 -- -test.v -bazel build //pkg/cmd/cockroach-short //pkg/cmd/dev //pkg/obsservice/cmd/obsservice //pkg/cmd/roachprod //pkg/cmd/roachtest --//build/toolchains:nogo_flag --local_cpu_resources=4 - -exec -dev lint pkg/cmd/dev pkg/spanconfig ----- -err: can only lint a single package (found pkg/cmd/dev, pkg/spanconfig) diff --git a/pkg/cmd/dev/testdata/recorderdriven/lint b/pkg/cmd/dev/testdata/recorderdriven/lint new file mode 100644 index 000000000000..8355e93f5eb0 --- /dev/null +++ b/pkg/cmd/dev/testdata/recorderdriven/lint @@ -0,0 +1,11 @@ +dev lint +---- +bazel info workspace --color=no +echo $HOME +bazel run @go_sdk//:bin/go --run_under=//build/bazelutil/whereis +bazel run //pkg/gen:code +bazel info workspace --color=no +bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' +which cc +bazel test //pkg/testutils/lint:lint_test --test_arg -test.v --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_env=HOME=/home/user --sandbox_writable_path=/home/user --test_output streamed --test_env=GO_SDK=/path/to/go/sdk --test_env=CC=/usr/bin/cc --test_env=CXX=/usr/bin/cc +bazel build //pkg/cmd/cockroach-short //pkg/cmd/dev //pkg/obsservice/cmd/obsservice //pkg/cmd/roachprod //pkg/cmd/roachtest --//build/toolchains:nogo_flag diff --git a/pkg/cmd/dev/testdata/recorderdriven/lint.rec b/pkg/cmd/dev/testdata/recorderdriven/lint.rec new file mode 100644 index 000000000000..c3ec7d9bfae3 --- /dev/null +++ b/pkg/cmd/dev/testdata/recorderdriven/lint.rec @@ -0,0 +1,23 @@ +echo $HOME +---- +/home/user + +bazel run @go_sdk//:bin/go --run_under=//build/bazelutil/whereis +---- +/path/to/go/sdk/bin/go + +bazel run //pkg/gen:code +---- + +bazel run //pkg/cmd/generate-cgo:generate-cgo '--run_under=cd crdb-checkout && ' +---- + +which cc +---- +/usr/bin/cc + +bazel test //pkg/testutils/lint:lint_test --test_arg -test.v --test_env=COCKROACH_WORKSPACE=crdb-checkout --test_env=HOME=/home/user --sandbox_writable_path=/home/user --test_output streamed --test_env=GO_SDK=/path/to/go/sdk --test_env=CC=/usr/bin/cc --test_env=CXX=/usr/bin/cc +---- + +bazel build //pkg/cmd/cockroach-short //pkg/cmd/dev //pkg/obsservice/cmd/obsservice //pkg/cmd/roachprod //pkg/cmd/roachtest --//build/toolchains:nogo_flag +---- diff --git a/pkg/testutils/lint/BUILD.bazel b/pkg/testutils/lint/BUILD.bazel index d9e7fb527676..1aa489dce63d 100644 --- a/pkg/testutils/lint/BUILD.bazel +++ b/pkg/testutils/lint/BUILD.bazel @@ -14,14 +14,18 @@ go_library( go_test( name = "lint_test", + size = "enormous", # keep srcs = [ "gen-lint_test.go", "nightly_lint_test.go", ], data = glob(["testdata/**"]) + [ + "//.github:CODEOWNERS", + "//build/bazelutil:nogo_config.json", + "//pkg/sql/opt/optgen/cmd/optfmt", "@co_honnef_go_tools//cmd/staticcheck", - "@go_sdk//:files", + "@com_github_cockroachdb_crlfmt//:crlfmt", ], embed = [":lint"], embedsrcs = ["gcassert_paths.txt"], @@ -38,6 +42,7 @@ go_test( "//pkg/sql/sem/builtins", "//pkg/testutils/datapathutils", "//pkg/testutils/skip", + "//pkg/util/envutil", "@com_github_cockroachdb_errors//:errors", "@com_github_ghemawat_stream//:stream", "@com_github_jordanlewis_gcassert//:gcassert", diff --git a/pkg/testutils/lint/lint_test.go b/pkg/testutils/lint/lint_test.go index 29ce85fa6418..e6b56de44a4a 100644 --- a/pkg/testutils/lint/lint_test.go +++ b/pkg/testutils/lint/lint_test.go @@ -32,6 +32,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/builtins" "github.com/cockroachdb/cockroach/pkg/testutils/datapathutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" + "github.com/cockroachdb/cockroach/pkg/util/envutil" "github.com/cockroachdb/errors" "github.com/ghemawat/stream" "github.com/jordanlewis/gcassert" @@ -46,11 +47,14 @@ var rawGcassertPaths string func init() { if bazel.BuiltWithBazel() { - gobin, err := bazel.Runfile("bin/go") - if err != nil { + goSdk := os.Getenv("GO_SDK") + if goSdk == "" { + panic("expected GO_SDK") + } + if err := os.Setenv("PATH", fmt.Sprintf("%s%c%s", filepath.Join(goSdk, "bin"), os.PathListSeparator, os.Getenv("PATH"))); err != nil { panic(err) } - if err := os.Setenv("PATH", fmt.Sprintf("%s%c%s", filepath.Dir(gobin), os.PathListSeparator, os.Getenv("PATH"))); err != nil { + if err := os.Setenv("GOROOT", goSdk); err != nil { panic(err) } } @@ -120,7 +124,14 @@ func vetCmd(t *testing.T, dir, name string, args []string, filters []stream.Filt // should be marked with t.Parallel(). func TestLint(t *testing.T) { var crdbDir, pkgDir string - { + if bazel.BuiltWithBazel() { + var set bool + crdbDir, set = envutil.EnvString("COCKROACH_WORKSPACE", 0) + if !set || crdbDir == "" { + t.Fatal("must supply COCKROACH_WORKSPACE variable (./dev lint does this for you automatically)") + } + pkgDir = filepath.Join(crdbDir, "pkg") + } else { cwd, err := os.Getwd() if err != nil { t.Fatal(err) @@ -135,10 +146,24 @@ func TestLint(t *testing.T) { pkgVar, pkgSpecified := os.LookupEnv("PKG") var nogoConfig map[string]any - nogoJSON, err := os.ReadFile(filepath.Join(crdbDir, "build", "bazelutil", "nogo_config.json")) - if err != nil { - t.Fatal(err) + var nogoJSON []byte + if bazel.BuiltWithBazel() { + nogoJSONPath, err := bazel.Runfile("build/bazelutil/nogo_config.json") + if err != nil { + t.Fatal(err) + } + nogoJSON, err = os.ReadFile(nogoJSONPath) + if err != nil { + t.Fatal(err) + } + } else { + var err error + nogoJSON, err = os.ReadFile(filepath.Join(crdbDir, "build", "bazelutil", "nogo_config.json")) + if err != nil { + t.Fatal(err) + } } + if err := json.Unmarshal(nogoJSON, &nogoConfig); err != nil { t.Error(err) } @@ -393,6 +418,16 @@ func TestLint(t *testing.T) { skip.IgnoreLint(t, "PKG specified") } + // If run outside of Bazel, we assume this binary must be in the PATH. + optfmt := "optfmt" + if bazel.BuiltWithBazel() { + var err error + optfmt, err = bazel.Runfile("pkg/sql/opt/optgen/cmd/optfmt/optfmt_/optfmt") + if err != nil { + t.Fatal(err) + } + } + cmd, stderr, filter, err := dirCmd(pkgDir, "git", "ls-files", "*.opt", ":!*/testdata/*") if err != nil { t.Fatal(err) @@ -409,7 +444,7 @@ func TestLint(t *testing.T) { stream.Map(func(s string) string { return filepath.Join(pkgDir, s) }), - stream.Xargs("optfmt", "-l"), + stream.Xargs(optfmt, "-l"), ), func(s string) { fmt.Fprintln(&buf, s) }); err != nil { @@ -1538,8 +1573,18 @@ func TestLint(t *testing.T) { if pkgSpecified { skip.IgnoreLint(t, "PKG specified") } + // If run outside of Bazel, we assume this binary must be in the PATH. + crlfmt := "crlfmt" + if bazel.BuiltWithBazel() { + var err error + crlfmt, err = bazel.Runfile("external/com_github_cockroachdb_crlfmt/crlfmt_/crlfmt") + if err != nil { + t.Fatal(err) + } + } + ignore := `zcgo*|\.(pb(\.gw)?)|(\.[eo]g)\.go|/testdata/|^sql/parser/sql\.go$|(_)?generated(_test)?\.go$|^sql/pgrepl/pgreplparser/pgrepl\.go$|^sql/plpgsql/parser/plpgsql\.go$` - cmd, stderr, filter, err := dirCmd(pkgDir, "crlfmt", "-fast", "-ignore", ignore, "-tab", "2", ".") + cmd, stderr, filter, err := dirCmd(pkgDir, crlfmt, "-fast", "-ignore", ignore, "-tab", "2", ".") if err != nil { t.Fatal(err) } @@ -2139,15 +2184,13 @@ func TestLint(t *testing.T) { t.Run("TestGCAssert", func(t *testing.T) { skip.UnderShort(t) - t.Parallel() - var gcassertPaths []string for _, path := range strings.Split(rawGcassertPaths, "\n") { path = strings.TrimSpace(path) if path == "" { continue } - gcassertPaths = append(gcassertPaths, fmt.Sprintf("../../%s", path)) + gcassertPaths = append(gcassertPaths, filepath.Join(crdbDir, "pkg", path)) } // Ensure that all packages that have '//gcassert' or '// gcassert' @@ -2181,7 +2224,7 @@ func TestLint(t *testing.T) { // and we want to extract the package path. filePath := s[:strings.Index(s, ":")] // up to the line number pkgPath := filePath[:strings.LastIndex(filePath, "/")] // up to the file name - gcassertPath := "../../" + pkgPath + gcassertPath := filepath.Join(crdbDir, "pkg", pkgPath) for i := range gcassertPaths { if gcassertPath == gcassertPaths[i] { return @@ -2200,10 +2243,10 @@ func TestLint(t *testing.T) { }) var buf strings.Builder - if err := gcassert.GCAssert(&buf, gcassertPaths...); err != nil { - t.Fatal(err) - } output := buf.String() + if err := gcassert.GCAssertCwd(&buf, crdbDir, gcassertPaths...); err != nil { + t.Fatalf("failed gcassert (%+v):\n%s", err, buf.String()) + } if len(output) > 0 { t.Fatalf("failed gcassert:\n%s", output) } @@ -2505,8 +2548,7 @@ func TestLint(t *testing.T) { co, err := codeowners.DefaultLoadCodeOwners() require.NoError(t, err) const verbose = false - repoRoot := filepath.Join("../../../") - codeowners.LintEverythingIsOwned(t, verbose, co, repoRoot, "pkg") + codeowners.LintEverythingIsOwned(t, verbose, co, crdbDir, "pkg") }) t.Run("cookie construction is forbidden", func(t *testing.T) {