Skip to content

Conversation

@andrew8er
Copy link
Contributor

@andrew8er andrew8er commented Nov 11, 2025

Short Description

Upgrades the required build JDK to 25.

Proposed Changes

  • Upgrade the required backend JDK to 25. The sourceCompatibility and targetCompatibility are still set to 24, since Kotlin does not support 25 at the moment. This will be resolved soon after updating to Kotlin 2.3.0, which is in beta now:
  • Removed the entitlementcard-jdk IntelliJ option. Let users just declare a "project SDK".
  • Update the CircleCI images for building/checking the backend.
  • Add a install_temurin command and add it to pack_backend job.
  • A few smaller improvements to the Gradle file:
    • Do not use maven group name for deducing package root: the Maven group name and the package root are two totally unrelated concepts and should not be conflated.
    • Extract CI config into object: put all environment variables set by the CI pipeline into an object for better clarity in the Gradle file. Also remove the usage of SENTRY_AUTH_TOKEN, use SENTRY_BACKEND_AUTH_TOKEN directly.
  • Remove this block from the Gradle file, which caused the "main" jar to not be included in the final distribution artifact:
    tasks.withType<Zip>().configureEach {
        enabled = false
    }
    
  • Update the setup docs.

Open questions

Side Effects

None

Testing

  • Run backend
  • Run tests

Resolved Issues

Fixes: #2281

@andrew8er andrew8er marked this pull request as draft November 11, 2025 09:18
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch 2 times, most recently from 71f5ec4 to 07cdc7d Compare November 11, 2025 13:03
@andrew8er andrew8er linked an issue Nov 11, 2025 that may be closed by this pull request
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch 2 times, most recently from 42cab4c to 0b6ec0d Compare November 11, 2025 15:00
@f1sh1918
Copy link
Contributor

  1. I think this variable should automatically resolve the correct jdk for this project, no? Actually for me its not working for couple of time so i don't mind removing it. I even never touched this configuration so maybe @michael-markl has some input
  • You can make a temporary commit and comment out the delivery steps(delivery_beta_all.yml):
  - deliver_server:
      production: false
      context:
        - credentials-ehrenamtskarte
      requires:
        - pack_administration
        - pack_backend
        - pack_martin
        - pack_meta

  - notify_release:
      production_delivery: false
      context:
        - deliverino
      requires:
        - deliver_android
        - deliver_ios
        - deliver_server
  • Then update the config.yml (package.json script)
  • trigger delivery_beta_all via circle ci GUI
    If you think it may make sense to create a "test" workflow you could also add one
    So far we didn't have to test that frequently.

@f1sh1918
Copy link
Contributor

f1sh1918 commented Nov 12, 2025

I tested your draft and it works well so far.
I think you could also enable virtual threads in this issue. I think its not worth creating a separate pr for that

spring.threads.virtual.enabled=true

I get some warnings during backend start but i guess you also realized this

WARNING: A terminally deprecated method in sun.misc.Unsafe has been called
WARNING: sun.misc.Unsafe::invokeCleaner has been called by nonapi.io.github.classgraph.utils.FileUtils (file:/Users/afischer/.gradle/caches/modules-2/files-2.1/io.github.classgraph/classgraph/4.8.174/d86b16ab77abd87118b51e6a2c61e2d44ce6deaa/classgraph-4.8.174.jar)
WARNING: Please consider reporting this to the maintainers of class nonapi.io.github.classgraph.utils.FileUtils
WARNING: sun.misc.Unsafe::invokeCleaner will be removed in a future release

@michael-markl
Copy link
Member

I think this variable should automatically resolve the correct jdk for this project, no? Actually for me its not working for couple of time so i don't mind removing it. I even never touched this configuration so maybe @michael-markl has some input

I am not sure which variable you mean. I don't have immediate feedback here. As long as everything works, these changes seem fine to me (without having checked them in depth :D )

@andrew8er
Copy link
Contributor Author

@f1sh1918

I think this variable should automatically resolve the correct jdk for this project, no? Actually for me its not working for couple of time so i don't mind removing it. I even never touched this configuration so maybe @michael-markl has some input

For me it mostly didn't work, though more often in the last few days, strangely enough. But yes, with this configuration, Gradle should chose the correct version automatically.

You can make a temporary commit and comment out the delivery steps(delivery_beta_all.yml):

Thanks, I'll try it.

I tested your draft and it works well so far.
I think you could also enable virtual threads in this issue. I think its not worth creating a separate pr for that

I would prefer to do this in a separate PR. The general advice here is to test this thoroughly.

I get some warnings during backend start but i guess you also realized this

I saw those, these are from one of our dependencies. We could suppress them, but I'd rather have them as a reminder to update our dependencies regularly.

@michael-markl

I am not sure which variable you mean. I don't have immediate feedback here. As long as everything works, these changes seem fine to me (without having checked them in depth :D )

The IntelliJ config option removed here: https://github.com/digitalfabrik/entitlementcard/pull/2657/files#diff-952b752681ded498853ce6363aa309aac9e6bd15fe12a907be0b8d2142d1fa21L8

@f1sh1918
Copy link
Contributor

I guess we also have to update the jdk on our Servers before doing a release

