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

suztomo · 2019-12-09T03:56:53Z

https://issues.apache.org/jira/projects/BEAM/issues/BEAM-8917

The Maven artifact org.apache.beam:beam-sdks-java-core, which contains org.apache.beam.sdk.schemas.FieldValueTypeInformation, should declare the dependency to com.google.code.findbugs:jsr305. The class needs Nullable class at runtime:

        .setNullable(field.isAnnotationPresent(Nullable.class))

Example project to produce an issue: https:/suztomo/beam-java-sdk-missing-nullable .

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

Choose reviewer(s) and mention them in a comment (R: @username).
Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	SDK	Apex	Dataflow	Gearpump	Samza	Spark
Go		---	---	---	---
Java
Python		---		---	---
XLang	---	---	---	---	---	---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website
Non-portable
Portable	---		---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

iemejia · 2019-12-09T09:17:31Z

sdks/java/core/build.gradle

@@ -69,6 +69,7 @@ dependencies {
 compile library.java.protobuf_java
 compile library.java.commons_compress
 compile library.java.commons_lang3
+ compile library.java.jsr305


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.

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

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

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

Sure, I can wait for BEAM-8858 cleared.

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

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.

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.

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?

@iemejia That explanation is correct.

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.

suztomo · 2019-12-09T14:58:00Z

Run Java PreCommit

suztomo · 2019-12-09T14:58:08Z

Run Python PreCommit

suztomo · 2019-12-09T17:50:27Z

Run Java PreCommit

suztomo · 2019-12-09T17:50:34Z

Run Python PreCommit

suztomo · 2019-12-09T19:17:28Z

Run BigQueryIO Streaming Performance Test Java

suztomo · 2019-12-09T21:10:49Z

Run Python PreCommit

suztomo · 2019-12-09T21:11:27Z

Run Dataflow ValidatesRunner

suztomo · 2019-12-10T02:00:30Z

Run Python PreCommit

suztomo · 2019-12-10T02:00:39Z

Run Dataflow ValidatesRunner

suztomo · 2019-12-10T04:20:53Z

Run Dataflow ValidatesRunner

suztomo · 2019-12-10T13:02:15Z

Run Python PreCommit

suztomo · 2019-12-10T15:29:40Z

Run Java PostCommit

suztomo

@kennknowles PTAL.

suztomo · 2019-12-11T13:45:58Z

sdks/java/core/build.gradle

@@ -69,6 +69,7 @@ dependencies {
 compile library.java.protobuf_java
 compile library.java.commons_compress
 compile library.java.commons_lang3
+ compile library.java.jsr305


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.

suztomo · 2019-12-11T15:49:44Z

Run CommunityMetrics PreCommit

suztomo · 2019-12-11T18:46:52Z

Run Dataflow ValidatesRunner

suztomo · 2019-12-11T18:47:18Z

Run Java PostCommit

suztomo · 2019-12-11T18:47:23Z

Run Java HadoopFormatIO Performance Test

suztomo · 2019-12-11T18:47:27Z

Run BigQueryIO Streaming Performance Test Java

suztomo · 2019-12-11T18:47:36Z

Run Spark ValidatesRunner

suztomo · 2019-12-11T18:47:49Z

Run SQL Postcommit

suztomo · 2019-12-11T18:47:54Z

Dataflow Runner Nexmark Tests

suztomo · 2019-12-11T18:48:23Z

Run Java Flink PortableValidatesRunner Batch

suztomo · 2019-12-11T18:48:41Z

Run Java Flink PortableValidatesRunner Streaming

suztomo · 2019-12-11T18:48:53Z

Direct Runner Nexmark Tests

suztomo · 2019-12-11T18:49:30Z

Run Java Spark PortableValidatesRunner Batch

suztomo · 2019-12-11T19:22:46Z

Run Spark ValidatesRunner

suztomo · 2019-12-11T21:19:56Z

Run Spark ValidatesRunner

suztomo · 2019-12-11T21:35:15Z

Run Spark ValidatesRunner

suztomo · 2019-12-11T23:28:15Z

Apache Spark Runner ValidatesRunner Tests don't succeed probably because of PR 10151

suztomo · 2019-12-12T05:35:20Z

Run Spark ValidatesRunner

suztomo · 2019-12-12T13:25:03Z

@iemejia @kennknowles All checks passed. Does this look good?

suztomo · 2019-12-12T20:17:31Z

Kenn, Thank you.

…10324) The Maven artifact org.apache.beam:beam-sdks-java-core, which contains org.apache.beam.sdk.schemas.FieldValueTypeInformation, should declare the dependency to com.google.code.findbugs:jsr305. The class needs Nullable class at runtime.

…ncy declaration for Nullable class (#10324)

iemejia · 2019-12-13T16:50:37Z

Just double checked with today's SNAPSHOTs after the merge and the pom of core is not modified, however the deps look good in master, not sure if the change was applied before the SNAPSHOT generation, but still to double check.
https://repository.apache.org/content/repositories/snapshots/org/apache/beam/beam-sdks-java-core/2.19.0-SNAPSHOT/beam-sdks-java-core-2.19.0-20191213.072102-9.pom

suztomo · 2019-12-13T19:17:01Z

Investigating that. I see the same problem in my local installation. /Users/suztomo/.m2/repository//org/apache/beam/beam-sdks-java-core/2.19.0-SNAPSHOT/beam-sdks-java-core-2.19.0-SNAPSHOT.pom does not have jsr305.

suztomo · 2019-12-13T20:41:36Z

PR to fix this #10382

[release-2.17.0][BEAM-8917][BEAM-8967] jsr305 dependency declaration for Nullable class (#10324)

suztomo · 2019-12-18T20:15:07Z

Fixed in the snapshot:
https://repository.apache.org/content/repositories/snapshots/org/apache/beam/beam-sdks-java-core/2.19.0-SNAPSHOT/beam-sdks-java-core-2.19.0-20191218.071919-14.pom

…10324) The Maven artifact org.apache.beam:beam-sdks-java-core, which contains org.apache.beam.sdk.schemas.FieldValueTypeInformation, should declare the dependency to com.google.code.findbugs:jsr305. The class needs Nullable class at runtime.

[BEAM-8917] jsr305 dependency declaration

9493e29

suztomo mentioned this pull request Dec 9, 2019

Linkage Checker: javax.annotation.Nullable false positive? GoogleCloudPlatform/cloud-opensource-java#1045

Closed

iemejia reviewed Dec 9, 2019

View reviewed changes

kennknowles requested review from kennknowles and iemejia December 10, 2019 03:02

suztomo added 2 commits December 10, 2019 10:07

Added description for JSR305 in BeamModulePlugin

8c1680c

Updated comment

e1c8629

Moved the description closer to spotbugs-annotations

6b4a098

suztomo commented Dec 11, 2019

View reviewed changes

iemejia mentioned this pull request Dec 11, 2019

[BEAM-8858] sdks/java/extensions/sql to declare used-but-undeclared dependencies #10332

Merged

3 tasks

kennknowles approved these changes Dec 12, 2019

View reviewed changes

kennknowles merged commit 160beb9 into apache:master Dec 12, 2019

suztomo deleted the nullable branch December 12, 2019 20:17

udim added a commit that referenced this pull request Dec 13, 2019

Merge pull request #10371: [release-2.18.0][BEAM-8917] jsr305 depende…

4b1bfc5

…ncy declaration for Nullable class (#10324)

suztomo mentioned this pull request Dec 13, 2019

[BEAM-8967] Declare JSR305 dependency as 'shadow' #10382

Merged

3 tasks

Ardagan added a commit that referenced this pull request Dec 17, 2019

Merge pull request #10370 from iemejia/release-2.17.0

5e3c975

[release-2.17.0][BEAM-8917][BEAM-8967] jsr305 dependency declaration for Nullable class (#10324)

	// 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",

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

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

Conversation

suztomo commented Dec 9, 2019 • edited Loading

Post-Commit Tests Status (on master branch)

Pre-Commit Tests Status (on master branch)

iemejia Dec 9, 2019 • edited Loading

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

Choose a reason for hiding this comment

suztomo Dec 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 9, 2019

suztomo commented Dec 10, 2019

suztomo commented Dec 10, 2019

suztomo commented Dec 10, 2019

suztomo commented Dec 10, 2019

suztomo commented Dec 10, 2019

suztomo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 11, 2019

suztomo commented Dec 12, 2019

suztomo commented Dec 12, 2019

suztomo commented Dec 12, 2019

iemejia commented Dec 13, 2019

suztomo commented Dec 13, 2019

suztomo commented Dec 13, 2019

suztomo commented Dec 18, 2019

suztomo commented Dec 9, 2019 •

edited

Loading

iemejia Dec 9, 2019 •

edited

Loading

suztomo Dec 11, 2019 •

edited

Loading