Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel: provide way to run lint in bazel #65518

Merged
merged 1 commit into from
May 27, 2021

Conversation

rickystewart
Copy link
Collaborator

@rickystewart rickystewart commented May 20, 2021

With this patch, lints can be run with bazel run build/bazelutil:lint.
The test binary takes the typical Go command-line arguments, so e.g.
bazel run build/bazelutil:lint -- -test.v.

We model this as a sh_binary that depends on the lint test binary as
well as all the dependencies that it has (roachvet, crlfmt, and
others). The shell script is a light wrapper that finds the actual
location of all the dependencies and stages them all in the PATH
appropriately. We also need to munge LDFLAGS because the linter calls
into the compiler in a couple places, and cgo wants to link into some
of the C libraries (libedit, libproj, etc.)

There are a couple tests that are still broken, which are skipped for
now. As a next step, we'll want to get this running in CI.

Closes #61555.

Release note: None

@rickystewart rickystewart requested a review from rail May 20, 2021 17:41
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart force-pushed the fulllint branch 3 times, most recently from 6a127dd to 0c6ffba Compare May 25, 2021 15:50
Copy link
Member

@rail rail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 9 of 9 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rail)


DEPS.bzl, line 2345 at r1 (raw file):

        build_file_proto_mode = "disable_global",
        importpath = "github.com/knz/go-libedit",
        patch_args = ["-p1"],

Interesting. Does it mean that you can't have multiple patches with different patch level? Sounds like a flaw in gazelle.

@rickystewart
Copy link
Collaborator Author

Interesting. Does it mean that you can't have multiple patches with different patch level? Sounds like a flaw in gazelle.

Yeah, it's weird. We haven't had to do too much with these patches so far, so it hasn't affected us.

bors r=rail

@craig
Copy link
Contributor

craig bot commented May 25, 2021

Build failed:

@rickystewart
Copy link
Collaborator Author

No idea what went wrong, let's try that again.

bors r=rail

@rickystewart
Copy link
Collaborator Author

bors r-

@craig
Copy link
Contributor

craig bot commented May 25, 2021

Canceled.

@rickystewart rickystewart force-pushed the fulllint branch 2 times, most recently from 085ebbe to 984877a Compare May 25, 2021 20:53
With this patch, lints can be run with `bazel run build/bazelutil:lint`.
The test binary takes the typical Go command-line arguments, so e.g.
`bazel run build/bazelutil:lint -- -test.v`.

We model this as a `sh_binary` that depends on the lint test binary as
well as all the dependencies that it has (`roachvet`, `crlfmt`, and
others). The shell script is a light wrapper that finds the actual
location of all the dependencies and stages them all in the `PATH`
appropriately. We also need to munge `LDFLAGS` because the linter calls
into the compiler in a couple places, and `cgo` wants to link into some
of the C libraries (`libedit`, `libproj`, etc.)

There are a couple tests that are still broken, which are skipped for
now. As a next step, we'll want to get this running in CI.

Closes cockroachdb#61555

Release note: None
@rickystewart
Copy link
Collaborator Author

bors r=rail

@craig
Copy link
Contributor

craig bot commented May 27, 2021

Build succeeded:

@craig craig bot merged commit 61d5d37 into cockroachdb:master May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bazel: add way to run lints w/ bazel-built binaries
3 participants