Skip to content

don't download JDK 17 during build #264

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

Closed
Ladicek opened this issue Sep 23, 2022 · 8 comments · Fixed by #265
Closed

don't download JDK 17 during build #264

Ladicek opened this issue Sep 23, 2022 · 8 comments · Fixed by #265
Assignees
Milestone

Comments

@Ladicek
Copy link
Contributor

Ladicek commented Sep 23, 2022

The current Jandex 3 codebase is meant to be built with JDK 8 and downloads JDK 17 during build for the purpose of the test-data module. This is hostile to downstream rebuilds in an air-gapped environment.

There are several possible solutions. The best one is probably to use JDK 17 to build the entire project and make sure that the core and maven-plugin are configured to emit Java 8 bytecode. Perhaps we should even add automated tests to make sure that the class files actually are Java 8 bytecode.

@Ladicek Ladicek added this to the 3.0.2 milestone Sep 23, 2022
@Ladicek Ladicek self-assigned this Sep 23, 2022
@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 23, 2022

The solution suggested above has one downside: it will no longer be possible to run tests in CI against JDK 8 and 11.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 23, 2022

Another possible solution, one that I've been trying to avoid, is to use Maven toolchains. It is unfriendly to contributors, but may be the best we can do (unless we're willing to abandon running tests with older JDKs...).

@MikeEdgar
Copy link
Member

Another option may be to run the JDK 17 build/tests (emitting Java 8 bytecode), and attach the test-data jar to the GitHub build. You could then have a secondary job in the same action that runs the JDK build/test matrix. You would need to fetch the jar attached to the build and install it to the local repo, etc. It's a hack, but would allow the primary build to be friendlier with Java 17 and keep the tests against older JDKs in place.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 23, 2022

That would be doable, but would require extracting the tests to a separate module, or doing some other hacks to convince Maven to run tests against code that wasn't built in the current Maven reactor...

@MikeEdgar
Copy link
Member

You should be able to only run a subset of the packages and it will use the artifacts in the local repo for the dependency. As long as the test-data and parent artifacts are installed, the others will be happy.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 23, 2022

I think I have a reasonable solution: if "current" JDK is < 17, don't change anything (keep downloading JDK 17 during build). If "current" JDK is >= 17, use the "current" JDK to build everything. Also, add tests to verify that Jandex bytecode is indeed Java 8. This way, downstreams can build using JDK 17 and no extra JDK will be downloaded. Perhaps we should do that in the release build, too.

@n1hility
Copy link
Contributor

One thing to look out for is that newer JDKs are dropping support in Javac to build older bytecode more aggressively than before.

@Ladicek
Copy link
Contributor Author

Ladicek commented Sep 26, 2022

I've read https://openjdk.org/jeps/182 and found the recent announcement that JDK 20 drops support for -source / -target / --release 7 pretty disturbing, because the JEP is in draft, yet they seem to be following it :-/ Version 8 is still supported, so we should be fine for a while. When they drop version 8 & when we need more recent JDK, we'll have to find another way.

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 a pull request may close this issue.

3 participants