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

Add rules_jvm_contrib to the bazel-contrib org #27

Closed
shs96c opened this issue Dec 1, 2021 · 23 comments
Closed

Add rules_jvm_contrib to the bazel-contrib org #27

shs96c opened this issue Dec 1, 2021 · 23 comments

Comments

@shs96c
Copy link

shs96c commented Dec 1, 2021

At work, we've been using a suite of java rules and macros that make working on a larger java project smoother when using Bazel. Our ruleset includes:

  • A JUnit5 runner
  • Linter integration for PMD, checkstyle, and spotbugs (three popular linters in the java world)
  • Support for automatically generating test suites from globs of java rules that integrate with the above.

As part of the process of Open Sourcing this, I've been given permission to host the repo here: https://github.com/shs96c/rules_jvm_contrib but I believe that this should live somewhere under bazel-contrib.

@alexeagle
Copy link
Contributor

I think this is the forcing function for us to address #3
If we know how we decided about adding this repo, hopefully that generalizes to deciding about others.

A couple q's on this one in particular:

  • Shouldn't https://github.com/bazelbuild/rules_java supply a test runner that works with modern JUnit? Is there something busted with the upstream repo? Replacing java_test seems like a workaround.
  • How will users discover this, and know that it's right for their use case? Should the SIG provide some high level "how to do Java with Bazel" that mentions this repo? Are there places in the canonical docs https://docs.bazel.build/versions/main/bazel-and-java.html where we'd wish for that signpost to exist?

@shs96c
Copy link
Author

shs96c commented Dec 2, 2021

Answers!

  • rules_java seems largely quiescent, and my impression is that since the native java rules are no longer being replaced with Starlark equivalents, it's somewhat redundant.
  • Providing a macro for java_test would be needed in any case, as it's the hook point that apple_rules_lint uses to introduce the linters (see library.bzl)
  • If bazel-contrib becomes a place for community-owned and developed rulesets, then one route for people finding this would be to just look at what repos are there. It also seems reasonable to add a link to rules_jvm_contrib from the docs of rules_jvm_external
  • Having said that, inclusion in https://awesomebazel.com would also help, as that's one place that users go to find this information.
  • Yes, it would be nice if companion rulesets for languages were mentioned in the main bazel docs for those languages, not just for the jvm.

@shs96c
Copy link
Author

shs96c commented Dec 2, 2021

And some comments about the relevant open questions in #3

  • What commitments do people make?

rules_jvm_contrib are a key part of our "jvm on bazel" story, and so will continue receiving updates and fixes. I believe our work with rules_jvm_external and rules_rust (amongst others), as well as with the bazel project itself, demonstrates that this isn't just a "throw it over the wall" dumping of code.

  • How do we mechanically perform the move such that everything correctly redirects

The only reason this repo exists under my own username on GitHub is to facilitate this discussion and the inclusion of the repo in bazel-contrib. There is nothing to redirect at the moment, and I intend to delete the original repo once the move is complete (reforking it from bazel-contrib so I can make upstream PRs :) )

@alexeagle
Copy link
Contributor

Ivo's talk at BazelCon gave me the impression that "starlarkification" is happening. I'd be curious to hear from someone on the Bazel team about whether we can land some of this functionality in rules_java. "quiescent" is a nice word for abandoned, and I'd hope we can push them to either archive the repo if they don't intend to move forward, or else give us a roadmap to indicate whether we can fix java_test there to do what you need.

@shs96c
Copy link
Author

shs96c commented Dec 2, 2021

My impression is that the "starlarkification" for the java rules is still quite a long way out, and I'm not sure that rules_java even accepts community patches (certainly of this kind) Clarification on timelines and whether this would be accepted would be helpful.

@fmeum
Copy link
Member

fmeum commented Dec 3, 2021

