Skip to content

update to java 17#2405

Closed
Sheikah45 wants to merge 5 commits intodevelopfrom
maintenance/java17
Closed

update to java 17#2405
Sheikah45 wants to merge 5 commits intodevelopfrom
maintenance/java17

Conversation

@Sheikah45
Copy link
Member

Fixes #2298

@Sheikah45 Sheikah45 requested a review from Brutus5000 October 23, 2021 13:18
@Sheikah45
Copy link
Member Author

Sheikah45 commented Oct 24, 2021

bah no official jacoco support for javaFX yet

Brutus5000
Brutus5000 previously approved these changes Nov 7, 2021
Copy link

@jnorthrup jnorthrup left a comment

Choose a reason for hiding this comment

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

i really want the benefit of raising the maxversion well beyond 17 since there is an aggressive java update tempo and this codebasee sees some pretty long iterations of updates.

if 15 works 15 should stay the minversion as well.

i'm not comprehending the role of the fx package here or at least the potential to spoil the whole build by failing to be exact in this matter.

i would suggest sdkman as a shell based jvm tool to install jdk's, which includes the suggested adoptjdk ones, while they current that is. again, i have no idea about fx and jacoco

@Sheikah45
Copy link
Member Author

You can technically modify the install script to allow greater than java 15 although this is only pertinent for linux installs as windows has the jre packaged with it

@jnorthrup
Copy link

what is the gap between having install4j and using gradle to make a shade/uberjar that just does the simplest thing expected?

@Sheikah45
Copy link
Member Author

install4j produces a native exe and also packages the necessary jre.

Using gradle would just produce a zip or a fat jar but users would still need to install the appropriate version of java to run it and also would have to invoke the java command themself

@jnorthrup
Copy link

jnorthrup commented Feb 27, 2022

the non-windows version requires rather lengthy shell-pasting detours to accomodate the window-binary preferences then, iiuc.

I have identified a bug in replays on linux i'd like to fix, where the client is invoking shell on the FA executable. i have not gone deeper than this github issue to understand the relative complexity required.

All told, it appears my install gist ( https://gist.github.com/Yoslincake/ddfa6f3f1aedfb6faa5f17ddd7d12f28 ) requires no less than 2 shell scripts and a jdk15 appendage and and a lot of non-windows directory magic like ~/.fafclient seperate from ~/games/faf

This would be simpler (e.g. not deflect FA players for years at a time who can't be bothered) as your described with an executable jarfile or a shell script with environment.

@Sheikah45
Copy link
Member Author

Sheikah45 commented Feb 27, 2022

The client infrastructure only officially supports windows as this is the supported platform for the game as well. There is no official support for linux as there are no devs who are actively working on it who also play on that environment to support it.

That is the reason why the linux path is more tedious simply because it doesn't have any support.

@Sheikah45
Copy link
Member Author

The unofficial support guide someone put together can be found here https://wiki.faforever.com/en/FAQ/Client-Setup. There is also a forum post that people have put some of their findings on that you can take a look at https://forum.faforever.com/topic/12/linux-support

@jnorthrup
Copy link

jnorthrup commented Feb 27, 2022 via email

@Brutus5000
Copy link
Member

GraalVM is not approachable and never will be. Even for simple backend Spring apps it mostly doesn't work yet.

We can't guarantee Java17 to work because our test suite fails on 17.

@Sheikah45
Copy link
Member Author

Superceded by #2623

@Sheikah45 Sheikah45 closed this Mar 20, 2022
@Sheikah45 Sheikah45 deleted the maintenance/java17 branch March 21, 2022 16:28
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.

Support Java 16+

3 participants