Increase maxwarns to -1

[fzakaria]

Please see bazelbuild/bazel#25927 (comment)

It's important not to filter out any warnings otherwise the lint warnings are not surfaced and then any attempt to do -Werror can fail with what may be observed as spurious (affect other parts of the codebase).

This is maybe a "breaking change" only if someone has enabled -Werror and may now see additional errors -- although that was in fact their intention...

[hvadehra]

The motivation for this sounds reasonable to me, but I think we should only be setting defaults that make sense for everyone, all the time. So, it's probably best if we actually trim down the default list to an empty one rather than expanding it (so the default javac behavior is what everyone gets out-of-the-box). If you'd like to have different behavior, consider registering a custom java_toolchain with those options.

[cushon]

The motivation for this sounds reasonable to me, but I think we should only be setting defaults that make sense for everyone, all the time. So, it's probably best if we actually trim down the default list to an empty one rather than expanding it (so the default javac behavior is what everyone gets out-of-the-box). If you'd like to have different behavior, consider registering a custom java_toolchain with those options.

FWIW I think this default sort of makes sense for everyone, the status quo is that errors may not be shown if there are too many warnings, and then there's no way to tell why the build failed from the log. There is an underlying bug causing that which should be fixed, but until then this default is helpful for everyone. And I also agree in theory it would be nice to use javac defaults, but in practice I'm not sure there's a strong reason to change this default upstream, the issue here is Bazel-specific.

[fzakaria]

Yea -- basically without this PR users of rules_java are subject to potential bugs that I've documented in the bug.
If someone (@cushon ) can recommend where to try and patch it upstream -- I can look into that as well; but for now this is a pretty reasonable change.

[hvadehra]

the issue here is Bazel-specific.

Is this because JavaBuilder does not propagate the -Werror to the compilation task? If so, ISTM we should perhaps not propagate -Xmaxwarns either (passing -1 instead), and do the limiting on the JavaBuilder side (after filtering ignored diagnostics). Is that right?

[cushon]

the issue here is Bazel-specific.

Is this because JavaBuilder does not propagate the -Werror to the compilation task?

It's related to that: bazelbuild/bazel#25927 (comment)

If so, ISTM we should perhaps not propagate -Xmaxwarns either (passing -1 instead), and do the limiting on the JavaBuilder side (after filtering ignored diagnostics). Is that right?

Hard-coding -Xmaxwarns=-1 in JavaBuilder instead of configuring it via javacopts would also work around the problem.