Fixes for duplicate jvm options

OpenLiberty · Dec 8, 2023 · bd15ef6 · bd15ef6
1 parent 03a2f0c
commit bd15ef6
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 13 deletions.
diff --git a/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml b/liberty-maven-plugin/src/it/dev-it/resources/basic-dev-project/pom.xml
@@ -13,6 +13,7 @@
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <maven.compiler.source>1.8</maven.compiler.source>
     <maven.compiler.target>1.8</maven.compiler.target>
+    <liberty.jvm.minheap>-Xms512m</liberty.jvm.minheap>
     <app.name>LibertyProject</app.name>
     <!-- tag::ports[] -->
     <testServerHttpPort>9080</testServerHttpPort>
@@ -212,6 +213,9 @@
             <default.https.port>${testServerHttpsPort}</default.https.port>
             <com.ibm.ws.logging.message.format>json</com.ibm.ws.logging.message.format>
           </bootstrapProperties>
+          <jvmOptions>
+            <param>-Xms512m</param>
+         </jvmOptions>
           <!-- ADDITIONAL_CONFIGURATION -->
         </configuration>
       </plugin>

diff --git a/...n/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java b/...n/src/it/dev-it/src/test/java/net/wasdev/wlp/test/dev/it/DevCopyTestDependenciesTest.java
@@ -42,7 +42,7 @@ public static void setUpBeforeClass() throws Exception {
 
       // add new parameter in first argument to skip install features on restart
       // in this case, it should not skip install feature because Liberty was not previously installed
-      startProcess("-DskipInstallFeature=true", true);
+      startProcess("-DskipInstallFeature=true -Dliberty.jvm.minHeapSize=-Xms512m", true);
    }
 
    @AfterClass
@@ -57,10 +57,32 @@ public void copyDependenciesTest() throws Exception {
 
       File f = new File(targetDir, "liberty/wlp/usr/servers/defaultServer/lib/global/postgresql-42.1.1.jar");
       assertFalse(f.exists());
+   }
 
+   @Test
+   public void skipInstallFeatureTest() throws Exception {
       assertTrue("The install-feature goal did not run: "+getLogTail(), verifyLogMessageExists("Running liberty:install-feature", 2000));
       assertFalse("The skip install-feature log message was found unexpectedly: "+getLogTail(), verifyLogMessageExists("Skipping liberty:install-feature", 2000));
-
    }
 
+   @Test
+   public void duplicatJvmOptionsTest() throws Exception {
+        File jvmOptionsFile = new File(tempProj,"/target/liberty/wlp/usr/servers/defaultServer/jvm.options");
+        assertTrue("The jvm.options file not found at path: "+jvmOptionsFile.getAbsolutePath(),jvmOptionsFile.exists());
+
+        String fileContents = org.codehaus.plexus.util.FileUtils.fileRead(jvmOptionsFile.getCanonicalPath()).replaceAll("\r","");
+
+        String[] fileContentsArray = fileContents.split("\\n");
+        assertTrue("fileContents", fileContentsArray.length == 2);
+
+        for (int i=0; i < fileContentsArray.length; i++) {
+            String nextLine = fileContentsArray[i];
+            // verify that a single -Xms512m is the only entry in the jvm.options file.
+            if (i == 0) {
+                assertTrue("comment not found on first line", nextLine.equals("# Generated by liberty-maven-plugin"));
+            } else if (i == 1) {
+                assertTrue("-Xms512m not found on last line", nextLine.equals("-Xms512m"));
+            }
+        }
+    }    
 }
diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties b/liberty-maven-plugin/src/it/server-param-pom-override-it/invoker.properties
@@ -1,2 +1,2 @@
 # prove that Liberty configuration provided by maven properties can be overridden on cmd line
