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.

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

avoiding more NoSuchMethod exceptions #6671

Merged
merged 1 commit into from
Jun 3, 2021
Merged

Conversation

krojew
Copy link
Contributor

@krojew krojew commented May 27, 2021

refs #6657

@github-actions github-actions bot added the java label May 27, 2021
@krojew
Copy link
Contributor Author

krojew commented May 27, 2021

@aardappel after rebuilding my test Android project from scratch, more methods started to fail. Smells like 2.0.2...

@aardappel
Copy link
Collaborator

Ugh.. I guess we have no easy way of testing this / adding to the CI? How do we avoid this in the future? I can't be making releases every time we forget this in Java code.

Do you have confidence this is the last of it?

Also, please update the version number in all the xml files, so it is ready to release.

@krojew
Copy link
Contributor Author

krojew commented May 27, 2021

To be honest, I don't know how to automate it. I tried to manually look for calls returning the subclass. Right now, everything seems to work, but it looked like this before. Might be a good idea for another pair of eyes to search for such problematic calls.

@aardappel
Copy link
Collaborator

@paulovap ?

@paulovap
Copy link
Collaborator

paulovap commented May 31, 2021

@paulovap ?

Sorry for the late reply, I was on vacation. I never face this problem before, but on Android we usually target Java 8 API.

@krojew if I understood it correct it only happens if you target Java 9 API? Is that it?

Yes, we can automate. Just run a unit-test against an Android Emulator. I haven't check if we have android specific tests, but this could be a good reason to start.

@krojew
Copy link
Contributor Author

krojew commented May 31, 2021

I really can't tell what exactly triggers this error. I'm targeting Java 8, which in theory, shouldn't be a problem. I know it started to appear after Android Studio update (which coincided with gradle update). That's why I'm not sure what would be a repeatable scenario for an automated test. Maybe any simple Android project, maybe some specific combination of settings.

@paulovap
Copy link
Collaborator

I really can't tell what exactly triggers this error. I'm targeting Java 8, which in theory, shouldn't be a problem. I know it started to appear after Android Studio update (which coincided with gradle update). That's why I'm not sure what would be a repeatable scenario for an automated test. Maybe any simple Android project, maybe some specific combination of settings.

Can you leave your gradle, android, etc versions and basic project config in a comment so we can try to reproduce later?

@krojew
Copy link
Contributor Author

krojew commented May 31, 2021

Android Studio: 4.2.1
gradle: 6.7.1
AS gradle plugin: 4.2.1
Java: 8
AndroidX: true
buildToolsVersion "30.0.2"
compileSdkVersion 30
minSdkVersion 26
targetSdkVersion 30

@krojew
Copy link
Contributor Author

krojew commented Jun 3, 2021

Can this be merged?

@aardappel aardappel merged commit 752c7b5 into google:master Jun 3, 2021
@krojew krojew deleted the fix-6657 branch June 14, 2021 05:10
@krojew
Copy link
Contributor Author

krojew commented Jun 15, 2021

@aardappel @paulovap will there be another patch release?

@aardappel
Copy link
Collaborator

release seems to have succeeded..

@aardappel
Copy link
Collaborator

https://stackoverflow.com/questions/68302798/is-there-a-way-to-use-flatc-generated-classes-within-a-java-8-runtime

The user seems to be claiming that the 2.0.2 package from Maven doesn't work for them because it was built with Java 9, even though it has all @krojew's fixes in it? I am confused how that works..

@krojew
Copy link
Contributor Author

krojew commented Jul 8, 2021

I took a look at the code from the error and there is an explicit cast there: https://github.com/google/flatbuffers/blob/master/java/com/google/flatbuffers/FlatBufferBuilder.java#L605

Maybe he's not using the latest package.

@bjornharrtell
Copy link
Collaborator

I suspect I'm also seeing this issue and think that perhaps jetty/jetty.project#3244 provides a lead on how to fix it.

@bjornharrtell
Copy link
Collaborator

Created follow up issue #6763 and PR #6764.

paulovap added a commit to paulovap/flatbuffers that referenced this pull request Aug 14, 2021
With the changes introduced on google#6729, google#6671, google#6658 it seems that using java
compiler version 8 is no longer possible. We are adjusting our CI build for
Kotlin to use Java 11 as compiler, while still emiting 1.8 bytecode.

The Kotlin CI action is also being breakdown into two: Mac & Linux.
Kotlin mac will test ios & mac builds while Linux will test js and JVM.
This change will improve build times for Kotlin on CI, which is currently
taking long times.
aardappel pushed a commit that referenced this pull request Aug 14, 2021
With the changes introduced on #6729, #6671, #6658 it seems that using java
compiler version 8 is no longer possible. We are adjusting our CI build for
Kotlin to use Java 11 as compiler, while still emiting 1.8 bytecode.

The Kotlin CI action is also being breakdown into two: Mac & Linux.
Kotlin mac will test ios & mac builds while Linux will test js and JVM.
This change will improve build times for Kotlin on CI, which is currently
taking long times.
bjornharrtell added a commit to bjornharrtell/flatbuffers that referenced this pull request Aug 14, 2021
aardappel pushed a commit that referenced this pull request Aug 30, 2021
* Revert "avoiding even more NoSuchMethod exceptions (#6729)"

This reverts commit 6fb2c90.

* Revert "avoiding more NoSuchMethod exceptions (#6671)"

This reverts commit 752c7b5.

* Revert "avoiding NoSuchMethod exception (#6658)"

This reverts commit 813d363.

* Use Java 8 for Kotlin Linux builds to verify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants