From 61b5cfa733559b2c631bd25eec1b89e63ba76620 Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sun, 24 Oct 2021 14:11:01 +0100 Subject: [PATCH] Improve run config argument handling, should work with spaces a bit better. (#524) --- .../loom/configuration/ide/RunConfig.java | 57 +++++++++---------- .../fabricmc/loom/task/AbstractRunTask.java | 33 +---------- .../loom/task/GenVsCodeProjectTask.java | 4 +- .../test/integration/RunConfigTest.groovy | 1 + .../loom/test/unit/RunConfigUnitTest.groovy | 38 +++++++++++++ .../projects/runconfigs/build.gradle | 4 ++ .../java/net/fabricmc/example/ExampleMod.java | 2 + 7 files changed, 76 insertions(+), 63 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/RunConfigUnitTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java index c25dc583..3e23dec3 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java @@ -28,10 +28,11 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.Objects; -import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.gson.JsonElement; import com.google.gson.JsonObject; @@ -44,7 +45,6 @@ import org.w3c.dom.Node; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.configuration.InstallerData; -import net.fabricmc.loom.util.OperatingSystem; public class RunConfig { public String configName; @@ -53,8 +53,8 @@ public class RunConfig { public String mainClass; public String runDirIdeaUrl; public String runDir; - public String vmArgs; - public String programArgs; + public List vmArgs = new ArrayList<>(); + public List programArgs = new ArrayList<>(); public SourceSet sourceSet; public Element genRuns(Element doc) { @@ -65,12 +65,12 @@ public class RunConfig { this.addXml(root, "option", ImmutableMap.of("name", "MAIN_CLASS_NAME", "value", mainClass)); this.addXml(root, "option", ImmutableMap.of("name", "WORKING_DIRECTORY", "value", runDirIdeaUrl)); - if (!Strings.isNullOrEmpty(vmArgs)) { - this.addXml(root, "option", ImmutableMap.of("name", "VM_PARAMETERS", "value", vmArgs)); + if (!vmArgs.isEmpty()) { + this.addXml(root, "option", ImmutableMap.of("name", "VM_PARAMETERS", "value", joinArguments(vmArgs))); } - if (!Strings.isNullOrEmpty(programArgs)) { - this.addXml(root, "option", ImmutableMap.of("name", "PROGRAM_PARAMETERS", "value", programArgs)); + if (!programArgs.isEmpty()) { + this.addXml(root, "option", ImmutableMap.of("name", "PROGRAM_PARAMETERS", "value", joinArguments(programArgs))); } return root; @@ -106,11 +106,10 @@ public class RunConfig { private static void populate(Project project, LoomGradleExtension extension, RunConfig runConfig, String environment) { runConfig.configName += extension.isRootProject() ? "" : " (" + project.getPath() + ")"; runConfig.eclipseProjectName = project.getExtensions().getByType(EclipseModel.class).getProject().getName(); - runConfig.vmArgs = ""; - runConfig.programArgs = ""; runConfig.mainClass = "net.fabricmc.devlaunchinjector.Main"; - runConfig.vmArgs = "-Dfabric.dli.config=" + encodeEscaped(extension.getFiles().getDevLauncherConfig().getAbsolutePath()) + " -Dfabric.dli.env=" + environment.toLowerCase(); + runConfig.vmArgs.add("-Dfabric.dli.config=" + encodeEscaped(extension.getFiles().getDevLauncherConfig().getAbsolutePath())); + runConfig.vmArgs.add("-Dfabric.dli.env=" + environment.toLowerCase()); } // Turns camelCase/PascalCase into Capital Case @@ -165,19 +164,9 @@ public class RunConfig { runConfig.sourceSet = sourceSet; // Custom parameters - for (String progArg : settings.getProgramArgs()) { - runConfig.programArgs += " " + progArg; - } - - for (String vmArg : settings.getVmArgs()) { - runConfig.vmArgs += " " + vmArg; - } - - runConfig.vmArgs += " -Dfabric.dli.main=" + getMainClass(environment, extension, defaultMain); - - // Remove unnecessary leading/trailing whitespaces we might have generated - runConfig.programArgs = runConfig.programArgs.trim(); - runConfig.vmArgs = runConfig.vmArgs.trim(); + runConfig.programArgs.addAll(settings.getProgramArgs()); + runConfig.vmArgs.addAll(settings.getVmArgs()); + runConfig.vmArgs.add("-Dfabric.dli.main=" + getMainClass(environment, extension, defaultMain)); return runConfig; } @@ -204,18 +193,26 @@ public class RunConfig { dummyConfig = dummyConfig.replace("%ECLIPSE_PROJECT%", eclipseProjectName); dummyConfig = dummyConfig.replace("%IDEA_MODULE%", ideaModuleName); dummyConfig = dummyConfig.replace("%RUN_DIRECTORY%", runDir); - dummyConfig = dummyConfig.replace("%PROGRAM_ARGS%", programArgs.replaceAll("\"", """)); - dummyConfig = dummyConfig.replace("%VM_ARGS%", vmArgs.replaceAll("\"", """)); + dummyConfig = dummyConfig.replace("%PROGRAM_ARGS%", joinArguments(programArgs).replaceAll("\"", """)); + dummyConfig = dummyConfig.replace("%VM_ARGS%", joinArguments(vmArgs).replaceAll("\"", """)); return dummyConfig; } - public static String getOSClientJVMArgs() { - if (OperatingSystem.getOS().equalsIgnoreCase("osx")) { - return " -XstartOnFirstThread"; + public static String joinArguments(List args) { + final var sb = new StringBuilder(); + boolean first = true; + + for (String arg : args) { + if (!first) { + sb.append(" "); + } + + first = false; + sb.append("\"").append(arg).append("\""); } - return ""; + return sb.toString(); } private static String getMainClass(String side, LoomGradleExtension extension, String defaultMainClass) { diff --git a/src/main/java/net/fabricmc/loom/task/AbstractRunTask.java b/src/main/java/net/fabricmc/loom/task/AbstractRunTask.java index c93c85a4..6bb63e53 100644 --- a/src/main/java/net/fabricmc/loom/task/AbstractRunTask.java +++ b/src/main/java/net/fabricmc/loom/task/AbstractRunTask.java @@ -26,7 +26,6 @@ package net.fabricmc.loom.task; import java.io.File; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.function.Function; @@ -46,39 +45,11 @@ public abstract class AbstractRunTask extends JavaExec { this.config = configProvider.apply(getProject()); setClasspath(config.sourceSet.getRuntimeClasspath()); + args(config.programArgs); } @Override public void exec() { - List argsSplit = new ArrayList<>(); - String[] args = config.programArgs.split(" "); - int partPos = -1; - - for (int i = 0; i < args.length; i++) { - if (partPos < 0) { - if (args[i].startsWith("\"")) { - if (args[i].endsWith("\"")) { - argsSplit.add(args[i].substring(1, args[i].length() - 1)); - } else { - partPos = i; - } - } else { - argsSplit.add(args[i]); - } - } else if (args[i].endsWith("\"")) { - StringBuilder builder = new StringBuilder(args[partPos].substring(1)); - - for (int j = partPos + 1; j < i; j++) { - builder.append(" ").append(args[j]); - } - - builder.append(" ").append(args[i], 0, args[i].length() - 1); - argsSplit.add(builder.toString()); - partPos = -1; - } - } - - args(argsSplit); setWorkingDir(new File(getProject().getRootDir(), config.runDir)); super.exec(); @@ -102,7 +73,7 @@ public abstract class AbstractRunTask extends JavaExec { public List getJvmArgs() { List superArgs = super.getJvmArgs(); List args = new ArrayList<>(superArgs != null ? superArgs : Collections.emptyList()); - args.addAll(Arrays.asList(config.vmArgs.split(" "))); + args.addAll(config.vmArgs); return args; } } diff --git a/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java b/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java index 2837cac0..ab6fc5fa 100644 --- a/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java +++ b/src/main/java/net/fabricmc/loom/task/GenVsCodeProjectTask.java @@ -103,8 +103,8 @@ public class GenVsCodeProjectTask extends AbstractLoomTask { VsCodeConfiguration(RunConfig runConfig) { this.name = runConfig.configName; this.mainClass = runConfig.mainClass; - this.vmArgs = runConfig.vmArgs; - this.args = runConfig.programArgs; + this.vmArgs = RunConfig.joinArguments(runConfig.vmArgs); + this.args = RunConfig.joinArguments(runConfig.programArgs); this.cwd = "${workspaceFolder}/" + runConfig.runDir; if (getProject().getRootProject() != getProject()) { diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/RunConfigTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/RunConfigTest.groovy index e893c352..46d8b3a3 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/RunConfigTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/RunConfigTest.groovy @@ -42,6 +42,7 @@ class RunConfigTest extends Specification implements GradleProjectTestTrait { then: result.task(":${task}").outcome == SUCCESS + result.output.contains("This contains a space") where: task | _ diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/RunConfigUnitTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/RunConfigUnitTest.groovy new file mode 100644 index 00000000..8a804d71 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/RunConfigUnitTest.groovy @@ -0,0 +1,38 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2021 FabricMC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package net.fabricmc.loom.test.unit + +import net.fabricmc.loom.configuration.ide.RunConfig +import spock.lang.Specification + +class RunConfigUnitTest extends Specification { + def "escape arguments"() { + when: + def args = RunConfig.joinArguments(["-Dfabric.test=123", "-Dfabric.test=abc 123"]) + + then: + args == '"-Dfabric.test=123" "-Dfabric.test=abc 123"' + } +} diff --git a/src/test/resources/projects/runconfigs/build.gradle b/src/test/resources/projects/runconfigs/build.gradle index dbdd21e5..9ed945c7 100644 --- a/src/test/resources/projects/runconfigs/build.gradle +++ b/src/test/resources/projects/runconfigs/build.gradle @@ -21,6 +21,10 @@ loom { vmArg "-Dfabric.autoTest" } } + + runConfigs.configureEach { + vmArg "-Dfabric.loom.test.space=This contains a space" + } } archivesBaseName = "fabric-example-mod" diff --git a/src/test/resources/projects/runconfigs/src/main/java/net/fabricmc/example/ExampleMod.java b/src/test/resources/projects/runconfigs/src/main/java/net/fabricmc/example/ExampleMod.java index 21928e56..f756397f 100644 --- a/src/test/resources/projects/runconfigs/src/main/java/net/fabricmc/example/ExampleMod.java +++ b/src/test/resources/projects/runconfigs/src/main/java/net/fabricmc/example/ExampleMod.java @@ -7,6 +7,8 @@ public class ExampleMod implements ModInitializer { public void onInitialize() { System.out.println("Hello Fabric world!"); + System.out.println(System.getProperty("fabric.loom.test.space")); + // Quit now, we dont need to load the whole game to know the run configs are works System.exit(0); }