You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by markhamstra <gi...@git.apache.org> on 2014/04/03 00:25:46 UTC
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
GitHub user markhamstra opened a pull request:
https://github.com/apache/spark/pull/307
[SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Should be a painless upgrade, and does offer some significant advantages should we want to leverage FindBugs more during the 1.0 lifecycle. http://findbugs.sourceforge.net/findbugs2.html
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/markhamstra/spark findbugs
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/307.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 #307
----
commit 2cce98bc02334b650e290b855402d7dcd4409f14
Author: Mark Hamstra <ma...@gmail.com>
Date: 2014-04-02T22:17:57Z
Bumped FindBugs 1 to FindBugs 2
----
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39494981
Merged build finished. All automated tests passed.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39518909
Hey, I think this is causing some problems with our travis and sbt builds:
On travis:
```
[warn] Class javax.annotation.Nullable not found - continuing with a stub.
[warn] Caught: java.lang.NullPointerException while parsing annotations in /home/travis/build/apache/spark/lib_managed/bundles/guava-14.0.1.jar(com/google/common/base/Optional.class)
[error] error while loading Optional, class file '/home/travis/build/apache/spark/lib_managed/bundles/guava-14.0.1.jar(com/google/common/base/Optional.class)' is broken
[error] (class java.lang.RuntimeException/bad constant pool index: 1025 at pos: 2801)
[warn] two warnings found
[error] one error found
[error] (core/compile:compile) Compilation failed
[error] Total time: 158 s, completed Apr 3, 2014 11:14:07 PM
```
Locally it doesn't completely fail the build, but does cause warnings:
```
[info] Compiling 35 Scala sources to /Users/marmbrus/workspace/spark/sql/core/target/scala-2.10/classes...
[warn] Class javax.annotation.Nullable not found - continuing with a stub.
[warn] one warning found
```
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39488315
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39418061
Actually, looking at this a little deeper, the smarter move might be to
bump our Guava version to 16.0.1 and eliminate the explicit dependency on
findbugs entirely. The findbugs dependency is <optional> in the 16.0.x
POM, so that should work. Furthermore, if we want to eliminate fastutil
and start using Guava more (for murmurhash3 and maybe other stuff), then
getting current with Guava makes sense (and shouldn't be a problem since we
barely use Guava at all right now.)
On Wed, Apr 2, 2014 at 10:49 PM, Mark Hamstra <ma...@gmail.com> wrote:
> Looks like we started pulling it in with Guava:
>
>
> https://github.com/apache/spark/commit/04567a1771ec02e1efcc3ce078291a975fbb4e68#diff-c3580fe26fb42eb3aac6e180ae11e947
> https://code.google.com/p/guava-libraries/issues/detail?id=1096
>
>
> On Wed, Apr 2, 2014 at 10:31 PM, Patrick Wendell <notifications@github.com
> > wrote:
>
>> Seems reasonable - just wondering though - what do we use this for? I
>> think at present we aren't using this in the jenkins build... not sure how
>> it is supposed to be used.
>>
>> --
>> Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/307#issuecomment-39414237>
>> .
>>
>
>
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39519282
I'm reverting this. Sorry jenkins was messed up :P
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39391997
Merged build started.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39505192
Merged - thanks @markhamstra !
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39519289
The local warning is expected, but the Travis failure is a problem and is symptomatic of not having the jsr305 annotations available. Looks like we'll either have to revert this or find a way to make the jsr305 available to the Travis tests.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39415000
Looks like we started pulling it in with Guava:
https://github.com/apache/spark/commit/04567a1771ec02e1efcc3ce078291a975fbb4e68#diff-c3580fe26fb42eb3aac6e180ae11e947
https://code.google.com/p/guava-libraries/issues/detail?id=1096
On Wed, Apr 2, 2014 at 10:31 PM, Patrick Wendell
<no...@github.com>wrote:
> Seems reasonable - just wondering though - what do we use this for? I
> think at present we aren't using this in the jenkins build... not sure how
> it is supposed to be used.
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/apache/spark/pull/307#issuecomment-39414237>
> .
>
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39391983
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39414237
Seems reasonable - just wondering though - what do we use this for? I think at present we aren't using this in the jenkins build... not sure how it is supposed to be used.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39429880
Ah I should have looked more at this. The dependency is just to bring in some annotations that were later standardized. We do not use them, and so do not need any copy, including from Guava. This could be omitted, as it does not enable running Findbugs anyway.
I have the formula for adding a Findbugs plugin to the build. It's not hard. As I say in the JIRA, it would not do much good in a Scala-based project.
(IntelliJ does have good Scala static analysis. I'd like to take a crack at fixing all the stuff it's found. These are big PR-busting changes unfortunately so I've not suggested many. It only gets harder later, so seems like worth doing soon if at all, but it's so so busy!)
Updating Guava is good per se; it's a bit perilous when you get near Hadoop since even current versions use Hadoop 11.0.2, and when executing code within a Hadoop classloader, you'll end up linking against it's old version in the parent classpath. I think we've had this discussion before -- it is somewhat less of an issue for Spark but I confess I haven't though through whether it is actually a non-issue. The project is already on 14, hmm.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39488330
Merged build started.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39519684
We could go back to bumping up to FindBugs 2, or just leave things as they have been. It's mostly a question of whether we want to try to avoid presenting a dependency annoyance for those using the current FindBugs.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39396555
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13701/
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39486891
Looks like just removing the findbugs jsr305 dependency entirely works fine, but bumping the Guava version not so much. I'm going to change the name of this JIRA and PR, just do the minor dependency cleanup, and leave it to @srowen and others to decide whether they want to change the Guava version in the fastutil or other PRs.
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/307
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Removed findbugs jsr305 dependenc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39494982
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13729/
---
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.
---
[GitHub] spark pull request: [SPARK-1398] Bumped FindBugs 1 to FindBugs 2
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/307#issuecomment-39396554
Merged build finished. All automated tests passed.
---
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.
---