[SPARK-1398] Removed findbugs jsr305 dependency

[markhamstra]

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

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished. All automated tests passed.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13701/

[pwendell]

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.

[markhamstra]

Looks like we started pulling it in with Guava:

04567a1#diff-c3580fe26fb42eb3aac6e180ae11e947
https://code.google.com/p/guava-libraries/issues/detail?id=1096

On Wed, Apr 2, 2014 at 10:31 PM, Patrick Wendell
[email protected]:

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 GitHubhttps://github.com//pull/307#issuecomment-39414237
.

[markhamstra]

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 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 [email protected] wrote:

Looks like we started pulling it in with Guava:

04567a1#diff-c3580fe26fb42eb3aac6e180ae11e947
https://code.google.com/p/guava-libraries/issues/detail?id=1096

On Wed, Apr 2, 2014 at 10:31 PM, Patrick Wendell <[email protected]

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 GitHubhttps://github.com//pull/307#issuecomment-39414237
.

[srowen]

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.

[markhamstra]

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.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[AmplabJenkins]

Merged build finished. All automated tests passed.

[AmplabJenkins]

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13729/

[pwendell]

Merged - thanks @markhamstra !

[marmbrus]

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

[pwendell]

I'm reverting this. Sorry jenkins was messed up :P

[markhamstra]

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.

[markhamstra]

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.