@andrew8er
Copy link
Contributor Author

The JDK update is tracked here: https://tasks.tuerantuer.org/projects/infra/work_packages/4793/activity

@f1sh1918

This comment was marked as resolved.

@andrew8er

This comment was marked as resolved.

@f1sh1918
Copy link
Contributor

f1sh1918 commented Nov 17, 2025

https://tasks.tuerantuer.org/projects/infra/work_packages/4793/activity

🙈 sure... looks like it's monday
Okay works fine

@andrew8er andrew8er force-pushed the 2444-upgrade-to-gradle-92 branch from 1945586 to e1d364b Compare November 17, 2025 13:23
Base automatically changed from 2444-upgrade-to-gradle-92 to main November 19, 2025 08:51
@f1sh1918 f1sh1918 added the not-testable For end users label Nov 20, 2025
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch from 0b6ec0d to 8d35c9c Compare November 21, 2025 16:14
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch 5 times, most recently from 4a0ef01 to 10c62a3 Compare November 25, 2025 14:00
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch 3 times, most recently from e65277a to 4e72bd8 Compare November 26, 2025 18:46
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch from f70b6f4 to 317fc9e Compare November 26, 2025 19:54
@andrew8er
Copy link
Contributor Author

So after literally hours of figuring out, why the "main" jar was not packaged into the backend.tar artifact, I found out that this was the cause (in build.gradle.kts):

tasks.withType<Zip>().configureEach {
    enabled = false
}

@seluianova
What was this actually supposed to do? I guess it should inhibit the building of the zip file when building the assemble or assembleDist task? For whatever reason, it caused a seemingly totally unrelated side effect. I removed it, building the distTar task is enough.

@andrew8er andrew8er marked this pull request as ready for review November 26, 2025 20:09
@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch from 317fc9e to f50a403 Compare November 26, 2025 20:16
@f1sh1918
Copy link
Contributor

f1sh1918 commented Nov 27, 2025

So after literally hours of figuring out, why the "main" jar was not packaged into the backend.tar artifact, I found out that this was the cause (in build.gradle.kts):

tasks.withType<Zip>().configureEach {
    enabled = false
}

@seluianova
What was this actually supposed to do? I guess it should inhibit the building of the zip file when building the assemble or assembleDist task? For whatever reason, it caused a seemingly totally unrelated side effect. I removed it, building the distTar task is enough.

I don't get why there are such side effects but okay.
I suggested adding this line because we don't need the zip file but the only tar?

I think with Java 17 there were no issues at all?

Didn't know that there is a particular job in gradle

Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i get only issue running distTar probably typo
Thx for improving docs

@andrew8er andrew8er force-pushed the 2281-upgrade-to-jdk25 branch from f50a403 to c042e3b Compare November 27, 2025 09:52
@andrew8er
Copy link
Contributor Author

@f1sh1918

I think with Java 17 there were no issues at all?

I tested this with JDK 17, 21 and 25 as the Gradle platform. All had the same effect, with

tasks.withType<Zip>().configureEach {
    enabled = false
}

the main jar in the dist tar is missing. So I'm pretty sure this did not ever work on a real deployment.

@f1sh1918
Copy link
Contributor

f1sh1918 commented Dec 1, 2025

Maybe it would be good to know which issues exactly you are talking about. @seluianova deployed it with java 17 on staging and we couldn't figure out issues directly after deployment

@andrew8er
Copy link
Contributor Author

andrew8er commented Dec 1, 2025

Steps to reproduce:

  • Build the backend with a clean env (delete /backend/build): SENTRY_AUTH_TOKEN=<sentry_token> ./gradlew assemble -Denv=prod
  • Unpack the dist tar file: /backend/build/distributions/backend.tar
  • Try to run resulting bin/backend. An error is produced:
Error: Could not find or load main class app.ehrenamtskarte.backend.EntryPointKt
Caused by: java.lang.ClassNotFoundException: app.ehrenamtskarte.backend.EntryPointKt

This is because the file lib/backend-0.0.1-SNAPSHOT-plain.jar is missing, which contains our code.

  • Remove the offending block
  • Delete /backend/build again
  • Retry

@seluianova
Copy link
Contributor

@seluianova deployed it with java 17 on staging and we couldn't figure out issues directly after deployment

actually I applied this suggestion after the deployment, so yes, this issue could happen.
good to find this now, thanks @andrew8er

Copy link
Contributor

@seluianova seluianova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@andrew8er
Copy link
Contributor Author

I'll wait with merging this PR until JDK 25 is available on our server: https://tasks.tuerantuer.org/projects/infra/work_packages/4793/activity

@andrew8er andrew8er added the blocked Task or user story can not be continued at the moment label Dec 1, 2025
@f1sh1918
Copy link
Contributor

f1sh1918 commented Jan 5, 2026

@andrew8er jdk25 is running on staging server. afaics it runs stable. I think we can merge this pr

@andrew8er andrew8er enabled auto-merge January 5, 2026 11:29
@andrew8er
Copy link
Contributor Author

It needs another review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked Task or user story can not be continued at the moment not-testable For end users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Upgrade to JDK 25

5 participants