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

Expose generated Go files to editors and IDEs #512

Open
jayconrod opened this issue Jun 6, 2017 · 111 comments
Open

Expose generated Go files to editors and IDEs #512

jayconrod opened this issue Jun 6, 2017 · 111 comments

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Jun 6, 2017

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
Copy link
Contributor Author

@jayconrod jayconrod commented Jun 6, 2017

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

TODO: update answer when this issue is resolved.

@excavador
Copy link

@excavador excavador commented Jul 22, 2017

What the status of the task?

@excavador
Copy link

@excavador excavador commented Jul 22, 2017

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
Copy link
Contributor Author

@jayconrod jayconrod commented Jul 25, 2017

@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
Copy link

@excavador excavador commented Jul 25, 2017

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

@raliste
Copy link

@raliste raliste commented May 6, 2018

Any updates?

@excavador
Copy link

@excavador excavador commented May 6, 2018

@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
Copy link
Contributor Author

@jayconrod jayconrod commented May 7, 2018

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
Copy link

@burdiyan burdiyan commented Jun 20, 2018

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

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Jun 20, 2018

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

@vitarb
Copy link

@vitarb vitarb commented Jul 13, 2018

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
Copy link
Contributor Author

@jayconrod jayconrod commented Jul 13, 2018

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
Copy link
Contributor

@akshayjshah akshayjshah commented Sep 7, 2018

@jayconrod Any updates on the proposed workspace abstraction layer?

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Sep 7, 2018

@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
Copy link
Contributor

@akshayjshah akshayjshah commented Sep 7, 2018

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
Copy link
Contributor Author

@jayconrod jayconrod commented Sep 7, 2018

@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
Copy link

@pierreis pierreis commented Nov 5, 2018

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

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Nov 5, 2018

@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
Copy link
Contributor Author

@jayconrod jayconrod commented Dec 13, 2018

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
Copy link

@vitarb vitarb commented Jan 17, 2019

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

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Jan 18, 2019

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).

@excavador
Copy link

@excavador excavador commented Jun 8, 2020

Just in case: I am wrote to this ticket, but after that I found IDEA plugin which solved my problems completely: https://plugins.jetbrains.com/plugin/8609-bazel

@ribrdb
Copy link

@ribrdb ribrdb commented Jun 16, 2020

The GoCompilePkg action should be as consolidated as possible. Bazel actions have a lot of overhead in some situations (particularly with remote execution and sandboxing on operating systems with slow file I/O). It's fine for the code to be split up and reused, it's just that we need to be able to build each package with a single action (or as close to that as possible). Indeed, the code that generates JSON blobs that the driver will read should be integrated into the builder.

I thought about having GoCompilePkg emit a JSON blob that the driver could use by default. That would work for configurations that don't need AST or type information. Not sure how common that is and whether it's worth optimizing for. Probably better to keep GoCompilePkg fast.

I've been digging in to this a little lately. The aspect approach does seem to be too slow, and it's causing bazel to run out of memory when I try to use it an a large go project. Adding the json generation to GoCompilePkg seems like a much better idea.

gopls seems to be the main client of go packages, and it always uses the same configuration. Maybe we should optimize for that first? It doesn't require types, so it seems like the go builder could just output the json files by default. I'm wondering if the driver even needs to run bazel. I get annoyed when I have to wait for the editor's bazel commands to finish so I can use bazel from the command line, and I'm pretty sure I don't have enough memory to have two copies of bazel running.

@nikunjy
Copy link

@nikunjy nikunjy commented Jul 28, 2020

I wrote a hacky library that can solve this https://github.com/nikunjy/golink for protobufs and can be extended wherever go_generated_srcs is there.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Sep 17, 2020

Hey @jmhodges, would you be willing to contribute your work on this so far? @tomlu is interested in continuing this, and your work would be a great starting point.

If you open a PR from the feature/gopackagesdriver branch in your fork, I can merge that onto a dev branch as-is, and we can take it from there. No need to update or rebase. That should keep our open source lawyercats happy.

@purkhusid
Copy link

