bazel-contrib / rules_go

Go rules for Bazel
Apache License 2.0
1.39k stars 664 forks source link

Expose generated Go files to editors and IDEs #512

Closed jayconrod closed 1 year ago

jayconrod commented 7 years ago

When a library is created with go_proto_library or some other mechanism that generates Go code (.pb.go files in this case), the generated sources files are not exposed to editors and IDEs. Consequently, code completion and refactoring tools don't work, lowering developer productivity.

The Go team is developing a workspace abstraction mechanism that will allow editors and Bazel (and other build systems) to communicate with each other without the need for direct integration. This is the long-term solution to this problem.

This issue will track progress on the workspace abstraction and will track editor awareness of code generated with Bazel.

jayconrod commented 7 years ago

This issue is in response to Bazel build, protobuf and code completion.

TODO: update answer when this issue is resolved.

excavador commented 7 years ago

What the status of the task?

excavador commented 7 years ago

So, I have proposal how to close this issue.

Suppose you generated code: bazel-genfiles/somedir/somefile.go bazel-genfiles/somedir/anotherfile.go

Let's add symlink src/somedir => ../bazel-genfiles/somedir

Use in your files import "somedir"

As result you will get workable code-completition AND workable build.

jayconrod commented 7 years ago

@excavador We're still developing a more complete solution to this (the workspace abstraction I mentioned above). Ideally, tools won't need to be aware of the project layout in use. I don't have any ETA for that yet, but we are making progress.

As you showed, it's possible to work around this with symlinks. You can also copy or symlink generated files into your source directory. Bazel will prefer a generated file if a source file of the same name is present. I'd recommend against checking generated files into VCS though.

excavador commented 7 years ago

Thank you for notify! Will wait. You can consider me as closed beta-tester :)

raliste commented 6 years ago

Any updates?

excavador commented 6 years ago

@raliste it works for me in IDEA Ultimate + Bazel plugin with following limitation: autocomplete works only if all files inside golang package are auto-generated. If mix together auto-generated and manually written file, autogenerated file would not be inspected

jayconrod commented 6 years ago

The workspace abstraction mentioned above is being actively worked on. We've gone through a lot of internal prototypes, and we're in the process of reviewing, testing, and merging one that we're pretty happy with. It's a huge body of code though, and it will take time. Following that, a large number of Go tools will need to be updated to use the workspace abstraction so they can function in "go build", vgo, Bazel, and other kinds of workspaces.

I know it seems like progress is slow, but it's an interface that many, many tools will interact with, and we want to be sure we get it right. It's a huge amount of work, but it is our highest priority task.

burdiyan commented 6 years ago

@jayconrod Are there any public discussions about that? Design docs perhaps?

jayconrod commented 6 years ago

Not yet, but soon. We're still working on simplifying and narrowing the interface.

vitarb commented 6 years ago

Can you briefly explain how this workspaces feature is going to work? Are there any upcoming changes in the 1.11 related to this?

jayconrod commented 6 years ago

The first public piece of this is golang.org/x/tools/go/packages. At the moment, that is based on go list and is intended to be used by tools that support go build and vgo. We're also working on an workspace abstraction layer that go/packages may eventually be built on. That workspace abstraction would be extensible and could include Bazel. Lot of work still happening there.

akshayjshah commented 6 years ago

@jayconrod Any updates on the proposed workspace abstraction layer?

jayconrod commented 6 years ago

@akshayjshah We've made some progress. The golang.org/x/tools/go/packages API is nearing completion. Our team has migrated a few tools to use it already, but the effort is ongoing.

If you set the GOPACKAGESDRIVER environment variable, or if you have a binary named gopackagesdriver in PATH, go/packages will shell out to that binary for queries. I have an implementation of that binary about 70% done. It works in most situations, but it needs polish, documentation, and tests.

So ideally, once that change is in, you'd be able to build and install the driver into your PATH, and tools that use go/packages will just work in Bazel workspaces (and they'll fall back to the default behavior if there's no WORKSPACE file).

akshayjshah commented 6 years ago

Thanks for the update! Would you be willing to push your driver to a branch so I can take a look? I'm not in a huge rush for you to officially release it, but I'd love to use your rough draft to better understand how these pieces should fit together.

jayconrod commented 6 years ago

@akshayjshah Sure, it's at https://github.com/jayconrod/rules_go/tree/dev-gopackagesdriver if you'd like to take a look. As I said, it's very rough right now.

pierreis commented 6 years ago

That looks awesome, thanks! Is there any update on this?

jayconrod commented 6 years ago

@pierreis Sorry, no updates since my last comment. I'm still planning to work on this this quarter. There are a lot of things going on, but this is one of the higher priority projects.

jayconrod commented 5 years ago

Generated .pb.go files are now available in the go_generated_srcs output group since #1827.

Leaving this issue open though since it's not documented yet.

vitarb commented 5 years ago

@jayconrod is package driver still planned as a feature or are we going to rely on the aspect-based approach mentioned above?

jayconrod commented 5 years ago

