java_toolchain ignores the encoding setting

[juhalindfors]

The encoding setting in java_toolchain appears to be ignored. For reference, see the stack overflow discussion here: http://stackoverflow.com/questions/43435772/bazel-java-library-character-encoding-via-javacopts-not-working

The problem seems to stem from the introduction of ClassloaderMaskingFileManager in commit 3c5e55f. It hard-codes UTF-8 in the file manager configuration provided to the SDK JavacTool implementation. The way the JDK base file manager is implemented prevents the encoding setting from taking place if the charset has been configured to anything other than a null reference.

A simple fix is to change the constructor call in CassloaderMaskingFileManager to pass a null reference as a charset argument. I've created a unit test that demonstrates this fix and would prevent future regression on the issue but am still in the process of trying to understand the gerrit submission process.

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

[juhalindfors]

Proposed fix: juhalindfors@81b1e9a

Related unit test: juhalindfors@8e94ccc

Note that the unit test currently depends on the patch set in https://bazel-review.googlesource.com/#/c/10530/ associated with issue #2897

[cushon]

That isn't the only place that assumes Java source files are UTF-8:

• bazel/src/java_tools/buildjar/java/com/google/devtools/build/buildjar/VanillaJavaBuilder.java

  Line 158 in a5ba0d3

  javaCompiler.getStandardFileManager(diagnosticCollector, ENGLISH, UTF_8);

• bazel/src/java_tools/buildjar/java/com/google/devtools/build/java/turbine/javac/JavacTurbineCompiler.java

  Line 59 in d7b774b

  JavacFileManager fm = new JavacFileManager(new Context(), false, UTF_8)) {

• https://github.com/google/turbine/blob/a5beb1786cb534d7bfe2b4cd5f8e31464e7b3e8e/java/com/google/turbine/main/Main.java#L105

I'm not sure how much value java_toolchain.encoding has in general, but if Bazel is going to support it, it should work everywhere.

We'll also need to make sure it isn't overridable for Blaze.

[GinFungYJF]

Any update on this？ Would it be fixed in the 0.5.0 release?

[juhalindfors]

@GinFungYJF

I think I found how the original proposed patch could potentially cause an issue with build reproducibility but I still need to verify this. If correct, it should be feasibile to modify the patch according to the review comments to ensure one fix does not cause another issue. You can follow the review process here: https://bazel-review.googlesource.com/c/10751/, https://bazel-review.googlesource.com/c/10750

[ddwolf]

is this problem fixed?

[ittaiz]

I think the current state of having a field which is ignored is really bad.
It sounds to me like we should either drop it and clearly document the encoding for javac is UTF-8 without being able to override or fix this issue.

[ittaiz]

@cushon I really think that some blatant documentation is needed about this

[sgowroji]

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.