Skip to content

Various updates and refactors to build platform in general #13790

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

Open
wants to merge 40 commits into
base: master
Choose a base branch
from

Conversation

touhidurrr
Copy link
Contributor

@touhidurrr touhidurrr commented Aug 7, 2025

What this PR contains:

  1. Gradle version 8.11.1 -> 9.0.0 (latest stable)
  2. remove some redundant dependencies
  3. update some actions versions in dockerPublish.yml
  4. Dockerfile improvements
  5. Kotlin version 2.1.20 -> 2.1.21

@touhidurrr touhidurrr changed the title Squash some more gradle warnings Squash some more deprecation warnings Aug 7, 2025
@yairm210
Copy link
Owner

yairm210 commented Aug 8, 2025

The docker failures seem real
Wet may need to upgrade gradle in the docker image

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 8, 2025

The docker failures seem real
Wet may need to upgrade gradle in the docker image

It is failing in my local gradle 9.0.0 also. Replacing tasks.create() with tasks.register() might not be so simple. I also wanted to try dependsOn() but the comments seem to be advicing against it.

For now, I will simply revert tasks.create() -> tasks.register() change later, rename this as a minor refactor pr. And maybe learn how tasks work in gradle in more detail and open another pr later.

@SomeTroglodyte
Copy link
Collaborator

Minor things: Isn't Enum.values() deprecated by now? And wouldn't the jre/jdk file name be more elegant as enum property than via when?

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 8, 2025

Minor things: Isn't Enum.values() deprecated by now? And wouldn't the jre/jdk file name be more elegant as enum property than via when?

Check d85257d tests. I used Enum.etries but it failed. It works on my pc though but fails here. I do use some different settings though.

Copy link

github-actions bot commented Aug 8, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the Conflicts label Aug 8, 2025
Copy link

github-actions bot commented Aug 8, 2025

Conflicts have been resolved.

@touhidurrr touhidurrr changed the title Squash some more deprecation warnings Set Github Actions Java Version to 21, update to latest kotlin patch, and some other refactors Aug 8, 2025
@touhidurrr touhidurrr changed the title Set Github Actions Java Version to 21, update to latest kotlin patch, and some other refactors set Github Actions Java version to 21, update to latest Kotlin patch, and some other refactors Aug 8, 2025
@touhidurrr
Copy link
Contributor Author

By the way, when did you guys revert Kotlin version.

@SomeTroglodyte
Copy link
Collaborator

when did you guys revert Kotlin version.

As far as I can see, it's still 2.1.20, unchanged since #13602

@touhidurrr
Copy link
Contributor Author

when did you guys revert Kotlin version.

As far as I can see, it's still 2.1.20, unchanged since #13602

I am confused, then what did I change in 6e9a874

@SomeTroglodyte
Copy link
Collaborator

then what did I change in 6e9a874

Not the main language level, but specific plugins I'd guess

@touhidurrr touhidurrr marked this pull request as draft August 8, 2025 19:23
@touhidurrr touhidurrr marked this pull request as ready for review August 8, 2025 19:50
@touhidurrr touhidurrr changed the title set Github Actions Java version to 21, update to latest Kotlin patch, and some other refactors update Kotlin to latest patch, actions Java version to 21, remove hardcoded kotlinVersion in build.gradle.kts & redundant dependencies caused by it Aug 8, 2025
@touhidurrr
Copy link
Contributor Author

Minor things: Isn't Enum.values() deprecated by now? And wouldn't the jre/jdk file name be more elegant as enum property than via when?

Ok so I was able to figure out why did it happen. Basically, gradle scripts run on their own kotlin version separate from the projects kotlin version. So, even if the IDE was showing a deprecation warning, it was not correct. I guess the IDE only loads one Kotlin version at a time and that is the projects version.

So, in short the way to make Enum.entries work is to increase the gradle version which in turn will increase the kotlin version that gradle uses. 9.0.0 just happens to use the same kotlin version series (2.1.x) that we currently use so that fixes it.

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 10, 2025

Gradle updates - More involved, unclear what the implications are

It can be downgraded to latest AGP minimum version 8.13 if you do not like it. It was just me trying to figure out why Enum.entries did not work and testing if 9.0.0 fixes that, which it did. Other than that it fixes nothing.

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 10, 2025

Docker changes - most of these are basically nothing, unclear why you feel compelled to make these changes

I touched it because some previous builds were only failing in Docker but not anywhere else. Then I realized that nothing there was fixing the issue of hardcoded kotlin version not working in docker. So, I just did some refractors that I saw possible and left it there. A list of such changes would be:

  1. Remove unnecessary gradle cannot detect jdk patch. The issue mentioned there was closed long ago and removing those lines did not affect gradle in any way.
  2. Use adoptium api to always fetch the latest version of JRE 11 instead of an old hardcoded version. Again harmless but something that I spotted and fixed.
  3. Fixed some docker complaints. Such as AS has to be the same case as FROM.
  4. Broke && chain into separate RUN calls. It just makes debugging a little easier.
  5. Removed openjdk11 installation because it was already being downloaded separately. Only installed openjdk21-jdk to remain consistent with github actions. The version here is only for gradle to run and is not shipped anywhere to my knowledge.

