-
Notifications
You must be signed in to change notification settings - Fork 396
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix JENKINS-54249 #216
base: master
Are you sure you want to change the base?
Fix JENKINS-54249 #216
Conversation
@@ -157,7 +151,7 @@ | |||
<dependency> | |||
<groupId>org.jenkins-ci.plugins</groupId> | |||
<artifactId>apache-httpcomponents-client-4-api</artifactId> | |||
<version>4.5.3-2.1</version> | |||
<version>4.5.5-3.0</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for hard requiring newer dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @KostyaSha and thanks for your reactivity reviewing this PR 👍
The purpose of the PR is to make the github-plugin
compliant with last versions of the git
Jenkins plugin (current version 4.0.0-rc
).
Here the version has been updated to be aligned with the version in use in library org.jenkins-ci.plugins:git:4.0.0-rc
.
Otherwise the following error is displayed at mvn install
.
[WARNING] Rule 4: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for org.jenkins-ci.plugins:apache-httpcomponents-client-4-api:4.5.3-2.1 paths to dependency are:
+-com.coravy.hudson.plugins.github:github:1.30.0-SNAPSHOT
+-org.jenkins-ci.plugins:apache-httpcomponents-client-4-api:4.5.3-2.1
and
+-com.coravy.hudson.plugins.github:github:1.30.0-SNAPSHOT
+-org.jenkins-ci.plugins:git:4.0.0-rc
+-org.jenkins-ci.plugins:git-client:3.0.0-rc
+-org.jenkins-ci.plugins:apache-httpcomponents-client-4-api:4.5.5-3.0
Nervetheless I did not saw the comment <!-- 4.5.3 used by rest-assured -->
above the dependency declaration in the pom.xml
.
Perhaps I could replace it by <!-- 4.5.3-3.0 used by org.jenkins-ci.plugins:git:4.0.0-rc -->
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think that 4.0.0-rc will be used in the nearest half year/year. This plugin is used on a lot of installations that will have no chance to update. If there is something really broken then it need to be fixed with the current code base...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I understand.
But i'm a little confused with my current Jenkins install, the current version of the git
plugin installed on our server is version 4.0.0-rc
.
I do not remember doing any particular install which would bring this version installed to our Jenkins (that's why i supposed using version 4.0.0-rc
in this PR would be ok).
Do you know what reason could lead to the install of version 4.0.0-rc
on our Jenkins server ? Could this be because of the install of an other plugin ? In this case is it possible to find what plugins use this version 4.0.0-rc
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because jenkins UpdateCenter treats -rc
as stable version. From experience git plugin had enough issues even between released versions and user may need to have ability to install the variety of versions. There is no need to have too fresh hard dep on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bgaillard thank you for addressing this issue!
BTW, we're encountering the same issue symptom with lower plugin versions (see NXBT-2867).
Our current versions are:
- CloudBees Jenkins Enterprise 2.164.3.2-rolling
- Git client plugin 2.7.6
- Git plugin 3.9.3
- SCM API Plugin 2.4.0
I'll comment on JENKINS-54249: it is not clear which issue you are actually submitting a fix for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jcarsique, I responded you here : https://issues.jenkins-ci.org/browse/JENKINS-54249
@KostyaSha, ok perhaps it's possible to update my PR to not depend on version 4.0.0-rc
of git-plugin
but I think it will require "ugly" tricks using reflection and i'll not be able to unit test it because the BuildDetails
class will not be available.
Would it be ok (i'll add clear comments to explain the intention in source code) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first of all restore binary compatibility, don't rename any java classes just to rename them. If you want to add new utils, then deprecate old...
I saw in issue that problem appeared since X version of plugin, is it possible to revert this piece of functionality?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can restore all library versions and make the plugin compliant with version 4.0.x of git-plugin without depending on it but it will be an "ugly" fix.
I'll need to use reflection to get informations from the BuildDetails class at runtime to then rebuild a BuildData object.
So I think the main problem is that Jenkins should not have proposed to install an RC version.
In the JIRA issue other users suggest to downgrade. Perhaps it's possible in some situations to downgrade but not always (because we could have other plugin depending on git-plugin 4.0.x and our jobs could use new features of those other plugins, making the downgrade of the other plugins not possible).
IMO and ideally Jenkins and it's libraries and plugins should respect something like SEMVER and be strict with it. But perhaps this would not solve everything... anyway we are on a standard Java JAR hell problem.
* @author Oleg Nenashev <o.v.nenashev@gmail.com> | ||
* @since 1.10 | ||
*/ | ||
public final class BuildDataHelper { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason for deleting class? If you want to get new methods, add them here and Deprecate old, either this will break binary compatibility for plugins that using it.
@@ -47,10 +47,10 @@ | |||
</issueManagement> | |||
|
|||
<properties> | |||
<jenkins.version>2.60.3</jenkins.version> | |||
<jenkins.version>2.107.3</jenkins.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise the following exception is encountered while executing the tests.
[ERROR] Tests run: 53, Failures: 0, Errors: 5, Skipped: 0, Time elapsed: 9.07 s <<< FAILURE! - in InjectedTest
[ERROR] testPluginActive(org.jvnet.hudson.test.PluginAutomaticTestBuilder$OtherTests) Time elapsed: 0.006 s <<< ERROR!
java.lang.Error: Plugin git-client failed to start
at org.jvnet.hudson.test.PluginAutomaticTestBuilder$OtherTests.testPluginActive(PluginAutomaticTestBuilder.java:99)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
...
...
Caused by: java.io.IOException: Jenkins Git client plugin v3.0.0-rc failed to load.
- You must update Jenkins from v2.60.3 to v2.107.3 or later to run this plugin.
at hudson.PluginWrapper.resolvePluginDependencies(PluginWrapper.java:626)
at hudson.PluginManager$2$1$1.run(PluginManager.java:516)
at org.jvnet.hudson.reactor.TaskGraphBuilder$TaskImpl.run(TaskGraphBuilder.java:169)
at org.jvnet.hudson.reactor.Reactor.runTask(Reactor.java:282)
at jenkins.model.Jenkins$7.runTask(Jenkins.java:1090)
at org.jvnet.hudson.reactor.Reactor$2.run(Reactor.java:210)
at org.jvnet.hudson.reactor.Reactor$Node.run(Reactor.java:117)
... 3 more
This is because org.jenkins-ci.plugins:git:4.0.0-rc
uses org.jenkins-ci.plugins:git-client:3.0.0-rc
(see https://github.com/jenkinsci/git-plugin/blob/git-4.0.0-rc/pom.xml#L83).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so all bumps because of git rc version, ok.
<groupId>org.jenkins-ci.plugins</groupId> | ||
<artifactId>scm-api</artifactId> | ||
<version>2.2.0</version> | ||
<version>4.0.0-rc</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too new dependency, github plugin wouldn't lock users to have release candidate as hard dependency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear lock for new dependency versions. Breaks binary compatibility.
Please find the way to fix without such things.
This PR fixes the following issue https://issues.jenkins-ci.org/browse/JENKINS-54249.
Few additional notes :
git
plugin to version4.0.0-rc
to have theBuildDetails
class (see comments associated to this commit for more details ttps://github.com/jenkinsci/git-plugin/commit/07cfa5ddef698838b01d4214915f98d4e902c0f8#diff-6cb4dc50342af417dc66c68b45c48fb1)scm-api
dependency in thepom.xml
because its a transitive dependency of thegit
Jenkins pluginBuildData
class has been removed as much as possible. So now we preferBuildDetails
.This is the first time I update a Jenkins plugin so do not hesitate to help me improve this before merging.
This change is