all: let interop test use shaded dependency correctly take 2#6792
all: let interop test use shaded dependency correctly take 2#6792dapengzhang0 merged 3 commits intogrpc:masterfrom
Conversation
…grpc#6780)"" This reverts commit 1df7d7e.
3683972 to
51a81ac
Compare
alts/build.gradle
Outdated
| // Use grpc-netty-shaded for transitive dependency. | ||
| compileOnly project(':grpc-netty') | ||
| shadow project(path: ':grpc-netty-shaded', configuration: 'shadow') | ||
| shadow configurations.compile.getDependencies() |
There was a problem hiding this comment.
This is doubly suspicious. It is strange to need to copy the dependencies, but wouldn't this also include compile-only dependencies?
There was a problem hiding this comment.
My understanding is:
DependencyHandler.project(path : 'grpc-alts', configuration: 'shadow')returns aDependencythat includes the output of:grpc-alts:shadowJarplus all dependencies added (manually) intoshadowconfiguration.
However,
-
The output of
:grpc-alts:shadowJardoes not depend on anything in the shadow configuration. -
The output pom.xml of maven publication does not include anything in the shadow configuration unless you modify
pom.withXml {}manually.
xds/build.gradle
Outdated
| // Use grpc-netty-shaded for transitive dependency. | ||
| compileOnly project(':grpc-netty') | ||
| shadow project(path: ':grpc-netty-shaded', configuration: 'shadow') | ||
| shadow configurations.compile.getDependencies() |
There was a problem hiding this comment.
This brings in library.pgv. It needs to be excluded. (maybe with minus() like used in grpc-all?)
I sort of think excluding may be better than using compileOnly for grpc-netty, since then both 'compile' configuration and 'shadow' configuration are functional outside of this project. It also seems to be a bit less likely to have surprises. But I can accept what's been done here.
ejona86
left a comment
There was a problem hiding this comment.
Looks good. One comment of something to clean up, but I'm fine if we submit this as-is and do it afterward (and it wouldn't need to be backported)
|
|
||
| compileOnly libraries.javax_annotation | ||
| compileOnly libraries.javax_annotation, | ||
| // At runtime use the epoll included in grpc-netty-shaded |
There was a problem hiding this comment.
Technically it would probably be best to do the same minus() thing here, but that can be a follow-up.
This include two commits: