-
Notifications
You must be signed in to change notification settings - Fork 196
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
Excluding a couple of libraries which should be picked up from Jenkins core (in older versions) #105
Conversation
…s core (in older versions).
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
I need to review it. |
@jglick Even when I build with 2.73.3, the dependency tree is fine. I see the dependency in https://github.com/jenkinsci/jenkins/blob/397b4fae8e7388e3c1c30957dfe9703a5e64185e/core/pom.xml#L249 , but it does not appear in the dependency tree at all. I'd guess it's because of the Jenkins plugin POM, latest:
Dependency tree...
I do not disagree with the proposed PR, but it's rather a symptom than the root cause. Something is wrong with the dependency graph, and IMHO we need to resolve it before the next release of Maven Plugin. If there is a conflict between versions, require upper bounds check must fail. |
@stephenc @aheritier ^^^ Do you know why it happens? |
Well from
(with the deps coming from
(with both deps coming from There is nothing wrong with the dependency graph; this is just how Maven works. As noted in documentation:
Now could |
…ins-ci.plugins:apache-httpcomponents-client-4-api:4.5.3-1.0 incorrectly requests 1.9.
If provided scope is in force, then that will not be transitive... except for version range constraints (and since nobody actually uses version ranges the version range constraints is probably not even tested by maven integration tests so may not actually work) |
Anyway, my summary would be:
|
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.
🐝
@reviewbybees done |
I tried prototyping this, using a (direct) dep on
What we want in this case is a way to enforce that a plugin’s dep on |
It is a follow-up to jenkinsci#105
Seems like #102 was not complete?
@reviewbybees