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

fix(java): update pom.xml to use javac 8 #1579

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stardustman
Copy link

fix using high version jdk compile to low version such as jdk8 using --release instead of -target
solve the #1577

Starting JDK 9, the javac executable can accept the --release option to specify against which Java SE release you want to build the project. For example, you have JDK 11 installed and used by Maven, but you want to build the project against Java 8. The --release option ensures that the code is compiled following the rules of the programming language of the specified release, and that generated classes target the release as well as the public API of that release. This means that, unlike the -source and -target options, the compiler will detect and generate an error when using APIs that don't exist in previous releases of Java SE.
https://maven.apache.org/plugins/maven-compiler-plugin/examples/set-compiler-release.html

fix using high version jdk compile to low version such as jdk8 using --release instead of -target
@chaokunyang chaokunyang changed the title Update pom.xml fix(java): update pom.xml to use javac 8 Apr 26, 2024
@chaokunyang
Copy link
Collaborator

Hi @stardustman , thanks for your contribution. Could you add a ByteBuffer ut for this ?

@chaokunyang
Copy link
Collaborator

Hi @stardustman , thanks for your contribution. Could you add a ByteBuffer ut for this ?

Seems this is not easy to add a test, we must compile use one java version, and run it with another java version.

@@ -127,6 +127,11 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<configuration>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I did a test, seems not work? Could you test it locally, you can compile it with java9+, then select to use java8 to run. And do we need to add this config to parent pom?

chaokunyang added a commit that referenced this pull request Apr 26, 2024
## What does this PR do?

This PR add a bytebuffer util and fix bytebuffer no such method error

## Related issues

Closes #1577 
#1579 

## Does this PR introduce any user-facing change?

<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/incubator-fury/issues/new/choose)
describing the need to do so and update the document if necessary.
-->

- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?


## Benchmark

<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach a benchmark data here.
-->
@stardustman
Copy link
Author

this 0.4.1 version in mvnrepository compile with jdk17? or other version?

@chaokunyang
Copy link
Collaborator

this 0.4.1 version in mvnrepository compile with jdk17? or other version?

The major in class file is 52, so it should be java8 already

@stardustman
Copy link
Author

this 0.4.1 version in mvnrepository compile with jdk17? or other version?

The major in class file is 52, so it should be java8 already
if this jar was compiled using jdk8's javac to get class file, this should not report error when execute with jdk8.
but I get error when I serialize the ByteBuffer, example code like this:

import io.fury.Fury;
import io.fury.ThreadLocalFury;
import io.fury.ThreadSafeFury;
import io.fury.config.Language;

import java.nio.ByteBuffer;

public class FuryTest {


    // Note that Fury instances should be reused between
    // multiple serializations of different objects.
    private static final ThreadSafeFury fury = new ThreadLocalFury(classLoader -> {
        Fury f = Fury.builder()
                .withLanguage(Language.JAVA)
                .withClassLoader(classLoader)
                .requireClassRegistration(false)
                .build();
        return f;
    });

    public static byte[] encoder(Object object) {
        return fury.serialize(object);
    }

    public static <T> T decoder(byte[] bytes) {
        return (T) fury.deserialize(bytes);
    }

    public static void main(String[] args) {
        byte len = 10;
        ByteBuffer byteBuffer = ByteBuffer.allocate(len);
        for (int i = 0; i < len; i++) {
            byteBuffer.put((byte) i);
        }
        byte[] bytes = FuryTest.encoder(byteBuffer);
        System.out.println(bytes.length);

    }

}

@chaokunyang
Copy link
Collaborator

chaokunyang commented Apr 28, 2024

I forgot which javac version I used to release jar.

I use following steps to guess javac version:

@stardustman
Copy link
Author

I use the branch releases-0.4.1 and jdk1.8.0_241 then execute the mvn package in the IDE to get the final jar file, and test using this jar, this time the test code can execute, but using the jar download from mvn central repo report error.
I also try to use jdk11 to execute the mvn package but get the error: package sun.misc does not exist.

@chaokunyang
Copy link
Collaborator

Hi @stardustman , does latest fury 0.8.0 works for you on JDK8?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants