Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Conform with OSS golang expectations
This looks like a huge change, but I promise it's quite simple. In fact it's so simple that I was able to sync it with several days of commit from your mainline repo and only had one merge conflict (`server/BUILD`, which I replaced entirely) + updating the new imports you had made to point at their new path. I promise this won't negatively impact your productivity. If a file previously was declaring a unique package, put it under the equivalent path. For example: `server/backends/cachedb.go` was creating a package `github.com/buildbuddy-io/buildbuddy/cachedb`, so I simply moved it to the directory that would be associated with it. Additionally, I manually mapped the `go_proto_library`s in `/proto` to have an `importpath` that was relative to the repo root (`github.com/buildbuddy-io/buildbuddy`). This means that Gazelle knows about the existance of these proto files and can automatically add dependencies on them. Why is this worth merging? * You will now be able to get the beautiful godoc.org docs when you're working on your project. * I made Gazelle work, so now you'll no longer have to manually update your build files. * You will also be able to use more of the go ecosystem around linting, code fixes (eg), and more! * Having code that uses the standard tools decreases the burden on external contributors to your project (like me) who would be delighted to pitch in and fix issues here and there. After all, this is the whole reason to do open source, right? But isn't `go_default_library` ugly? * Yes, yes it is. It's also a hack that has been in place in `rules_go` for a while that is not beloved by the maintainer. In the olden days of rules_go, before there was even a Gazelle, the package name was inferred from the path to the file. You would declare your repo root when you loaded rules_go in your WORKSPACE. This, over time, was deemed to be insufficiently flexible for open-source where you can't make modifications in a `third_party` context. They have a different, now deprecated system called `vendors` that behaves similarly, but differently enough that it caused problems. In order to address this, `importpath` was added as a stopgap to allow the file path and the `importpath` to be inconsistent. Then there was an issue around creating crosscompilation targets for linux from osx. This issue has since been resolved in the master for rules_go using transitions, and as of the next release of rules_go. Fixing `go_default_library` is now just a matter of someone going into gazelle and updating it to have a flag to select naming things name things `go_default_library` or the directory that contains them. I think that is probably something I'm going to do in the near future. Testing strategy: I've ensured that the entire repo is able to build/test completely. Note that there are no tests checked into this repo, so the `bazel test` returns failure. The `bazel build` was successful, however. ``` $ bazelisk test //... INFO: Invocation ID: e4f2626b-88ef-4e32-a52c-1c0bdbe10780 INFO: Streaming build results to: https://app.buildbuddy.io/invocation/e4f2626b-88ef-4e32-a52c-1c0bdbe10780 WARNING: Streamed test output requested. All tests will be run locally, without sharding, one at a time INFO: Analyzed 117 targets (0 packages loaded, 0 targets configured). INFO: Found 117 targets and 0 test targets... INFO: Elapsed time: 0.262s, Critical Path: 0.00s INFO: 0 processes. ERROR: No test targets were found, yet testing was requested INFO: Streaming build results to: https://app.buildbuddy.io/invocation/e4f2626b-88ef-4e32-a52c-1c0bdbe10780 INFO: Build completed successfully, 1 total action $ bazelisk build //... INFO: Invocation ID: d1abc034-6186-4f9c-9b0a-0763c3725dc3 INFO: Streaming build results to: https://app.buildbuddy.io/invocation/d1abc034-6186-4f9c-9b0a-0763c3725dc3 INFO: Build option --test_sharding_strategy has changed, discarding analysis cache. INFO: Analyzed 117 targets (0 packages loaded, 16507 targets configured). INFO: Found 117 targets... INFO: Elapsed time: 1.096s, Critical Path: 0.07s INFO: 0 processes. INFO: Streaming build results to: https://app.buildbuddy.io/invocation/d1abc034-6186-4f9c-9b0a-0763c3725dc3 INFO: Build completed successfully, 1 total action ``` Fixed: buildbuddy-io#16
- Loading branch information