feat: Server library cleanup on paper install

[harrisonablack]

Adds support for itzg/mc-image-helper#748

Adds flag $CLEAN_SERVER_LIBRARIES which defaults to true. When set appends --clean-libraries to mc-image-helper call.

master already sets MC_HELPER_VERSION=1.57.2 > minimum 1.57.1 for the feature.

Two questions, do you want the feature true or false by default? And should documentation be added iteratively on each jar type support, or done at the end? if iteratively, added under Misc Options or would you suggest a better location?

[harrisonablack]

One thing of note, i found the warn on "Failed to read installed libraries" when initially installing noisy and unnecessary

mc-1  | [init] Changing ownership of /data to 1000 ...
mc-1  | [init] Running as uid=1000 gid=1000 with /data as 'drwxr-xr-x 2 1000 1000 64 May  7 12:59 /data'
mc-1  | [init] Image info: buildtime=local,version=local,revision=local
mc-1  | [init] Resolving type given paper
mc-1  | [mc-image-helper] 13:00:00.105 INFO  : Downloaded /data/paper-1.19.2-307.jar
mc-1  | [mc-image-helper] 13:00:00.112 WARN  : Failed to read installed libraries
mc-1  | java.nio.file.NoSuchFileException: /data/libraries
mc-1  | 	at java.base/sun.nio.fs.UnixException.translateToIOException(Unknown Source)
mc-1  | 	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(Unknown Source)

I realised however a check can be performed if the manifest exists from a previous install.
If the manifest exists, check for the libraries, if not safely skip as this will be an initial install.

That way the warn can be removed, which isn't really helpfull.
Can get a pr for this before proceeding with mc-image-helper support for more types

[itzg]

i found the warn on "Failed to read installed libraries" when initially installing noisy and unnecessary

Yep, I was wondering about some of these scenarios. Yes, a PR to address that first would be great. I hadn't yet cut a release for Purpur library cleaning, so that can be included at same time.

[itzg]

Two questions, do you want the feature true or false by default?

The way you have it, true by default makes sense to me. Then setting it to false can be a way to mitigate any unexpected behavior.

And should documentation be added iteratively on each jar type support, or done at the end? if iteratively, added under Misc Options or would you suggest a better location?

I like the non-specific variable name you introduced since it can be utilized across server types. As such, yeah that misc options area seems good.

[harrisonablack]

i found the warn on "Failed to read installed libraries" when initially installing noisy and unnecessary

Yep, I was wondering about some of these scenarios. Yes, a PR to address that first would be great. I hadn't yet cut a release for Purpur library cleaning, so that can be included at same time.

itzg/mc-image-helper#759

[harrisonablack]

Just added documentation under misc-options

Anything further needed for the PR?

[itzg]

All looks good. Thanks.

[itzg]

Ah, java8 test fails because I maintain a separate mc-image-helper branch and have pinned the compiler at Java 8 there. I now realize it makes a mess of functional changes like this.

In this case, cherry picking into https://github.com/itzg/mc-image-helper/tree/java8 and bumping the release for that will ultimately work out. It seems annoying for me to do it that way...

[itzg]

Maybe the image also needs a slow changing, maintenance java8 branch.

[harrisonablack]

Ah, java8 test fails because I maintain a separate mc-image-helper branch and have pinned the compiler at Java 8 there. I now realize it makes a mess of functional changes like this.

How does the versioning system of the Java8 branch actually work?

docker-minecraft-server/.github/workflows/build.yml

Line 95 in 5015523

mcHelperVersion: 1.51.1

I can see in the build CI pipeline the pinned version, but how is this version differentiated from master releases?

From my understanding, pinning the version at 1.51.1 is effectively saying "this release is compatible with and able to be built with java 8".

I feel like it would be more appropriate to follow x.x.x-java8 version tagging for legacy / maintenance, this would also support future maintenance branches, like the mention of Java11 and can decouple the numbering from the main release if you only wants java 8/11 to target maintenance and no feature releases

[itzg]

I feel like it would be more appropriate to follow x.x.x-java8 version tagging for legacy / maintenance, this would also support future maintenance branches, like the mention of Java11 and can decouple the numbering from the main release if you only wants java 8/11 to target maintenance and no feature releases

Sorry, I just now noticed this. I like this idea.

[harrisonablack]

Do you need anything further for the pull into master? itzg/mc-image-helper#766 was merged and the java8 version bump #4056, so if CI re-runs should pass all builds. Otherwise i'm happy to continue further supporting other jar types

[itzg]

I can see in the build CI pipeline the pinned version, but how is this version differentiated from master releases?

All image variants are built from the same mainline branch. Previously I got lucky and there weren't any features that were affected. Yours and another coming up for OCI support have caused my luck to run out 😄

To address that, I have added a new subcommand in https://github.com/itzg/mc-image-helper/releases/tag/1.51.3-java8 and https://github.com/itzg/mc-image-helper/releases/tag/1.58.0 :

https://github.com/itzg/mc-image-helper#has-feature

that will allow for a single mainline branch in https://github.com/itzg/docker-minecraft-server but allow the scripts to query if a feature is available, such as the new subcommand in this PR. For the Java 8 - 16 image variants, using the https://github.com/itzg/mc-image-helper/releases/tag/1.51.3-java8 type releases, that query will return exit code 1 since we won't backport things that required Java 17. I'm thinking the script call point can gracefully fail the startup indicating that the requested feature isn't available for that image java variant.

[itzg]

...I can make that change in a separate PR or push to yours, since this is a newly added thing.

[itzg]

I can see in the build CI pipeline the pinned version, but how is this version differentiated from master releases?

All image variants are built from the same mainline branch. Previously I got lucky and there weren't any features that were affected. Yours and another coming up for OCI support have caused my luck to run out 😄

To address that, I have added a new subcommand in https://github.com/itzg/mc-image-helper/releases/tag/1.51.3-java8 and https://github.com/itzg/mc-image-helper/releases/tag/1.58.0 :

https://github.com/itzg/mc-image-helper#has-feature

that will allow for a single mainline branch in https://github.com/itzg/docker-minecraft-server but allow the scripts to query if a feature is available, such as the new subcommand in this PR. For the Java 8 - 16 image variants, using the https://github.com/itzg/mc-image-helper/releases/tag/1.51.3-java8 type releases, that query will return exit code 1 since we won't backport things that required Java 17. I'm thinking the script call point can gracefully fail the startup indicating that the requested feature isn't available for that image java variant.

nm, all this. I forgot that you had cherry picked the feature over to java8 branch of mc-image-helper.

[itzg]

Remaining build failure is just GitHub rate limit since I end up with so many concurrent builds with this image.