[JENKINS-64185] Replace use of NamedArgsAndClosure as return value for invokeDescribable #399
Conversation
…dArgsAndClosure as return value for invokeDescribable
...main/java/org/jenkinsci/plugins/workflow/cps/UninstantiatedDescribableWithInterpolation.java
Show resolved
Hide resolved
...main/java/org/jenkinsci/plugins/workflow/cps/UninstantiatedDescribableWithInterpolation.java
Show resolved
Hide resolved
car-roll
changed the title
|
Tested here and it worked again, tested manually triggering the job and also using Gerrit. |
| @@ -544,8 +542,6 @@ private static int preallocatedHashmapCapacity(int elementsToHold) { | |||
| final Closure body; | |||
| final List<String> msgs; | |||
| final Set<String> interpolatedStrings; | |||
| // UninstantiatedDescribable is set when the associated symbol is being built as the parameter of a step to be invoked. | |||
| UninstantiatedDescribable uninstantiatedDescribable = null; | |||
|
|
|||
| private NamedArgsAndClosure(Map<?,?> namedArgs, Closure body, @Nonnull Set<String> foundInterpolatedStrings) { | |||
There was a problem hiding this comment.
@car-roll Do we need to do any cleanup in NamedArgsAndClosure now? I guess we should remove implements Serializable and remove "The instance is returned to the Groovy program ..." from the Javadoc.
There was a problem hiding this comment.
Technically the whole paragraph might have to go, right? Because we are now using the UninstantiatedDescribable again. Maybe that paragraph should go to UninstantiatedDescribableWithInterpolation?
There was a problem hiding this comment.
Yeah you are right, everything other than "This class holds the argument map and optional body of the step that is to be invoked." is no longer accurate. Feel free to move it to the new class if you want.
| import java.util.Map; | ||
| import java.util.Set; | ||
|
|
||
| public class UninstantiatedDescribableWithInterpolation extends UninstantiatedDescribable { |
There was a problem hiding this comment.
We should restrict this class (some imports will need to be added along with this suggestion):
| public class UninstantiatedDescribableWithInterpolation extends UninstantiatedDescribable { | |
| @Restricted(NoExternalUse.class) | |
| public class UninstantiatedDescribableWithInterpolation extends UninstantiatedDescribable { |
…uesMap Version v2.86 workflow-cps plugin extended the UninstantiatedDescribable class from the structs plugin to store Groovy interpolated strings. This commit ensures we are able to handle both classes when building in Jenkins. jenkinsci/workflow-cps-plugin#399 https://github.com/jenkinsci/workflow-cps-plugin/blob/1ff8b0f5ec3479f27a7b23cab892b312a9d2d78a/src/main/java/org/jenkinsci/plugins/workflow/cps/UninstantiatedDescribableWithInterpolation.java
There are some conditions in which the NamedArgsAndClosure class is passed to other packages where it is not recognized. This fix here goes back to an earlier implementation in #370 where we create a subclass of
UninstantiatedDescribablethat stores interpolated strings. This should ensure that existing code that used to look forUninstantiatedDescribablecan continue to work as expected.Note: not sure what tests to add to reflect this problem. Did manual testing with the Gerrit plugin