android-interop-testing: include android interop testing in main build #6829

voidzcy · 2020-03-13T20:55:39Z

~~TODO: temporarily disabled code shrinking for release buildType, need to fix proguard rules to before being able to do code shrinking.~~
The problem was caused by pulling in org.apache.httpcomponents.htttpcore by google auth2 lib. According to Proguard:

In Android development, sloppy libraries may contain duplicates of classes that are already present in the Android run-time (notably org.w3c.dom, org.xml.sax, org.xmlpull.v1, org.apache.commons.logging.Log, org.apache.http, and org.json). You must remove these classes from your libraries, since they are possibly inconsistent, and the run-time libraries would get precedence anyway.

~~TODO: there are tons of errorprone warnings and Proguard notes. Although they do not cause build fail, it would be better to fix/suppress them.~~
Proguard notes seem to be fine. There are some compiler warnings coming from javalite generated code, nothing can be done to fix that. A couple of errorprone warnings left, but unclear if it is necessary to fix them.

After the change, generated code will be checked in. So this PR contains generated code. Reviewers should only need to review .gradle, .md files, proguard-rules.pro, InteropTask.java and TesterActivity.java.

…ode minification due to proguard issue.

creamsoup

not fully reviewed, sending what i have now
i ran ./gradlew :grpc-android-interop-testing:{test,checkStyleMain} those tasks don't work.

RELEASING.md

voidzcy · 2020-03-16T20:58:42Z

not fully reviewed, sending what i have now
i ran ./gradlew :grpc-android-interop-testing:{test,checkStyleMain} those tasks don't work.

You should be able to run ./gradlew :grpc-android-interop-testing:test. checkStyle tasks do not exist in android modules (same for grpc-android and grpc-cronet). They are only added to modules with java plugin. We could add checkStyle tasks (if we are able to) for android modules either manually in each modules buildscript (like what we are currently doing for javadoc tasks) or find a solution to configure globally in grpc's main buildscript. Anyway, checkStyle issue would be addressed separately, together with grpc-android and grpc-cronet.

creamsoup · 2020-03-16T21:17:50Z

You should be able to run ./gradlew :grpc-android-interop-testing:test.

yes, it turns out that it was my local environmental issue.

build.gradle

creamsoup

LGTM

dapengzhang0 · 2020-03-17T22:39:39Z

@voidzcy My local run of ./gradlew grpc-android-interop-testing:build -PskipAndroid=false fails as follows

> Task :grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease
Request to incrementing alive workforce from 0. Current workforce (dead or alive) 0
thread-pool size=4

