[FIXED JENKINS-29577] Ensure classpath is the first option

jenkinsci · Jul 24, 2015 · 031b83f · 031b83f
1 parent cff114d
commit 031b83f
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 14 deletions.
diff --git a/src/main/java/hudson/plugins/groovy/Groovy.java b/src/main/java/hudson/plugins/groovy/Groovy.java
@@ -25,6 +25,7 @@
 import net.sf.json.JSONObject;
 
 import org.apache.commons.exec.CommandLine;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.StaplerRequest;
 
@@ -89,20 +90,14 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
                     envVars.put(e.getKey(),e.getValue());
                 }
 
-                if(properties != null) {
-                    String origJavaOpts = build.getBuildVariables().get("JAVA_OPTS");
-                    StringBuffer javaOpts = new StringBuffer((origJavaOpts != null) ? origJavaOpts : "");
-                    for (Entry<Object, Object> entry : parseProperties(properties).entrySet()) {
-                        cmd.add(1, "-D" + entry.getKey() + "=" + entry.getValue());
-                    }
-
-                    //Add javaOpts at the end
-                    if(this.javaOpts != null) //backward compatibility
-                        javaOpts.append(" " + this.javaOpts);
-
-                    envVars.put("JAVA_OPTS", javaOpts.toString());
-                }
-                envVars.put("$PATH_SEPARATOR",":::");
+                String origJavaOpts = build.getBuildVariables().get("JAVA_OPTS");
+                StringBuilder javaOpts = new StringBuilder((origJavaOpts != null) ? origJavaOpts : "");
+                //Add javaOpts at the end
+                if(this.javaOpts != null) //backward compatibility
+                    javaOpts.append(' ').append(this.javaOpts);
+                envVars.put("JAVA_OPTS", javaOpts.toString());
+
+                envVars.put("$PATH_SEPARATOR",":::"); //TODO why??
 
                 result = launcher.launch().cmds(cmd.toArray(new String[] {})).envs(envVars).stdout(listener).pwd(ws).join();
             } catch (IOException e) {
@@ -280,6 +275,13 @@ protected List<String> buildCommandLine(AbstractBuild<?,?> build, BuildListener
             }
             list.add(sb.toString());
         }
+
+        //Add java properties
+        if(StringUtils.isNotBlank(properties)) {
+            for (Entry<Object, Object> entry : parseProperties(properties).entrySet()) {
+                list.add("-D" + entry.getKey() + "=" + entry.getValue());
+            }
+        }
 
         //Add groovy parameters
         if(parameters != null && !parameters.isEmpty()) {

diff --git a/src/test/java/hudson/plugins/groovy/ClassPathTest.java b/src/test/java/hudson/plugins/groovy/ClassPathTest.java
@@ -5,12 +5,14 @@
 import hudson.model.Result;
 import hudson.model.FreeStyleProject;
 
+import java.io.File;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 
 public class ClassPathTest {
@@ -38,6 +40,23 @@ public void testDirectoryOnClassPath() throws Exception {
         assertTrue(containsString(p.scheduleBuild2(0).get().getLog(100), testJar));
     }
 
+    @Issue("JENKINS-29577")
+    @Test
+    public void testClassPathAndProperties() throws Exception {
+        final String testJar = "groovy-cp-test.jar";
+        StringBuilder sb = new StringBuilder();
+        final ScriptSource script = new StringScriptSource(
+                sb.append("import App\n")
+                .append("println System.getProperty(\"aaa\")")
+                .toString()
+                );
+        Groovy g = new Groovy(script,"(Default)", "", "","aaa=\"bbb\"", "", this.getClass().getResource("/lib").getPath() + File.separator + testJar);
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.getBuildersList().add(g);
+        assertEquals(Result.SUCCESS, p.scheduleBuild2(0).get(10,TimeUnit.SECONDS).getResult());
+        assertTrue(containsString(p.scheduleBuild2(0).get().getLog(100), "bbb"));
+    }
+
     private boolean containsString(List<String> input, String searchStr) {
         boolean isPresent = false;
         for(String str : input) {