-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Update java pom.xml file to allow java 8 compatibility #7894
Conversation
…be compiled under java 8, and pulled in as a dependency to a project using java 8. (google#7893)
@paulovap Can you take a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bjornharrtell can you also take a look, since you wrote the aforementioned change?
I still don't understand why we need to keep separated profiles, but having added an additional profile for java8 is the sensible solution for now.
I don't remember much about where I was inspired from when I made the changes and I agree it doesn't look entirely sensible (more like Java 9+ compatible). But I find it strange because at the time I was interested in Java 8 compatibility and it looks like I had it working. I'm not opposed to fixing it but the there are more changes in this PR that seem unrelated so I find it hard to review. |
I have a project where I use flatbuffers in which I expose a reference implementation library for multiple languages. I think it was probably here I did manual testing against Java 8. Automated testing was against Java 17 but I just now added also for Java 11 and Java 8 and it appears to be working so I'm a bit at a loss here. See https://app.circleci.com/pipelines/github/flatgeobuf/flatgeobuf/902/workflows/809f20df-415a-43b0-aba9-1e8fb54d9c6e/jobs/3773 for a test run that brings in flatbuffers from maven and have no issue passing on Java 8, unless I'm mistaken. |
The changes made were to solve 2 problems I ran into. 1) including flat buffers through a maven dependency and using copy-dependencies in the maven-dependencies-plugin in a project building under JDK 8. 2) Building flat buffers under JDK 8. I would think just a normal dependency would include would fail the same way, but it doesn't look that way. My code that pulls it in as a dependency seems to be fine. Which is probably what you are doing. The changes that I made: 1) Move all plugins out of the the Java 9 profile to the main plugin section. This fixed the bundle issue. 2) That exposed a second issue when building flat buffers under JDK 8, which is that the maven-compiler-plugin under java 8 doesn't support the --release flag, which can be fixed by including different options between jdk8 and jdk9 onwards. For the final issue I added the compiler plugin to the 2 different profiles with separate compatible options. If you don't care if flatbuffers actually compiles under jdk8, this fix probably isn't needed, and you can just move all the plugins out to the main section, and remove the jdk9 section all together. |
Just to clarify, I have no issues with this being merged |
…be compiled under java 8, and pulled in as a dependency to a project using java 8. (google#7893) (google#7894) Co-authored-by: Derek Bailey <derekbailey@google.com> Co-authored-by: Paulo Pinheiro <paulovictor.pinheiro@gmail.com>
…be compiled under java 8, and pulled in as a dependency to a project using java 8. (google#7893) (google#7894) Co-authored-by: Derek Bailey <derekbailey@google.com> Co-authored-by: Paulo Pinheiro <paulovictor.pinheiro@gmail.com>
We are trying to upgrade from Flatbuffers 1.x to 2.0.8. Some of our products still use java 8.
With this change a7b527d the pom file only includes the bundle plugin (and others) if it is java 9 or later. This causes the maven-dependency plugin to fail under java 8, because the "bundle" isn't known unless the plugin is defined.
One solution is to move the bundle plugin out of the java 9 profile, and directly into the build section in the flat buffers pom.xml, but that would require an update in flatbuffers.
How is this working for anyone using java 8 for flat buffers 2.0.3 forward, or was that the point of this. But reading through the pull request thread (#6764), it looks like the point was to regain java 8 compatibility not disable it.
Also no matter what I do to try and add the plugin, from my own project, I still get the unknown bundle error:
Failed to execute goal org.apache.maven.plugins:maven-dependency-plugin:3.1.2:copy-dependencies (copy-dependencies) on project icore-release: Some problems were encountered while processing the POMs:
[ERROR] [ERROR] Unknown packaging: bundle @ line 7, column 14
[ERROR] : 1 problem was encountered while building the effective model for com.google.flatbuffers:flatbuffers-java:2.0.8
[ERROR] [ERROR] Unknown packaging: bundle @ line 7, column 14
Anyone have flat buffers 2.0.3 onwards working in a project pulling it in through the maven dependency and using java 8?
Can the bundle plugin be moved out into the main build section instead of the jdk9 profile?
Can the profile be switched to jdk8 instead?
If the updates were for java 8 bytecode compatibility, why is java 8 excluded?
This pull request fixes the pom.xml file to allow flatbuffers-java to compile under jdk8, as well as allow for it to be pulled in as a dependency to a project using jdk8.