Starlarkification is actually progressing quite nicely (see https://github.com/bazelbuild/bazel/tree/master/src/main/starlark/builtins_bzl/common/java for Java), just not in rules_java. But of course it's still a long way until java_common has been starlarkified and it's unclear to me whether rules_java will become the source of truth before that transformation has been completed.

@shs96c
Copy link
Author

shs96c commented Dec 4, 2021

So does this mean that rules_java is effectively moribund? Or that the focus of rules_java will shift, and landing these contributions there would be acceptable?

If the answer is that the process will take more than a month or two, I think it would be better to accept rules_jvm_contrib into the bazel-contrib org.

@fmeum
Copy link
Member

fmeum commented Dec 4, 2021

I don't know. @comius Could you explain what the short- to l mid-term plan for rules_java is? Does it accept external contributions?

@shs96c
Copy link
Author

shs96c commented Dec 6, 2021

@comius, if you could also comment about whether the changes in this repository would be accepted into rules_java, that would be helpful too. To summarise, these are:

  • JUnit 5 test runner
  • A java_junit5_test that uses the junit5 runner
  • A java_test_suite macro that expands a glob of sources into individual java_test or java_junit5_test targets and supporting library code.
  • Integration of linters for:
    • checkstyle
    • pmd
    • spotbugs

@alexeagle
Copy link
Contributor

Based on the lack of reply from @comius I'm guessing Google doesn't have a plan for rules_java. I wish they would just archive it for now, which can be reversed once there's a plan. @cushon are you still active in Bazel-Java?

That aside, I'm supportive of adding your rules_jvm_contrib. Any other @bazel-contrib/rules-authors who can voice support?

A bit of bikeshedding, does it make sense to have "contrib" appear twice in the "bazel-contrib/rules_jvm_contrib"? We could just have a rules_jvm, into which more Java/Scala stuff would land

@jsharpe
Copy link
Member

jsharpe commented Dec 8, 2021

A bit of bikeshedding, does it make sense to have "contrib" appear twice in the "bazel-contrib/rules_jvm_contrib"? We could just have a rules_jvm, into which more Java/Scala stuff would land

Would a standard WORKSPACE naming convention of contrib_<repo_name> be a good idea for bazel-contrib repos? so this would have a workspace name of contrib_rules_jvm?

@shs96c
Copy link
Author

shs96c commented Dec 8, 2021

I can rename this anyway that people think is best, but my preference is to keep rules_jvm_contrib. The reasons are:

  • Prefixing rules_ is the recommendation from the bazel team
  • jvm because I think kotlin and scala stuff can live equally happily in this repo (we have some kotlin rules that can generate test suites with kotlin sources, for example)
  • contrib to differentiate it from rules_jvm_external without seeming to take precedence.

I agree that it's a little ungainly to have "contrib" twice when looking at the GitHub URL, but the workspace name that people import into the projects will just be @rules_jvm_contrib so it's not quite as bad as it may originally appear.

Having said all that, @jsharpe's suggestion has all these properties, but in a different order :)

In short, I'll happily go along with any reasonable decision.

@alexeagle
Copy link
Contributor

alexeagle commented Dec 8, 2021

My preference is GH bazel-contrib/rules_jvm -> workspace contrib_rules_jvm

Bazel style guide says that rulesets should have workspace name in reverse DNS

If your rules belong to the bazelbuild organization, you must use rules_<lang> (e.g. rules_mockascript). Otherwise, you should name your repository <org>_rules_<lang>

@shs96c
Copy link
Author

shs96c commented Dec 8, 2021

I'll rename the workspace.

@illicitonion
Copy link

Any other @bazel-contrib/rules-authors who can voice support?

+1 - it's super useful to have a space to put this stuff, and if there's not an official space for it, this feels like the right approach.

My preference is GH bazel-contrib/rules_jvm -> workspace contrib_rules_jvm

SGTM. My instinct is to do the repetitive thing (repo name == workspace name is a quite nice pattern), but I care much less about the colour of the bikeshed than that it's built.

@restingbull
Copy link

restingbull commented Dec 8, 2021

+1 -- Google is intending to push out a rules_kotlin at some point so we'll end up moving the current one.

@gravypod
Copy link
Member

gravypod commented Dec 9, 2021

My preference is GH bazel-contrib/rules_jvm -> workspace contrib_rules_jvm

Fully agreed. It will also make it easier to search for help.

@keith
Copy link
Member

keith commented Dec 9, 2021

Lots of interesting topics here, some thoughts:

  • From what I understand of google's plan the native java rules will become starlark in bazel, and then potentially be moved to another repo, so I don't think we can be certain that the rules_java is entirely dead (but that's still far enough out I wouldn't wait for it either)
  • Even if rules_java was the home of the rules, I think having a separate repo has some benefits from the maintainership side, and it shouldn't be something users should consider a downside (outside of the normal concerns of WORKSPACE management and the risks new dependencies add). so I think having this separate is practical for where we are now regardless of google's plans
  • I think especially in the case of these rules, and the integration with various linters and another 3rd party rulesets means google would never accept this into a repo they owned anyways
  • I think the biggest decision point for moving stuff like this to this org is making sure we don't just move everything and become a bit of a wasteland for low usage repos. obviously for a new one like this it's more of a bet, and I'm willing to take that be hoping to centralize some of the effort into this, but it will still mostly hinge on the existing contributors to keep the lights on, so I think that has to be clear to everyone at least at the moment when there isn't yet a clear financial / investment benefit from moving them to this org.

@shs96c
Copy link
Author

shs96c commented Dec 10, 2021

In response to the last point: these rules are the ones we're using at work for our Java development. They're widely used by teams, and we plan on continuing to support these rules as part of our regular development processes. My team and I are active participants of other rulesets, including rules_jvm_external and the rust rules, part of the remote execution working group, and active in the Bazel community.

TL;DR: we'll keep the lights on :)

@shs96c
Copy link
Author

shs96c commented Dec 10, 2021

The ruleset has been renamed to contrib_rules_jvm: bazel-contrib/rules_jvm@3370977

@alexeagle
Copy link
Contributor

It has been over a week, and I haven't seen any concerns from @bazel-contrib/rules-authors other than we aren't sure how to make this process repeatable, which is #3
I don't think we need to block @shs96c on that, so I think we should go ahead and move rules_jvm_contrib into the org.

@alexeagle
Copy link
Contributor

https://github.com/bazel-contrib/rules_jvm is here now.

@comius
Copy link

comius commented Dec 15, 2021

Hey all, I'm back from vacations.

True, rules_java seems abandoned and it contains at the moment hollow implementations (that call-back native implementation).

The short term plan for rules_java is to resurrect it with Starlark implementation of Java rules that matches the native implementation together with unit and integration tests written in Starlark.

Anything beyond this is not defined at the moment (branching, maintaining, converging, diverging).

I'm open to contributions. rules_java will need to reach and sustain a very high code quality to make contributions possible though. I believe there are big monorepos that depend on native Java ruleset and even small changes come at a high price.

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

9 participants