rules_scala icon indicating copy to clipboard operation
rules_scala copied to clipboard

Question on behavioral changes with scalac and setting of `--javabase`/`--host_javabase`

Open adam-singer opened this issue 4 years ago • 8 comments

Context

I'm working off of a fork of scala_rules https://github.com/twitter-forks/rules_scala. Changes to that fork are focused around scrooge changes https://github.com/bazelbuild/rules_scala/compare/master...twitter-forks:master. When attempting to rebase to latest bazelbuild/rules_scala I think I've noticed some change in behavior around which JDK JAVABIN is used durning scalac actions. Would like to clarify what used to happen and what is expected to happen.

I'm noticing that the behavior of how --javabase and --host_javabase might have changed between the upgrade where scalac action is (latest master) using the --host_javabase when generating the execroot/__main__/bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac script. In the previous behavior scalac action was using the --javabase (afaict).

This was problematic for because of having two different JDK versions for --javabase and --host_javabase (ex build --javabase=//tools/jdks:twitter_jdk8 --host_javabase=//tools/jdks:twitter_jdk11) can lead to incompatiblities in generated bytecode. For example I'm seeing some byte code generated where interfaces have known to be broken for example https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-4774077 / https://github.com/AdoptOpenJDK/openjdk-jdk9/commit/d9d7e875470bf478110b849315b4fff55b4c35cf , is happening on the latest master where it didn't happen the past

1) testExtractByteBuffer(com.twitter.io.BufCompilationTest)
java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer;
    at com.twitter.io.Buf$ByteBuffer$Shared$.copy(Buf.scala:953)
    at com.twitter.io.Buf$ByteBuffer$Shared$.apply(Buf.scala:957)
    at com.twitter.io.Bufs.sharedBuf(Bufs.java:72)
    at com.twitter.io.BufCompilationTest.testExtractByteBuffer(BufCompilationTest.java:126)
2) testByteBuffer(com.twitter.io.BufCompilationTest)
java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer;
    at com.twitter.io.Buf$ByteBuffer$Shared$.copy(Buf.scala:953)
    at com.twitter.io.Buf$ByteBuffer$Shared$.apply(Buf.scala:957)
    at com.twitter.io.Bufs.sharedBuf(Bufs.java:72)
    at com.twitter.io.BufCompilationTest.testByteBuffer(BufCompilationTest.java:90)

Previous Behavior (f28107017c5f2a3857d983988b663ea37cd4d00f)

The previous code I was working with before bazelbuild/scala_ruels commit f28107017c5f2a3857d983988b663ea37cd4d00f, https://github.com/twitter-forks/rules_scala/commit/a41a98648508b887c4e3513c45c2afef6bd9552b / https://github.com/twitter-forks/rules_scala/pull/3#commits-pushed-f281070 / https://github.com/bazelbuild/rules_scala/commit/f28107017c5f2a3857d983988b663ea37cd4d00f

The .bazelrc contained the following configurations

build --javabase=//tools/jdks:twitter_jdk8 --host_javabase=//tools/jdks:twitter_jdk11

The byte code generated for a class that calls java.nio.ByteBuffer.flip() is as follows

  Compiled from "Buf.scala"
public class com.twitter.io.Buf$ByteBuffer$Shared$
  minor version: 0
  major version: 52

  [...]

   #48 = NameAndType        #47:#31       // put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #49 = Methodref          #34.#48       // java/nio/ByteBuffer.put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #50 = Utf8               flip
   #51 = Utf8               ()Ljava/nio/Buffer;
   #52 = NameAndType        #50:#51       // flip:()Ljava/nio/Buffer;
   #53 = Methodref          #34.#52       // java/nio/ByteBuffer.flip:()Ljava/nio/Buffer;
   #54 = Utf8               Ljava/nio/ByteBuffer;
   #55 = Utf8               this
   #56 = Utf8               apply

Looking at the generated scalac wrapper it had picked up

$ cat [..]/execroot/__main__/bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac|grep JAVABIN
JAVABIN=${JAVABIN:-/Library/Java/JavaVirtualMachines/TwitterJDK/Contents/Home/bin/java}

Current Behavior (4dd3013e5006b33d457a33d66e2ccc9480621981)

Recently rebased from https://github.com/twitter-forks/rules_scala/commit/43a742bb7b3912a16aaeaf09b6ba217c3b655843 to our master https://github.com/twitter-forks/rules_scala/commits/master and using https://github.com/twitter-forks/rules_scala/commit/4dd3013e5006b33d457a33d66e2ccc9480621981. We noticed the behavior difference is now the scalac wrapper is picking up the JDK defined in --host_javabase instead of --javabase. This leads to byte code being generated that is not compatible when running code generated (if the JDK are note byte code compatible for some APIs). Our example is similar to above but added more flags and defined java tool chains

The .bazelrc contained the following configurations

build --java_language_version=8
build --tool_java_language_version=8
build --java_toolchain=//tools/jdks:twitter_jdk8_toolchain
build --javabase=//tools/jdks:twitter_jdk8
build --host_javabase=//tools/jdks:twitter_jdk11
build --host_java_toolchain=//tools/jdks:twitter_jdk11_toolchain

The tools/jdks/BUILD.bazel contains the following definitions

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

java_runtime(
    name = "twitter_jdk11",
    java_home = select({
        ":linux": "/usr/lib/jvm/java-11-twitter",
        ":mac": "/Library/Java/JavaVirtualMachines/TwitterJDK11/Contents/Home",
    }),
)

java_runtime(
    name = "twitter_jdk8",
    java_home = select({
        ":linux": "/usr/lib/jvm/java-1.8.0-twitter",
        ":mac": "/Library/Java/JavaVirtualMachines/TwitterJDK/Contents/Home",
    }),
)

default_java_toolchain(
    name = "twitter_jdk8_toolchain",
    jvm_opts = JDK8_JVM_OPTS,
    java_runtime = "//tools/jdks:twitter_jdk8",
    bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath"],
    source_version = "8",
    target_version = "8",
    visibility = ["//visibility:public"],
)

default_java_toolchain(
    name = "twitter_jdk11_toolchain",
    jvm_opts = JDK9_JVM_OPTS,
    java_runtime = "//tools/jdks:twitter_jdk11",
    bootclasspath = ["@bazel_tools//tools/jdk:platformclasspath"],
    source_version = "8",
    target_version = "8",
    visibility = ["//visibility:public"],
)

When looking at the bytecode generated the following was created, when this code executes with JDK8 it will throw an exception because the return type signature is flip:()Ljava/nio/ByteBuffer, meaning it was compiled with JDK9+. JDK8 would generate the following return type signature flip:()Ljava/nio/Buffer;.

  Compiled from "Buf.scala"
public class com.twitter.io.Buf$ByteBuffer$Shared$
  minor version: 0
  major version: 52

  [...]


   #44 = Utf8               ()Ljava/nio/ByteBuffer;
   #45 = NameAndType        #43:#44       // duplicate:()Ljava/nio/ByteBuffer;
   #46 = Methodref          #34.#45       // java/nio/ByteBuffer.duplicate:()Ljava/nio/ByteBuffer;
   #47 = Utf8               put
   #48 = NameAndType        #47:#31       // put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #49 = Methodref          #34.#48       // java/nio/ByteBuffer.put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #50 = Utf8               flip
   #51 = NameAndType        #50:#44       // flip:()Ljava/nio/ByteBuffer;
   #52 = Methodref          #34.#51       // java/nio/ByteBuffer.flip:()Ljava/nio/ByteBuffer;
   #53 = Utf8               Ljava/nio/ByteBuffer;
   #54 = Utf8               this

When looking at the scalac wrapper it contains the following code was generated. So leads me to think that the compiler used is the one defined with --host_javabase.

JAVABIN=${JAVABIN:-/Library/Java/JavaVirtualMachines/TwitterJDK11/Contents/Home/bin/java}

To confirm behavior when I set --javabase and --host_javabase to the same version JDK the problem goes away, regardless if its JDK8 or JDK11.

The .bazelrc contained the following configurations

build --java_language_version=8
build --tool_java_language_version=8
build --java_toolchain=//tools/jdks:twitter_jdk8_toolchain
build --javabase=//tools/jdks:twitter_jdk8
build --host_javabase=//tools/jdks:twitter_jdk8
build --host_java_toolchain=//tools/jdks:twitter_jdk11_toolchain

Generated byte code which is using a JDK8 sigature for flip:()Ljava/nio/Buffer;.

  Compiled from "Buf.scala"
public class com.twitter.io.Buf$ByteBuffer$Shared$
  minor version: 0
  major version: 52

  [...]

   #48 = NameAndType        #47:#31       // put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #49 = Methodref          #34.#48       // java/nio/ByteBuffer.put:(Ljava/nio/ByteBuffer;)Ljava/nio/ByteBuffer;
   #50 = Utf8               flip
   #51 = Utf8               ()Ljava/nio/Buffer;
   #52 = NameAndType        #50:#51       // flip:()Ljava/nio/Buffer;
   #53 = Methodref          #34.#52       // java/nio/ByteBuffer.flip:()Ljava/nio/Buffer;
   #54 = Utf8               Ljava/nio/ByteBuffer;
   #55 = Utf8               this

Generated scalac wrapper picks up JDK8 JAVABIN.

JAVABIN=${JAVABIN:-/Library/Java/JavaVirtualMachines/TwitterJDK/Contents/Home/bin/java}

Question

I'm wondering if this is exepcted behavior (bazel n00b here) for the host_javabase to be used (if thats what really happening) the compiling of scala code? Also wondering if I have incorrectly registered a JDK or setup some variable incorrectly. I've been debugging this mostly with bazel test -s --sandbox_debug --toolchain_resolution_debug and picking at the scripts directly / modifying scala_rules to print information out, if there are better ways to approach displaying all the environment information to know what should be or is being used at the point scalac is being called relative to javac that would be great, still new to this tooling.

adam-singer avatar Feb 27 '21 18:02 adam-singer

IIUC the crux is really about which JDK should the scalac wrapper invoke.

Per https://docs.bazel.build/versions/master/user-manual.html#flag--java_toolchain

--java_toolchain label
This option specifies the label of the java_toolchain used to compile Java source files.

--host_java_toolchain label
If not specified, bazel uses the value of --java_toolchain to compile code in the host configuration, i.e., tools run during the build. The main purpose of this flag is to enable cross-compilation.

--javabase (label)
This option sets the label of the base Java installation to use for bazel run, bazel test, and for Java binaries built by java_binary and java_test rules. The JAVABASE and JAVA "Make" variables are derived from this option.

--host_javabase label
This option sets the label of the base Java installation to use in the host configuration, for example for host build tools including JavaBuilder and Singlejar.

This does not select the Java compiler that is used to compile Java source files. The compiler can be selected by settings the --java_toolchain option.

It seems most consistent with what --java_toolchain aims to do, i.e. we should use the same version of JDK to compile Java and Scala sources, since a project can depend on both java and scala code.

@ittaiz @liucijus would you agree with this assessment?

wisechengyi avatar Mar 01 '21 01:03 wisechengyi

@adam-singer any chance you have a repro where I can see how behavior changes between versions? Does it work with Java targets?

liucijus avatar Mar 02 '21 06:03 liucijus

The repo I have is not viewable, but if I can find some example code that compiles/tests both scala/java targets I might be able to setup the same conditions

adam-singer avatar Mar 02 '21 06:03 adam-singer

I just wander if this can be related to incompatible_use_toolchain_transition = True,. If possible, could you try changing them to False and see if it helps?

liucijus avatar Mar 02 '21 08:03 liucijus

@liucijus hopefully this give an idea of what configuration we might of had that that leads to the difference in behavior

Previous behavior: Setting the javabase/java_toolchain to JDK8 while setting the host_javabase to JDK11 (f28107017c5f2a3857d983988b663ea37cd4d00f)

Note: My local jdk is jdk8 @bazel_tools//tools/jdk:jdk afaict.

Based on f28107017c5f2a3857d983988b663ea37cd4d00f added a simple example to show what the previous behavior with these settings are https://github.com/twitter-forks/rules_scala/tree/adams/scalac-behavior-with-f28107017/examples/testing/scalatest_repositories

cd <repo>/examples/testing/scalatest_repositories
bazel clean && bazel test -s --sandbox_debug --toolchain_resolution_debug --javabase=@bazel_tools//tools/jdk:jdk --java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 --host_javabase=@bazel_tools//tools/jdk:remote_jdk11 --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 //example:example 
INFO: Elapsed time: 12.757s, Critical Path: 11.28s
INFO: 35 processes: 13 internal, 12 darwin-sandbox, 10 worker.
INFO: Build completed successfully, 35 total actions
//example:example                                                        PASSED in 1.3s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 35 total actions
cat $(bazel info output_base)/execroot/scalatest_repositories/bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac |grep JAVABIN=\\$
JAVABIN=${JAVABIN:-${JAVA_RUNFILES}/local_jdk/bin/java}

Current behavior: Setting the javabase/java_toolchain to JDK8 while setting the host_javabase to JDK11

Based on https://github.com/twitter-forks/rules_scala/compare/master...twitter-forks:adams/github-issue-1233 shows example of NoSuchMethodError on java.nio.ByteBuffer.flip() due to scalac compiling with jdk11 but running tests on jdk8.

cd <repo>/examples/testing/scalatest_repositories
bazel clean && bazel test -s --sandbox_debug --toolchain_resolution_debug --javabase=@bazel_tools//tools/jdk:jdk --java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 --host_javabase=@bazel_tools//tools/jdk:remote_jdk11 --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 //example:example 
$ head -n 20 $(bazel info output_base)/execroot/scalatest_repositories/bazel-out/darwin-fastbuild/testlogs/example/example/test.log
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //example:example
-----------------------------------------------------------------------------
Discovery starting.
Discovery completed in 94 milliseconds.
Run starting. Expected test count is: 1
ExampleTest:
Exmaple
test1
*** RUN ABORTED *** (154 milliseconds)
  java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer;
  at examples.testing.scalatest_repositories.example.ExampleTest.$anonfun$new$1(ExampleTest.scala:10)
  at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
  at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
  at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
  at org.scalatest.Transformer.apply(Transformer.scala:22)
  at org.scalatest.Transformer.apply(Transformer.scala:20)
  at org.scalatest.flatspec.AnyFlatSpecLike$$anon$5.apply(AnyFlatSpecLike.scala:1683)
  at org.scalatest.TestSuite.withFixture(TestSuite.scala:196)
  at org.scalatest.TestSuite.withFixture$(TestSuite.scala:195)
# Path may differ, but I was looking inside the scalac script
cat $(bazel info output_base)/execroot/scalatest_repositories/bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac|grep JAVABIN=\\$
JAVABIN=${JAVABIN:-${JAVA_RUNFILES}/remotejdk11_macos/bin/java}

Current behavior: Setting the javabase/java_toolchain/host_javabase to JDK11

Based on https://github.com/twitter-forks/rules_scala/compare/master...twitter-forks:adams/github-issue-1233 shows example of successfully compiling and running on the same jdk11.

cd <repo>/examples/testing/scalatest_repositories
bazel clean && bazel test -s --sandbox_debug --toolchain_resolution_debug --javabase=@bazel_tools//tools/jdk:remote_jdk11 --java_toolchain=@bazel_tools//tools/jdk:toolchain_java11 --host_javabase=@bazel_tools//tools/jdk:remote_jdk11 --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java11 //example:example 
INFO: Elapsed time: 18.649s, Critical Path: 17.21s
INFO: 174 processes: 61 internal, 103 darwin-sandbox, 10 worker.
INFO: Build completed successfully, 174 total actions
//example:example                                                        PASSED in 1.3s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 174 total actions
cat $(bazel info output_base)/execroot/scalatest_repositories/bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac|grep JAVABIN=\\$
JAVABIN=${JAVABIN:-${JAVA_RUNFILES}/remotejdk11_macos/bin/java}

Current behavior: Setting the javabase/java_toolchain/host_javabase to JDK8

Based on https://github.com/twitter-forks/rules_scala/compare/master...twitter-forks:adams/github-issue-1233 shows example of successfully compiling and running on the same jdk8.

cd <repo>/examples/testing/scalatest_repositories
bazel clean && bazel test -s --sandbox_debug --toolchain_resolution_debug --javabase=@bazel_tools//tools/jdk:jdk --java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8 --host_javabase=@bazel_tools//tools/jdk:jdk --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_hostjdk8 //example:example 
INFO: Elapsed time: 11.427s, Critical Path: 11.19s
INFO: 29 processes: 8 internal, 11 darwin-sandbox, 10 worker.
INFO: Build completed successfully, 29 total actions
//example:example                                                        PASSED in 1.2s

Executed 1 out of 1 test: 1 test passes.
INFO: Build completed successfully, 29 total actions
cat $(bazel info output_base)/execroot/scalatest_repositories/bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/io_bazel_rules_scala/src/java/io/bazel/rulesscala/scalac/scalac|grep JAVABIN=\\$
JAVABIN=${JAVABIN:-${JAVA_RUNFILES}/local_jdk/bin/java}

adam-singer avatar Mar 02 '21 09:03 adam-singer

I just wander if this can be related to incompatible_use_toolchain_transition = True,. If possible, could you try changing them to False and see if it helps?

I will give that a try

adam-singer avatar Mar 02 '21 09:03 adam-singer

Tried the --incompatible_override_toolchain_transition flag on the example provided above based on https://github.com/twitter-forks/rules_scala/compare/master...twitter-forks:adams/github-issue-1233 shows example of NoSuchMethodError on java.nio.ByteBuffer.flip() due to scalac compiling with jdk11 but running tests on jdk8.

cd <repo>/examples/testing/scalatest_repositories
bazel clean && bazel test -s --sandbox_debug --toolchain_resolution_debug --javabase=@bazel_tools//tools/jdk:jdk --java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 --host_javabase=@bazel_tools//tools/jdk:remote_jdk11 --host_java_toolchain=@bazel_tools//tools/jdk:toolchain_java8 --incompatible_override_toolchain_transition //example:example

Ended up with same issue

[scalatest_repositories (adams/github-issue-1233)]$ head -n 20 /private/var/tmp/_bazel_adams/4e97845988238be1dc0fa3c00c4f74d0/execroot/scalatest_repositories/bazel-out/darwin-fastbuild/testlogs/example/example/test.log
exec ${PAGER:-/usr/bin/less} "$0" || exit 1
Executing tests from //example:example
-----------------------------------------------------------------------------
Discovery starting.
Discovery completed in 87 milliseconds.
Run starting. Expected test count is: 1
ExampleTest:
Exmaple
test1
*** RUN ABORTED *** (143 milliseconds)
  java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer;
  at examples.testing.scalatest_repositories.example.ExampleTest.$anonfun$new$1(ExampleTest.scala:10)
  at org.scalatest.OutcomeOf.outcomeOf(OutcomeOf.scala:85)
  at org.scalatest.OutcomeOf.outcomeOf$(OutcomeOf.scala:83)
  at org.scalatest.OutcomeOf$.outcomeOf(OutcomeOf.scala:104)
  at org.scalatest.Transformer.apply(Transformer.scala:22)
  at org.scalatest.Transformer.apply(Transformer.scala:20)
  at org.scalatest.flatspec.AnyFlatSpecLike$$anon$5.apply(AnyFlatSpecLike.scala:1683)
  at org.scalatest.TestSuite.withFixture(TestSuite.scala:196)
  at org.scalatest.TestSuite.withFixture$(TestSuite.scala:195)

I had also tested the flag on our internal repo and same issue. @liucijus do you think this is a toolchain resolution issue?

adam-singer avatar Mar 02 '21 16:03 adam-singer

@adam-singer I've created a repro https://github.com/simuons/rules_scala/tree/javabase-example/examples/jdk and got same failure java.lang.NoSuchMethodError: java.nio.ByteBuffer.flip()Ljava/nio/ByteBuffer; for command bazel clean && bazel run --javabase=:jdk8 --host_javabase=:jdk11 :MainScala

I made it pass by changing cfg="exec" to cfg="target" at https://github.com/bazelbuild/rules_scala/blob/master/scala/private/common_attributes.bzl#L104

~~Could you try that on your fork?~~

UPDATE: Please ignore this. Changing cfg="exec" to cfg="target" is not the right way to solve this.

simuons avatar Mar 04 '21 14:03 simuons