-invoker.goals.1 = clean install -Dliberty.var.someVariable2=myCmdLineValue -Dliberty.var.new.CmdLine.Var=newCmdLineValue
+invoker.goals.1 = clean install -Dliberty.var.someVariable2=myCmdLineValue -Dliberty.var.new.CmdLine.Var=newCmdLineValue -Dliberty.jvm.minHeapSize=-Xms512m
diff --git a/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options b/liberty-maven-plugin/src/it/server-param-pom-override-it/src/main/liberty/config/jvm.options
@@ -1 +1,2 @@
--Xmx1024m
+-Xmx2056m
+-Xms512m
diff --git a/...-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java b/...-param-pom-override-it/src/test/java/net/wasdev/wlp/maven/test/app/PluginConfigXmlIT.java
@@ -170,10 +170,15 @@ public void testJvmOptionsFileElements() throws Exception {
                     assertTrue("-Xmx768m not found on last line", nextLine.equals("-Xmx768m"));
                 } else {
                     if (nextLine.equals("-Xms512m")) {
+                        assertFalse("jvm option found more than once: "+nextLine,myXms512mFound);
                         myXms512mFound = true;
                     } else if (nextLine.equals("-Xmx1024m")) {
+                        assertFalse("jvm option found more than once: "+nextLine,myXmx1024mFound);
                         myXmx1024mFound = true;
+                    } else if (nextLine.equals("-Xmx2056m")) {
+                        fail("jvm option found that should not be present: "+nextLine);
                     } else if (nextLine.equals("-javaagent:/path/to/some/jar.jar")) {
+                        assertFalse("jvm option found more than once: "+nextLine,myArgLineValueFound);
                         myArgLineValueFound = true;
                     } else if (nextLine.equals("@{undefined}")) {
                         myUndefinedVarFound = true;

diff --git a/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java b/liberty-maven-plugin/src/main/java/io/openliberty/tools/maven/BasicSupport.java
@@ -718,14 +718,16 @@ protected Map<String,File> getLibertyDirectoryPropertyFiles() {
 
     protected void setContainerEngine(AbstractContainerSupportUtil util) throws PluginExecutionException {
         String LIBERTY_DEV_PODMAN = "liberty.dev.podman";
-        Properties mergedProperties = project.getProperties();
-        mergedProperties.putAll(System.getProperties()); //System/command line properties will take precedence
-        if (!mergedProperties.isEmpty() && mergedProperties.containsKey(LIBERTY_DEV_PODMAN)) {
-            Object isPodman = mergedProperties.get(LIBERTY_DEV_PODMAN);
-            if (isPodman != null) {
-                util.setIsDocker(!(Boolean.parseBoolean(isPodman.toString())));
-                getLog().debug("liberty.dev.podman was set to: " + (Boolean.parseBoolean(isPodman.toString())));
-            }
+        Object podmanPropValue = null;
+        if (System.getProperties().containsKey(LIBERTY_DEV_PODMAN)) {
+            podmanPropValue = System.getProperties().get(LIBERTY_DEV_PODMAN);
+        } else if (project.getProperties().containsKey(LIBERTY_DEV_PODMAN)) {
+            podmanPropValue = project.getProperties().get(LIBERTY_DEV_PODMAN);
+        }
+
+        if (podmanPropValue != null) {
+            util.setIsDocker(!(Boolean.parseBoolean(podmanPropValue.toString())));
+            getLog().debug("liberty.dev.podman was set to: " + (Boolean.parseBoolean(podmanPropValue.toString())));
         }
     }
 }
diff --git a/...y-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java b/...y-maven-plugin/src/main/java/io/openliberty/tools/maven/server/StartDebugMojoSupport.java
@@ -824,7 +824,7 @@ private void loadLibertyConfigFromProperties(Properties props) {
                                      break;
                     case BOOTSTRAP:  bootstrapMavenProps.put(suffix, value);
                                      break;
-                    case JVM:        jvmMavenProps.add(value);
+                    case JVM:        if (!jvmMavenProps.contains(value)) { jvmMavenProps.add(value); } // avoid exact duplicates
                                      break;
                     case VAR:        varMavenProps.put(suffix, value);
                                      break;
@@ -952,6 +952,7 @@ private void writeJvmOptions(File file, List<String> options, List<String> maven
                 combinedJvmOptions = new ArrayList<String> ();
                 // add the maven properties first so that they do not take precedence over the options specified with jvmOptions
                 combinedJvmOptions.addAll(mavenProperties);
+                combinedJvmOptions.removeAll(options); // remove any exact duplicates before adding all the jvmOptions
                 combinedJvmOptions.addAll(options);
             }
         } else {