-
Notifications
You must be signed in to change notification settings - Fork 51
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
JKNS-571: Bump jenkins from 2.426.2 to 2.440.3 #213
base: master
Are you sure you want to change the base?
Conversation
@kevydotvinu: This pull request references JKNS-571 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
@kevydotvinu: This pull request references JKNS-571 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
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.
/approve
Good start!
I added my idea on how we can get around the dependency management for guava
.
pom.xml
Outdated
<dependency> | ||
<groupId>com.google.guava</groupId> | ||
<artifactId>guava</artifactId> | ||
<version>31.1-android</version> | ||
</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.
🤔 here - in the <dependencyManagement>
section of this file, can you try removing the parts where we pin the dependencies for the Jenins plugins? (this bit below):
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>mailer</artifactId>
<version>463.vedf8358e006b_</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
<version>3.2.1</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials</artifactId>
<version>1311.vcf0a_900b_37c2</version>
</dependency>
This will a) ensure that we are using newer versions of the upstream plugins, and b) potentially remove the need to declare guava
a managed transitive 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.
I have tried that.
git --no-pager diff
diff --git a/pom.xml b/pom.xml
index 58427f4..ce36b4d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -108,32 +108,12 @@
<artifactId>google-oauth-client-java6</artifactId>
<version>1.35.0</version>
</dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>mailer</artifactId>
- <version>463.vedf8358e006b_</version>
- </dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>matrix-auth</artifactId>
- <version>3.2.1</version>
- </dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>credentials</artifactId>
- <version>1311.vcf0a_900b_37c2</version>
- </dependency>
<dependency>
<groupId>org.easymock</groupId>
<artifactId>easymock</artifactId>
<version>5.1.0</version>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- <version>31.1-android</version>
- </dependency>
</dependencies>
</dependencyManagement>
However, it throws the below error.
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.4.1:enforce (display-info) on project openshift-login:
[ERROR] Rule 5: org.apache.maven.enforcer.rules.dependency.RequireUpperBoundDeps failed with message:
[ERROR] Failed while enforcing RequireUpperBoundDeps. The error(s) are [
[ERROR] Require upper bound dependencies error for com.google.errorprone:error_prone_annotations:2.18.0 paths to dependency are:
[ERROR] +-org.openshift.jenkins:openshift-login:1.1.0.999999-SNAPSHOT
[ERROR] +-com.google.http-client:google-http-client-jackson2:1.43.3
[ERROR] +-com.google.http-client:google-http-client:1.43.3
[ERROR] +-com.google.errorprone:error_prone_annotations:2.18.0 (managed) <-- com.google.errorprone:error_prone_annotations:2.18.0
[ERROR] and
[ERROR] +-org.openshift.jenkins:openshift-login:1.1.0.999999-SNAPSHOT
[ERROR] +-com.google.http-client:google-http-client-jackson2:1.43.3
[ERROR] +-com.google.http-client:google-http-client:1.43.3
[ERROR] +-com.google.guava:guava:33.0.0-jre (managed) <-- com.google.guava:guava:30.1.1-android
[ERROR] +-com.google.errorprone:error_prone_annotations:2.18.0 (managed) <-- com.google.errorprone:error_prone_annotations:2.23.0
[ERROR] ]
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
Declaring the managed transitive dependency com.google.errorprone:error_prone_annotations:2.23.0
resolved the issue.
git --no-pager diff
diff --git a/pom.xml b/pom.xml
index 58427f4..6f6523a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -108,21 +108,6 @@
<artifactId>google-oauth-client-java6</artifactId>
<version>1.35.0</version>
</dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>mailer</artifactId>
- <version>463.vedf8358e006b_</version>
- </dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>matrix-auth</artifactId>
- <version>3.2.1</version>
- </dependency>
- <dependency>
- <groupId>org.jenkins-ci.plugins</groupId>
- <artifactId>credentials</artifactId>
- <version>1311.vcf0a_900b_37c2</version>
- </dependency>
<dependency>
<groupId>org.easymock</groupId>
<artifactId>easymock</artifactId>
@@ -130,9 +115,9 @@
<scope>test</scope>
</dependency>
<dependency>
- <groupId>com.google.guava</groupId>
- <artifactId>guava</artifactId>
- <version>31.1-android</version>
+ <groupId>com.google.errorprone</groupId>
+ <artifactId>error_prone_annotations</artifactId>
+ <version>2.23.0</version>
</dependency>
</dependencies>
</dependencyManagement>
Should we check any of the other possible combinations?
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'm fine resolving the transitive dependency with an item for error_prone_annotations
. Please add an XML comment (enclose in <!-- -->
brackets) explaining why we need to pin the version of this library. This will help in the future if/when this needs to be updated (or think we can remove the explicit version pin in the future).
Resolving transitive dependencies is a necessary evil in all software development - an art more so than a science. It helps to know which libraries are "big" (like guava
) and those that are small, which are less likely to cause breaking changes when upgraded/version pinned.
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 have made that change. Another question: how can we differentiate between large and small libraries? Can we use the file size information available on Maven Repository to do this?
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.
Can we use the file size information available on Maven Repository to do this?
Perhaps. This is honestly more "art" than science, and experienced Java developers tend to have an intuitive sense of which dependencies are large/commonly referenced.
- Bump jenkins from 2.426.2 to 2.440.3 - Bump jenkins bom from 2705.vf5c48c31285b_ to 3413.v0d896b_76a_30d - Add transitive dependency com.google.errorprone:error_prone_annotations:2.23.0 - Remove transitive dependencies \ org.jenkins-ci.plugins:mailer:463.vedf8358e006b_ \ org.jenkins-ci.plugins:matrix-auth:3.2.1 \ org.jenkins-ci.plugins:credentials:1311.vcf0a_900b_37c2 \ Signed-off-by: Vinu K <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adambkaplan, kevydotvinu The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@kevydotvinu: This pull request references JKNS-571 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "4.18.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/retest |
1 similar comment
/retest |
@kevydotvinu: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
/remove-lifecycle stale |
jenkins
from2.426.2
to2.440.3
jenkins
bom
from2705.vf5c48c31285b_
to3413.v0d896b_76a_30d
com.google.errorprone:error_prone_annotations:2.23.0
org.jenkins-ci.plugins:mailer:463.vedf8358e006b_
org.jenkins-ci.plugins:matrix-auth:3.2.1
org.jenkins-ci.plugins:credentials:1311.vcf0a_900b_37c2