Feature request: Setting default javacopts for all java_library rules in workspace

[kevingessner]

I'd like to set some javacopts on all java_library rules in my project. In particular, I want to enable certain Error Prone checks that aren't on by default, with the -Xep flags. However, I only want these flags to apply to Java targets in my project, not to builds of Java code in external repositories that are included with e.g. http_archive. That external code may not adhere to our internal standards, so I can't have these additional error checks enabled there---it would cause the build to fail.

I've tried several approaches to make this happen (all with Bazel 0.5.2), and each of them has drawbacks that make them infeasible:

1. Set flags in tools/bazel.rc

Since javacopts can be set both on the java_library rule and via the --javacopt flag to bazel build, I created a tools/bazel.rc like this

build --javacopt=" \
--XepDisableWarningsInGeneratedCode \
--Xep:MissingCasesInEnumSwitch:ERROR \
# etc.

Pros:

• Automatically applied to all rules
• Pretty clear how it works to other devs: bazel.rc is well-documented, and the flags are printed with bazel build -s

Cons:

• Applies to all builds, including builds of libraries in http_archives, so external builds fail

2. Enforce setting via out-of-band validation

Create tools/build_rules/java.bzl defining a list, JAVACOPTS = ['-XepDisableWarningsInGeneratedCode', '-Xep:MissingCasesInEnumSwitch:ERROR', …]. In each BUILD file, add load("//tools/build_roles/java.bzl", "JAVACOPTS") and set javacopts=JAVACOPTS in the java_library call. Finally, add a check in CI that all BUILD files that contain a java_library rule pass the imported JAVACOPTS.

I could put the load call in bazel_prelude, but that has its own discoverability problems (see #1612 and #1674).

Pros:

• Very clear what flags are applied where, because it is explicit
• Only applies to code in my project, not to external projects

Cons:

• We have dozens of java_library rules across dozens of files, each of which needs to have the load and argument added and maintained
• Using the prelude to reduce duplication adds magic
• Requires out-of-band enforcement (e.g. in CI) that all new and existing BUILD files include these stanzas

3. Wrap java_library with a macro

Similarly to 2, create tools/build_rules/java.bzl that defines JAVACOPTS and also a my_java_library macro, e.g.:

JAVACOPTS = [...]

def my_java_library(javacopts=[], **kwargs):
    javacopts += JAVACOPTS
    native.java_library(javacopts=javacopts, **kwargs)

In each BUILD file, load and use my_java_library in place of the native rule.

Pros:

• Very clear what flags are applied where, because the rule name is customized
• Only applies to code in my project, not to external projects

Cons:

• Like Fix up README #2, requires changes in every existing and new BUILD file, with CI enforcement

4. Shadow java_library with a macro

Like #3, but name the macro java_library so it shadows the built-in, and load it in the prelude so it automatically applies to all targets. The macro can check native.repository_name and apply different changes to my project's code vs external code:

JAVACOPTS = [...]

def java_library(javacopts=[], **kwargs):
    if native.repository_name() == '@':
        # Only apply these opts to my project, not to external projects.
        javacopts += JAVACOPTS
    native.java_library(javacopts=javacopts, **kwargs)

Pros:

• Automatically applied to all rules without changing each BUILD file
• repository_name check makes this only apply to targets in my project, not external code

Cons:

• Magical and hard to discover---the prelude is not
• Possibly brittle in the face of changes to the java_library rule in future versions
• Is the additional macro call going to be slow?

Feature request

None of the above solutions are ideal: (1) causes build failures in external code; (2) and (3) require extra checks and documentation outside of Bazel; and (4) is excessively magical and difficult to discover & maintain.

It would be great if Bazel had a supported, clear way of setting default arguments to rules in a workspace. The defaults should apply only to rules defined in that workspace (not to external repositories loaded at build time). I can't find anything like this that exists now.

That would solve for my need of default javacopts, and support other needs for default arguments (e.g. default global test resources; I'm sure there are others).

Example proposal

For example, this could be implemented via a new workspace rule called default_arguments, used like so:

default_arguments('java_library', javacopts=[...])

At analysis time, these arguments would be mixed in to each target's arguments for a given rule. This could be used for any rule and argument, not just javacopts and java_library.

I'm not sure what the best logic would be for resolving the existing default arguments vs ones set in the workspace vs arguments passed to each rule; that would have to be decided and documented.

[kevingessner]

This is similar to #2358, with one major difference: that FR wants the defaults to apply to external repositories' code, whereas I don't want. I haven't experimented with using a custom toolchain to solve this, so I'm not sure if that would be a feasible solution.

[cushon]

I haven't experimented with using a custom toolchain to solve this, so I'm not sure if that would be a feasible solution.

You can use the (somewhat poorly named) java_toolchain.misc to set default javacopts. I think it has the same drawback as (1) of applying to external builds, though.

[gorset]

We ended up using extra actions to run more checks for our java code. Maybe that could be an option? With extra actions you can use --experimental_extra_action_filter=-@ to skip running them on external dependencies. We do this to run checkstyle, ecj and findbugs - and it would obviously be possible to run error prone. Maybe not an optimal solution, but should work :)

It's quite annoying that external dependencies will be compiled with the same copts as the local rules as default, which makes it hard to take full advantage of non default error prone checks and javac lint rules.

(sorry if you got duplicate notification. Posted with the wrong github user first).

[alexeagle]

Discussed with @dslomov a while ago: I wanted to have a similar feature for TypeScript to set compiler default options, and ran into the same issues. Even for a rule written in skylark (not a native rule like java_library) there is no good way to apply defaults just to the local repository.

[cushon]

There's now another option for setting javacopts based on package specifications. It wasn't created specifically with this issue in mind, but it may be better than some of the existing options.

The configuration uses java_toolchain.package_configuration:

load("@bazel_tools//tools/jdk:default_java_toolchain.bzl", "default_java_toolchain")

# this is a convenience macro that inherits values from Bazel's default java_toolchain
default_java_toolchain(
    name = "toolchain",
    package_configuration = [
        ":error_prone",
    ],
    visibility = ["//visibility:public"],
)

# this associates a set of javac flags with a set of packages
java_package_configuration(
    name = "error_prone",
    javacopts = [
        "-Xep:MissingOverride:ERROR",
    ],
    packages = ["error_prone_packages"],
)

# this is a regular package_group, which is used to specify a set of packages to apply flags to
package_group(
    name = "error_prone_packages",
    packages = [
        "//foo/...",
        "-//foo/bar/...", # this is an exclusion
    ],
)

bazel build --java_toolchain //:toolchain //foo
...
foo/Foo.java:2: error: [MissingOverride] hashCode overrides method in Object; expected @Override
  public int hashCode() {
             ^
    (see http://errorprone.info/bugpattern/MissingOverride)
  Did you mean '@Override public int hashCode() {

[unlikely-leo]

Thanks @cushon, the setup you describe suits our use case well! Could you point me to the fix you're referring to in your comment?

bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath.jar"], # this line shouldn't be necessary, I have a fix out for review

[cushon]

@unlikely-leo I think that was af24688. The example I provided should work without having to set default_java_toolchain.bootclasspath.

[unlikely-leo]

Thanks for checking @cushon! FYI in Bazel version 0.14 (build timestamp 1527858416), default_java_toolchain.bootclasspath is still required for a successful build as far as I can tell. Not a problem, just thought I'd let you know.

[dslomov]

Please do not assign more than one team to an issue.

[comius]

This seems to be resolves with java_package_configuration.