@purkhusid purkhusid commented Nov 12, 2020

Is there any work going on to make this a reality?

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Nov 12, 2020

Sorry, no, not at the moment.

@burdiyan
Copy link

@burdiyan burdiyan commented Nov 12, 2020

I guess Go modules and their replace statements can solve some of these issues. But still, it would be nice to have something that would work more nicely.

@ribrdb
Copy link

@ribrdb ribrdb commented Nov 13, 2020

I'm hoping to have time to work on this next month.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Nov 13, 2020

@ribrdb This is not a small project. Please read this comment.

@bobg
Copy link

@bobg bobg commented Nov 13, 2020

There is another, much simpler way: include generated code in the source tree. Then everything Just Works.

I'm aware that this is contrary to the Bazel philosophy, but I consider the idea that some code should exist only at build time to be a misguided antipattern. As the aphorism says, code - even generated code - is primarily for humans to read, and only secondarily for computers to execute. And as the various other kinds of tooling breakage shows, there's more to do with source code than simply compile it into a binary. It belongs in the source tree.

This does mean changes to rules_go. In particular it means that where rules_go presently generates code, it should validate generated code instead. Builds should fail if the source-tree copy of any generated code is out of date with respect to its upstream source (such as a .proto file). Ideally the same logic that validates generated code at bazel build time could also update it with the right bazel run command. I'm working on exactly this right now - it's the motivation behind this Slack question - but my skill with writing Bazel rules is low. It would make my life a lot easier if rules_go went this way, and it would obviate the many difficulties outlined in Jay's long comment.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Nov 13, 2020

@bobg Copying generated code back to the source tree is a fine workaround. For folks that need to interoperate with other build systems like go build that don't support code generation, it's necessary.

rules_go (and Bazel rules in general) cannot do that though. Output files are always declared in a separate tree. Actions may run on a different machine. Even when they're run locally, they're usually sandboxed to prevent that that kind of thing.

Generated files could be copied by a tool executed with bazel run, but that doesn't ned to be part of rules_go.

@duarten
Copy link

@duarten duarten commented Nov 13, 2020

I'm using generated_file_test from rules_nodejs which takes two files, one under version control, and another that's generated by some arbitrary rule, and generates a test target that validates the two match. It also generates a ".update" target which updates the VCS file.

@bobg
Copy link

@bobg bobg commented Nov 13, 2020

Ooh, that looks super handy, thanks @duarten. What are the prospects for adding something like that to rules_go?

@duarten
Copy link

@duarten duarten commented Nov 13, 2020

Another possibility would be to try and get generated_file_test into bazel-skylib or something.

@burdiyan
Copy link

@burdiyan burdiyan commented Nov 16, 2020

rules_go (and Bazel rules in general) cannot do that though. Output files are always declared in a separate tree. Actions may run on a different machine. Even when they're run locally, they're usually sandboxed to prevent that that kind of thing.

@jayconrod I totally get the premise of Bazel to restrict outputs into a separate tree. But often times, especially with this use case for generated content, I'm struggling to understand what would be the technical concerns for having this restriction. Why actions couldn't declare outputs in the source tree, optionally and maybe only available for people that know what they are doing? I mean, what could go wrong in this case, considering that the action itself is "pure" and reproducible?

I'm thinking of this as maybe an additional feature of Bazel, which I'm pretty sure would never be accepted :)

@jmillikin-stripe
Copy link
Contributor

@jmillikin-stripe jmillikin-stripe commented Nov 16, 2020

Polluting the source tree with generated build outputs would go against the design of Bazel, since it is intended to be both reproducible and hermetic.

@burdiyan
Copy link

@burdiyan burdiyan commented Nov 16, 2020

@jmillikin-stripe Yeah, I totally get it. That's why I said that I don't see this being accepted by Bazel at all :) But for the purpose of a thought experiment, where reproducibility and hermeticity would break if actions are allowed to output into the source tree? Considering that same inputs always produce same outputs.

@jmillikin-stripe
Copy link
Contributor

@jmillikin-stripe jmillikin-stripe commented Nov 16, 2020