Note: the configuration refers to the unknown field 'com.google.android.gms.common.api.internal.BasePendingResult$ReleasableResultGuardian mResultGuardian' in class 'com.google.android.gms.common.api.internal.BasePendingResult'
Note: the configuration keeps the entry point 'com.google.android.gms.common.api.internal.LifecycleCallback { com.google.android.gms.common.api.internal.LifecycleFragment getChimeraLifecycleFragmentImpl(com.google.android.gms.common.api.internal.LifecycleActivity); }', but not the descriptor class 'com.google.android.gms.common.api.internal.LifecycleActivity'
Note: the configuration keeps the entry point 'com.google.android.gms.common.images.internal.LoadingImageView { void setOnImageLoadedListener(com.google.android.gms.common.images.ImageManager$OnImageLoadedListener); }', but not the descriptor class 'com.google.android.gms.common.images.ImageManager$OnImageLoadedListener'
Note: the configuration keeps the entry point 'com.google.android.gms.common.images.internal.LoadingImageView { void setClipPathProvider(com.google.android.gms.common.images.internal.LoadingImageView$ClipPathProvider); }', but not the descriptor class 'com.google.android.gms.common.images.internal.LoadingImageView$ClipPathProvider'
Note: there were 1 references to unknown classes.
      You should check your configuration for typos.
      (http://proguard.sourceforge.net/manual/troubleshooting.html#unknownclass)
Note: there were 1 references to unknown class members.
      You should check your configuration for typos.
Note: there were 3 unkept descriptor classes in kept class members.
      You should consider explicitly keeping the mentioned classes
      (using '-keep').
      (http://proguard.sourceforge.net/manual/troubleshooting.html#descriptorclass)
Note: there were 26 unresolved dynamic references to classes or interfaces.
      You should check if you need to specify additional program jars.
      (http://proguard.sourceforge.net/manual/troubleshooting.html#dynamicalclass)
Warning: there were 3 unresolved references to library class members.
         You probably need to update the library versions.
         (http://proguard.sourceforge.net/manual/troubleshooting.html#unresolvedlibraryclassmember)
Warning: Exception while processing task java.io.IOException: Please correct the above warnings first.
Thread(Tasks limiter_1): destruction

> Task :grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease'.
> Job failed, see logs for details

voidzcy · 2020-03-17T22:44:27Z

@voidzcy My local run of ./gradlew grpc-android-interop-testing:build -PskipAndroid=false fails as follows

> Task :grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease
Request to incrementing alive workforce from 0. Current workforce (dead or alive) 0
thread-pool size=4

Note: the configuration refers to the unknown field 'com.google.android.gms.common.api.internal.BasePendingResult$ReleasableResultGuardian mResultGuardian' in class 'com.google.android.gms.common.api.internal.BasePendingResult'
Note: the configuration keeps the entry point 'com.google.android.gms.common.api.internal.LifecycleCallback { com.google.android.gms.common.api.internal.LifecycleFragment getChimeraLifecycleFragmentImpl(com.google.android.gms.common.api.internal.LifecycleActivity); }', but not the descriptor class 'com.google.android.gms.common.api.internal.LifecycleActivity'
Note: the configuration keeps the entry point 'com.google.android.gms.common.images.internal.LoadingImageView { void setOnImageLoadedListener(com.google.android.gms.common.images.ImageManager$OnImageLoadedListener); }', but not the descriptor class 'com.google.android.gms.common.images.ImageManager$OnImageLoadedListener'
Note: the configuration keeps the entry point 'com.google.android.gms.common.images.internal.LoadingImageView { void setClipPathProvider(com.google.android.gms.common.images.internal.LoadingImageView$ClipPathProvider); }', but not the descriptor class 'com.google.android.gms.common.images.internal.LoadingImageView$ClipPathProvider'
Note: there were 1 references to unknown classes.
      You should check your configuration for typos.
      (http://proguard.sourceforge.net/manual/troubleshooting.html#unknownclass)
Note: there were 1 references to unknown class members.
      You should check your configuration for typos.
Note: there were 3 unkept descriptor classes in kept class members.
      You should consider explicitly keeping the mentioned classes
      (using '-keep').
      (http://proguard.sourceforge.net/manual/troubleshooting.html#descriptorclass)
Note: there were 26 unresolved dynamic references to classes or interfaces.
      You should check if you need to specify additional program jars.
      (http://proguard.sourceforge.net/manual/troubleshooting.html#dynamicalclass)
Warning: there were 3 unresolved references to library class members.
         You probably need to update the library versions.
         (http://proguard.sourceforge.net/manual/troubleshooting.html#unresolvedlibraryclassmember)
Warning: Exception while processing task java.io.IOException: Please correct the above warnings first.
Thread(Tasks limiter_1): destruction

> Task :grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':grpc-android-interop-testing:transformClassesAndResourcesWithProguardForRelease'.
> Job failed, see logs for details

Did you pull all commits in this PR? That's caused by ProGuard and I should have fixed it in commit 61a3d75

dapengzhang0 · 2020-03-17T23:36:33Z

Yes, I pulled the PR up to f7c160c . However, I tested build in master and it failed similarly. I figured out the actual failure is

Warning: io.grpc.internal.CompositeReadableBuffer$4: can't find referenced method 'java.nio.ByteBuffer limit(int)' in library class java.nio.ByteBuffer
Warning: io.grpc.internal.ReadableBuffers$ByteReadableBufferWrapper: can't find referenced method 'java.nio.ByteBuffer position(int)' in library class java.nio.ByteBuffer
Warning: io.grpc.internal.ReadableBuffers$ByteReadableBufferWrapper: can't find referenced method 'java.nio.ByteBuffer limit(int)' in library class java.nio.ByteBuffer

I think it is because I was using jdk 11. I believe it's not a problem from your change.

voidzcy · 2020-03-17T23:38:38Z

Yes, I pulled the PR up to f7c160c . However, I tested build in master and it failed similarly. I figured out the actual failure is
Warning: io.grpc.internal.CompositeReadableBuffer$4: can't find referenced method 'java.nio.ByteBuffer limit(int)' in library class java.nio.ByteBuffer
Warning: io.grpc.internal.ReadableBuffers$ByteReadableBufferWrapper: can't find referenced method 'java.nio.ByteBuffer position(int)' in library class java.nio.ByteBuffer
Warning: io.grpc.internal.ReadableBuffers$ByteReadableBufferWrapper: can't find referenced method 'java.nio.ByteBuffer limit(int)' in library class java.nio.ByteBuffer
I think it is because I was using jdk 11. I believe it's not a problem from your change.

Yes yes. You need to recompile everything with jdk 8.

ericgribkoff · 2020-03-18T18:08:13Z

Yes yes. You need to recompile everything with jdk 8.

I'm a little confused about the intended support for JDK versions here. Currently I can build grpc-java at master with JDK 11. With this PR, I need to use JDK 8 (JDK 11 fails) and it sounds like this is intentional. I'm not sure why this would be a desired change, but if it is, should this be explained and documented somewhere?

dapengzhang0 · 2020-03-18T18:20:07Z

Java 9 made an ABI incompatible change in java.nio.ByteBuffer See apache/felix#114

I'm a little confused about the intended support for JDK versions here. Currently I can build grpc-java at master with JDK 11. With this PR, I need to use JDK 8 (JDK 11 fails) and it sounds like this is intentional. I'm not sure why this would be a desired change, but if it is, should this be explained and documented somewhere?

voidzcy · 2020-03-18T18:21:56Z

Yes yes. You need to recompile everything with jdk 8.

I'm a little confused about the intended support for JDK versions here. Currently I can build grpc-java at master with JDK 11. With this PR, I need to use JDK 8 (JDK 11 fails) and it sounds like this is intentional. I'm not sure why this would be a desired change, but if it is, should this be explained and documented somewhere?

I suspect this is due to Android. You cannot use JDK 11 to build some part of the source (i.e., grpc-core, in which java.nio.ByteBuffer is used) and use Android SDK to build grpc-android-interop-testing which depends on grpc-core. This causes the method not found problem.

Do you know if there is any good solution to mitigate this?

ericgribkoff · 2020-03-18T18:46:01Z

My concern was just that a user who tries to build the grpc-java project with JDK 11 will start getting a build error, the cause of which is not very obvious (at least to me, I didn't immediately think JDK incompatibility). Building with skipAndroid=true works fine with JDK 11, so maybe that's fine for most people; not sure. But that would mean that my normal workflow would have to change to using JDK 8 or keep using JDK 11 but skipping Android builds whenever I build the project. Both of those are probably fine, but this doesn't seem like it will be at all obvious to newcomers to the project.

voidzcy · 2020-03-18T19:14:25Z

My concern was just that a user who tries to build the grpc-java project with JDK 11 will start getting a build error, the cause of which is not very obvious (at least to me, I didn't immediately think JDK incompatibility). Building with skipAndroid=true works fine with JDK 11, so maybe that's fine for most people; not sure. But that would mean that my normal workflow would have to change to using JDK 8 or keep using JDK 11 but skipping Android builds whenever I build the project. Both of those are probably fine, but this doesn't seem like it will be at all obvious to newcomers to the project.

Alright. This is actually an issue when ProGuard is trying to remove unused code, not really bytecode incompatibility (we have sourceCompatibility = 1.7 in the main build file, so real method invocation should be fine). In this case, we could simply add -dontwarn java.nio.ByteBuffer to grpc-android-interop-testing's ProGuard rules to ignore that warning. I tried this locally and it seems to work for JDK 11.

…ile against JDK 9+.

voidzcy · 2020-03-18T19:22:22Z

Edited ProGuard rules, build (skipAndroid=false) should succeed for JDK 9+.

dapengzhang0 · 2020-03-18T19:23:39Z

Good point. I'm trying to fix this in general in #6839

My concern was just that a user who tries to build the grpc-java project with JDK 11 will start getting a build error, the cause of which is not very obvious (at least to me, I didn't immediately think JDK incompatibility). Building with skipAndroid=true works fine with JDK 11, so maybe that's fine for most people; not sure. But that would mean that my normal workflow would have to change to using JDK 8 or keep using JDK 11 but skipping Android builds whenever I build the project. Both of those are probably fine, but this doesn't seem like it will be at all obvious to newcomers to the project.

build.gradle

dapengzhang0

LGTM

android-interop-testing/proguard-rules.pro

…hen compile against JDK 9+." This reverts commit 46b407f.

…roid_interop_testing_into_main_build

voidzcy added 6 commits March 13, 2020 13:07

Include android-interop-testing in main buildscript.

fc656b1

Flatten android-interop-testing project structure.

c94de96

Add android application plugin in the main build's configuration.

613e6f0

Clean up android-interop-testing's buildscript. Temporarily disable c…

3497fa7

…ode minification due to proguard issue.

Clean up TODO in README.

2116297

Completely exclude apache.http dependencies.

61a3d75

voidzcy marked this pull request as ready for review March 16, 2020 09:17

voidzcy added 2 commits March 16, 2020 10:47

Remove android/cronet/android-interop-testing from RELEASING.md

d5e5855

Fix small errorprone issues.

c9956a2

voidzcy force-pushed the build/add_android_interop_testing_into_main_build branch from a6b2739 to c9956a2 Compare March 16, 2020 18:49

voidzcy requested review from creamsoup, ericgribkoff, ejona86 and dapengzhang0 March 16, 2020 19:00

creamsoup reviewed Mar 16, 2020

View reviewed changes

RELEASING.md Show resolved Hide resolved

Add checkStyle tasks.

f7c160c

creamsoup reviewed Mar 17, 2020

View reviewed changes

build.gradle Show resolved Hide resolved

creamsoup approved these changes Mar 17, 2020

View reviewed changes

Supress ProGuard warning for java.nio.ByteBuffer limit(int) when comp…

46b407f

…ile against JDK 9+.

voidzcy mentioned this pull request Mar 18, 2020

core: fix java.nio.ByteBuffer Java 9+ incompatible usage #6839

Merged

ericgribkoff approved these changes Mar 18, 2020

View reviewed changes

dapengzhang0 reviewed Mar 18, 2020

View reviewed changes

build.gradle Show resolved Hide resolved

dapengzhang0 approved these changes Mar 18, 2020

View reviewed changes

android-interop-testing/proguard-rules.pro Outdated Show resolved Hide resolved

voidzcy added 2 commits March 18, 2020 16:19

Revert "Supress ProGuard warning for java.nio.ByteBuffer limit(int) w…

ddcbf2b

…hen compile against JDK 9+." This reverts commit 46b407f.

Merge branch 'master' of github.com:grpc/grpc-java into build/add_and…

afeb153

…roid_interop_testing_into_main_build

voidzcy merged commit d537ade into grpc:master Mar 19, 2020

This was referenced Mar 20, 2020

android-interop-testing: set minSdkVersion to 14 in manifest #6846

Merged

android-interop-testing spews lots of warnings #6866

Closed

lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

android-interop-testing: include android interop testing in main build #6829

android-interop-testing: include android interop testing in main build #6829

voidzcy commented Mar 13, 2020 •

edited

creamsoup left a comment

voidzcy commented Mar 16, 2020 •

edited

creamsoup commented Mar 16, 2020

creamsoup left a comment

dapengzhang0 commented Mar 17, 2020 •

edited

voidzcy commented Mar 17, 2020

dapengzhang0 commented Mar 17, 2020 •

edited

voidzcy commented Mar 17, 2020

ericgribkoff commented Mar 18, 2020

dapengzhang0 commented Mar 18, 2020

voidzcy commented Mar 18, 2020 •

edited

ericgribkoff commented Mar 18, 2020

voidzcy commented Mar 18, 2020

voidzcy commented Mar 18, 2020

dapengzhang0 commented Mar 18, 2020

dapengzhang0 left a comment

android-interop-testing: include android interop testing in main build #6829

android-interop-testing: include android interop testing in main build #6829

Conversation

voidzcy commented Mar 13, 2020 • edited

creamsoup left a comment

Choose a reason for hiding this comment

voidzcy commented Mar 16, 2020 • edited

creamsoup commented Mar 16, 2020

creamsoup left a comment

Choose a reason for hiding this comment

dapengzhang0 commented Mar 17, 2020 • edited

voidzcy commented Mar 17, 2020

dapengzhang0 commented Mar 17, 2020 • edited

voidzcy commented Mar 17, 2020

ericgribkoff commented Mar 18, 2020

dapengzhang0 commented Mar 18, 2020

voidzcy commented Mar 18, 2020 • edited

ericgribkoff commented Mar 18, 2020

voidzcy commented Mar 18, 2020

voidzcy commented Mar 18, 2020

dapengzhang0 commented Mar 18, 2020

dapengzhang0 left a comment

Choose a reason for hiding this comment

voidzcy commented Mar 13, 2020 •

edited

voidzcy commented Mar 16, 2020 •

edited

dapengzhang0 commented Mar 17, 2020 •

edited

dapengzhang0 commented Mar 17, 2020 •

edited

voidzcy commented Mar 18, 2020 •

edited