And yes, I did not test if it actually runs. I will try when I go back home and give an update.

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 10, 2025

ok @yairm210, I tried the latest stable version of Unciv and it simply does not work on windows. It simply runs for eternity without displaying anything. And I cannot build PR so do not know if it works. Maybe only Linux users can verify it to begin with.

@yairm210
Copy link
Owner

@touhidurrr
Copy link
Contributor Author

Did you connect via vnc? https://github.com/yairm210/Unciv?tab=readme-ov-file#run-with-docker-

Ok it worked. But very weird way to play Unciv for sure. I was expecting a window to pop up.

@touhidurrr
Copy link
Contributor Author

But anyways, building the image locally in Windows is still an impossibility. Thus I am unable to test if it works.

@SeventhM
Copy link
Collaborator

But anyways, building the image locally in Windows is still an impossibility

I have no clue if this is a factor and it probably shouldn't be, but I should ask...

What version of Android Studio do you have?

@SeventhM
Copy link
Collaborator

SeventhM commented Aug 11, 2025

Ok, double checking stuff on my end. I'm mostly just lazily doing stuff through Studio, I have no clue the implications for docker, nor am I looking at the fixes or whatever for docker or detekt. Just seeing if I can build it. Side note: I only ask about the Android Studio version because I noticed recently that older versions of Studio could no longer build Unciv (I didn't keep track of the error message, but it failed on Gradle sync itself, saying something was an incorrect version)

Kotlinx-coroutines 1.8.1 -> 1.10.2
Gdx 1.13.1 -> 1.13.5
Kotlin hardcoded values updated to 2.1.21
Gradle 8.11.1 -> 8.14.3 (nothing is asking me to go to 9.0.0, so first attempt not bothering)
Android Gradle plugin 8.9.1 -> 8.12.0

Builds fine, no issues (after disabling purity). Maybe there's a separate problem on your end? It's either that or the Gradle version (which I doubt is the culprit, but you could never be too sure)

Edit: Gradle version 9.0.0 is working fine as well, though it's now suggesting to use Kotlin 2.2.0

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

Kotlinx-coroutines 1.8.1 -> 1.10.2
Gdx 1.13.1 -> 1.13.5

I did not increment these versions though.

Gradle 8.11.1 -> 8.14.3

And gradle sync will fail in desktop and server build.gradle.kts script without gradle < 9.0.0. To fix these you have to replace Platform.entries with Platfom.values()

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

Android Gradle plugin 8.9.1 -> 8.12.0

Also, any AGP version >=8.10 will also fail. This is because some additional permission checks are required for these versions. This is why I did not increase AGP version here because it is likely better to this in another PR. patch doesn’t affect anything though.

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

though it's now suggesting to use Kotlin 2.2.0

Weird. From patch notes I am pretty sure Gradle 9 itself use Kotlin 2.1.x. So I wonder why are they suggesting 2.2. It did not suggest this to me.

Edit: Ok my bad.

Gradle 9 embeds the latest stable release of Kotlin 2.2.x runtime and uses Kotlin language version 2.2.
https://gradle.org/whats-new/gradle-9/#kotlin-and-kotlin-dsl

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

What version of Android Studio do you have?

Latest feature drop. I updated it a few days ago. Also, this is likely because of how Docker Desktop works and on Windows Home and not for Android Studio. It runs inside WSL. My guess is it cannot access Android Sdk binaries from WSL.

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

Builds fine, no issues

Can you kindky verify if the docker image build and runs fine with exactly this PR's content? You do not need to disable purity.

@yairm210
Copy link
Owner

LibGDX we're not upgrading, as per the maintainers' advice, since it have Android problems
Already discussed in another PR somewhere... #13553 ?

@SeventhM
Copy link
Collaborator

SeventhM commented Aug 11, 2025

Can you kindky verify if the docker image build and runs fine with exactly this PR's content? You do not need to disable purity.

Tbh, I have zero clue how to do anything with docker at all

I did not increment these versions though.

I know and that's not my point. My point is that, at least through Studio, things builds fine on my end, so it must be something more docker specific and not just related to upgrading versions. I don't expect us to touch everything I did, I was just seeing what's possible

@touhidurrr
Copy link
Contributor Author

touhidurrr commented Aug 11, 2025

Tbh, I have zero clue how to do anything with docker at all

Me before opening this PR: Everyone use Docker. I am the only guy who doesn’t. I am so useless.
Me after this PR: What is Docker and why do the maintainers even want me to test if stuff runs on it. Who uses such niche technology that crashes everywhere.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

Conflicts have been resolved.

yairm210 added a commit that referenced this pull request Aug 15, 2025
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

Copy link

Conflicts have been resolved.

@touhidurrr
Copy link
Contributor Author

So, after 1919b50 I guess what remains for discussion is if thr Docker image actually runs and what Gradle version can we upgrade to and should we.

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.

4 participants