You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by jbonofre <gi...@git.apache.org> on 2016/03/18 00:32:22 UTC
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
GitHub user jbonofre opened a pull request:
https://github.com/apache/spark/pull/11803
SPARK-13991 - Extend the enforcer plugin Maven version rule
## What changes were proposed in this pull request?
Extend of the Maven version in the enforcer plugin configuration.
## How was this patch tested?
Full build with build/mvn -DskipTests clear package
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jbonofre/spark SPARK-13991
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11803.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #11803
----
commit 7ae973b652f0220313d0a601763dddbfea8dee1d
Author: Jean-Baptiste Onofré <jb...@apache.org>
Date: 2016-03-17T23:30:13Z
SPARK-13991 - Extend the enforcer plugin Maven version rule
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-199326547
OK yes let's close this PR then @jbonofre
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198152999
**[Test build #53479 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53479/consoleFull)** for PR 11803 at commit [`7ae973b`](https://github.com/apache/spark/commit/7ae973b652f0220313d0a601763dddbfea8dee1d).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198142427
Hi Josh,
it means that we force users to use a specific Maven version. For instance, I didn't have yet update to Maven 3.3.9, I'm still using 3.3.3.
If we force a version, it's OK for me (I will update on my machine), but I don't think it will be easy to maintain (updating for each new Maven release).
Maybe, we can at least specify [${maven.version},) (again, meaning that we really force the minimal Maven version).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198153139
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/11803#discussion_r56646795
--- Diff: pom.xml ---
@@ -1733,7 +1733,7 @@
<configuration>
<rules>
<requireMavenVersion>
- <version>${maven.version}</version>
+ <version>[3.3,)</version>
--- End diff --
Actually, that's what the existing specification already means:
https://maven.apache.org/enforcer/enforcer-rules/versionRanges.html
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198314377
As you explained in #8598, I understand the need to have Maven 3.3.x version (instead of Maven 3.2.x). It makes complete sense. The point in this PR is the fact that we "force" the latest Maven 3.3.x version, whereas the build works fine with another version in the Maven 3.3.x serie.
@srowen it's weird as build/mvn doesn't download the latest version of Maven on my box, it uses the one installed:
spark$ ./build/mvn -DskipTests clean package
Using `mvn` from path: /opt/apache-maven-3.3.3/bin/mvn
...
[WARNING] Rule 0: org.apache.maven.plugins.enforcer.RequireMavenVersion failed with message:
Detected Maven Version: 3.3.3 is not in the allowed range 3.3.9.
I think using enforcer [3.3,) rule for Maven version would be more "flexible".
Let me know what it makes sense, else I will close this PR.
Thanks !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/11803#discussion_r56646677
--- Diff: pom.xml ---
@@ -1733,7 +1733,7 @@
<configuration>
<rules>
<requireMavenVersion>
- <version>${maven.version}</version>
+ <version>[3.3,)</version>
--- End diff --
This should just be `[${maven.version},)` Because this minimum version is provided by the build it's OK to require it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11803
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198140635
Why should we do this? It's one less variable / environment thing for us to have to worry about if we can rely on a fixed Maven version.
Alternatively, why not treat `${maven.version}` as a lower-bound?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198291151
`build/mvn` already downloads the latest for you, hence it's required. Just making it a lower bound is fine. I'd rather keep plugins and build tools up to date and enforce that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198153142
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53479/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by jbonofre <gi...@git.apache.org>.
Github user jbonofre commented on a diff in the pull request:
https://github.com/apache/spark/pull/11803#discussion_r56646955
--- Diff: pom.xml ---
@@ -1733,7 +1733,7 @@
<configuration>
<rules>
<requireMavenVersion>
- <version>${maven.version}</version>
+ <version>[3.3,)</version>
--- End diff --
${maven.version} and [${maven.version},) are actually the same think for an enforcer perspective. So, if we don't go to [3.3,), then I will close this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198132452
**[Test build #53479 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53479/consoleFull)** for PR 11803 at commit [`7ae973b`](https://github.com/apache/spark/commit/7ae973b652f0220313d0a601763dddbfea8dee1d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: SPARK-13991 - Extend the enforcer plugin Maven...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/11803#issuecomment-198224277
@jbonofre thanks for the patch. However, I don't think things have really changed since your last patch https://github.com/apache/spark/pull/8598
The problem is that there are subtle differences from version to version, and the easiest way for us to ensure build reproducibility is to fix the version here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org