I suspect allowing Bazel to corrupt the source tree would cause problems for targets defined with a glob() that would match output files, targets where an output file would overwrite a different input file, and remote execution of actions (since non-dependency portions of the source tree would need to be reified).

If someone wants generated files in the source tree, why use Bazel at all? Wouldn't make be a better fit?

@burdiyan
Copy link

@burdiyan burdiyan commented Nov 16, 2020

@jmillikin-stripe Agree with your point about globs. That's why I said "for people that know what they are doing" :) Basically, you'd have to take more care when you output in the source tree.

As for why use Bazel at all, I think it's because everything else about Bazel is very compelling, and getting the same workflow with Make Is cumbersome/impossible. Would be great if Bazel could allow more flexibility (like Ninja or Make) for users that know what they are doing, or maybe requiring that these actions are only ran locally and with no sandbox. Or even requiring that these actions are only ran manually, not included into //.... Generally speaking, for achieving a fluid workflow with Bazel you almost always need to wrap it with some script. Otherwise you have to learn 5 different tools (Gazelle, ibazel, etc.).

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Nov 16, 2020

This is drifting pretty far off-topic. Bazel rules cannot write files into the source tree. rules_go cannot change that. An external tool can copy files into the source tree (or verify they're up to date), but rules_go is not the place for that.

@leoluk
Copy link

@leoluk leoluk commented Nov 16, 2020

How about a fuse filesystem overlay which maps generated code into the source tree? This would be compatible with all editor/IDEs, not specific not any particular language, and would maintain the clean separation between source and output.

Perhaps https://github.com/linuxerwang/gobazel would be a good starting point?

@dmayle
Copy link

@dmayle dmayle commented Dec 16, 2020

Subscribing so I can update https://github.com/plezentek/rules_wire with this solution when it is implemented

@alexeagle
Copy link

@alexeagle alexeagle commented Dec 18, 2020

@duarten re

Another possibility would be to try and get generated_file_test into bazel-skylib or something

There is https://github.com/bazelbuild/bazel-skylib/blob/master/rules/diff_test.bzl but it doesn't

  • show a unified diff of the two files (useful only for text of course)
  • print a bazel run command to accept the new generated output

@jmillikin-stripe re

Polluting the source tree with generated build outputs would go against the design of Bazel, since it is intended to be both reproducible and hermetic.

I don't think there is anything non reproducible or non-hermetic with checking in generated files like generated_file_test
The trade-off involved isn't reproducibility or determinism, it's that you have to "accept" new generated files whenever they change (both in your local vcs and when again in code review) in fact Google uses exactly this pattern to check in generated files when it's useful to code review changes to them. Golden files for screenshot diff tests are an example. I think Bazel is a great fit for this pattern and there are plenty of good scenarios to use it.

That said, most of us here would find it a nuisance to have to accept files like .pb.go.

No one on the thread has suggested to use replace directive (for those using Go Modules). In cases where a given package is entirely generated (same restriction as mentioned above for the IntelliJ plugin), you could

replace com.myorg.package.genfiles v0.0.0 => ./bazel-bin/package/genfiles

couldn't you? Seems like the only problem with this proposal is that it's a mess to maintain all the replace directives, but that could probably be tooled around. Maybe I'm missing something.

@burdiyan
Copy link

@burdiyan burdiyan commented Dec 19, 2020

@alexeagle I talked about the replace directive above here: #512 (comment). I haven't tried it myself, but I think it definitely should work. The only thing is that you have to generate the go.mod file as well in the generated package. And it will not work when you mix proto files for within the source Go files in one package. The whole thing needs to be generated for this to work (exactly what you're saying too), but sometimes this is not always feasible. Many projects mix proto files and go files in one package.

@alexeagle
Copy link

@alexeagle alexeagle commented Dec 21, 2020

I happened across an example in Bazel itself of using a test target to verify that a generated .go file is up-to-date, as another example of how this is a valid practice. (and maybe the underlying rule might be useful to someone reading this issue)
https://github.com/bazelbuild/buildtools/blob/master/build_proto/BUILD.bazel#L3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.