-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-8316] Upgrade to Maven 3.3.3 #6770
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
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.
I removed this method because I found it unnecessary and hard to understand.
CC @brennonyork who I believe wrote this, in case I missed something.
|
cc @srowen |
|
Test build #34732 has finished for PR 6770 at commit
|
|
I believe we'd also need to require Maven 3.3 in the build, and in enforcer plugin? to actually fail if used with a local version that's not high enough? 3.3 is fairly new and most dev environments won't have it. I suppose that's why Not enforcing this risks people hitting the issue that prompted this, I suppose, without knowing. then again, otherwise, earlier versions of Maven appear fine. Hm. What do you guys think about the tradeoffs? |
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.
I believe we'd also need to require Maven 3.3 in the build, and in enforcer plugin? to actually fail if used with a local version that's not high enough?
@srowen - We can always install Maven at this step and ignore any existing Maven installs. Would that be sufficient?
|
@pwendell may have some thoughts on this. As I noted in a line comment, we can always install and use our own version of Maven, ignoring any existing installs on the user's machine. That should, I believe, guarantee that we're always using the version of Maven that we want. |
|
Has this issue ever affected anyone before now? If it's a relatively obscure issue I think failing builds with Maven < 3.3 might be a big inconvenience for developers. |
|
Heh, well we have this problem right now: #6492 Of course if we didn't support Hadoop 1, this would go away. (See what I did there?) I think the only reasonable way forward on this one right now is to, yes, download and use the latest Maven when that is requested, but not actually fail the build if 3.3 isn't used. This does mean that someone with 3.2 on their machine can run into #6492, but hey, that only affects people building for Hadoop 1. Maybe close enough for now. |
|
We could also set a warning to print if a user already has |
|
Is it an inconvenience if we just always download (if not already downloaded) and use Maven, instead of checking for a local install first? We already do that for Zinc and Scala, so I don't see that we need to ever fail the build for using an out of date version of Maven. As long as we always download our own version of Maven, then the only time a developer might still use an out-of-date version is if they already downloaded one using |
I dunno if it counts as obscure, but the issue described here means at the very least that we will generate broken Spark builds for Hadoop 1 when using versions of Maven older than 3.3. |
|
Other discussion aside, I think this is OK to merge, since it merely updates the version of Maven that is downloaded to a new version, which should be a generally good thing. |
Versions of Maven older than 3.3.0 apparently have [a bug in how they handle transitive dependencies](apache#6492 (comment)). I confirmed that upgrading to Maven 3.3.3 resolves at least the particular manifestation of this bug that I ran into. Author: Nicholas Chammas <[email protected]> Closes apache#6770 from nchammas/maven-333 and squashes the following commits: 6bed2d9 [Nicholas Chammas] upgrade to Maven 3.3.3
Versions of Maven older than 3.3.0 apparently have a bug in how they handle transitive dependencies.
I confirmed that upgrading to Maven 3.3.3 resolves at least the particular manifestation of this bug that I ran into.