A driver is still planned, but I don't have any update to share yet. Implementation-wise, the data will be produced by the builder and accessed through an output group (but the driver will hide those details).

clanstyles commented 5 years ago

I assume the driver is going to be something integrated into the IDE?

robbertvanginkel commented 5 years ago

@jayconrod rebased your branch and updated it with the latest changes from the go/packages API to play around a bit with the driver concept. I managed to get it to a point where I can use the bazel driver to power some of the module enabled tools like godef-mod or gopls. You can find it at https://github.com/robbertvanginkel/rules_go/tree/dev-gopackagesdriver.

Some of the code is a bit gnarly, needs some tests and there's still a few TODO's in there, but I'd love to help out and get the driver added to rules_go. Since the branch is mostly based on your changes, do you have a suggestion on how we could collaborate on this?

jayconrod commented 5 years ago

@robbertvanginkel I'm probably not going to keep most of the code on that branch. I don't think gathering information through separate actions via an aspect will be fast enough, and it adds a lot of complication. Instead, I'm thinking of having the compile action emit JSON metadata, then the driver can just "build" an output group and collect the results.

But a fair bit of refactoring is needed on the builder side of things first. I have a half-written PR to pull everything into a single binary and have the toolchain own that.

In short, I'm not ready to collaborate on this yet.

robbertvanginkel commented 5 years ago

Ok, thanks for the update! Please me know if you reach a point where someone can help out.

clanstyles commented 5 years ago

@jayconrod I'd love to help out too.

asv commented 5 years ago

@jayconrod I’m newbie in Skylark (and bazel concepts) yet. Can you tell how to copy (or link) files described in OutputGroup go_generated_src to some folder inside the workspace? In other words, can I now generate .pb.go a la protoc + protoc-gen-go using rules_go?

jayconrod commented 5 years ago

@asv This is something I wish were simpler in Bazel. You can build with the command line argument --output_groups=go_generated_src (or any comma separated list of group names). To get the paths to the files in a machine-readable format, you can use --build_event_json_file or --build_event_proto_file, depending on whether you want to parse JSON or proto. build_event_stream.proto describes the format. You'll be looking for an event with a TargetComplete payload.

asv commented 5 years ago

@jayconrod I'm got it!

Small fix to your post: --output_groups=go_generated_srcs. Name of group go_generated_srcs, not go_generated_src. It's a pity that bazel does not complain on error in name.

clanstyles commented 5 years ago

What's the state of this ticket? It looks like the necessary functions were added to Bazel. It also looks like there's a go package to support it. What's missing?

jayconrod commented 5 years ago

@clanstyles Nothing external is blocking this work anymore, but I'm working on landing some architectural changes first that will simplify this and fix a number of other issues.

In particular, I'm working on moving nearly all of the logic to build individual packages into the execution phase in a single action. This will dramatically reduce the loading / analysis burden for cgo, and it should enable better cross-compilation in the future. It should also lower remote execution overheads. One of the outputs of this action will be json files that can be picked up by a gopackagesdriver binary and presented through the golang.org/x/tools/go/packages API. The work so far is on my fork in the dev-compilepkg branch if you're curious.

As you can imagine, rewriting anything related to cgo is complicated, and it takes time to get it right. These days, I'm primarily working on module support in the Go command and tools to validate module releases. rules_go and Gazelle are still active of course, but most of the time I have allocated each week goes to answering questions, fixing critical bugs, and reviewing PRs.

TennyZhuang commented 5 years ago

Is there any workaround rule or example to copy or link the generated .pb.go to the sourse directory by bazel run?

kalbasit commented 5 years ago

@TennyZhuang I use a basic bash script to symlink the generated proto files: https://gist.github.com/kalbasit/3c9607333f5f3c794d2bd96114184301

clanstyles commented 5 years ago

@kalbasit when are you triggering this?

kalbasit commented 5 years ago

@clanstyles manually. I know that it sounds like a lot to remember, but it does become a habit. If I'm changing a protobuf, I ran kt proto. If I'm changing an asset, I ran kt gen. For context, kt stands for KeepTruckin and is a rich CLI to interact with our mono-repo, driven by Nix/Bazel.

You can probably get something integrated with your editor triggered upon saving. I do not recommend doing that if it's going to add latency to your overall editing experience.

strican commented 5 years ago

@jayconrod - It seems like you had made a bunch of progress on this issue, but you've walked it back to do the architectural changes first. Would there be value in making the feature available (at least to collaborators) so more people can get value sooner? Then the architectural changes can come in under the covers and everyone who is using the interim system will benefit.

jayconrod commented 5 years ago

@strican Architectural changes landed in #2027. I'll implement this as soon as I have bandwidth.

akshayjshah commented 5 years ago

Jay, are you open to a PR for this?

jayconrod commented 5 years ago

@akshayjshah No, probably not for this. I don't think there's going to be a lot of code, but it needs to be carefully written across several layers.

ChinmayR commented 5 years ago

@jayconrod any update on this? what is the timeline we should track for this change?

jayconrod commented 5 years ago

