Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix JENKINS-54249 #216

wants to merge 1 commit into from

Conversation

bgaillard
Copy link

@bgaillard bgaillard commented Jun 15, 2019

This PR fixes the following issue https://issues.jenkins-ci.org/browse/JENKINS-54249.

Few additional notes :

  • The code update the git plugin to version 4.0.0-rc to have the BuildDetails class (see comments associated to this commit for more details ttps://github.com/jenkinsci/git-plugin/commit/07cfa5ddef698838b01d4214915f98d4e902c0f8#diff-6cb4dc50342af417dc66c68b45c48fb1)
  • I removed the scm-api dependency in the pom.xml because its a transitive dependency of the git Jenkins plugin
  • Usage of the BuildData class has been removed as much as possible. So now we prefer BuildDetails.

This is the first time I update a Jenkins plugin so do not hesitate to help me improve this before merging.


This change is Reviewable

bgaillard referenced this pull request in bgaillard/github-plugin Jun 16, 2019
@@ -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>
Copy link
Member

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?

Copy link
Author

@bgaillard bgaillard Jun 16, 2019

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 --> ?

Copy link
Member

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...

Copy link
Author

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 ?

Copy link
Member

@KostyaSha KostyaSha Jun 17, 2019

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.

Copy link
Contributor

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.

Copy link
Author

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) ?

Copy link
Member

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?

Copy link
Author

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 {
Copy link
Member

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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reason?

Copy link
Author

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).

Copy link
Member

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>
Copy link
Member

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.

Copy link
Member

@KostyaSha KostyaSha left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants