-
Notifications
You must be signed in to change notification settings - Fork 385
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
Proposal: move language extensions to rules_{go,proto} #1030
Comments
cc @bazelbuild/go-maintainers The history until now:
That last part hasn't worked out really well for a couple reasons, mainly that the people developing an extension aren't usually the same as the people who own the rule set repository. It would be fine for rules_go (except for the dependency issue which caused Gazelle to be moved here originally) but not rules_proto. In many cases, the repository owners don't write Go every day and don't want to be responsible for maintaining an extension. Perhaps a better approach is to make Gazelle more "batteries included" and host more extensions here. That would certainly make a lot of things easier for users. My main concern about that is increasing the maintenance load on bazel-gazelle maintainers. I can't speak for others, but these days I'm unable to do much more than triage issues and review small PRs every couple weeks. Maintaining a C++ or Python extension would be out of the question for me. |
We could help out with the maintenance in some way. I imagine we can use a "contrib" model where some subfolders here are documented as being owned and maintained by a different team, to try to set expectations about governance and responsiveness. @f0rmiga and I are both working at Aspect which intends to make a long-term investment in the rules ecosystem, and "batteries included" is exactly what we'd like to build. I think the primary reason for choosing a repo should be CI. We'd want to make sure that rules changes don't break the generator for those rules, and making an integration test covering both is a lot easier if they're co-located. We can ensure users don't observe the wrong runtime dependencies by shipping two release artifacts. Another note is that there's one other language supported by gazelle whose plugin is in a canonical BazelBuild org, https://github.com/bazelbuild/bazel-skylib/tree/main/gazelle/bzl - in case that gives any weight to that proposal. |
I would be all for this if it were just a matter of getting people in an OWNERS file. In practice, it's been a bit more complicated on GitHub. Contributors need to be added to the bazelbuild org manually by an org admin. GitHub supports CODEOWNERS files for assigning reviews, but it looks like everyone in them must have write access to the entire repo, and I'd be hesitant to grant that level of access to anyone without more review infrastructure. The Gazelle extension in bazel-skylib is actually a case in point for this. @achew22 are in CODEOWNERS there but we don't have write access, so we can't actually fix anything without getting a review from someone else, and that hasn't been easy. |
We actually do have write permission in that repo now. There was some configuration issue where they needed to click a button but we are good to go in that repo |
@achew22 Ah, looks like you're right. I don't think I actually knew that was fixed. So maybe this isn't as difficult as I'm thinking. What would actually be needed for a
Do people with write access have other privileges that need to be limited? |
The issue I see with multiple languages being built into
A project would no longer be able to upgrade Gazelle and be stuck with This is why I think it's so valuable to keep the Gazelle plugins locked in step with Independent of all of this, I do think it is too hard to install a plugin. I just realized that I wrote To give a straw man on making this more automatic, what would you think about having Then have that make a new target known_repos = {
"rules_proto": {
"plugin_path": "@rules_proto//gazelle",
},
"rules_go": {
"plugin_path": "@rules_go//gazelle",
},
# ...
} I think there is another problem here which has gone unstated. There just aren't any other plugins for I don't wish to speak for any community that isn't my own, but the impression I have is that Since I'm a big fan of strawmen (strawmans?), what would you think about making a "template" of a language and including it in Gazelle as a starting place. We could have a fake language that has a boring import syntax parsed by regexp and generates very normal Additionally |
Let me share some of our experience of using Gazelle in Uber's Go monorepo.
The situation is not as pessimistic as you thought @achew22 . Uber has a long list of Gazelle extensions internally: ThriftRW, Apache Thrift, gomock, to name a few that are based on publicly available code generators and generate internal Bazel rules that calls these generators. We didn't open source them because they makes some assumptions on Uber's Go monorepo structure and conventions. In addition, as those Bazel rules are not open sourced yet, there is no point to open source the Gazelle extensions either. Implicitly, we are having Bazel rules and Gazelle extensions in the same repo as proposed in this thread. Most Bazel rule authors in Uber don't find it hard to write a Gazelle extension. As a central developer experience team, we receive a new Gazelle extension from random teams at Uber every once a while, asking us to review. We have to be very careful, because the top level |
@linzhp With Gomock you mean that Gazelle is automatically capable of generating mock library targets for individual packages? That sounds pretty awesome! I would love it if Gazelle supported something like this out of the box in the future. I'm currently using bazel_gomock. This works, but has the downside that it's a lot of manual effort. |
It makes more sense to host the gomock extension in bazel_gomock. But bazel_gomock seems to be inactive. |
Sorry for jumping into this late, but I can also provide some context from Skydio, where we also have a pretty extensive list (~8) of internal Gazelle plugins, including Python and C++ rules we use in production. We expect to increase that list over time, likely including Java, Kotlin, Objective C, and Swift once we get our mobile build working under Bazel. In theory, it would be ideal to upstream the helper libraries and maybe the rules themselves, but in practice we're probably going to continue treating our plugins as "internal-first" for the foreseeable future, and won't really be willing to make many changes in response to upstream requirements. |
Since the extensions generate targets compatible with a certain rules_{go,proto} version, at a hight-level, it sounds like their home should be with what they generate. This is true for other rules maintainers if they want BUILD file generation using Gazelle.
Thoughts? Any technical reasoning I'm missing?
The text was updated successfully, but these errors were encountered: