Use `io.jenkins.plugins:commons-lang3-api` #2433

viceice · 2023-04-27T09:52:59Z

Description

Use io.jenkins.plugins:commons-lang3-api instead of org.apache.commons:commons-lang3.
All tests still pass. I don't think this needs new tests.

I also updated these plugins to fix upper bound errors

org.apache.maven:maven-artifact
org.jenkins-ci.plugins:jira
org.jenkins-ci.plugins:github-api
io.jenkins.plugins:okhttp-api

Submitter checklist

Link to JIRA ticket in description, if appropriate.
Change is code complete and matches issue description
Appropriate unit or acceptance tests or explanation to why this change has no tests
Reviewer's manual test instructions provided in PR description. See Reviewer's first task below.

Reviewer checklist

Run the changes and verified the change matches the issue description
Reviewed the code
Verified that the appropriate tests have been written or valid explanation given

viceice · 2023-04-27T10:13:52Z

https://github.com/jenkinsci/code-coverage-api-plugin/issues/647#issuecomment-1523920637

viceice

Failed tests seems unrelated to my changes 😕

blueocean-pipeline-editor/pom.xml

viceice · 2023-04-27T15:27:59Z

any idea why those tests failed?

pom.xml

blueocean-commons/pom.xml

viceice · 2023-04-28T06:57:33Z

pom.xml

-            <groupId>com.squareup.okhttp3</groupId>
-            <artifactId>okhttp</artifactId>
-            <version>3.12.12</version>


Removed, as it's a transitive dep of io.jenkins.plugins:okhttp-api which should be used instead

viceice · 2023-04-28T07:57:39Z

@olamy The failed tests seems to be unrelated to my change. Looks like a jenkins agent issue:

WARNING: Caught unhandled exception with ID e2654f2d-78dd-429a-92a0-008d7409e5fe
javax.servlet.ServletException: hudson.plugins.git.GitException: Command "git fetch --tags --force --progress --prune -- origin +refs/heads/*:refs/remotes/origin/*" returned status code 128:
stdout: 
stderr: Unable to negotiate with 127.0.0.1 port 37891: no matching host key type found. Their offer: ssh-rsa
fatal: Could not read from remote repository.
Please make sure you have the correct access rights
and the repository exists.

pom.xml

viceice · 2023-04-29T17:59:14Z

@olamy anything else todo here? i really need this fixed

jglick · 2023-05-08T13:40:08Z

pom.xml

+                            <bannedDependencies>
+                                <excludes>
+                                    <exclude>org.apache.commons:commons-lang3</exclude>
+                                    <exclude>com.squareup.okhttp3:okhttp</exclude>


Fixed properly in #2435.

thanks 🙏

btw: I've uninstalled blue ocean as the development was stopped.

jglick · 2023-05-08T13:40:37Z

pom.xml

+                        <rules>
+                            <bannedDependencies>
+                                <excludes>
+                                    <exclude>org.apache.commons:commons-lang3</exclude>


Why? If there are errors they should be fixed, not suppressed.

was not my idea, was told to add that rule. 🤷‍♂️

#2433 (comment) I guess? Not sure I follow.

The idea is: we don't want to have dependency org.apache.commons:commons-lang3 here because we are now using commons-lang3-api so we ban it and if for any reason a new dependency or an upgrade of a current dependency bring it back to the dependency tree the build will fail as the dependency is now banned.
Otherwise the dependency can come back and nobody will notice that (upperBound maybe but not if it's back in the tree with the same or higher version)

I was mixing this up with requireUpperBoundsDeps. That said, not sure I follow the logic—if we are using commons-lang3-api then it is harmless if the same artifact is available via some other dependency trail.

the origin of the change comes from https://github.com/jenkinsci/code-coverage-api-plugin/issues/647

Oh definitely we should be using the API wrapper plugin. I am just trying to understand the need for a bannedDependencies rule on top of that, since depending on the API wrapper plugin ought to ensure that the library JAR does not get bundled here even if it is accessible via some other dependency trail.

you can definitely add manually a dependency to org.apache.commons:commons-lang3 (at least same version as te one coming from the wrapper api) in any pom and it will be added to the hpi. Or coming from transitive (but still with same or higher version because of upperBound.

Try this

diff --git a/blueocean-pipeline-editor/pom.xml b/blueocean-pipeline-editor/pom.xml index 14f3c6ac7..85f43ddc1 100644 --- a/blueocean-pipeline-editor/pom.xml +++ b/blueocean-pipeline-editor/pom.xml @@ -73,12 +73,17 @@ <artifactId>okhttp</artifactId> </exclusion>  - <exclusion> - <groupId>org.apache.commons</groupId> - <artifactId>commons-lang3</artifactId> - </exclusion> + + + + </exclusions> </dependency> + <dependency> + <groupId>org.apache.commons</groupId> + <artifactId>commons-lang3</artifactId> + <version>3.12.0</version> + </dependency> <dependency> <groupId>org.jenkins-ci.plugins</groupId> <artifactId>ant</artifactId> diff --git a/pom.xml b/pom.xml index 2fd6c28dd..26507faef 100644 --- a/pom.xml +++ b/pom.xml @@ -764,7 +764,7 @@ <rules> <bannedDependencies> <excludes> - <exclude>org.apache.commons:commons-lang3</exclude> +  </excludes> <searchTransitive>false</searchTransitive> </bannedDependencies>

mvn clean install -pl :blueocean-pipeline-editor -DskipTests ➜ blueocean-plugin git:(master) ✗ unzip -l blueocean-pipeline-editor/target/blueocean-pipeline-editor.hpi|grep jar 587402 11-17-2022 09:07 WEB-INF/lib/commons-lang3-3.12.0.jar 1061316 05-09-2023 19:30 WEB-INF/lib/blueocean-pipeline-editor.jar

In this case it is same version so no problem at all but having some higher version potentially can be a problem.
not really sure how to prevent this automatically except writing some special enforcer rules to fail in such case or manually added banned dependency rule such this one.

If you added that dependency explicitly to the plugin POM then it is not surprising it appears in the *.hpi, but do you have an example of

Or coming from transitive

? If true, there may be a bug in maven-hpi-plugin since it is supposed to be filtering out jar-packaging dependencies which are also available via some dependency trail involving another plugin.

Somewhere I proposed just forcing plugin POMs to explicitly declare a list of all jar-packaging artifactIds they intend to package, since this is such a frequent source of mistakes. I can try to dig up references and propose the change. Would be incompatible but easy to adapt to.

viceice added 5 commits April 27, 2023 10:18

Use commons-lang3-api jenkins plugin

e989a15

Another org.apache.commons:commons-lang3 exclusion

3b34a48

More updates and exclusions

81f514b

Bump jira from 3.0.14 to 3.9

d6f39ed

Bump maven-artifact from 3.5.0 to 3.8.6

c6be298

viceice commented Apr 27, 2023

View reviewed changes

blueocean-pipeline-editor/pom.xml Show resolved Hide resolved

viceice marked this pull request as ready for review April 27, 2023 11:06

viceice requested a review from a team as a code owner April 27, 2023 11:06

viceice changed the title ~~Use commons-lang3-api jenkins plugin~~ Use io.jenkins.plugins:commons-lang3-api Apr 27, 2023

uhafner reviewed Apr 27, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

olamy requested changes Apr 27, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

blueocean-commons/pom.xml Show resolved Hide resolved

viceice added 5 commits April 28, 2023 06:58

Use exclusions instead of dependency updates

3937640

Ban org.apache.commons:commons-lang3

7f90409

Revert version bump and add exclusion

5e70f3f

Revert version

cc17a19

Move enforcer rule

8bf277a

viceice commented Apr 28, 2023

View reviewed changes

viceice requested review from uhafner and olamy April 28, 2023 06:58

uhafner approved these changes Apr 28, 2023

View reviewed changes

pom.xml Show resolved Hide resolved

olamy added the dependencies Pull requests that update a dependency file label May 2, 2023

olamy merged commit eb2e850 into jenkinsci:master May 3, 2023

viceice deleted the feat/commons-lang3-api branch May 3, 2023 04:34

jglick reviewed May 8, 2023

View reviewed changes

olamy mentioned this pull request May 8, 2023

Bump BOM and explicitly depend on caffeine-api to fix LoadingCache.refresh #2435

Merged

jglick mentioned this pull request Nov 27, 2023

Explicit list of packaged libraries jenkinsci/maven-hpi-plugin#557

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `io.jenkins.plugins:commons-lang3-api` #2433

Use `io.jenkins.plugins:commons-lang3-api` #2433

viceice commented Apr 27, 2023 •

edited

Loading

viceice commented Apr 27, 2023

viceice left a comment

viceice commented Apr 27, 2023

viceice Apr 28, 2023

viceice commented Apr 28, 2023

viceice commented Apr 29, 2023

jglick May 8, 2023

viceice May 8, 2023

jglick May 8, 2023

viceice May 8, 2023

jglick May 8, 2023

olamy May 8, 2023

jglick May 9, 2023

olamy May 9, 2023

jglick May 9, 2023 •

edited

Loading

olamy May 9, 2023

jglick May 9, 2023

Use io.jenkins.plugins:commons-lang3-api #2433

Use io.jenkins.plugins:commons-lang3-api #2433

Conversation

viceice commented Apr 27, 2023 • edited Loading

Description

Submitter checklist

Reviewer checklist

viceice commented Apr 27, 2023

viceice left a comment

Choose a reason for hiding this comment

viceice commented Apr 27, 2023

Choose a reason for hiding this comment

viceice commented Apr 28, 2023

viceice commented Apr 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jglick May 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Use `io.jenkins.plugins:commons-lang3-api` #2433

Use `io.jenkins.plugins:commons-lang3-api` #2433

viceice commented Apr 27, 2023 •

edited

Loading

jglick May 9, 2023 •

edited

Loading