Merge branch 'master' into logging-exMTL

jimklimov · web-flow · commit d781a3bb40fe · 2024-04-24T11:26:15.000+02:00
diff --git a/dgm-builder/src/main/java/com/cloudbees/groovy/cps/tool/Translator.java b/dgm-builder/src/main/java/com/cloudbees/groovy/cps/tool/Translator.java
@@ -169,7 +169,7 @@ private String mangledName(ExecutableElement e) {
      */
     public void translate(String fqcn, String outfqcn, String sourceJarName) throws JClassAlreadyExistsException {
         final JDefinedClass $output = codeModel._class(outfqcn);
-        $output.annotate(Generated.class).param("value", Translator.class.getName()).param("date", new Date().toString()).param("comments", "based on " + sourceJarName);
+        $output.annotate(Generated.class).param("value", Translator.class.getName()).param("comments", "based on " + sourceJarName);
         $output.annotate(SuppressWarnings.class).param("value", "rawtypes");
         $output.constructor(JMod.PRIVATE);
 
diff --git a/lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java b/lib/src/main/java/com/cloudbees/groovy/cps/Continuable.java
@@ -23,6 +23,9 @@
  */
 public class Continuable implements Serializable {
 
+    /**
+     * Users of this library must pass (at least) these to {@link GroovyCategorySupport#use(List, Closure)} during all operations.
+     */
     @SuppressWarnings("rawtypes")
     public static final List<Class> categories = List.of(
         CpsDefaultGroovyMethods.class,
@@ -133,31 +136,21 @@ public Object runByThrow(Throwable arg) throws InvocationTargetException {
         return run0(new Outcome(null,arg)).wrapReplay();
     }
 
-    @Deprecated
-    public Outcome run0(final Outcome cn) {
-        return run0(cn, categories);
-    }
-
     /**
      * Resumes this program by either returning the value from {@link Continuable#suspend(Object)} or
      * throwing an exception
      */
-    public Outcome run0(final Outcome cn, List<Class> categories) {
-        return GroovyCategorySupport.use(categories, new Closure<>(null) {
-            @Override
-            public Outcome call() {
-                Next n = cn.resumeFrom(e,k);
-
-                while(n.yield==null) {
-                    n = n.step();
-                }
-
-                e = n.e;
-                k = n.k;
-
-                return n.yield;
-            }
-        });
+    public Outcome run0(final Outcome cn) {
+        Next n = cn.resumeFrom(e,k);
+
+        while(n.yield==null) {
+            n = n.step();
+        }
+
+        e = n.e;
+        k = n.k;
+
+        return n.yield;
     }
 
     /**
diff --git a/lib/src/main/java/com/cloudbees/groovy/cps/impl/ContinuationGroup.java b/lib/src/main/java/com/cloudbees/groovy/cps/impl/ContinuationGroup.java
@@ -21,7 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
-import javax.annotation.CheckReturnValue;
+import edu.umd.cs.findbugs.annotations.CheckReturnValue;
 import org.codehaus.groovy.runtime.InvokerInvocationException;
 import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
 import org.codehaus.groovy.runtime.callsite.CallSite;
diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/CpsDefaultGroovyMethodsTest.java
@@ -334,8 +334,6 @@ public static Iterable<Object[]> generateParameters() {
             asList("uniqueSet", "([1, 2, -2, 3] as HashSet).unique { i -> i * i }.collect { it.abs() } as HashSet", asList(1, 2, 3) as HashSet),
             */
 
-            // TODO: use?
-
             // TODO: with?
 
             // .withDefault
diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/CpsTransformerTest.java
@@ -769,7 +769,7 @@ public void rehydrateClosure() throws Throwable {
 
     @Issue("https://github.com/cloudbees/groovy-cps/issues/16")
     @Test
-    @Ignore
+    @Ignore("cannot easily be supported")
     public void category() throws Throwable {
         assertEvaluate("FOO",
             "class BarCategory {\n" +
diff --git a/lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java b/lib/src/test/java/com/cloudbees/groovy/cps/impl/NotBlockTest.java
@@ -7,7 +7,6 @@
 import groovy.lang.Script;
 import java.io.InputStream;
 import java.io.ObjectInputStream;
-import java.util.Collections;
 import org.junit.Test;
 
 import static org.hamcrest.CoreMatchers.equalTo;
@@ -38,6 +37,6 @@ public void serialFormBackwardsCompatibility() throws Throwable {
             c = (Continuable)ois.readObject();
         }
         assertTrue(c.isResumable());
-        assertThat(c.run0(new Outcome(false, null), Collections.emptyList()).replay(), equalTo(true));
+        assertThat(c.run0(new Outcome(false, null)).replay(), equalTo(true));
     }
 }
diff --git a/plugin/pom.xml b/plugin/pom.xml
@@ -247,7 +247,7 @@
         <dependency>
             <groupId>org.testcontainers</groupId>
             <artifactId>testcontainers</artifactId>
-            <version>1.19.6</version>
+            <version>1.19.7</version>
             <scope>test</scope>
             <exclusions>
                 <!-- Provided by Jenkins core -->
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecution.java
@@ -84,7 +84,6 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -256,7 +255,7 @@ public class CpsFlowExecution extends FlowExecution implements BlockableResume {
      * Any additional scripts {@linkplain CpsGroovyShell#parse(GroovyCodeSource) parsed} afterward, keyed by
      * their FQCN.
      */
-    /*package*/ /*final*/ Map<String,String> loadedScripts = new HashMap<>();
+    /*package*/ /*final*/ Map<String,String> loadedScripts = new LinkedHashMap<>();
 
     private final boolean sandbox;
     private transient /*almost final*/ FlowExecutionOwner owner;
@@ -706,12 +705,6 @@ private CpsScript parseScript() throws IOException {
         }
 
         s.execution = this;
-        if (false) {
-            System.out.println("scriptName="+s.getClass().getName());
-            System.out.println(List.of(s.getClass().getInterfaces()));
-            System.out.println(List.of(s.getClass().getDeclaredFields()));
-            System.out.println(List.of(s.getClass().getDeclaredMethods()));
-        }
         return s;
     }
 
@@ -1939,7 +1932,7 @@ public Object unmarshal(HierarchicalStreamReader reader, final UnmarshallingCont
                     }
 
                     if (result.loadedScripts == null) {
-                        result.loadedScripts = new HashMap<>();   // field added later
+                        result.loadedScripts = new LinkedHashMap<>(); // field added later
                     }
                     result.liveTimings = result.timings == null ?
                         new ConcurrentHashMap<>() :
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java
@@ -48,6 +48,8 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
+
 import jenkins.model.Jenkins;
 import jenkins.scm.api.SCMFileSystem;
 import jenkins.security.HMACConfidentialKey;
@@ -218,7 +220,8 @@ private String getFilePathSuffix() {
         public Collection<? extends SCMDescriptor<?>> getApplicableDescriptors() {
             StaplerRequest req = Stapler.getCurrentRequest();
             Job<?,?> job = req != null ? req.findAncestorObject(Job.class) : null;
-            return SCM._for(job);
+            return SCM._for(job).stream().filter(d -> !"org.jenkinsci.plugins.multiplescms.MultiSCM".equals(d.getId())).collect(Collectors.toList());
+
         }
 
         // TODO doCheckLightweight impossible to write even though we have SCMFileSystem.supports(SCM), because form validation cannot pass the SCM object
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsThread.java
@@ -26,7 +26,6 @@
 
 import com.cloudbees.groovy.cps.Continuable;
 import com.cloudbees.groovy.cps.Outcome;
-import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.FutureCallback;
 import java.io.IOException;
 import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
@@ -43,7 +42,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
-import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
 
 import static java.util.logging.Level.FINE;
 import static org.jenkinsci.plugins.workflow.cps.persistence.PersistenceContext.PROGRAM;
@@ -162,11 +160,6 @@ public StepExecution getStep() {
         this.step = step;
     }
 
-    private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
-            .addAll(Continuable.categories)
-            .add(IteratorHack.class)
-            .build();
-
     /**
      * Executes CPS code synchronously a little bit more, until it hits
      * the point the workflow needs to be dehydrated.
@@ -184,7 +177,7 @@ public StepExecution getStep() {
             LOGGER.fine(() -> "runNextChunk on " + resumeValue);
             final Outcome o = resumeValue;
             resumeValue = null;
-            outcome = program.run0(o, CATEGORIES);
+            outcome = program.run0(o);
             if (outcome.getAbnormal() != null) {
                 LOGGER.log(FINE, "ran and produced error", outcome.getAbnormal());
             } else {
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsVmExecutorService.java
@@ -1,8 +1,9 @@
 package org.jenkinsci.plugins.workflow.cps;
 
+import com.cloudbees.groovy.cps.Continuable;
 import com.cloudbees.groovy.cps.impl.CpsCallableInvocation;
+import com.google.common.collect.ImmutableList;
 import hudson.Main;
-import hudson.model.Computer;
 import hudson.remoting.SingleLaneExecutorService;
 import hudson.security.ACL;
 import java.io.IOException;
@@ -11,9 +12,21 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
+import groovy.lang.Closure;
 import hudson.model.TaskListener;
+import hudson.util.DaemonThreadFactory;
+import hudson.util.ExceptionCatchingThreadFactory;
+import hudson.util.NamingThreadFactory;
+import java.util.List;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ThreadFactory;
 import jenkins.model.Jenkins;
+import jenkins.security.ImpersonatingExecutorService;
+import jenkins.util.ContextResettingExecutorService;
+import jenkins.util.ErrorLoggingExecutorService;
 import jenkins.util.InterceptingExecutorService;
+import org.codehaus.groovy.runtime.GroovyCategorySupport;
+import org.jenkinsci.plugins.workflow.cps.persistence.IteratorHack;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.steps.StepExecution;
 
@@ -24,27 +37,55 @@
  * @see CpsVmThreadOnly
  */
 class CpsVmExecutorService extends InterceptingExecutorService {
+
+    @SuppressWarnings("rawtypes")
+    private static final List<Class> CATEGORIES = ImmutableList.<Class>builder()
+        .addAll(Continuable.categories)
+        .add(IteratorHack.class)
+        .build();
+
+    private static ThreadFactory categoryThreadFactory(ThreadFactory core) {
+        return r -> core.newThread(() -> {
+            LOGGER.fine("spawning new thread");
+            GroovyCategorySupport.use(CATEGORIES, new Closure<Void>(null) {
+                @Override
+                public Void call() {
+                    r.run();
+                    return null;
+                }
+            });
+        });
+    }
+
+    private static final ExecutorService threadPool = new ContextResettingExecutorService(
+        new ImpersonatingExecutorService(
+            new ErrorLoggingExecutorService(
+                Executors.newCachedThreadPool(
+                    categoryThreadFactory(
+                        new ExceptionCatchingThreadFactory(
+                            new NamingThreadFactory(
+                                new DaemonThreadFactory(),
+                                "CpsVmExecutorService"))))),
+            ACL.SYSTEM2));
+
     private CpsThreadGroup cpsThreadGroup;
 
-    public CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
-        super(new SingleLaneExecutorService(Computer.threadPoolForRemoting));
+    CpsVmExecutorService(CpsThreadGroup cpsThreadGroup) {
+        super(new SingleLaneExecutorService(threadPool));
         this.cpsThreadGroup = cpsThreadGroup;
     }
 
     @Override
     protected Runnable wrap(final Runnable r) {
-        return new Runnable() {
-            @Override
-            public void run() {
-                ThreadContext context = setUp();
-                try {
-                    r.run();
-                } catch (final Throwable t) {
-                    reportProblem(t);
-                    throw t;
-                } finally {
-                    tearDown(context);
-                }
+        return () -> {
+            ThreadContext context = setUp();
+            try {
+                r.run();
+            } catch (final Throwable t) {
+                reportProblem(t);
+                throw t;
+            } finally {
+                tearDown(context);
             }
         };
     }
@@ -89,18 +130,15 @@ private void reportProblem(Throwable t) {
 
     @Override
     protected <V> Callable<V> wrap(final Callable<V> r) {
-        return new Callable<>() {
-            @Override
-            public V call() throws Exception {
-                ThreadContext context = setUp();
-                try {
-                    return r.call();
-                } catch (final Throwable t) {
-                    reportProblem(t);
-                    throw t;
-                } finally {
-                    tearDown(context);
-                }
+        return () -> {
+            ThreadContext context = setUp();
+            try {
+                return r.call();
+            } catch (final Throwable t) {
+                reportProblem(t);
+                throw t;
+            } finally {
+                tearDown(context);
             }
         };
     }
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/SandboxContinuable.java
@@ -7,7 +7,6 @@
 import hudson.console.ConsoleAnnotator;
 import hudson.console.ConsoleNote;
 import java.io.IOException;
-import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox;
@@ -24,8 +23,7 @@ class SandboxContinuable extends Continuable {
     }
 
     @SuppressWarnings("rawtypes")
-    @Override
-    public Outcome run0(final Outcome cn, final List<Class> categories) {
+    public Outcome run0(final Outcome cn) {
         CpsFlowExecution e = thread.group.getExecution();
         if (e == null) {
             throw new IllegalStateException("JENKINS-50407: no loaded execution");
@@ -48,7 +46,7 @@ public Outcome run0(final Outcome cn, final List<Class> categories) {
             trustedShell.getClassLoader(),
             shell.getClassLoader()));
         try (GroovySandbox.Scope scope = sandbox.enter()) {
-            return SandboxContinuable.super.run0(cn, categories);
+            return SandboxContinuable.super.run0(cn);
         }
     }
 
diff --git a/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/Snippetizer.java b/plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/Snippetizer.java
@@ -574,10 +574,9 @@ public static class LocalAction extends Snippetizer {
             return Messages.Pipeline_Syntax();
         }
 
-        public String getIconClassName() {
-            return "icon-help";
+        @Override public String getIconFileName() {
+            return "symbol-help-circle";
         }
-
     }
 
     private static final Logger LOGGER = Logger.getLogger(Snippetizer.class.getName());
diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionMemoryTest.java
@@ -25,6 +25,7 @@
 package org.jenkinsci.plugins.workflow.cps;
 
 import groovy.lang.MetaClass;
+import hudson.model.Computer;
 import java.lang.ref.WeakReference;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
@@ -67,6 +68,7 @@ public static void register(Object o) {
     }
 
     @Test public void loaderReleased() throws Exception {
+        Computer.threadPoolForRemoting.submit(() -> {}).get(); // do not let it be initialized inside the CpsVmExecutorService thread group
         WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
         r.jenkins.getWorkspaceFor(p).child("lib.groovy").write(CpsFlowExecutionMemoryTest.class.getName() + ".register(this)", null);
         p.setDefinition(new CpsFlowDefinition(CpsFlowExecutionMemoryTest.class.getName() + ".register(this); node {load 'lib.groovy'; evaluate(readFile('lib.groovy'))}", false));
diff --git a/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionTest.java b/plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowExecutionTest.java
diff --git a/pom.xml b/pom.xml

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,6 @@`
`7`	`7`	`import groovy.lang.Script;`
`8`	`8`	`import java.io.InputStream;`
`9`	`9`	`import java.io.ObjectInputStream;`
`10`		`-import java.util.Collections;`
`11`	`10`	`import org.junit.Test;`
`12`	`11`
`13`	`12`	`import static org.hamcrest.CoreMatchers.equalTo;`
`@@ -38,6 +37,6 @@ public void serialFormBackwardsCompatibility() throws Throwable {`
`38`	`37`	`c = (Continuable)ois.readObject();`
`39`	`38`	`}`
`40`	`39`	`assertTrue(c.isResumable());`
`41`		`- assertThat(c.run0(new Outcome(false, null), Collections.emptyList()).replay(), equalTo(true));`
	`40`	`+ assertThat(c.run0(new Outcome(false, null)).replay(), equalTo(true));`
`42`	`41`	`}`
`43`	`42`	`}`