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

Small RFE for dependency #12

Closed
wants to merge 1 commit into from
Closed

Small RFE for dependency #12

wants to merge 1 commit into from

Conversation

@slide
Copy link
Member

slide commented Mar 1, 2018

  • Make jCIFS dependency local instead of relying on a dependency in core
  • Add Jenkinsfile for building
- Make jCIFS dependency local instead of relying on a dependency in core
- Add Jenkinsfile for building
@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 1, 2018

@slide I'd guess it's a follow-up to jenkinsci/jenkins#3318 . Could you please clarify what you're trying to achieve in this PR? Even if we have a library locally, there is no guarantee that classloading will actually happen from it

@slide
Copy link
Member Author

slide commented Mar 11, 2018

@oleg-nenashev, yes, it's a follow up to that PR. I am trying to localize the dependency and I understand that without a change in core, the version from core will be loaded for now. This is just to allow removing the dependency from core at some future date.

@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 13, 2018

Even if you define a dependency here, you cannot guarantee that it will be actually loaded from here. Depends on the class loading order in this and other plugins. Once the core dependency changes, it may lead to upper bound dependency checks here.

As we discussed in jenkinsci/jenkins#3318 , a correct way would be to detach the jcifs API Plugin from the core and then add a dependency on it in this plugin.

@slide
Copy link
Member Author

slide commented Mar 13, 2018

The plan would be to remove the core dependency, there should be no reason to keep it.

@oleg-nenashev
Copy link
Member

oleg-nenashev commented Mar 13, 2018

@escoem also pointed me to https://issues.jenkins-ci.org/browse/JENKINS-46255, which is kinda related. Maybe the plugin should just move out to another lib

@slide
Copy link
Member Author

slide commented Mar 13, 2018

I am using jcifs-ng (https://github.com/AgNO3/jcifs-ng) now in po-cifs which has support for SMBv2

@slide slide closed this Jul 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.