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

package old JNA version 3.3.0 with the plugin #10

Merged
merged 2 commits into from Feb 1, 2015

Conversation

g-k-r
Copy link
Contributor

@g-k-r g-k-r commented Jan 11, 2015

fixes #JENKINS-12523
https://issues.jenkins-ci.org/browse/JENKINS-12523
needs removable of slf4j references

@@ -3,7 +3,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.557</version>
<version>1.586</version>
Copy link
Member

Choose a reason for hiding this comment

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

Is this really necessary? This will prevent use of the plugin in current LTS (1.580.x) releases.

See https://wiki.jenkins-ci.org/display/JENKINS/Choosing+Jenkins+version+to+build+against for more pros/cons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok repeated test with 1.580 - works too, so i'll update that.

Copy link
Member

Choose a reason for hiding this comment

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

Not everybody uses latest LTS, downgrade to 1.565.3 at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KostyaSha would you expect this to be tested solely on 1.565.3 or on all versions inbetween ? Note that this patch only fixes an incompatibility with jenkins introduced in 1.586. There is no added functionality, so what would be the point supporting even older versions than current LTS ?

Copy link
Member

Choose a reason for hiding this comment

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

Future plugin development will would available to recent versions unless a backwards-compatible branch is maintained with extra effort (like Subversion plugin's 2.4.x branch while 2.5 aims at Jenkins 1.577 or so)

@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

g-k-r added a commit that referenced this pull request Feb 1, 2015
package old JNA version 3.3.0 with the plugin
@g-k-r g-k-r merged commit f01db46 into jenkinsci:master Feb 1, 2015
@g-k-r g-k-r deleted the JENKINS-12523 branch February 1, 2015 21:21
@rodrigc
Copy link

rodrigc commented Feb 7, 2015

This is a bad change.
I would rather see the libvirt-slave-plugin updated to be compatible with JNA 4.1.

JNA 3.3 had bugs which broke certain platforms.
I took a pass through a lot of plugins to update them to use JNA 4.1, so it would be a shame to move backwards.

@g-k-r
Copy link
Contributor Author

g-k-r commented Feb 10, 2015

hi @rodrigc as long as this dependency is not compatible with jna 4.1 i will not invest in upgrading
http://libvirt.org/git/?p=libvirt-java.git;a=blob_plain;f=pom.xml.in;hb=HEAD

feel free to send a PR though

@rodrigc
Copy link

rodrigc commented Feb 10, 2015

I don't understand, you are not interested in actually fixing the plugin, but
you are OK with breaking things?

@g-k-r
Copy link
Contributor Author

g-k-r commented Feb 10, 2015

this plugin has been requiring 3.3 since before jenkins packaged a newer version with release 1.586, which broke this plugin. so to keep this plugin running on platforms that were working before it now packages its own jna. i don't see which existing installations would break because of this.
could you elaborate please.

@rodrigc
Copy link

rodrigc commented Feb 10, 2015

Look at this bug: https://issues.jenkins-ci.org/browse/JENKINS-24521
and the related 17 issues. Moving from JNA 3.3.0 to JNA 4.1.0 fixed those issues and platforms.

Your patch may have solved your local problem, but is moving things backwards.

@g-k-r
Copy link
Contributor Author

g-k-r commented Feb 10, 2015

well it keeps the thing running until the used library is fit for 4.1.0. Don't get me wrong, I agree it would be much nicer to not use this hackish pluginFirstClassLoader, but it doesn't work. yet.

@daniel-beck
Copy link
Member

Your patch may have solved your local problem, but is moving things backwards.

This plugin currently does not work at all in recent Jenkins versions. Making it work again on most platforms, i.e. restoring the functionality that was there when used with older Jenkins versions, is a step forward.

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