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

Update Java and fix AppVeyor #1663

Merged
merged 2 commits into from Dec 5, 2021
Merged

Conversation

Jikoo
Copy link
Collaborator

@Jikoo Jikoo commented Nov 17, 2021

  • Target Java 16 for compilation
    • GP requires MC 1.17 to run and MC 1.17 requires Java 16, ergo GP requires Java 16.
    • This may be a bad change in that I'm not sure if the plugin.yml is read before or after the classloader checks class versions, this may make startup errors for GP on old servers with old Java less explicit.
  • Run AppVeyor CI using Java 17
    • Requires new VS 2022 image: https://www.appveyor.com/updates/2021/11/09/
    • Certain dependencies cannot be upgraded to current versions (see Correct WorldGuard integration #1630) because they are compiled with later Java versions. It's possible to target lower Java versions for compilation, but the Maven runner must be using a Java version equal to or greater than that of the dependency. Better to be ahead of the curve, particularly since 1.18 will require Java 17.
    • JDK 16 had several annotation-related compiler issues, preferable to just avoid
  • Use provided AppVeyor Maven installation instead of manually installing
    • AppVeyor bundles and sets up Maven.
    • The mirror link used in manual setup is no longer valid and the AppVeyor cache has been cleared. This causes all AppVeyor builds to fail.
  • Remove Java 7-specific memory settings

Target Java 16
Run CI using Java 17
Use provided Maven installation instead of relying on mirror link
Remove Java 7 memory limitation settings
Jitpack uses Java 8 by default. While their (very minimal) explanation of settings suggests specifying `jdk: [ openjdk17 ]` would be enough, it does not appear to work. Not sure if that's just an issue with their image not including JDK17 yet or something, but this does function.
@RoboMWM
Copy link

RoboMWM commented Dec 5, 2021

Thank you! Been struggling to get this one going because the docs don't seem to note any of the older config stuff.

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.

None yet

2 participants