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

Proposal: move language extensions to rules_{go,proto} #1030

Open
f0rmiga opened this issue Apr 15, 2021 · 10 comments
Open

Proposal: move language extensions to rules_{go,proto} #1030

f0rmiga opened this issue Apr 15, 2021 · 10 comments

Comments

@f0rmiga
Copy link
Contributor

f0rmiga commented Apr 15, 2021

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?

@jayconrod
Copy link
Contributor

cc @bazelbuild/go-maintainers

The history until now:

  • Gazelle was originally part of rules_go. It had hard coded support for Go.
  • It moved to this repository so that rules_go could have fewer dependencies and so it would be easier to use rules_go without Gazelle.
  • Support for proto_library and go_proto_library was added.
  • Gazelle was refactored to support language extensions. Go and proto support were converted into extensions. We encouraged other extensions to be hosted in separate repositories (ideally with the related rule sets).

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.

@alexeagle
Copy link
Contributor

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.

@jayconrod
Copy link
Contributor

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.

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.

@achew22
Copy link
Member

achew22 commented May 26, 2021

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

@jayconrod
Copy link
Contributor

@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 contrib directory in this repo? I think the goal would be for someone to be able to fix something in their own extension without needing a review from someone in go-maintainers (we can't necessarily review details for languages we don't know), ideally still needing a review from another owner of the extension, though I'm not sure that should be forced.

  • Add contributors to bazelbuild org; admins need to do this.
  • Grant contributors write access to bazel-gazelle.
  • Write CODEOWNERS so that for any PR that touches files outside of contrib a review from go-maintainers is required; otherwise, extension owners can review changes within their extensions.

Do people with write access have other privileges that need to be limited?

@achew22
Copy link
Member

achew22 commented May 27, 2021

The issue I see with multiple languages being built into bazelbuild/bazel-gazelle is that the versions of the gazelle plugin and the rules version may be out of step if there are multiple languages. For example:

  • T+0: Project installs Gazelle in repo to manage rules_foo and rules_bar
  • T+1: rules_bar releases v2 which changes the way imports work and you now have to write them backward (or any other unpleasant LSC you can think of)
  • T+2: rules_foo adds a new field fast_mode = True in every rule in v2 and thus upgrades their plugin in Gazelle.

A project would no longer be able to upgrade Gazelle and be stuck with rules_foo's fast mode set to False until they are able to upgrade rules_bar to the latest version.

This is why I think it's so valuable to keep the Gazelle plugins locked in step with rules_x. In the case of loading the plugin from @rules_x, as you upgrade the rules_x http_archive the Gazelle plugin is upgraded in lock step keeping them synchronized.


Independent of all of this, I do think it is too hard to install a plugin. I just realized that I wrote skylib's language extension but it isn't even installed in my project!

To give a straw man on making this more automatic, what would you think about having gazelle_dependencies in the WORKSPACE detect a list of language plugins.

Then have that make a new target @gazelle//:gazelle which would be addressable by its shortened name to @gazelle. The other thing we could do would be to use whatever trick the maybe macro does to automatically detect that rules_go is present in the repo and automatically install the plugin by having a list like:

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 gazelle that are worth having. If there was a vibrant ecosystem of plugins who were blossoming then I think the situation would be clear how to advance.

I don't wish to speak for any community that isn't my own, but the impression I have is that rules_x authors look at Gazelle, think it's cool, but ultimately decide the effort of learning/working/bugfixing in a new and unfamiliar paradigm (be it language, framework, or other) isn't worth having their rules generated automatically. Having written a few Gazelle plugins myself, I will say I think it is quite an undertaking. I don't know what we could do to make it easier to write a plugin, but it feels like something could be done to make that easier.

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 fake_library rules. This would be a good cp -rable template for people to work off of. Most people understand regexes well enough that they can probably hack something together.

Additionally skylib is an approximately hermetic plugin that has tests onboard. With a small bit of refactoring we could absorb the test helper into gazelle so that a plugin author only needs to bother with modifying a small number of lines of go to do the parsing and adding testcases becomes trivial?

@linzhp
Copy link
Contributor

linzhp commented May 28, 2021

Let me share some of our experience of using Gazelle in Uber's Go monorepo.

the impression I have is that rules_x authors look at Gazelle, think it's cool, but ultimately decide the effort of learning/working/bugfixing in a new and unfamiliar paradigm (be it language, framework, or other) isn't worth having their rules generated automatically. Having written a few Gazelle plugins myself, I will say I think it is quite an undertaking. I don't know what we could do to make it easier to write a plugin, but it feels like something could be done to make that easier.

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 //:gazelle target would run on CI to check every diff. If any Gazelle extension breaks, it would affect many related teams. Sometimes we accept such extensions to //:gazelle, but disable it from root, and ask the maintainer to enable it on directories they need. Then they have to track down their transitive dependencies across different organizations in the repository to enable their extensions. Sometimes we reject such extensions because they didn't meet our quality bar, then they ended up creating //some/org:gazelle with their own extensions. Since it's not run by developers outside their org, if some transitive dependency change requires rerunning //some/org:gazelle, things would break randomly. Neither is ideal, but we don't have good solutions yet.

@EdSchouten
Copy link
Contributor

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

@linzhp
Copy link
Contributor

linzhp commented Jul 7, 2021

It makes more sense to host the gomock extension in bazel_gomock. But bazel_gomock seems to be inactive.

@eric-skydio
Copy link
Contributor

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.
We have found that it took a lot longer to write the first Gazelle plugin than subsequent ones, mostly because our existing set of plugins serves as a fairly effective template for new plugin authors, but also because we've made a couple helper libraries to simplify and standardize some boilerplate.
We definitely have ambitions of open-sourcing our plugins, but likely would do so in a way decoupled from the underlying rules. For example, we could make a github.com/Skydio/gazelle_plugins repo that contains all of our open-source plugins (and the helper libraries they share), and encourage downstream users to construct their own Gazelle binary with some of our plugins and some from other places.
For things like Python and C++, our goal would be to provide sufficient configuration in the Gazelle plugin that it can generate rules for any rule implementation (i.e. rules_python vs dbx_build_tools) that's reasonably similar.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants