-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[ZEPPELIN-1407] Fix Scala 2.11 build #1400
Conversation
68da065
to
3fb1b06
Compare
@bzz @Leemoonsoo Please review, this is more like a hotfix for the build. |
This should go for both master and 0.6x release. |
build failed I think because of
|
f136021
to
2c56423
Compare
@bzz Sorry, I had put the profiles activation as if scala 2.11 was the default, have fixed it now. |
2c56423
to
bba1274
Compare
@bzz the build errors are related to RAT issues
The build has been terminated |
Avoid activating the Scala 2.10 profile when building for Scala 2.11
bba1274
to
3fa489b
Compare
CI green, any more comments? |
@felixcheung @Leemoonsoo @bzz Any more comments here ? Should we merge this to 0.6 branch and master, as it's kind blocking to 2.11 |
### What is this PR for? Avoid activating the Scala 2.10 profile when building for Scala 2.11 ### What type of PR is it? [Bug Fix] ### What is the Jira issue? * (https://issues.apache.org/jira/browse/ZEPPELIN-1407)[https://issues.apache.org/jira/browse/ZEPPELIN-1407] ### How should this be tested? Perform Scala 2.10 and 2.11 builds starting from a maven repository that does not have org.apache.zeppelin artifacts. Author: Luciano Resende <lresende@apache.org> Closes #1400 from lresende/scala-profile and squashes the following commits: 3fa489b [Luciano Resende] [ZEPPELIN-1407] Fix Scala 2.11 build (cherry picked from commit 724ef6f) Signed-off-by: Felix Cheung <felixcheung@apache.org>
Done. master and branch-0.6 - thanks! |
I'm sorry my comment is bit late. code is already been merged. But let my write it for the records Because of all our documents and scripts uses Build instruction travis ci script release script If we want to use |
@Leemoonsoo The issue wasn't about -P versus -D, but that the Scala 2.10 was with activation true by default, so even when you did only -Pscala2.11 the Scala 2.10 profile was being activated. |
BTW, the properties on the profile activation, only enables the "default" to be activated by default, so mvn clean install will not have properties defined, so will start Scala 2.10... |
@lresende You can see followings from http://maven.apache.org/guides/introduction/introduction-to-profiles.html.
So once -Pscala-2.11 is defined, |
But unfortunately this wasn't what maven was enforcing, and Scala 2.11 build was failing when -PScala2.11 was being used, and when invoking dependency:tree with -Pscala2.11 there were few dependencies that were being resolved to _2.10 version of the dependency and thus causing the build failure. This approach seems to not rely on that maven feature and make the build work on both scenarios. |
@lresende In the issue description, your build command used I could reproduce the error when i use |
It looks like -D is in fact setting property and this change is adding such a check for the property? |
As moon mentioned, if you use build instruction in https://github.com/apache/zeppelin#example it doesn't bring any issue. |
I have verified that with that by reverting my changes -PScala2.10/Scala2.11 have the proper behavior and my original issue was that indeed my build alias had -D for the profile activation. Sorry for the confusion, please go ahead and revert this change. |
@lresende thank you for verification. I am reverting this PR then. |
What is this PR for?
Avoid activating the Scala 2.10 profile when building for Scala 2.11
What type of PR is it?
[Bug Fix]
What is the Jira issue?
How should this be tested?
Perform Scala 2.10 and 2.11 builds starting from a maven repository that does not have org.apache.zeppelin artifacts.