Expose default/warn strict_java_deps mode to skylark

[ittaiz]

Description of the problem / feature request / question:

I'm integrating java_common.compile into rules_scala where we want to use strict_java_deps flag for compilation of java code and of scala code.
The beginning of this started in #3295.
While trying to integrate with it I get an error if I try to pass warn as an input.
I then saw that default isn't allowed as well.

I think it's obvious why I'd want warn and I need default since to the best of my understanding that is one of the possible values I can get from ctx.fragments.java.strict_java_deps (IIUC this will be the value when no value is given).

Environment info

• Bazel version (output of bazel info release):
  0.5.3

[ittaiz]

@iirina this is the issue we discussed

[ittaiz]

btw, is it wise to have OFF as the default value? This feels like anti bazel idioms
command line defaults to "default"

[ittaiz]

oh, now I see that the default value is ERROR but the documentation is just wrong.

      @Param(
        name = "strict_deps",
        defaultValue = "'ERROR'",
        positional = false,
        named = true,
        type = String.class,
        doc = "A string that specifies how to handle strict deps. Possible values: 'OFF' (silently"
          + " allowing referencing transitive dependencies) and 'ERROR' (failing to build when"
          + " transitive dependencies are used directly). By default 'OFF'."
      ),

[ittaiz]

@iirina
I can send a PR to allow warn and default to the switch

bazel/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java

Line 463 in 2abc654

switch (strictDepsMode) {

I wasn't able to find tests though so I can send the PR as is.
FYI what I tried was to try changing the switch below to switch("") and ran the test bazel test //src/test/java/... and nothing fails.

[iirina]

Hi Ittai, I already have a change for this. I'm waiting for it to be pushed. It should be in bazel today.

[ittaiz]

Awesome, thanks!

[ittaiz]

BTW, did you include upper-casing? Since the command line docs are for lowercase but the switch on the string is upper-case.

[ittaiz]

Also another problem I saw when I hacked on this was that I got an exception from DependencyModule#StrictJavaDeps that there is no DEFAULT value in that enum because it's different than BuildConfiguration.StrictDepsMode

[iirina]

BTW, did you include upper-casing? Since the command line docs are for lowercase but the switch on the string is upper-case.

Yes

Also another problem I saw when I hacked on this was that I got an exception from DependencyModule#StrictJavaDeps that there is no DEFAULT value in that enum because it's different than BuildConfiguration.StrictDepsMode

Yes you are right, the DEFAULT value is missing from DependencyModule#StrictJavaDeps. The description of BuildConfiguration#StrictDepsMode#DEFAULT says "When no flag value is specified on the command line". From this I understand that when nothing is specified on the command line, the default value for strict deps for each language should be use. In our case, that value should be ERROR. I propose we use ERROR when passing DEFAULT to java_common.compile. WDYT?

[ittaiz]

sounds good. Whatever the default means for java_* rules I think it should apply here as well.

[ittaiz]

Hi,
Any update?

[iirina]

Yes, it should be here with the next push to GH. We had some problems with submitting changes internally.