What's the motivation for this? Was there a problem without this patch?

Actually it seems that I can run minecraft without the JAVA_HOME line at all, so we should probably remove it.

What's the motivation for this? Was there a problem without this patch?

When launching Minecraft through the minecraft-launcher it to run java. This patch fixes the value of JAVA_HOME set in the wrapper, so that minecraft-launcher can use the appropriate JRE.

Unable to locate the Java runtime.
Error details: Success
Filename on disk: java
Path: /nix/store/gn29h5q4jkwqh634r2aw38vj2hcs17xw-openjdk-14.0.2-ga/lib/openjdk/bin/bin/java
Exists: Nonexistent

Actually it seems that I can run minecraft without the JAVA_HOME line at all, so we should probably remove it.

When I removed the JAVA_HOME line from the wrapper completely, minecraft-launcher seems to try and use /usr/bin/java which also doesn't exist.

Unable to locate the Java runtime.
Error details: Success
Filename on disk: java
Path: /usr/bin/java
Exists: Nonexistent

With no java installed globally on my system, the JAVA_HOME env in the wrapper is necessary for minecraft-launcher to detect which JRE to use.

Hm I can't reproduce this. For the parent commit of this PR (336fd62) I can build and run minecraft without problems:

$ nix-build -A minecraft
/nix/store/ndrz8jb5bjdg54102vpprd5ymm6l3v2b-minecraft-launcher-2.2.741
$ /nix/store/ndrz8jb5bjdg54102vpprd5ymm6l3v2b-minecraft-launcher-2.2.741/bin/minecraft-launcher
[1206/222512.902949:INFO:main_context.cpp(107)] CEF initialized successfully.

In the launcher, I hit "Play" with the latest version, which runs just fine, and I don't have any java installed globally.

Can you confirm that you tested with the exact same store path? If you get different behavior, something weird is going on. Also, let's compare nix-info -m output, mine is:

• system: "x86_64-linux"
• host os: Linux 5.4.70, NixOS, 21.03pre-git (Okapi)
• multi-user?: yes
• sandbox: yes
• version: nix-env (Nix) 2.3.7
• nixpkgs: /etc/nixpkgs

That version works for me too.

$ nix-info -m
warning: unknown setting 'extra-sandbox-paths'
 - system: `"x86_64-linux"`
 - host os: `Linux 5.9.8, NixOS, 21.03.20201116.4f3475b (Okapi)`
 - multi-user?: `yes`
 - sandbox: `yes`
warning: unknown setting 'extra-sandbox-paths'
 - version: `nix-env (Nix) 2.4pre20201102_550e11f`
warning: unknown setting 'extra-sandbox-paths'
 - channels(ryantm): `""`
warning: unknown setting 'extra-sandbox-paths'
 - channels(root): `"nixos-19.03pre166987.bc41317e243"`
warning: unknown setting 'extra-sandbox-paths'
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

Hm I can't reproduce this. For the parent commit of this PR (336fd62) I can build and run minecraft without problems:

$ nix-build -A minecraft
/nix/store/ndrz8jb5bjdg54102vpprd5ymm6l3v2b-minecraft-launcher-2.2.741
$ /nix/store/ndrz8jb5bjdg54102vpprd5ymm6l3v2b-minecraft-launcher-2.2.741/bin/minecraft-launcher
[1206/222512.902949:INFO:main_context.cpp(107)] CEF initialized successfully.

Building from 336fd62 gives me the same store path (/nix/store/ndrz8jb5bjdg54102vpprd5ymm6l3v2b-minecraft-launcher-2.2.741). The launcher starts and works, but when hitting Play on minecraft 1.16.4 it fails to launch.

In the launcher, I hit "Play" with the latest version, which runs just fine, and I don't have any java installed globally.

Did you by any chance have set a java runtime in the minecraft launcher, for the minecraft version you run? I confirmed by issue with a clean system (no ~/.minecraft). After downloading 1.16.4 it fails to launch. With this patch JAVA_HOME is set correctly and it launches.

Can you confirm that you tested with the exact same store path? If you get different behavior, something weird is going on. Also, let's compare nix-info -m output, mine is:

Sure. With nixpkgs set to the parent commit, mine is:

 - system: `"x86_64-linux"`
 - host os: `Linux 5.9.12, NixOS, 21.03pre256292.296793637b2 (Okapi)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.3.9`
 - channels(bart): `""`
 - channels(root): `"home-manager, nixos-21.03pre256292.296793637b2"`
 - nixpkgs: `/nix/store/1pz6mkrv78hxa8gk9jfavin0k7skqpz1-336fd6292016e41140c29b1657545071db87e6fd.tar.gz`

Besides the point that for me the launcher is broken without a correct JAVA_HOME, I feel that setting JAVA_HOME in the wrapper is necessary to ensure minecraft uses the JRE set by the derivation, and makes overrides work. Other java based programs (e.g. maven [1]) set JAVA_HOME too in their wrapper.

[1] - https://github.com/NixOS/nixpkgs/blob/336fd6292016e41140c29b1657545071db87e6fd/pkgs/development/tools/build-managers/apache-maven/builder.sh

I also can't reproduce this issue, with a clean $HOME (no .minecraft) and no java on the system. And I confirm that it runs for me without JAVA_HOME set. Digging some more, I see that this was introduced in a8edf35, and from the commit log it looks like it was added to improve purity, not because it was needed.
For bonus, I tried to remove java from the PATH in the wrapper, and it fails to start unless I apply your change :)
So while I agree that your fix is valid, it would be nice to understand why only you seem to require it. Can you double check your environment? Are you using some kind of modding that might affect this?

I also can't reproduce this issue, with a clean $HOME (no .minecraft) and no java on the system. And I confirm that it runs for me without JAVA_HOME set.

Been doing some more investigation on this. Normally I run minecraft in a sandbox (with firejail), and it turns out it's due to firejail restrictions that the launcher cannot use java from the path. It does some weird logic for finding a java install (rather than just running java) and that is blocked by firejail.

Sorry for not testing it unsandboxed before, I could have figured this out earlier.

So while I agree that your fix is valid, it would be nice to understand why only you seem to require it.

Agreed. For purity the patch makes sense. As a bonus for me it runs in a sandbox with this patch too :)

Again sorry for not realizing this earlier on. Thank you all for helping trace this in the end.