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-10961] Enable strict dependency analysis on all Java modules #12938

Closed
wants to merge 68 commits into from
Closed

[BEAM-10961] Enable strict dependency analysis on all Java modules #12938

Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
53940fd
Integrated Nebula plugin
sonam-vend Sep 25, 2020
dbab64a
Unused exclude dependencies
sonam-vend Sep 25, 2020
e3d2be0
Excluded unused dependencies
sonam-vend Oct 1, 2020
b974636
Enabled strict dependency over all java modules
sonam-vend Oct 12, 2020
9fa8a1d
Removed nebula plugin
sonam-vend Oct 12, 2020
ce8a592
Delete replay_pid52172.log
sonam-vend Oct 13, 2020
6ef876f
Delete replay_pid54062.log
sonam-vend Oct 13, 2020
0552aeb
Delete replay_pid79535.log
sonam-vend Oct 13, 2020
d0d6c33
Reverted to org.gradle.jvmargs=-Xss10240k
sonam-vend Oct 13, 2020
bb67adb
Merge branch 'dev-beam' of https://github.com/sonam-vend/beam into de…
sonam-vend Oct 13, 2020
a85aab1
code refactored: removed additional/unwanted comments
sonam-vend Oct 13, 2020
dffde94
Code Refactored: Removed commented out dependencies and created dep v…
sonam-vend Oct 15, 2020
343c171
Resolved merge conflicts
sonam-vend Oct 19, 2020
accf6e7
Merge branch 'master' into dev-beam
sonam-vend Oct 19, 2020
ea972f3
Fixed zstd issue
sonam-vend Oct 20, 2020
40f04af
Merge branch 'dev-beam' of https://github.com/sonam-vend/beam into de…
sonam-vend Oct 20, 2020
fd61007
added com.github.luben:zstd-jni artifact
sonam-vend Oct 20, 2020
1f18118
Merge remote-tracking branch 'upstream/master' into dev-beam
sonam-vend Oct 23, 2020
872b9df
Fixed checker issues
sonam-vend Oct 26, 2020
4c5041c
Fixed zetaqsl tests issue
sonam-vend Oct 26, 2020
205d42a
Fixed zetaqsl dependency issue
sonam-vend Oct 26, 2020
b23845d
fixed Java pre-commit tests issues
sonam-vend Oct 27, 2020
a0ff883
fixed 'run Java pre-commit' tests issues
sonam-vend Oct 27, 2020
aae94a7
fixed jms test run issues
sonam-vend Oct 27, 2020
5ed53e5
Fixed tika, xml, mqtt dependency issue
sonam-vend Oct 27, 2020
f73b7e2
Fixed derby issues
sonam-vend Oct 27, 2020
907544f
Fixed commit issues
Nov 3, 2020
a6ed6cb
modified artifacts into the prefered format
Nov 4, 2020
e73cdf0
Fixed direct runner dependency issue
Nov 4, 2020
d3d030a
Fixed merge conflicts with the main branch
Nov 5, 2020
d14d168
Modified slf4j-api artifact to prefered format
sonam-vend Nov 5, 2020
d08dbd9
Modified beam-vendor-grpc artifact to the prefered format
sonam-vend Nov 5, 2020
4bbe3e5
Modified vendored_guava artifact to the preferred value
sonam-vend Nov 5, 2020
51e4364
Modified joda-time artifact to the preferred format
sonam-vend Nov 5, 2020
efc5b5f
modified avro artifact to the perferred format
sonam-vend Nov 5, 2020
e6a1360
modified jackson-core dependency to the preferred format
sonam-vend Nov 5, 2020
000eb99
fixed flink runner dependency version issues
sonam-vend Nov 5, 2020
657fcc4
Merge branch 'master' of https://github.com/apache/beam into dev-beam
sonam-vend Nov 9, 2020
16d46ad
Resolved merge conflicts
sonam-vend Nov 24, 2020
342a315
Modified slf4j dependency to the preffered format
sonam-vend Nov 25, 2020
c65966e
Removed duplicate guava dependency
sonam-vend Nov 25, 2020
0b42185
Daemon=true
sonam-vend Nov 25, 2020
47c8295
moved flink dependencies to flink_runner.gradle
sonam-vend Nov 25, 2020
0a31ca4
moved flink_job_Server dependencies to flink_job_Server.gradle
sonam-vend Nov 25, 2020
606e3d5
removed permitUnusedDeclared com.google.auto.value:auto-value-annotat…
sonam-vend Dec 9, 2020
c9add73
Added jackson dependencies to suitable format
sonam-vend Dec 9, 2020
3b00f47
Fixed checker-qual dependency
sonam-vend Dec 9, 2020
c27ae5e
Merge branch 'master' of https://github.com/apache/beam into dev-beam
sonam-vend Dec 10, 2020
7868898
resolved flink 1.12 dependencies
sonam-vend Dec 14, 2020
5dd2771
resolving java precommit run
sonam-vend Dec 14, 2020
23a5609
corrected options3 typo
sonam-vend Dec 14, 2020
10cfaef
resolved spotless issues
sonam-vend Dec 14, 2020
7721512
resolved Run Python_PVR_Flink PreCommit
sonam-vend Dec 14, 2020
5068d90
upgraded kafka-clients version to 2.4.1
sonam-vend Dec 14, 2020
7d388fa
excluded kafka-schema-registry-client
sonam-vend Dec 15, 2020
3af3802
resolved check failures issues
sonam-vend Dec 15, 2020
d52d009
removed * dependencies
sonam-vend Dec 21, 2020
61cb207
downgraded xz version to 1.5
sonam-vend Dec 29, 2020
658f482
Merge branch 'master' into dev-beam
sonam-vend Dec 29, 2020
ab8fb61
fixed style violations
sonam-vend Dec 29, 2020
be64412
resolved dependencies
sonam-vend Dec 30, 2020
aa22d1b
Fixed code standards issues
sonam-vend Dec 31, 2020
9a773af
fix for precommits checks
sonam-vend Jan 1, 2021
20703a1
code cleaning
sonam-vend Jan 4, 2021
c207c6a
code format fixing
sonam-vend Jan 4, 2021
766e0eb
code clean
sonam-vend Jan 4, 2021
b47662e
fixed code format issues
sonam-vend Jan 4, 2021
7cc00b0
upstream with main repo
sonam-vend Jan 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 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 @@ -21,27 +21,26 @@ package org.apache.beam.gradle
import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
import groovy.json.JsonOutput
import groovy.json.JsonSlurper
import org.gradle.api.attributes.Category
import org.gradle.api.GradleException
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.Task
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.ProjectDependency
import org.gradle.api.attributes.Category
import org.gradle.api.file.FileCollection
import org.gradle.api.file.FileTree
import org.gradle.api.plugins.quality.Checkstyle
import org.gradle.api.publish.maven.MavenPublication
import org.gradle.api.tasks.PathSensitivity
import org.gradle.api.tasks.JavaExec
import org.gradle.api.tasks.Copy
import org.gradle.api.tasks.Delete
import org.gradle.api.tasks.Exec
import org.gradle.api.tasks.JavaExec
import org.gradle.api.tasks.bundling.Jar
import org.gradle.api.tasks.compile.JavaCompile
import org.gradle.api.tasks.javadoc.Javadoc
import org.gradle.api.tasks.testing.Test
import org.gradle.api.tasks.PathSensitive
import org.gradle.api.tasks.PathSensitivity
import org.gradle.testing.jacoco.tasks.JacocoReport

