bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.36k stars 647 forks source link

doc: Document how to run external vetters #626

Open kevinburke opened 7 years ago

kevinburke commented 7 years ago

I like running this tool on Go codebases: https://github.com/dominikh/go-tools/tree/master/cmd/megacheck

I'm not sure the best way to do it. I'm downloading it using go_repository but I'm not sure how to invoke it, whether I have to compile it, or where it installs to.

(I'm new to Bazel but I assume the instructions will be useful to all Go Bazel developers)

spxtr commented 7 years ago

Related: #511

We would also like guidance on this.

kevinburke commented 7 years ago

Okay I have something roughly working: this in WORKSPACE:

go_repository(
    name = "com_github_kisielk_gotool", 
    importpath = "github.com/kisielk/gotool",
    commit = "0de1eaf82fa3f583ce21fde859f1e7e0c5e9b220",
)

go_repository(
    name = "megacheck", 
    importpath = "honnef.co/go/tools",
    commit = "f583b587b6ff1149f9a9b0c16ebdda74da44e1a2",
)

and then this in BUILD.bazel:

sh_test(
    name = "go_html_boilerplate_megacheck",
    srcs = [
        "@megacheck//cmd/megacheck:megacheck",
    ],
    args = [
        ".",
    ],
    data = [
        ":go-srcs",
    ],
    timeout = "short",
)

However, I'm running into a problem where GOPATH isn't set when I run megacheck so standard library imports fail. Here's a small sample:

/private/var/tmp/_bazel_kevin/0261fe5f79c7721cd61b010c29e100ac/bazel-sandbox/5134534126330854747/execroot/__main__/bazel-out/darwin_x86_64-fastbuild/bin/go_html_boilerplate_megacheck.runfiles/__main__/crypto.go:6:2: could not import crypto/rand (cannot find package "crypto/rand" in any of:
    /usr/local/go/src/crypto/rand (from $GOROOT)
    ($GOPATH not set. For more details see: 'go help gopath'))
/private/var/tmp/_bazel_kevin/0261fe5f79c7721cd61b010c29e100ac/bazel-sandbox/5134534126330854747/execroot/__main__/bazel-out/darwin_x86_64-fastbuild/bin/go_html_boilerplate_megacheck.runfiles/__main__/crypto.go:7:2: could not import encoding/base64 (cannot find package "encoding/base64" in any of:
    /usr/local/go/src/encoding/base64 (from $GOROOT)
    ($GOPATH not set. For more details see: 'go help gopath'))
/private/var/tmp/_bazel_kevin/0261fe5f79c7721cd61b010c29e100ac/bazel-sandbox/5134534126330854747/execroot/__main__/bazel-out/darwin_x86_64-fastbuild/bin/go_html_boilerplate_megacheck.runfiles/__main__/crypto.go:8:2: could not import encoding/hex (cannot find package "encoding/hex" in any of:
    /usr/local/go/src/encoding/hex (from $GOROOT)
    ($GOPATH not set. For more details see: 'go help gopath'))

How can I set that? It seems like Bazel doesn't really support env vars. I'd rather not need to add a shell script to get this test to work.

kjc-stripe commented 7 years ago

@kevinburke From my limited understanding, you shouldn't be downloading the source code for megacheck. Instead, you should be downloading the precompiled binaries, similar to how these rules install the go runtime in the sandbox.

kevinburke commented 7 years ago

Sure, but I think I'd still run into the same problem; megacheck expects GOPATH and GOROOT to be set so it can find e.g. the standard library as part of running checks. I'm pretty sure the other tools do as well - vet, unused, etc.

ianthehat commented 7 years ago

GOROOT should be correctly set, and is where things find the standard library. In general, GOPATH cannot be set by bazel, because there is no requirement in bazel for anything to be even in a structure that would be valid on a GOPATH, you could (but please don't) put all your packages in a single directory with radically different import paths, if you really wanted, and externals are not checked out into their import paths.

The other problem is that generated source files are not in the same place as non generated code, which is one of the selling points of Bazel (for me, I hate checking in generated code) but is not something that fits in the directory per package model of GOPATH either.

We have a longer term plan to support tooling better by abstracting the location of source files from the tools inside the build package, when we get that working all the tools that use go/build will start working. There are people actively working on this, and it's very important to us, but I don't have a timeline for you at the moment.

In the slightly shorter term, I also want to add a system for recreating things that look like valid GOPATH entries as an "export" from bazel, this would allow you to create something that go get would understand from a bazel build (in file export mode), and also allow things like ide's and command line tools to work with the source (in soft/hard link mode). I have a prototype of this, but it needs more work before I can ship it, and making cross-compile, protos and cgo all work properly are higher priority.

To some extent you can make it work anyway right now, if you have your checkout on something that looks like a valid go path, and you run the sh_test in local non sandbox mode, and you have your dependancies also on your GOPATH somewhere. You can look a the gazelle rules for an example of how to do this (it is also building from source, I think it's the right choice if you can), it's fairly ugly and fragile though (I am reading soft links to work out the original workspace directory)

kevinburke commented 7 years ago

GOROOT should be correctly set, and is where things find the standard library.

The sh_test command I had was:

sh_test(
    name = "go_html_boilerplate_megacheck",
    timeout = "short",
    srcs = [
        "@megacheck//cmd/megacheck:megacheck",
    ],
    args = [
        ".",
    ],
    data = [
        ":go-srcs",
    ],
)

I modified the sh_test command to dump the environment:

sh_test(
    name = "go_html_boilerplate_megacheck",
    timeout = "short",
    srcs = [
        "foo.sh",
    ],
    args = [
        ".",
    ],
    data = [
        ":go-srcs",
    ],
)

GOROOT wasn't present in the list of set environment variables...

ianthehat commented 7 years ago

Ah, yes, you are using sh_test. It's set for go_test, we have no control over sh_test.

kevinburke commented 7 years ago

OK so if I am reading correctly there is essentially no way to run automated AST parsers/checkers like megacheck or unused that require the GOPATH to be set? Unless you run some sh_test script that can figure out the correct way to set it for the environment, or call one of those binaries from e.g. TestMegacheck(t *testing.T).

Helcaraxan commented 5 years ago

I am in a relative similar situation. I have a Go binary that generates mocks (an internal tool that uses vektra/mockery). I want to run this tool on our codebase using bazel run in order to ensure full reproducibility. However the tool needs (for the same reasons as megacheck) access to the standard library sources.

How should I go about setting the appropriate GOROOT (i.e. the one used for compiling the binary) when it is invoked via bazel run.

There is a potentially insane hack of having the tools go_binary depend on io_bazel_rules_go//:go_info and have the tool parse the output in the platform dependent bazel-bin/external/io_bazel_rules_go/<platform>_stripped/go_info%/go_info_report file, retrieve the GOROOT and set the environment variable after doing so... but I'd really rather not do that.

jayconrod commented 5 years ago

@Helcaraxan Your best bet is probably to write a Bazel rule, ideally one that produces .go files that can be embedded into a go_library without needing the files to be checked in. There's some documentation on this in go/toolchains.rst, but I don't have a really good example for you. You may also find go_path useful.

Helcaraxan commented 5 years ago

Ideally I would indeed do that (had already started on it, basing myself on the existing work for gomock support). However in our case we need the files to be checked-in for the development flow in our IDEs to not be broken, we do the same for generated files from proto / grpc. I am aware of the ongoing work on the workspace abstraction but until that is complete we will unfortunately need to rely on checked-in files.

I've looked at go_path indeed and that would be definitely a way to go for an intermediary step: bazelify the generation of mocks (and our proto's which is already done) and then generate the relevant files to check-in via go_path.

Thank you for the advice and for the amazing work that you have been doing on Gazelle and rules_go over the years. 👍

jayconrod commented 5 years ago

Re-triaging old issues. nogo is the intended way to run static analysis tools.

We still need better documentation on integrating external vetters.