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

CompiledJavaVersionBuildStep may load a wrong class number with gradle #42884

Closed
Malandril opened this issue Aug 29, 2024 · 4 comments · Fixed by #42897
Closed

CompiledJavaVersionBuildStep may load a wrong class number with gradle #42884

Malandril opened this issue Aug 29, 2024 · 4 comments · Fixed by #42897
Labels
area/gradle Gradle kind/bug Something isn't working
Milestone

Comments

@Malandril
Copy link
Contributor

Malandril commented Aug 29, 2024

Describe the bug

CompiledJavaVersionBuildStep will try to produce a CompiledJavaVersionBuildItem by walking the build directory and finding the first java class it can get a java version from.
This is problematic as gradle often uses the build/tmp/.cache directory to unzip temporary files, that may contain classes and the version of these classes may be loaded instead of the ones from the project.

This can prevent some builds from using virtual threads with @RunOnVirtualThread for example.

Expected behavior

The java version should be retrieved from a class compiled by the gradle project.

Actual behavior

The java version is retrieved from the first found class, even from some temporary build directory

How to Reproduce?

Reproducer her https://github.com/Malandril/quarkus-bug-virtual-jacoco.git

clone it then run
./gradlew test jacocoTestReport

It should fail, because the CompiledJavaVersionBuildStep will load the version from a class in build/tmp/.cache/expanded/zip_zipchecksum/org/jacoco/agent/package-info.class/package-info.class which is of java 5 instead of java 21

Output of java -version

openjdk version "21.0.2" 2024-01-16

Quarkus version or git rev

3.14.1

Build tool (ie. output of mvnw --version or gradlew --version)

Gradle 8.8

Additional information

Maybe we could simply exclude hidden folders from the SimpleFileVisitor in the CompiledJavaVersionBuildStep ?
This would not ensure that a project class will be loaded, but would solve the issue with jacoco at lease

@Malandril Malandril added the kind/bug Something isn't working label Aug 29, 2024
@quarkus-bot quarkus-bot bot added the area/gradle Gradle label Aug 29, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 29, 2024

/cc @glefloch, @quarkusio/devtools

@gsmet
Copy link
Member

gsmet commented Aug 30, 2024

While I'm all for the quick fix, I would have expected the build tools to pass this information to the build instead of us relying on the compiled classes to determine this information.

@aloubyansky is there something that prevents us to do that?

@Malandril that being said, I'm all for getting a quick fix in. I would rather limit the scan to classes and generated, rather than excluding /tmp.

@gsmet
Copy link
Member

gsmet commented Aug 30, 2024

Ah, apparently, @aloubyansky created a PR already :).

@aloubyansky
Copy link
Member

We could read it from Maven/Gradle project configs. In the meantime, I opened a PR with a quick fix. One issue with the current approach is that it looks only in the main module, which sometimes has no classes but there could be other modules in the project that do have them. I could include them in a follow up if the main module does not include any classes.

@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Aug 30, 2024
@gsmet gsmet modified the milestones: 3.16 - main, 3.14.2 Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/gradle Gradle kind/bug Something isn't working
Projects
None yet
3 participants