import java.util.concurrent.atomic.AtomicInteger
Expand Down Expand Up @@ -101,7 +100,7 @@ class BeamModulePlugin implements Plugin<Project> {
boolean checkerTooSlowOnTests = false

/** Controls whether the dependency analysis plugin is enabled. */
boolean enableStrictDependencies = false
boolean enableStrictDependencies = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

enabled dependency check via wfhartford plugin

Copy link
Member

Choose a reason for hiding this comment

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

Great. Once it is enabled we can delete the option, too.


/** Override the default "beam-" + `dash separated path` archivesBaseName. */
String archivesBaseName = null
Expand Down Expand Up @@ -850,7 +849,7 @@ class BeamModulePlugin implements Plugin<Project> {
// configurations because they are never required to be shaded or become a
// dependency of the output.
def compileOnlyAnnotationDeps = [
"com.google.auto.value:auto-value-annotations:1.7",
"com.google.auto.value:auto-value-annotations:1.7.2",
Copy link
Member

Choose a reason for hiding this comment

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

Around here, you can add these dependencies to permitUnusedDeclared so you don't have to add it to all modules.

Copy link
Member

Choose a reason for hiding this comment

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

You gave a thumbs up but did not add the lines here :-)

Here is what I mean: Right now you have this line in many build.gradle files:

permitUnusedDeclared "com.google.auto.value:auto-value-annotations:1.7.2"

Instead, move that line here. This way the build.gradle files are cleaner and do not have to have exclusions for things they do not control.

"com.google.auto.service:auto-service-annotations:1.0-rc6",
"com.google.j2objc:j2objc-annotations:1.3",
// These dependencies are needed to avoid error-prone warnings on package-info.java files,
Expand Down
34 changes: 1 addition & 33 deletions buildSrc/src/main/groovy/org/apache/beam/gradle/GrpcVendoring_1_26_0.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,40 +43,13 @@ class GrpcVendoring_1_26_0 {
static def npn_api_version = "1.1.1.v20141010"
static def jboss_marshalling_version = "1.4.11.Final"
static def jboss_modules_version = "1.1.0.Beta1"

/** Returns the list of compile time dependencies. */
static List<String> dependencies() {
return [
"com.google.guava:guava:$guava_version",
Copy link
Member

Choose a reason for hiding this comment

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

The change here concerns me - does it introduce linkage errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kennknowles ./gradlew analyzeDependencies is successfull. Apparently, there are no linkage errors.

Copy link
Member

Choose a reason for hiding this comment

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

It is not quite so simple. See https://cwiki.apache.org/confluence/display/BEAM/Dependency+Upgrades

Specifically,

./gradlew -Ppublishing -PjavaLinkageArtifactIds=beam-vendor-grpc-1_26_0  :checkJavaLinkage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we need to run this for all the artifacts?

"com.google.protobuf:protobuf-java:$protobuf_version",
"com.google.protobuf:protobuf-java-util:$protobuf_version",
"com.google.code.gson:gson:$gson_version",
"io.grpc:grpc-auth:$grpc_version",
"io.grpc:grpc-core:$grpc_version",
"io.grpc:grpc-context:$grpc_version",
"io.grpc:grpc-netty:$grpc_version",
"io.grpc:grpc-protobuf:$grpc_version",
"io.grpc:grpc-stub:$grpc_version",
"io.netty:netty-transport-native-epoll:$netty_version",
// tcnative version from https://github.com/grpc/grpc-java/blob/master/SECURITY.md#netty
"io.netty:netty-tcnative-boringssl-static:2.0.33.Final",
"com.google.auth:google-auth-library-credentials:$google_auth_version",
"io.grpc:grpc-testing:$grpc_version",
"com.google.api.grpc:proto-google-common-protos:$proto_google_common_protos_version",
"io.opencensus:opencensus-api:$opencensus_version",
"io.opencensus:opencensus-contrib-grpc-metrics:$opencensus_version",
"io.perfmark:perfmark-api:$perfmark_version",
"com.github.jponge:lzma-java:$lzma_java_version",
"com.google.protobuf.nano:protobuf-javanano:$protobuf_javanano_version",
"com.jcraft:jzlib:$jzlib_version",
"com.ning:compress-lzf:$compress_lzf_version",
"net.jpountz.lz4:lz4:$lz4_version",
"org.bouncycastle:bcpkix-jdk15on:$bouncycastle_version",
"org.bouncycastle:bcprov-jdk15on:$bouncycastle_version",
"org.eclipse.jetty.alpn:alpn-api:$alpn_api_version",
"org.eclipse.jetty.npn:npn-api:$npn_api_version",
"org.jboss.marshalling:jboss-marshalling:$jboss_marshalling_version",
"org.jboss.modules:jboss-modules:$jboss_modules_version"
"io.grpc:grpc-api:$grpc_version",
]
}

Expand All @@ -86,12 +59,7 @@ class GrpcVendoring_1_26_0 {
*/
static List<String> runtimeDependencies() {
return [
'com.google.errorprone:error_prone_annotations:2.3.3',
'commons-logging:commons-logging:1.2',
'org.apache.logging.log4j:log4j-api:2.6.2',
'org.slf4j:slf4j-api:1.7.30',
// TODO(BEAM-9288): Enable relocation for conscrypt
"org.conscrypt:conscrypt-openjdk-uber:$conscrypt_version"
]
}

Expand Down
6 changes: 5 additions & 1 deletion examples/java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ dependencies {
compile library.java.joda_time
compile library.java.proto_google_cloud_datastore_v1
compile library.java.slf4j_api
compile library.java.slf4j_jdk14
compile "com.google.api.grpc:proto-google-cloud-language-v1:1.81.4"
compile library.java.jsr305
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't add jsr305 as a compile dependency; it should be a compileOnly dependency. Also, since the transitive dependency on jsr305 is introduced by spotBugs, jsr305 should be included in compileOnlyAnnotationDeps. Including jsr305 in compileOnlyAnnotationDeps will remove the need to add it in every build file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for guidance here, resolved.

compile "com.google.oauth-client:google-oauth-client:1.31.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Use library.java.google_oauth_client instead.

permitUnusedDeclared "com.google.auto.value:auto-value-annotations:1.7.2"
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding dependencies to permitUnusedDeclared or permitTestUnusedDeclared causes the dependency analyzer to ignore cases where the dependencies are declared but not used.

Copy link
Member

Choose a reason for hiding this comment

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

We prefer to use references to library.auto_value_annotations, etc. You may have to add some.

Copy link
Contributor Author

@sonam-vend sonam-vend Nov 5, 2020 •
edited

Choose a reason for hiding this comment

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

@kennknowles modified almost all the newly added dependencies to the "library.auto_value_annotations" format now.

Copy link
Member

Choose a reason for hiding this comment

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

... but not this one?

I wonder if you forgot to push the new commit?

runtime project(path: ":runners:direct-java", configuration: "shadow")
testCompile project(":sdks:java:io:google-cloud-platform")
testCompile project(":sdks:java:extensions:ml")
Expand Down
12 changes: 5 additions & 7 deletions examples/kotlin/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,17 @@ dependencies {
compile library.java.google_api_services_pubsub
compile library.java.google_auth_library_credentials
compile library.java.google_auth_library_oauth2_http
compile library.java.google_cloud_datastore_v1_proto_client
compile library.java.google_http_client
compile library.java.joda_time
compile library.java.proto_google_cloud_datastore_v1
compile library.java.slf4j_api
compile library.java.slf4j_jdk14
compile "com.google.guava:guava:25.1-jre"
compile "org.jetbrains.kotlin:kotlin-stdlib:1.3.72"
compile "org.jetbrains:annotations:13.0"
runtime project(path: ":runners:direct-java", configuration: "shadow")
testCompile project(":sdks:java:io:google-cloud-platform")
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.junit
testCompile library.java.mockito_core
permitUnusedDeclared "com.google.auto.value:auto-value-annotations:1.7.2"
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"

// Add dependencies for the PreCommit configurations
// For each runner a project level dependency on the examples project.
Expand All @@ -87,7 +86,6 @@ dependencies {
sparkRunnerPreCommit project(":sdks:java:io:hadoop-file-system")
sparkRunnerPreCommit library.java.spark_streaming
sparkRunnerPreCommit library.java.spark_core
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8"
}

/*
Expand Down
3 changes: 3 additions & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,6 @@ javaVersion=1.8

docker_image_default_repo_root=apache
docker_image_default_repo_prefix=beam_

org.gradle.daemon=true
file.encoding=utf-8
3 changes: 3 additions & 0 deletions model/fn-execution/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ ext.summary = "Portable definitions for execution user-defined functions."
dependencies {
// We purposely depend on the unshaded classes for protobuf compilation and
// export the shaded variant as the actual runtime dependency.
compile "com.google.guava:guava:28.1-android"
compile project(path: ":model:pipeline", configuration: "unshaded")
runtime project(path: ":model:pipeline", configuration: "shadow")
permitUnusedDeclared library.java.vendored_grpc_1_26_0
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
}
3 changes: 3 additions & 0 deletions model/job-management/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,7 @@ dependencies {
// export the shaded variant as the actual runtime dependency.
compile project(path: ":model:pipeline", configuration: "unshaded")
runtime project(path: ":model:pipeline", configuration: "shadow")
permitUnusedDeclared library.java.vendored_grpc_1_26_0
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
compile "com.google.guava:guava:28.1-android"
Copy link
Member

Choose a reason for hiding this comment

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

This is interesting. If generated protos depend on this version of Guava, we may need to do something about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kennknowles something like, do we need to need to declare it any central location?

}
6 changes: 6 additions & 0 deletions model/pipeline/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,9 @@ configurations {
artifacts {
unshaded jar
}


dependencies {
permitUnusedDeclared library.java.vendored_grpc_1_26_0
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
}
1 change: 0 additions & 1 deletion release/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ repositories {

dependencies {
compile library.groovy.groovy_all
compile 'commons-cli:commons-cli:1.2'
}

task runJavaExamplesValidationTask {
Expand Down
5 changes: 3 additions & 2 deletions runners/core-construction-java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,9 @@ dependencies {
compile library.java.jackson_databind
compile library.java.joda_time
compile library.java.slf4j_api
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
compile "com.fasterxml.jackson.core:jackson-annotations:2.10.2"
sonam-vend marked this conversation as resolved.
Show resolved Hide resolved
compile library.java.jsr305
compile library.java.avro
testCompile library.java.junit
testCompile library.java.mockito_core
testCompile library.java.jackson_annotations
Expand Down
10 changes: 5 additions & 5 deletions runners/core-java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,17 @@ test {
dependencies {
compile project(path: ":model:pipeline", configuration: "shadow")
compile project(path: ":sdks:java:core", configuration: "shadow")
compile project(path: ":model:fn-execution", configuration: "shadow")
compile project(path: ":model:job-management:", configuration: "shadow")
compile project(":runners:core-construction-java")
compile project(":sdks:java:fn-execution")
permitUsedUndeclared project(":sdks:java:fn-execution")
sonam-vend marked this conversation as resolved.
Show resolved Hide resolved
compile library.java.vendored_guava_26_0_jre
compile library.java.joda_time
compile library.java.jsr305
compile library.java.vendored_grpc_1_26_0
compile library.java.slf4j_api
testCompile project(path: ":sdks:java:core", configuration: "shadowTest")
testCompile library.java.junit
testCompile library.java.hamcrest_core
testCompile library.java.hamcrest_library
testCompile library.java.mockito_core
testCompile library.java.slf4j_api
testCompile library.java.jackson_dataformat_yaml
testRuntimeOnly library.java.slf4j_simple
}
14 changes: 7 additions & 7 deletions runners/direct-java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ def dependOnProjects = [":runners:core-construction-java",
":runners:core-java",
":runners:local-java",
":runners:java-fn-execution",
":sdks:java:fn-execution"]
":sdks:java:fn-execution"
]

applyJavaNature(
automaticModuleName: 'org.apache.beam.runners.direct',
Expand Down Expand Up @@ -72,17 +73,16 @@ dependencies {
shadow library.java.vendored_grpc_1_26_0
shadow library.java.joda_time
shadow library.java.slf4j_api
shadow library.java.args4j
permitUnusedDeclared library.java.vendored_grpc_1_26_0
permitUnusedDeclared project(":runners:java-fn-execution")
permitUnusedDeclared project(":sdks:java:fn-execution")
Copy link
Member

Choose a reason for hiding this comment

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

These seem wrong. The direct runner should either use these or not depend on them.

Copy link
Contributor Author

@sonam-vend sonam-vend Nov 5, 2020 •
edited

Choose a reason for hiding this comment

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

If we completely remove these project-project dependencies, Java Wordcount Direct Runner check is failing.

Copy link
Member

Choose a reason for hiding this comment

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

I see. So these are really testRuntime dependencies. If we had a shadowTestRuntime configuration we would exclude it from checking I guess. Since we created the shadowTest configuration I don't think there is a shadowTestRuntime configuration. Please add a comment explaining these exclusions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kennknowles , these dependency are indirectly being declared i.e there is transitive dependency for these projects. But the module does not require them, so we need to declare them as permitUnusedDeclared .

Copy link
Member

Choose a reason for hiding this comment

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

Transitive dependencies are automatically included. So there is something else that makes these transitive dependencies not work. If the compile step succeeds but the test step does not succeed, that means it is a runtime dependency of the test. I think the only thing we can do is to add the exclusion, for now. Please add a comment that these are runtime dependencies of the tests.

compile "com.fasterxml.jackson.core:jackson-databind:2.10.2"
compile library.java.jsr305
provided library.java.hamcrest_core
provided library.java.junit
shadowTest project(path: ":sdks:java:core", configuration: "shadowTest")
shadowTest project(path: ":runners:core-java", configuration: "testRuntime")
shadowTest library.java.slf4j_jdk14
shadowTest library.java.mockito_core
shadowTest library.java.stax2_api
shadowTest library.java.woodstox_core_asl
shadowTest library.java.google_cloud_dataflow_java_proto_library_all
shadowTest library.java.jackson_dataformat_yaml
needsRunner project(path: ":runners:core-construction-java", configuration: "testRuntime")
needsRunner project(path: ":runners:core-java", configuration: "testRuntime")
needsRunner project(path: ":sdks:java:core", configuration: "shadowTest")
Expand Down
3 changes: 2 additions & 1 deletion runners/extensions-java/metrics/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,11 @@ description = "Apache Beam :: Runners :: Extensions Java :: Metrics"
ext.summary = "Beam Runners Extensions Metrics provides implementations of runners core metrics APIs."

dependencies {
compile library.java.vendored_guava_26_0_jre
compile project(path: ":sdks:java:core", configuration: "shadow")
compile library.java.jackson_databind
compile library.java.jackson_datatype_joda
compile library.java.jackson_core
compile library.java.joda_time
testCompile library.java.joda_time
testCompile library.java.junit
}
4 changes: 4 additions & 0 deletions runners/flink/1.10/job-server/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ project.ext {

// Load the main build script which contains all build logic.
apply from: "$basePath/flink_job_server.gradle"

dependencies {
permitUnusedDeclared project(":runners:flink:1.10")
Copy link
Member

Choose a reason for hiding this comment

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

This seems strange. The Flink 1.10 job server should definitely actually use :runners:flink:1.10. And same for the other versions. Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

project(":runners:flink:1.10") is transitively being declared, though we do not need it here. If I dont add

dependencies {
permitUnusedDeclared project(":runners:flink:1.10")
}

I have been getting,
What went wrong:
Execution failed for task ':runners:flink:1.10:job-server:analyzeClassesDependencies'.

Dependency analysis found issues.
unusedDeclaredArtifacts:

  • beam.runners.flink:1.10:2.27.0-SNAPSHOT@jar

same is true for other versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment explaining that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:runners:flink:1.10:job-server:analyzeClassesDependencies task fails if we don't add permitUnusedDeclared project(":runners:flink:1.10")

Task :runners:flink:1.10:job-server:analyzeClassesDependencies FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':runners:flink:1.10:job-server:analyzeClassesDependencies'.
> Dependency analysis found issues.
  unusedDeclaredArtifacts: 
   - beam.runners.flink:1.10:2.28.0-SNAPSHOT@jar

As it is transitively being declared, but we do not need it here. Thats why it is important to declare it permitUnsedDeclared.

@ibzib

}
5 changes: 5 additions & 0 deletions runners/flink/1.11/job-server/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ project.ext {

// Load the main build script which contains all build logic.
apply from: "$basePath/flink_job_server.gradle"


dependencies {
permitUnusedDeclared project(":runners:flink:1.11")
}
1 change: 0 additions & 1 deletion runners/flink/1.8/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/

def basePath = '..'

sonam-vend marked this conversation as resolved.
Show resolved Hide resolved
/* All properties required for loading the Flink build script */
project.ext {
// Set the version of all Flink-related dependencies here.
Expand Down
4 changes: 4 additions & 0 deletions runners/flink/1.8/job-server/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ project.ext {

// Load the main build script which contains all build logic.
apply from: "$basePath/flink_job_server.gradle"

dependencies {
permitUnusedDeclared project(":runners:flink:1.8")
}
1 change: 1 addition & 0 deletions runners/flink/1.9/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ project.ext {

// Load the main build script which contains all build logic.
apply from: "$basePath/flink_runner.gradle"

Copy link
Contributor

Choose a reason for hiding this comment

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

stray change - please revert

4 changes: 4 additions & 0 deletions runners/flink/1.9/job-server/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ project.ext {

// Load the main build script which contains all build logic.
apply from: "$basePath/flink_job_server.gradle"

dependencies {
permitUnusedDeclared project(":runners:flink:1.9")
}
14 changes: 14 additions & 0 deletions runners/flink/flink_runner.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ description = "Apache Beam :: Runners :: Flink $flink_version"
evaluationDependsOn(":sdks:java:core")
evaluationDependsOn(":runners:core-java")

def checker_qual_version = "2.0.0"
/*
* Copy & merge source overrides into build directory.
*/
Expand Down Expand Up @@ -169,6 +170,19 @@ dependencies {
validatesRunner project(path: ":runners:core-java", configuration: "testRuntime")
validatesRunner project(project.path)
miniCluster "org.apache.flink:flink-runtime-web_2.11:$flink_version"
compile project(path: ":model:fn-execution", configuration: "shadow")
compile project(path: ":model:pipeline", configuration: "shadow")
compile project(path: ":model:job-management:", configuration: "shadow")
compile project(":sdks:java:fn-execution")
compile project(path: ":vendor:sdks-java-extensions-protobuf", configuration: "shadow")
permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
permitUnusedDeclared library.java.jackson_annotations
compile library.java.jackson_databind
compile library.java.jsr305
compile "org.apache.flink:flink-annotations:$flink_version"
compile "org.apache.flink:flink-optimizer_2.11:$flink_version"
permitUnusedDeclared "org.apache.flink:flink-clients_2.11:$flink_version"
compile "org.checkerframework:checker-qual:$checker_qual_version"
}

class ValidatesRunnerConfig {
Expand Down
2 changes: 2 additions & 0 deletions runners/flink/job-server/flink_job_server.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ dependencies {
runtimeOnly project(":sdks:java:io:google-cloud-platform")
// SqlTransform
runtimeOnly project(":sdks:java:extensions:sql:expansion-service")
permitUnusedDeclared project(":runners:flink:1.8")
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't you already do this in runners/flink/1.8/job-server/build.gradle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed it from here.

permitUnusedDeclared "org.checkerframework:checker-qual:3.7.0"
}

// NOTE: runShadow must be used in order to run the job server. The standard run
Expand Down
16 changes: 11 additions & 5 deletions runners/google-cloud-dataflow-java/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,26 @@ dependencies {
compile library.java.google_api_client
compile library.java.google_api_services_clouddebugger
compile library.java.google_api_services_dataflow
compile library.java.google_api_services_storage
testImplementation 'com.google.apis:google-api-services-storage:v1-rev20200611-1.30.10'
compile library.java.google_auth_library_credentials
compile library.java.google_auth_library_oauth2_http
compile library.java.google_http_client
compile library.java.google_http_client_jackson2
testImplementation 'com.google.http-client:google-http-client-jackson2:1.34.0'
compile library.java.jackson_annotations
compile library.java.jackson_core
compile library.java.jackson_databind
compile library.java.joda_time
compile library.java.slf4j_api
compile library.java.vendored_grpc_1_26_0
testCompile library.java.hamcrest_core
compile library.java.jsr305
compile "org.checkerframework:checker-qual:2.0.0"
compile "org.hamcrest:hamcrest:2.1"
compile "com.google.auth:google-auth-library-credentials:0.19.0"
permitUnusedDeclared "com.google.guava:guava:25.1-jre"
permitUnusedDeclared "io.opencensus:opencensus-api:0.24.0"
compile "org.checkerframework:checker-qual:2.0.0"
permitUnusedDeclared "org.conscrypt:conscrypt-openjdk-uber:2.5.1"
permitUnusedDeclared "com.google.api.grpc:proto-google-common-protos:1.17.0"
testCompile library.java.guava_testlib
testCompile library.java.junit
testCompile project(path: ":sdks:java:io:google-cloud-platform", configuration: "testRuntime")
Expand All @@ -99,8 +107,6 @@ dependencies {
testCompile library.java.google_cloud_dataflow_java_proto_library_all
testCompile library.java.jackson_dataformat_yaml
testCompile library.java.mockito_core
testCompile library.java.proto_google_cloud_datastore_v1
testCompile library.java.slf4j_jdk14
validatesRunner project(path: ":sdks:java:core", configuration: "shadowTest")
validatesRunner project(project.path)
validatesRunner project(path: project.path, configuration: "testRuntime")
Expand Down