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

Require 2.452.4 and use API plugins #152

Merged
merged 2 commits into from
Jan 5, 2025

Conversation

jonesbusy
Copy link
Contributor

@jonesbusy jonesbusy commented Jan 4, 2025

Partially automated. With current implementation, API plugins are added only when direct dependencies.

Since they are brought by opencsv it cause causing upper bound issue

  • Use the Jenkins core BOM to declare that commons-beanutils is provided by Jenkins core

@jonesbusy jonesbusy changed the title Require 2.452.4 and link againts API plugin Require 2.452.4 and link againts API plugins Jan 4, 2025
https://www.jenkins.io/doc/developer/plugin-development/dependency-management/#jenkins-core-bom says:

> Jenkins core provides a Maven Bill Of Materials (BOM) that centrally
> defines versions of various libraries used by Jenkins. If you are using
> Maven to build your plugin, then you can simplify dependency management
> by importing this BOM, at jenkins.version. Dependency versions are then
> automatically synchronized with the version of Jenkins against which
> you are building.

Since commons-beanutils is included in the Jenkins core BOM, we can
reference it as "provided" and since the Jenkins core BOM is now
included, we can rely on the Jenkisn core BOM to manage the version
of commons-beanutils.

That seemed to resolve the upper bounds dependency error and let me
learn more about the Jenkins core BOM.

Testing done:

* Tested with Jenkins 2.462.3 by creating a line plot that used a file
  named "plot-b.csv" with data that I added after each build.  The plot
  was shown correctly when the plot was defined in the freestyle project
  as a post build action.

* Tested with Jenkins 2.462.3 by creating a line plot as a build step
  and that failed with tip of the master branch and with the changes in
  this pull request.  I don't know if that is a recent failure or if it has
  been there a long time.  It has not been reported in the issue tracker.
@MarkEWaite MarkEWaite changed the title Require 2.452.4 and link againts API plugins Require 2.452.4 and use API plugins Jan 5, 2025
@MarkEWaite MarkEWaite added the chore General project maintenance label Jan 5, 2025
Copy link
Contributor

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

Change looks good though I want to perform more interactive testing to confirm that the plugin has no worse behavior now than it did before these changes.

@MarkEWaite MarkEWaite merged commit 3726a04 into jenkinsci:master Jan 5, 2025
16 checks passed
@jonesbusy jonesbusy deleted the feature/depends-api-plugins branch January 5, 2025 04:58
@jonesbusy
Copy link
Contributor Author

Thanks.

I think exclusion are still necessary.

If not, the hpi will have transitive dependencies

[INFO] Bundling direct dependency opencsv-5.9.jar
[WARNING] Bundling transitive dependency commons-beanutils-1.9.4.jar (via opencsv)
[WARNING] Bundling transitive dependency commons-collections4-4.4.jar (via opencsv)
  • The provided commons-beanutils will not be used if available on jenkins core
  • The transitive commons-collections will not be used since it's provided by commons-text-api
[INFO] +- io.jenkins.plugins:commons-text-api:jar:1.12.0-129.v99a_50df237f7:compile
[INFO] |  \- org.apache.commons:commons-text:jar:1.12.0:compile
[INFO] |  \- org.apache.commons:commons-collections4:jar:4.4:compile

This would reduce the hpi size and prevent unexpected runtime issue

@jonesbusy jonesbusy mentioned this pull request Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore General project maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants