[SPARK-27250][test-maven][BUILD] Scala 2.11 maven compile should target Java 1.8

[jzhuge]

What changes were proposed in this pull request?

Fix Scala 2.11 maven build issue after merging SPARK-26946.

How was this patch tested?

Maven Scala 2.11 and 2.12 builds with -Phadoop-provided -Phadoop-2.7 -Pyarn -Phive -Phive-thriftserver.

[cloud-fan]

ok to test

[cloud-fan]

add to whitelist

[SparkQA]

Test build #103836 has finished for PR 24184 at commit 284ad6a.

• This patch fails to build.
• This patch merges cleanly.
• This patch adds no public classes.

[jzhuge]

build failure:
[error] 'jvm-1.8.0_191' is not a valid choice for '-target'
[error] 'jvm-1.8.0_191' is not a valid choice for '-target'
[error] bad option: '-target:jvm-1.8.0_191'
[error] bad option: '-target:jvm-1.8.0_191'

[cloud-fan]

shall we just use jvm-1.8?

[jzhuge]

Hate to hard-code it but might have to without other choice. I am not fluent in SBT, is it normal for it to set `java.version` to 1.8.0_191?

…

On Fri, Mar 22, 2019 at 2:23 PM Wenchen Fan ***@***.***> wrote: shall we just use jvm-1.8? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#24184 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABy-pEgQIElkMlxKUYK9DHXJJvBmijVpks5vZUnfgaJpZM4cEXCp> .

-- John Zhuge

[vanzin]

"java.version" is a system property set by the JVM itself.

[jzhuge]

So it is not reading property "java.version" from root pom, but what about this line in SparkBuild.java: javaVersion := SbtPomKeys.effectivePom.value.getProperties.get("java.version").asInstanceOf[String], Shouldn't SbtPomKeys.effectivePom read the pom.xml from project base directory?

…

On Fri, Mar 22, 2019 at 4:50 PM Marcelo Vanzin ***@***.***> wrote: "java.version" is a system property set by the JVM itself. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#24184 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABy-pO_fmMNafoTJEdtnTGIX-u8O4EARks5vZWxGgaJpZM4cEXCp> .

-- John Zhuge

[jzhuge]

I have a workaround by adding another Maven property "target.java.version" specifically for "-target:jvm-${target.java.version}" only. This doesn't solve the SBT mystery, but at least get us going. What do you guys think?

…

On Fri, Mar 22, 2019 at 5:34 PM John Zhuge ***@***.***> wrote: So it is not reading property "java.version" from root pom, but what about this line in SparkBuild.java: javaVersion := SbtPomKeys.effectivePom.value.getProperties.get("java.version").asInstanceOf[String], Shouldn't SbtPomKeys.effectivePom read the pom.xml from project base directory? On Fri, Mar 22, 2019 at 4:50 PM Marcelo Vanzin ***@***.***> wrote: > "java.version" is a system property set by the JVM itself. > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#24184 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABy-pO_fmMNafoTJEdtnTGIX-u8O4EARks5vZWxGgaJpZM4cEXCp> > . > -- John Zhuge

-- John Zhuge

[HyukjinKwon]

So, is the issue java.version is not respected from root pom? If so can we investigate the root cause?

[HyukjinKwon]

I have observed (I assume) this issue which caused an error in the thirdparty library I worked on but just decided to workaround within the thirdparty project. It would be awesome if root cause is identified and fixed.

[HyukjinKwon]

BTW, let's do maven build as well before getting this in since we now observed the difference between SBT and Maven. If we can fix it in both SBT and Maven without identifying root cause (if that's difficult), that's fine to me too.

[srowen]

This is a change to the Maven build, not SBT. java.version is defined in pom.xml so I'm surprised. Is this failing when SBT interprets the pom via a plugin and uses the JVM java.version? Well, I think hard-coding "1.8" is fine for now, myself. I'm about to drop 2.11 support anyway and can remove it.

[felixcheung]

SGTM but need to update and re-run test

[jzhuge]

Passed maven 2.11/2.12 and sbt 2.11/2.12 builds locally, after reverting SPARK-25196.

[HyukjinKwon]

retest this please

[SparkQA]

Test build #4656 has started for PR 24184 at commit 28aa9cb.

[SparkQA]

Test build #103842 has finished for PR 24184 at commit 28aa9cb.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[srowen]

I'll merge it fairly soon to get the 2.11 build healthy one last time

[jzhuge]

Thanks @srowen.

FWIW, I understand the SBT mystery a little better. Scale compile options come from maven scala-maven-plugin.configuration.args and the following code in SparkBuild.scala:

    scalacOptions in Compile ++= Seq(
      s"-target:jvm-${javaVersion.value}",
      "-sourcepath", (baseDirectory in ThisBuild).value.getAbsolutePath  // Required for relative source links in scaladoc
    ),

Evidence in the generated file inc_compile-2.12:

compile options:
12 items
00 -> -unchecked
01 -> -deprecation
02 -> -feature
03 -> -explaintypes
04 -> -Yno-adapted-args
05 -> -target:jvm-1.8.0_181
06 -> -P:genjavadoc:out=/Users/jzhuge/Repos/upstream-spark/common/network-yarn/target/java
07 -> -P:genjavadoc:strictVisibility=true
08 -> -Xplugin:/Users/jzhuge/.ivy2/cache/com.typesafe.genjavadoc/genjavadoc-plugin_2.12.8/jars/genjavadoc-plugin_2.12.8-0.11.jar
09 -> -target:jvm-1.8
10 -> -sourcepath
11 -> /Users/jzhuge/Repos/upstream-spark

The line -target:jvm-${java.version} in maven scalac args was parsed as -target:jvm-1.8.0_191, based on JVM system property, while the SparkBuild code set -target:jvm correctly because it got javaVersion from effective pom. It seems maven property java.version was not honored when extracting maven scalec options.

@seancxmao Any insight?

[dongjoon-hyun]

+1, LGTM, too.
After 01e6305 , I rebased this PR to the master and was able to build with Scala-2.11 on both maven/sbt cleanly.

[srowen]

Merged to master