@ChinmayR I've done some prototyping on https://github.com/jayconrod/rules_go/tree/dev-gopackagesdriver a few weeks ago. A few specific cases are close to working, but as a whole, it's not all that close.

My time is split between several projects. Of the time I'm able to spend on rules_go, most of it goes to maintaining compatibility with new versions of Bazel. #2079 and #2089 for example are taking all my time this week and last week. I'm also working on closing release blockers like #2058 before the next major release.

Sorry, I know this isn't moving as fast as everyone would like, but stability comes first.

dancompton commented 5 years ago

@jayconrod I know you're busy, but what items are remaining on this? Would it be possible for the community to contribute?

Does anyone have gopls working (reliably) with vim+bazel?

codesuki commented 5 years ago

As Dan said above, is there a specific task list that we could help with? If it's a lot of explorational work I could understand that you have to work on it alone, but if there would be a design doc or similar.. :)

jayconrod commented 5 years ago

To recap and add to what's already been said: this is the most important feature request currently pending in rules_go. However, I prioritize maintenance work (especially Go and Bazel compatibility) and bug fixing over features, and lately, there's been a lot of the former. Bazel has been shipping a lot of breaking changes before their 1.0 release planned for this fall, and for the last month or two, it's been taking 40-50% of my work week to keep up with that, plus operational stuff like tagging releases, triaging issues, and answering questions. My time budget for rules_go and Gazelle is ~20%, so that has meant mornings and weekends.

While this feature is high priority, it's also very large and complicated. I'd guess it's on the order of 15-30 eng-days for the main implementation, plus a long tail of bug fixing and support. The main chunk of work is a binary that implements the driver interface of golang.org/x/tools/go/packages. The driver is invoked by packages, and it needs to run bazel build with some output group or aspect. From there, it will extract the output and print JSON objects for each package. The objects may contain anything from source file names to fully parsed ASTs with type information, depending on what's requested. There are a number of complications for the driver:

Anyone implementing this needs to be intimately familiar with Bazel rules, aspects, golang.org/x/tools/go/packages, and all layers of rules_go stack. It wouldn't hurt to know about how go list and gopls both work. A lot of design work needs to go into this. I have a rough idea of what needs to be done, but I don't have a blueprint.

If someone is willing to learn all this and implement it, I'd be willing to accept PRs only if you're willing to take at least partial ownership of rules_go. There will definitely be a lot of bugs in anything this big and complicated. Some of those may actually be gopls or packages bugs that start out here, but the whole stack needs to be debugged. Many of the reported issues will be pretty vague. I'll only invest time in mentoring and reviewing someone if they're willing to stay and help with support, bug fixing, and code health long-term.

akshayjshah commented 5 years ago

Thanks for the detailed reply, Jay!

I'll only invest time in mentoring and reviewing someone if they're willing to stay and help with support, bug fixing, and code health long-term.

This is a really, really generous offer. I'll poke through the network of people I know are interested in this issue and see if I can find someone who's funded to help develop rules_go as part of their day job.

jayconrod commented 5 years ago

I've written a design doc on the wiki at Editor and tool integration. I think I have a better understanding of how to deal with test packages and overlays, which were the biggest area of uncertainty for me before.

Folks who are interested in working on this, please take a look and let me know (here or on Gophers Slack) if you have thoughts or questions.

I have some code that parses the configuration, runs bazel, parses the Bazel proto output, and formats package output. I'll clean that up and put it on a branch next week. There's still a lot of work to do after that though.

jmhodges commented 5 years ago

@jayconrod Is https://github.com/jayconrod/rules_go/tree/dev-gopackagesdriver the right branch to be looking at for your latest work? I was curious about helping out.

jayconrod commented 5 years ago

@jmhodges feature/gopackagesdriver has a skeleton implementation. That has as much code from the dev-gopackagesdriver branch as I thought was usable.

@vitarb was working on this a while ago. Not sure if he's made progress.

jmhodges commented 5 years ago

Plunked down a commit that adds an initial failing go_bazel_test on my own feature/gopackagesdriver branch.

I'm going to pick this back up tomorrow, and, since I've paused, I wanted to see if this was the kind of thing you meant for the testing strategy in the design doc.

jmhodges commented 5 years ago

So, I've gotten a good bit further.

@jayconrod The design doc talks about distinguishing between test targets and non-test targets but I'm not sure how we'd do this from inside the aspect in order to get the IDs set correctly. We, of course, don't have a GoTest provider currently. Have I missed an API we can call inside skylark to grab the target's kind or should we create a GoTest provider for this? Or some other better option I didn't see?

Relatedly, as I worked on this, I realized we probably don't want the non-recursive aspects (e.g. gopackagesdriver_files) to produce output on go_binary targets with embed set because we'd duplicate the work done on the original go_library that was embedded and possibly confuse downstream tooling.

Do you have any thoughts on how to distinguish go_binary from go_test in the aspect without also accidentally no longer working in the go_test case itself?

There's a GoBinary provider we can check the target for, but no equivalent GoTest provider. Perhaps I'm missing something obvious!