Details
-
Bug
-
Status: Resolved (View Workflow)
-
Blocker
-
Resolution: Fixed
-
Jenkins 2.241, all plugins updated
-
-
Jenkins 2.248
Description
Hello,
after updating jenkins from 2.239 to 2.241 our jobs started to fail:
BUILD SUCCESSFUL Total time: 10minutes 49seconds New run name is '20.3.0.1-#40123' ERROR: Build step failed with exception java.lang.IllegalArgumentException: Method perform not found in hudson.plugins.analysis.core.HealthAwareRecorder (or it is private, finalor static) at hudson.Util.getMethod(Util.java:1462) at hudson.Util.getMethod(Util.java:1455) at hudson.Util.getMethod(Util.java:1455) at hudson.Util.isOverridden(Util.java:1439) at jenkins.tasks.SimpleBuildStep.perform(SimpleBuildStep.java:110) at hudson.tasks.BuildStepCompatibilityLayer.perform(BuildStepCompatibilityLayer.java:78) at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20) at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:741) at hudson.model.AbstractBuild$AbstractBuildExecution.performAllBuildSteps(AbstractBuild.java:690) at hudson.model.Build$BuildExecution.post2(Build.java:186) at hudson.model.AbstractBuild$AbstractBuildExecution.post(AbstractBuild.java:635) at hudson.model.Run.execute(Run.java:1905) at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43) at hudson.model.ResourceController.execute(ResourceController.java:97) at hudson.model.Executor.run(Executor.java:428) Build step '[Deprecated] Scan workspace for open tasks'marked build as failure Archiving artifacts Recording fingerprints [Java] Skipping execution of recorder since overall result is 'FAILURE'
In the german version of the GUI this particular post build step is not marked as deprecated, only in the build log it is possible to see that this build step seems to be deprecated and therefor maybe problematic.
Is there any alternative step available, that i am not aware of? I guess the exception happend above should not occur as well,right?
Greetings from Berlin,
Heiko
Attachments
Issue Links
- is caused by
-
-
- Resolved
-
-
JENKINS-29144 SimpleBuildStep to receive EnvVars
-
- Resolved
-
- links to
Activity
Core regression also apparently affecting phabricator-plugin and perhaps many others.
I suspect the root cause is JENKINS-30002. Should be possible to verify in IsOverriddenTest.
Tim Van Holder
added a comment - Yay for poor API docs.
{Util.isOverridden()}is documented as "Checks if the method defined on the base type with the given arguments is overridden in the given derived type."
There is/was no reason to expect it would throw for any reason (other than real failures).
Nor does it specify that it disregards final methods, which seems weird to me: In inheritance chain A > B > C, if I ask whether C overrides method M from A, I expect the answer to be yes when B and/or C override M, regardless of finality.
In addition, the logic assumes subclassing only - it does not handle cases where the base is an interface (it walks superclasses only, so never sees the interface method).
I'll look at adding an alternative to isOverridden() (maybe isDefaulted()?) that handles this use case. And I'll improve the docs for isOverridden() while I'm at it.
Tim Van Holder
added a comment - Yay for poor API docs.
{Util.isOverridden()}
is documented as "Checks if the method defined on the base type with the given arguments is overridden in the given derived type."
There is/was no reason to expect it would throw for any reason (other than real failures).
Nor does it specify that it disregards final methods, which seems weird to me: In inheritance chain A > B > C, if I ask whether C overrides method M from A, I expect the answer to be yes when B and/or C override M, regardless of finality.
In addition, the logic assumes subclassing only - it does not handle cases where the base is an interface (it walks superclasses only, so never sees the interface method).
I'll look at adding an alternative to isOverridden() (maybe isDefaulted()?) that handles this use case. And I'll improve the docs for isOverridden() while I'm at it. Filed a unit test reproducing a similar stack trace. Passing back to you for a fix and Javadoc improvements.
I do not think we need a new API method, just a corrected implementation of isOverridden.
Tim Van Holder
added a comment - I suppose I could just check whether base is an interface and then use a different underlying approach from what there is now.
I probably will add an overload that takes a boolean, to decide whether or not final methods should be considered overrides.
Tim Van Holder
added a comment - I suppose I could just check whether base is an interface and then use a different underlying approach from what there is now.
I probably will add an overload that takes a boolean, to decide whether or not final methods should be considered overrides. Tim Van Holder
added a comment - (Also: how do I grab your unit test? Add your repo as a remote and cherry-pick that commit into my branch for a PR?)
Tim Van Holder
added a comment - (Also: how do I grab your unit test? Add your repo as a remote and cherry-pick that commit into my branch for a PR?) I could just check whether base is an interface
I think it would suffice to amend Method not found in clazz, let's search in superclasses to also check interfaces in a depth-first recursion. Common idiom in Java reflection.
add an overload that takes a boolean, to decide whether or not final methods should be considered overrides
There is no use case for that. final methods should be considered overrides. The intent of the final check was to apply to the method lookup on the base class, not the derived class.
how do I grab your unit test? Add your repo as a remote
No need to do that, nor cherry-pick. Just go to your local branch and
git pull https://github.com/jglick/jenkins isOverridden-JENKINS-62723
Tim Van Holder
added a comment - Ah that makes sense.
It's unfortunate that there was/is no guarantee/requirement that base itself declares the method in question, so I do have to keep the lookup-including-base-classes logic for that too.
Tim Van Holder
added a comment - Ah that makes sense.
It's unfortunate that there was/is no guarantee/requirement that base itself declares the method in question, so I do have to keep the lookup-including-base-classes logic for that too. Tim Van Holder
added a comment - Going to have to see what happens if for A > B > C, both A and C declare themselves as implementing interface I, while B contains the method override.
In that case, isOverridden(I, C) should probably still return true.
So when looking at interfaces (and their super-interfaces) it shouldn't give up when base is encountered, unless it's the last class in the hierarchy implementing that interface.
So I guess the logic should be: if not found in the current class, get the superclass. If that does not implement base, and base is an interface, go through the interfaces for the current class.
And abstract methods (for abstract classes implementing an interface) shouldn't be considered overrides either.
Tim Van Holder
added a comment - Going to have to see what happens if for A > B > C, both A and C declare themselves as implementing interface I, while B contains the method override.
In that case, isOverridden(I, C) should probably still return true.
So when looking at interfaces (and their super-interfaces) it shouldn't give up when base is encountered, unless it's the last class in the hierarchy implementing that interface.
So I guess the logic should be: if not found in the current class, get the superclass. If that does not implement base, and base is an interface, go through the interfaces for the current class.
And abstract methods (for abstract classes implementing an interface) shouldn't be considered overrides either. Tim Van Holder
added a comment - Added a PR with what should be a fix for this issue.
Still looking at a further fix for a remaining edge case (where the override for an interface method is provided by a default implementation on a derived interface)
Tim Van Holder
added a comment - Added a PR with what should be a fix for this issue.
Still looking at a further fix for a remaining edge case (where the override for an interface method is provided by a default implementation on a derived interface) Tim Van Holder
added a comment - Further fix applied. Tim Van Holder
added a comment - Fix was merged, should appear in 2.245 early next week. 
Orgad Shaneh
added a comment - Still happens with 2.245. 
Thijs Wenker
added a comment - It seems that it was merged after the 2.245 release. When can we expect the next release with this fix?
Thijs Wenker
added a comment - It seems that it was merged after the 2.245 release. When can we expect the next release with this fix? Tim Van Holder
added a comment - Ah ok, I see that while there was a 2.245 tagged, it was not published as a release at https://github.com/jenkinsci/jenkins/releases/ - so when I checked there to see which version to report here, it looked like 2.245 was still next.
That means it'll be in 2.246 instead, which will likely get tagged today or tomorrow.
Tim Van Holder
added a comment - Ah ok, I see that while there was a 2.245 tagged, it was not published as a release at https://github.com/jenkinsci/jenkins/releases/ - so when I checked there to see which version to report here, it looked like 2.245 was still next.
That means it'll be in 2.246 instead, which will likely get tagged today or tomorrow. 
Thijs Wenker
added a comment - Released as Jenkins 2.248. Will mark as lts-candidate, just in case we select older version as a new LTS baseline
These plugins are end-of-life since Spring 2019. Please upgrade to https://github.com/jenkinsci/warnings-ng-plugin.