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

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

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

iam-afk
Copy link
Contributor

@iam-afk iam-afk commented Aug 11, 2023

Resolves #137

@Ladicek
Copy link
Collaborator

Ladicek commented Aug 14, 2023

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

@iam-afk
Copy link
Contributor Author

iam-afk commented Aug 15, 2023

😃 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 (5f044c1). 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
Copy link
Collaborator

Ladicek commented Aug 15, 2023

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.

@Ladicek Ladicek changed the title Configure persistent index format version in the Maven plugin Allow configuring persistent index format version in the Maven plugin Aug 15, 2023
@Ladicek Ladicek added this to the 3.1.3 milestone Aug 15, 2023
@Ladicek Ladicek merged commit d04d79e into smallrye:main Aug 15, 2023
39 checks passed
@iam-afk
Copy link
Contributor Author

iam-afk commented Aug 21, 2023

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
Copy link
Collaborator

Ladicek commented Aug 21, 2023

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

@Ladicek
Copy link
Collaborator

Ladicek commented Aug 25, 2023

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.

@iam-afk
Copy link
Contributor Author

iam-afk commented Aug 26, 2023

Thanks, I'll try after the weekend.

@iam-afk
Copy link
Contributor Author

iam-afk commented Aug 28, 2023

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...

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.

Configurable persistent index format version in the Maven plugin
2 participants