Devin Nusbaum

Results 105 comments of Devin Nusbaum

@witokondoria Do you think you will have any time to look into adding some tests?

I have write permission to this repository for historical reasons, so my approval is green, but I do not actively maintain the plugin (I don't think anyone is really actively...

> > I do wonder about potential performance impact. > > Any particular concerns? My worry is just that this is inner loop code that looks at every byte that...

Thanks for the investigation! I checked your proposed change and indeed that does seem to fix this specific issue. From a very brief look I could not reproduce a similar...

> Sorry, what sort of check are you referring to? I mean that if this approach turns out to not be useful in practice due to things like `@NonCPS` blocks,...

> Perhaps, though I am guessing that most library developers do not even know this annotation exists. Generally speaking, I agree, but for the kinds of libraries we are concerned...

Implementations of `DeclarativeAgentScript` are supposed to register themselves automatically. Not sure what is happening here.

See [`WithScriptDescriptor.getScriptResource` and `.getScriptClass`](https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/f921b4e72c73e4b60564b8f6610f35bf8bbe9720/pipeline-model-extensions/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/withscript/WithScriptDescriptor.java#L67-L82) along with [`WithScriptDescriptor$WithScriptAllowlist`](https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/f921b4e72c73e4b60564b8f6610f35bf8bbe9720/pipeline-model-extensions/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/withscript/WithScriptDescriptor.java#L105-L117), and then [`KubernetesDeclarativeAgentImpl$DescriptorImpl`](https://github.com/jenkinsci/kubernetes-plugin/blob/4230d0ccd951f44657b16fd8f867defb03180fc6/src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesDeclarativeAgent.java#L415C25-L415C39) extends [`DeclarativeAgentDescriptor`](https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/f921b4e72c73e4b60564b8f6610f35bf8bbe9720/pipeline-model-extensions/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/agent/DeclarativeAgentDescriptor.java#L45) which extends `WithScriptDescriptor`. ~Maybe some kind of class loading problem [here](https://github.com/jenkinsci/pipeline-model-definition-plugin/blob/f921b4e72c73e4b60564b8f6610f35bf8bbe9720/pipeline-model-extensions/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/withscript/WithScriptDescriptor.java#L80) and it needs to use `uberClassLoader`?~ Never...

Yes, I am looking at this but will file a PR to revert things if I don't know what is happening in the next half hour or so.