Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BEAM-8917] jsr305 dependency declaration for Nullable class #10324

Merged
merged 4 commits into from
Dec 12, 2019
Merged

[BEAM-8917] jsr305 dependency declaration for Nullable class #10324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9493e29
[BEAM-8917] jsr305 dependency declaration
suztomo Dec 9, 2019
8c1680c
Added description for JSR305 in BeamModulePlugin
suztomo Dec 10, 2019
e1c8629
Updated comment
suztomo Dec 10, 2019
6b4a098
Moved the description closer to spotbugs-annotations
suztomo Dec 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ class BeamModulePlugin implements Plugin<Project> {
jackson_module_scala : "com.fasterxml.jackson.module:jackson-module-scala_2.11:$jackson_version",
jaxb_api : "javax.xml.bind:jaxb-api:$jaxb_api_version",
joda_time : "joda-time:joda-time:2.10.3",
jsr305 : "com.google.code.findbugs:jsr305:3.0.2",
junit : "junit:junit:4.13-beta-3",
kafka : "org.apache.kafka:kafka_2.11:$kafka_version",
kafka_clients : "org.apache.kafka:kafka-clients:$kafka_version",
Expand Down Expand Up @@ -738,6 +739,10 @@ class BeamModulePlugin implements Plugin<Project> {
// spotbugs-annotations artifact is licensed under LGPL and cannot be included in the
// Apache Beam distribution, but may be relied on during build.
// See: https://www.apache.org/legal/resolved.html#prohibited
// Special case for jsr305 (a transitive dependency of spotbugs-annotations):
// sdks/java/core's FieldValueTypeInformation needs javax.annotations.Nullable at runtime.
// Therefore, the java core module declares jsr305 dependency (BSD license) as "compile".
// https:/findbugsproject/findbugs/blob/master/findbugs/licenses/LICENSE-jsr305.txt
"com.github.spotbugs:spotbugs-annotations:3.1.12",
"net.jcip:jcip-annotations:1.0",
]
Expand Down
1 change: 1 addition & 0 deletions sdks/java/core/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ dependencies {
compile library.java.protobuf_java
compile library.java.commons_compress
compile library.java.commons_lang3
compile library.java.jsr305
Copy link
Member

@iemejia iemejia Dec 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot do this a compile time dependency due to the license (LGPL) of findbugs/spotbugs. So far I think we have gotten around it because of the scope.

beam/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy

Lines 738 to 741 in 8066d78

// spotbugs-annotations artifact is licensed under LGPL and cannot be included in the
// Apache Beam distribution, but may be relied on during build.
// See: https://www.apache.org/legal/resolved.html#prohibited
"com.github.spotbugs:spotbugs-annotations:3.1.12",

CC: @kennknowles who probably knows more of the details.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For extra ref the licenses we cannot include in Apache Software Foundations (ASF) projects is known as the Category X dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responded to you in JIRA ticket.

https://issues.apache.org/jira/browse/BEAM-8917?focusedCommentId=16991659&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16991659

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good info thanks! We probably still would need to go with spotbugs anyway to be consistent with the previous upgrade, but maybe it is worth to focus on the big issue that's on https://issues.apache.org/jira/browse/BEAM-8858

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can wait for BEAM-8858 cleared.

What is "the previous upgrade" you're referring?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add it to the list of compileOnlyAnnotationDeps where you see spotbugs-annotations?

I am actually sort of surprised this works, since spotbugs operates on bytecode, not during compilation. I guess the annotations are retained and inspected in a way that does not require having the annotation library present. I have heard that any annotation with a class name (insensitive) of "nullable" will work, for example.

Noting in yet another place that spotbugs nullability analysis appears to be nonfunctional anyhow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In findbugsproject/findbugs#128 (comment), iloveeclipse confirmed that JSR305 has BSD license and was unexpectedly uploaded to Maven Central with a wrong license.

I moved the JSR305 explanation closer to spotbugs-annotations.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that it is not a compileOnlyAnnotation anymore because it is used to resolve nullability of Schema based fields per 788ce61#diff-504e2e0131eda09163b086becec92f3cR91 so it is needed at runtime too, or am I misreading this?

Copy link
Contributor Author

@suztomo suztomo Dec 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iemejia That explanation is correct.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. The LGPL annotations are spotbugs-annotations which have things like SuppresFBWarnings and a few other utility annotations. jsr305 is fine. The landscape of these annotations and what they do is actually quite strange, incomplete, and also changed.

shadow library.java.jackson_core
shadow library.java.jackson_annotations
shadow library.java.jackson_databind
Expand Down