Create a new subproject for annotations #107

KengoTODA · 2017-01-24T03:52:10Z

Similar with #106, but it also contains:

Stop making jar file which contains jcip annotations and jsr305 annotations. We've switched to transitive dependency.
Delete jsr305.jar from findbugs/lib, and use jsr305-3.0.1.jar on Maven central instead.

Note that currently the artifactId for annotation jar is spotbugs-annotations.

this revision has one problem: it package two annotation jar files into distZip.

…notations

coveralls · 2017-01-24T03:58:26Z

Coverage increased (+0.003%) to 56.573% when pulling 47d9a90 on KengoTODA:subproject-for-annotations into 7b693a2 on spotbugs:master.

jsotuyod · 2017-01-24T16:30:21Z

findbugs/build.gradle

        exclude 'junit.jar'

        include '**/*.jar'
      }
-      from([jar, spotbugsAnnotationsJar]) {
+      from([jar]) {


no need to use a list anymore

jsotuyod · 2017-01-24T16:33:42Z

findbugs/src/test/java/edu/umd/cs/findbugs/DetectorsTest.java

-                File file = new File(cpStr);
-                if (file.isDirectory()) {
-                    project.addAuxClasspathEntry(file.getCanonicalPath());
+        File annotationsLib = new File("../spotbugs-annotations/build/libs");


I'm not comfortable hardwiring this into the code. Specially when it's a build dir, and therefore a run from IDE may fail if the annotations project was cleaned from CLI.

What was wrong with the old code?

It couldn't find the jar file of spotbugs-annotation. So nullness check couldn't run as expected.

That's weird... having the project dependency, it should be in the classpath for both Gradle and Eclipse / IntelliJ runs.... we definitely need some way around this

I guess we can simply delete if (file.isDirectory()), because gradle added jar file path to CLASSPATH.

System.out.println("CP:" + System.getProperty("java.class.path")); -> ~/spotbugs/findbugs/build/classes/test: ... ~/spotbugs/spotbugs-annotations/build/libs/spotbugs-annotations-3.1.0-SNAPSHOT.jar ...

jsotuyod · 2017-01-24T16:37:41Z

eclipsePlugin/build.gradle

-    transitive = false
-  }
+  includedLibs project(':spotbugs-annotations')
+  includedLibs 'com.google.code.findbugs:jsr305:3.0.1'


if you do this, you should probably remove jsr305.jar from the libs folder, remove it from line 31 of this build script, and change the manifest accordingly.

jsotuyod · 2017-01-24T16:39:19Z

findbugs/build.gradle

@@ -213,11 +130,12 @@ distributions {

        // TODO : These exclusions can be removed when we move dependencies from lib to dependency management
        exclude 'AppleJavaExtensions.jar'
+        exclude 'hamcrest-all-1.3.jar'


who is adding hamcrest as part of the compile configuration? AFAIK it's only used as a test dependency.

It comes from lib directory.
https://github.com/spotbugs/spotbugs/tree/47d9a90d697542d1cef48a7a0d4104d6b128817d/findbugs/lib

Refs: f038a08

I'll delete it in #104.

coveralls · 2017-01-25T03:44:51Z

Coverage increased (+0.003%) to 56.573% when pulling 0b23ad2 on KengoTODA:subproject-for-annotations into 7b693a2 on spotbugs:master.

jsotuyod · 2017-01-25T12:14:03Z

spotbugs-annotations/build.gradle

@@ -1,7 +1,5 @@
 apply from: "$rootDir/gradle/checkstyle.gradle"
 apply from: "$rootDir/gradle/javadoc.gradle"
-apply plugin: 'maven'
-apply plugin: 'signing'


don't you need these plugins to be able to sign and publish to maven central the annotations artifacts?

coveralls · 2017-01-25T13:48:29Z

Coverage increased (+0.1%) to 56.712% when pulling f07026a on KengoTODA:subproject-for-annotations into 7b693a2 on spotbugs:master.

jsotuyod · 2017-01-25T14:07:03Z

findbugs/src/test/java/edu/umd/cs/findbugs/DetectorsTest.java

-                if (file.isDirectory()) {
-                    project.addAuxClasspathEntry(file.getCanonicalPath());
-                }
+                project.addAuxClasspathEntry(file.getCanonicalPath());


have you tested this running from the IDE? Just curious, I clearly runs from Gradle... I don't remember if it was getting this way, or classpath was null on that scenario

It works in my Eclipse.
But I have another problem: test-harness project introduced a cyclic dependency.

Gradle has no problem because it computes dependency based on tasks. However Eclipse computes it based on projects...

We should specify fixed version instead of project(':test-harness'). I'll handle this after we release 3.1.0.

jsotuyod · 2017-01-25T14:08:38Z

findbugs/build.gradle

@@ -286,19 +204,6 @@ publishing {
        classifier 'javadoc'
      }
    }
-
-    annotations(MavenPublication) {


we do need publishing config / maven + signing plugins in the new project to publish annotations artifacts.

Yes we need. I'm working on another branch issue-30.

https://github.com/KengoTODA/spotbugs/commits/issue-30

jsotuyod · 2017-01-25T14:09:12Z

This is looking really good

KengoTODA · 2017-01-25T14:47:55Z

I've commented to all review comments, please check and merge.

henrik242

Lgtm

KengoTODA added 4 commits January 23, 2017 22:27

create sub project for test-harness

4b9e2fe

create subproject for annotations

2ba018e

this revision has one problem: it package two annotation jar files into distZip.

clean up jar files in distribution ZIP file

a0be893

Merge remote-tracking branch 'spotbugs/master' into subproject-for-an…

47d9a90

…notations

KengoTODA added the enhancement label Jan 24, 2017

KengoTODA self-assigned this Jan 24, 2017

jsotuyod reviewed Jan 24, 2017

View reviewed changes

KengoTODA added 2 commits January 25, 2017 10:05

remove needless plugin

8c38589

no more list to wrap task

0b23ad2

jsotuyod reviewed Jan 25, 2017

View reviewed changes

KengoTODA added 2 commits January 25, 2017 21:27

use jar file in lib directory

605d89f

revert changes, and simply add jar file to aux classpath

f07026a

jsotuyod reviewed Jan 25, 2017

View reviewed changes

KengoTODA mentioned this pull request Jan 25, 2017

Break cyclic dependency among Eclipse projects #108

Closed

KengoTODA added this to the SpotBugs 3.1.0 milestone Jan 25, 2017

henrik242 approved these changes Jan 26, 2017

View reviewed changes

henrik242 merged commit 4c6c6c5 into spotbugs:master Jan 26, 2017

KengoTODA deleted the subproject-for-annotations branch January 26, 2017 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create a new subproject for annotations #107

Create a new subproject for annotations #107

KengoTODA commented Jan 24, 2017

coveralls commented Jan 24, 2017

jsotuyod Jan 24, 2017

jsotuyod Jan 24, 2017

KengoTODA Jan 25, 2017

jsotuyod Jan 25, 2017

KengoTODA Jan 25, 2017

jsotuyod Jan 24, 2017

jsotuyod Jan 24, 2017

KengoTODA Jan 25, 2017 •

edited

KengoTODA Jan 25, 2017

jsotuyod Jan 25, 2017

coveralls commented Jan 25, 2017

jsotuyod Jan 25, 2017

coveralls commented Jan 25, 2017

jsotuyod Jan 25, 2017

KengoTODA Jan 25, 2017

jsotuyod Jan 25, 2017

KengoTODA Jan 25, 2017

jsotuyod commented Jan 25, 2017

KengoTODA commented Jan 25, 2017

henrik242 left a comment

Create a new subproject for annotations #107

Create a new subproject for annotations #107

Conversation

KengoTODA commented Jan 24, 2017

coveralls commented Jan 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KengoTODA Jan 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 25, 2017

Choose a reason for hiding this comment

coveralls commented Jan 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsotuyod commented Jan 25, 2017

KengoTODA commented Jan 25, 2017

henrik242 left a comment

Choose a reason for hiding this comment

KengoTODA Jan 25, 2017 •

edited