-
Notifications
You must be signed in to change notification settings - Fork 887
Description
I would like to run google-java-format
in an (open source) project which uses Bazel as build system.
I currently have it set up using https://github.com/dustinsand/pre-commit-jvm (with https://pre-commit.com) but it strikes me as "odd" having to install https://get-coursier.io and relying on dustinsand/pre-commit-jvm
when it should be possible to "just" have a (Maven) dependency on this in Bazel and "run it" - I'm just not entirely sure HOW one would be run a Java based tool like this one from Bazel...
... I have briefly explored something like what this post on bazel-discuss
may have been after, and started to write my own GoogleJavaFormatTest
using this project's API, but it doesn't seem to be intended to used like that (UsageException
is package local only, even though Main#format(String... args) throws UsageException
) - and integrated this as a b test //...
is perhaps that's the wrong way to go about this anyway?
How would one best run google-java-format from Bazel? You would want it "run whenever any *.java changed", and one would want it to fail the build if the formatting was wrong - but re-write (correct) the source files. Is this possible with Bazel? How does one even "run a main() method from a dependency" for something like this?
@cushon perhaps you have some wise guidance?
Activity
cushon commentedon Mar 10, 2023
Can you say more about what the desired workflow would be? Would you like to be able to
bazel run //some/target
and have that format the files? Or a test that would fail if the files weren't formatted, and maybe print a separate command you could run to format them? I don't think there's a good way to do the first thing, the second one should be do-able.vorburger commentedon Mar 10, 2023
@cushon anything that works, really; if there is no good way to do via
bazel run
, then abazel test
approach SGTM!After staring at
Main.java
a bit, I had actually started to write my ownGoogleJavaFormatTest
- but got a bit stuck, as one couldn't call itspublic static void main(String[] args)
from such a test because it has aSystem.exit(result)
. So then I started to explore invokingpublic int format(String... args)
, but that's a PITA because itthrows UsageException
- which is not visible (that could perhaps be a neat thing to detect e.g. inerror-prone
, perhaps?).Would you welcome a contrib (PR) proposing some (backwards compatible) minor refactoring to
class Main
to make this possible / easier?I guess it's not even Bazel related, per se... such a test could "dog food" and even run within this project!
cushon commentedon Mar 10, 2023
If you want a
main
-like interface, I'd look at using theGoogleJavaFormatToolProvider
. There's a unit test for it with a usage example.vorburger commentedon Mar 10, 2023
works! Now I just need to figure out to... hm, what? I would probably want some Blaze
java_test
rule to get "all of the project's**/.*.java
file paths" as its input, to pass to myGoogleJavaFormatTest
?I can't pass an argument to a test. Set a system property? (Not sure if that could run into length limitations...) Probably best to somehow get a file with all Java classes to format written, possibly with a fixed name, and
@
that as an argument to theGoogleJavaFormatToolProvider
?But how do you do that -
glob
for files and have Bazel write that into a file? And would thatjava_test
have to be in aBUILD.bazel
file at the root of the project, in order to be "allowed" to find/see ALL**/.*.java
files?I don't know how to do the Bazel part... 😭 Will read up and ask around - and welcome any pointers & tips!
cushon commentedon Mar 10, 2023
Note that you don't need to instantiate the class directory, you can also use the ToolProvider SPI:
google-java-format/core/src/test/java/com/google/googlejavaformat/java/GoogleJavaFormatToolProviderTest.java
Line 42 in 1a00656
For the Bazel part, tests can only see files if they're inputs in the build graph. I don't know a standard way to make all of the
.java
files in a project inputs. If you want to check all of the sources that are inputs to a particularjava_binary
there's a_deploy-src.jar
artifact that rolls up the transitive sources, but that's just the inputs to that binary, not the entire project. Another approach would be to have a CI rule that tried to build everything, and used an aspect to check if sources were formatted.jbduncan commentedon Mar 10, 2023
A proposed solution for porting Spotless to Bazel is to use the third-party https://github.com/apple/apple_rules_lint, so I wanted to let you both know if this possibility in case you hadn't considered it already. :)
vorburger commentedon Mar 10, 2023
FYI I personally won't be actively pursuing this further, because I've settled on using https://github.com/macisamuele/language-formatters-pre-commit-hooks/ instead of a Bazel integrated approach, for my current project, for now. (I suspect it is more "incremental" than a Bazel-based solution could be; as https://pre-commit.com uses git to check only changed files.) But I'm leaving this issue open just in case this analysis/discussion is ever of any use to others in the future.
vorburger commentedon Feb 10, 2024
#216 (for bazel-contrib/rules_jvm#23) seems to be related to this...
davido commentedon Jun 4, 2024
I think the main reason to move
google-java-format
check to the build system, is the actual version of the GJF used.Say your project started to use GJF 1.7 years ago, and due to it evolution, we are currently on version 1.22 (at the time of writing this comment).
Now consider that your project has 42 stable branches (any many of them are still active and continue to become fixes and need to be formatted with the right GJF version used on that stable branch). What branch is re-formatted with which version of GJF?
So similarly, how baselisk is using
.bazelversion
to fetch and run the right Bazel version for this branch we could add somesetup_gjf.sh
script to do it for us, that we have done in Gerrit: [1].Or alternatively, just use Bazel to fetch and run the right version of GJF on the right branch. In that case the upgrade would only be GJF version bump in some Bazel build file (similarly to
.bazelversion
version bump):Actually the both approaches would be great to have.
I added the first option in this Gerrit change: [2] and can do something like that (see discussion in bazelbuild/bazel#3325 to understand why
--run_under
option is needed here):The above invocation could be simplified, if we split it in two steps: 1/ retrieve executable script and 2/ invoke it:
The second option to add a test rule would be more challenging, because one need to provide the sources, see this comment: bazelbuild/bazel#3325 (comment):
This only works, because this gerrit plugin has only one single Bazel package with all java sources located under this root (that wouldn't be the case for any non trivial Bazel project though):
Having said that, I think this issue is more justified for Bazel related projects and not for GJF itself.
[1] https://gerrit-review.googlesource.com/c/gerrit/+/428139/6/tools/setup_gjf.sh
[2] https://gerrit-review.googlesource.com/c/gerrit/+/428138