-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-28759][BUILD] Upgrade scala-maven-plugin to 4.2.0 and fix build profile on AppVeyor #25633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
cc @dongjoon-hyun, @srowen and @wangyum |
|
4.2.0 has davidB/scala-maven-plugin#358 fix too FWIW. |
|
Oh nice, so this possibly enables cross-compilation from JDK 11 to JDK 8 now? great |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM. (Pending Jenkins)
Thank you for the upstream scala-maven-plugin bug fix and updating this, @HyukjinKwon !
|
Test build #109950 has finished for PR 25633 at commit
|
|
Merged to master. Thank you all! |
| - cmd: mvn -DskipTests -Psparkr -Phive package | ||
| # '-Djna.nosys=true' is required to avoid kernel32.dll load failure. | ||
| # See SPARK-28759. | ||
| - cmd: mvn -DskipTests -Psparkr -Phive -Djna.nosys=true package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to document this as "building spark on windows" then? ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, do you mean https://github.com/apache/spark/blob/master/R/WINDOWS.md#building-sparkr-on-windows ? Actually this flag fixed an issue specific to Appveyor. So regular Windows build won't need this flag.
Actually, I have some more additional and required information to document there. Building Spark on Windows requires bash (by Windows 11 or Cygwin) due to, at least, here:
Line 464 in 9ea37b0
| <arg value="${project.basedir}/../build/spark-build-info"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me document this it later in few days.
What changes were proposed in this pull request?
This PR proposes to upgrade scala-maven-plugin from 3.4.4 to 4.2.0.
Upgrade to 4.1.1 was reverted due to unexpected build failure on AppVeyor.
The root cause seems to be an issue specific to AppVeyor - loading the system library 'kernel32.dll' seems being failed.
By setting
-Djna.nosys=true, it directly loads the library from the jar instead of system's.In this way, the build seems working fine.
Why are the changes needed?
It upgrades the plugin to fix bugs and fixes the CI build.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
It was tested at #25497