Update ServerInstaller.java #127
Conversation
Added a warning message for installation via sudo.
|
What "issues with the application" do you mean? There's obviously some problems if the user then runs the server as non-root due to file permissions, but that's to be expected. |
|
I just worked on the issue #112 |
|
Yeah, this is something that we should warn about. |
There was a problem hiding this comment.
This should be done for the client install only as thats where most of the issues come from. I would take a look at how the client installer shows a warning when the vanilla launcher is open.
It may also be a nice idea to show the same warning when running as an admin on windows?
I would also recommended building your PR to make sure that it passes the checkstyle rules. 👍 Let me know if you have any questions.
| public static boolean isSudo() { | ||
| String user = System.getProperty("user.name"); | ||
| return "root".equals(user); | ||
| } |
There was a problem hiding this comment.
I guess this is fine, maybe just limit it to macos and Linux?
| @@ -115,7 +142,8 @@ public static void install(Path dir, LoaderVersion loaderVersion, String gameVer | |||
| Path libraryFile = libsDir.resolve(library.getPath()); | |||
|
|
|||
| if (library.inputPath == null) { | |||
| progress.updateProgress(new MessageFormat(Utils.BUNDLE.getString("progress.download.library.entry")).format(new Object[]{library.name})); | |||
| progress.updateProgress(new MessageFormat(Utils.BUNDLE.getString("progress.download.library.entry")) | |||
There was a problem hiding this comment.
Dont change lines you havent changed.
| libraries.add(new Library(String.format("net.fabricmc:fabric-loader:%s", loaderVersion.name) | ||
| , null, loaderVersion.path)); | ||
| libraries.add(new Library(String.format("net.fabricmc:intermediary:%s", gameVersion) | ||
| , "https://maven.fabricmc.net/", null)); |
| Json json = FabricService.queryMetaJson(String.format("v2/versions/loader/%s/%s/server/json" | ||
| , gameVersion | ||
| , loaderVersion.name)); |
| progress.updateProgress(new MessageFormat(Utils.BUNDLE.getString("progress.installing.server")) | ||
| .format(new Object[]{String.format("%s(%s)", loaderVersion.name, gameVersion)})); |
| public static void install(Path dir | ||
| , LoaderVersion loaderVersion | ||
| , String gameVersion | ||
| , InstallerProgress progress) throws IOException { |
There was a problem hiding this comment.
This is very weird formatting, why change it? Please make sure it passes the checkstyle checks.
There was a problem hiding this comment.
Sorry, actually while I code I have a habit of wanting everything in front of my eyes and hate to scroll sideways. In the case of previous formatting, I had to scroll sideways, so I changed it.
| , String gameVersion | ||
| , InstallerProgress progress) throws IOException { | ||
|
|
||
| Scanner sc = new Scanner(System.in); |
There was a problem hiding this comment.
No, this will basically break when installing via the GUI. It should show a warning message box. Take a look at the client one for when the launcher is already open.
There was a problem hiding this comment.
No problem, I will improve the code according to what is needed.
| System.out.println("WARNING: Running this installer with sudo might cause issues with the application."); | ||
| System.out.print("Do you really want to continue? (y/n): "); |
There was a problem hiding this comment.
Could you please describe it a bit in detail.
| } else { | ||
| System.out.println("Invalid Response!!"); | ||
| } |
There was a problem hiding this comment.
Just assume the user aborted it. This will be removed anyway as a Scanner isnt the right solution
| install(dir, loaderVersion, gameVersion, progress, launchJar); | ||
| } else if (RESPONSE_TO_OPTION.equalsIgnoreCase("n")){ | ||
| throw new RuntimeException("Installation aborted by the user."); | ||
| } else { | ||
| System.out.println("Invalid Response!!"); | ||
| } | ||
| } else { | ||
| install(dir, loaderVersion, gameVersion, progress, launchJar); |
There was a problem hiding this comment.
You could clean this code up a lot to remove the 2 calls to install.
if (isSudo() && !proceedWithSudoInstall()) {
return;
}
install(dir, loaderVersion, gameVersion, progress, launchJar);
Added a warning message for installation via sudo.