-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
[SPARK-34341][BUILD] Skip zinc related operations on aarch64 #31454
Conversation
Can one of the admins verify this patch? |
58fb4ce
to
b96b8fd
Compare
Do you have any issue link in zinc for this? It would be great if we can confirm that zinc is not supported in aarch64. |
@HyukjinKwon Thanks for you super fast reply. : )
I think it's a little diffcult to get this link, because the latest standalone version zinc (0.3.15, spark used), it is already deprecated, so we have no chance to raise a issue to confirm the Arm support from zinc officially [1].
But I can confirmed the 0.3.15 zinc, will call And another info is that I check the sbt/zinc [4], looks like we can only use the scalor-maven-plugin to enable zinc with maven, but not the stand alone style. I do some try on this plugin demo[5] on aarch64, it works. IMO, maybe the solution is that first we skip the zinc operations on aarch64 in here , and switch standalone zinc to [1] https://github.com/typesafehub/zinc |
cc @huangtianhua, do you have any input on this? |
Sorry reply late. Yes, we found standalone zinc is not supported well on aarch64 before, but it doesn't affect mvn building/testing, so we ignored it. To skip zinc related commands on aarch64 first is ok to me, so +1 :) thanks |
Merged to master. |
@HyukjinKwon @huangtianhua, I was actually interested in enablement of "zinc" operations in aarch64 as well. Are you planning to add this in future? @Yikun I've tried modifying the pom.xml to to add scalor-maven-plugin to enable zinc, but since it is getting compiled and tests are running without zinc also, How can we test that it is running the build using zinc? Let me know if i am m missing something here. |
Manual test is fine (just showing the logs in the PR description). If we can fix it not invasively, I think it's fine to add zinc back in aarch64 too. |
@odidev Sorry for late reply, and looks like there is some misleading content in my last reply. Actually, the scala-maven-plugin v4.0.0 already support the new version Zinc 1.x with multi-arch support rather than scalor-maven-plugin. [1] So, you can just use the scala-maven-plugin v4.X to enable the Zinc incremental compiler in aarch64 and also x86.
@HyukjinKwon And when I do investigation on the Zinc multi-arch support, I found there is bug SPARK-34539, looks like the zinc server code can be cleanup completely after we bump the scala-maven-plugin to v4.X in SPARK-28759, because the zinc increamental compiler has been enabled by default[2] in scala-maven-plugin 4.X. [1] davidB/scala-maven-plugin#321 |
What changes were proposed in this pull request?
Skip the zinc related installation operations on aarch64 platform.
Why are the changes needed?
The standalone zinc is not supported well on aarch64, so that the error ouput,
cannot execute binary file: Exec format error
dumped after build/mvn is called.This patch try to skip the zinc installation and related operations on aarch64 to make sure the error output doesn't print again on aarch64.
Does this PR introduce any user-facing change?
No
How was this patch tested?
simple cmd:
build/mvn -v
, see no error ouput again in aarch64, and nothing changed on x86