smallrye / jandex

Java Annotation Indexer
Apache License 2.0
394 stars 94 forks source link

Allow configuring persistent index format version in the Maven plugin #320

Closed iam-afk closed 1 year ago

iam-afk commented 1 year ago

Resolves #137

Ladicek commented 1 year ago

Out of curiosity, is this something you actually need, or did you implement it just "for fun"?

iam-afk commented 1 year ago

😃 In short the change did not help me to solve my original issue.

We have a project where we use Jandex 1 or 2 (because of Hibernate depends on Jandex and Wildfly provide Jandex module). But once jandex-maven-plugin stopped working with latest JDK ea builds (5f044c136129c4d8b57188fea4f01f76f1d4ea88). I thought I could use latest jandex-maven-plugin 3 to create persistent index compatible with Jandex 2. But despite the fact that the versions of the formats have become the same, Jandex 2 could not read the index (unknown annotation tag value, if I remember it right)...

Since there is the issue #137 (someone else need something like this) and I have already made changes, why not make a PR.

Ladicek commented 1 year ago

OK, that is interesting. If you can provide a reproducer, I can take a look.

In the meantime, I'll try to find some time to write a test for this and then merge.

iam-afk commented 1 year ago

OK, that is interesting. If you can provide a reproducer, I can take a look.

In the meantime, I'll try to find some time to write a test for this and then merge.

I was able to reproduce a problem (or maybe one case) with this example:

import java.util.List;

public interface X<T extends List<T>> { }
javac --release 11 X.java

Test runner scratch.java:

import org.jboss.jandex.Index;
import org.jboss.jandex.IndexReader;
import org.jboss.jandex.IndexWriter;
import org.jboss.jandex.Indexer;

import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Paths;

class Scratch {
    public static void main(String[] args) throws IOException {
        if (args.length > 0 && "create".equals(args[0])) {
            Indexer indexer = new Indexer();
            try (InputStream inputStream = Files.newInputStream(Paths.get("X.class"))) {
                indexer.index(inputStream);
            }
            Index index = indexer.complete();

            try (OutputStream outputStream = Files.newOutputStream(Paths.get("jandex.idx"))) {
                IndexWriter indexWriter = new IndexWriter(outputStream);
                indexWriter.write(index, 9);
            }
        } else {
            try (InputStream inputStream = Files.newInputStream(Paths.get("jandex.idx"))) {
                Index index = new IndexReader(inputStream).read();
                System.out.println("index.getKnownClasses().size() = " + index.getKnownClasses().size());
            }
        }
    }
}

Create index with Jandex 3, version 9 (max for version 2.2.3):

java -cp jandex-3.1.2.jar --source 11 scratch.java create

Try to read index with Jandex 2:

java -cp jandex-2.2.3.Final.jar --source 11 scratch.java

Get error:

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Index 3 out of bounds for length 1
    at org.jboss.jandex.IndexReaderV2.readAnnotations(IndexReaderV2.java:208)
    at org.jboss.jandex.IndexReaderV2.readTypeEntry(IndexReaderV2.java:341)
    at org.jboss.jandex.IndexReaderV2.readTypeTable(IndexReaderV2.java:175)
    at org.jboss.jandex.IndexReaderV2.read(IndexReaderV2.java:109)
    at org.jboss.jandex.IndexReader.read(IndexReader.java:76)
    at Scratch.main(scratch.java:27)

But this works fine:

java -cp jandex-3.1.2.jar --source 11 scratch.java
index.getKnownClasses().size() = 1
Ladicek commented 1 year ago

I'll take a look. Apparently we broke compatibility, which is bad. Thanks for the reproducer!

Ladicek commented 1 year ago

OK, the problem is that Jandex 3 added a new kind of types, type variable references, and the serialization code doesn't account for that when serializing to an older version. Fortunately, there's a simple fix -- what Jandex 3 represents as a type variable reference, Jandex 2 represented as an unresolved type variable, so we'll just resort to writing an unresolved type variable for older versions of the serialization format. Here's a PR: https://github.com/smallrye/jandex/pull/324 If you could build Jandex from that branch and check, that would be great. It passes your reproducer above.

iam-afk commented 1 year ago

Thanks, I'll try after the weekend.

iam-afk commented 1 year ago

OK, the problem is that Jandex 3 added a new kind of types, type variable references, and the serialization code doesn't account for that when serializing to an older version. Fortunately, there's a simple fix -- what Jandex 3 represents as a type variable reference, Jandex 2 represented as an unresolved type variable, so we'll just resort to writing an unresolved type variable for older versions of the serialization format. Here's a PR: #324 If you could build Jandex from that branch and check, that would be great. It passes your reproducer above.

Yes, now it works on the whole project! Thanks! Wait for release...