Windows service installation should allow multiple values in ES_JAVA_OPTS (#64254)

* Add tests for using ES_JAVA_OPTS with windows service

* Relocate ES_JAVA_OPTS delimiter munging

* Don't use equals for -Xmx and -Xms args

* Write newlines in temporary configs

distribution/src/bin/elasticsearch-service.bat

@@ -120,9 +120,6 @@ rem     jvm.options.d/*.options
 rem   - third, JVM options from ES_JAVA_OPTS are applied
 rem   - fourth, ergonomic JVM options are applied
 
-if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS: =;%
-if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS:;;=;%
-
 @setlocal
 for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_PATH_CONF!" "!ES_HOME!"/plugins ^|^| echo jvm_options_parser_failed`) do set ES_JAVA_OPTS=%%a
 @endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%ES_JAVA_OPTS%" & set ES_JAVA_OPTS=%ES_JAVA_OPTS%
@@ -131,7 +128,14 @@ if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
   exit /b 1
 )
 
-if not "%ES_JAVA_OPTS%" == "" set ES_JAVA_OPTS=%ES_JAVA_OPTS: =;%
+rem The output of the JVM options parses is space-delimited. We need to
+rem convert to semicolon-delimited and avoid doubled semicolons.
+@setlocal
+if not "%ES_JAVA_OPTS%" == "" (
+  set ES_JAVA_OPTS=!ES_JAVA_OPTS: =;!
+  set ES_JAVA_OPTS=!ES_JAVA_OPTS:;;=;!
+)
+@endlocal & set ES_JAVA_OPTS=%ES_JAVA_OPTS%
 
 if "%ES_JAVA_OPTS:~-1%"==";" set ES_JAVA_OPTS=%ES_JAVA_OPTS:~0,-1%
 

qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java

@@ -36,6 +36,7 @@ import java.util.Arrays;
 import static com.carrotsearch.randomizedtesting.RandomizedTest.assumeTrue;
 import static org.elasticsearch.packaging.util.Archives.installArchive;
 import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
+import static org.elasticsearch.packaging.util.FileUtils.append;
 import static org.elasticsearch.packaging.util.FileUtils.mv;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -271,10 +272,27 @@ public class WindowsServiceTests extends PackagingTestCase {
         assertThat(result.stdout, containsString("Unknown option \"bogus\""));
     }
 
+    public void test80JavaOptsInEnvVar() throws Exception {
+        sh.getEnv().put("ES_JAVA_OPTS", "-Xmx2g -Xms2g");
+        sh.run(serviceScript + " install");
+        assertCommand(serviceScript + " start");
+        assertStartedAndStop();
+        sh.getEnv().remove("ES_JAVA_OPTS");
+    }
+
+    public void test81JavaOptsInJvmOptions() throws Exception {
+        withCustomConfig(tempConf -> {
+            append(tempConf.resolve("jvm.options"), "-Xmx2g" + System.lineSeparator());
+            append(tempConf.resolve("jvm.options"), "-Xms2g" + System.lineSeparator());
+            sh.run(serviceScript + " install");
+            assertCommand(serviceScript + " start");
+            assertStartedAndStop();
+        });
+    }
+
     // TODO:
     // custom SERVICE_USERNAME/SERVICE_PASSWORD
     // custom SERVICE_LOG_DIR
     // custom LOG_OPTS (looks like it currently conflicts with setting custom log dir)
-    // install and run with java opts
     // install and run java opts Xmx/s (each data size type)
 }