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

Modernizing POM #14

Merged
merged 2 commits into from
Aug 8, 2022
Merged

Modernizing POM #14

merged 2 commits into from
Aug 8, 2022

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 18, 2022

Unclear how this will all work at runtime; without RealJenkinsRule it is hard to predict. https://www.jenkins.io/doc/developer/plugin-development/dependencies-and-class-loading/ The dependencies in this plugin seem to overlap heavily with things supplied by Jenkins core.

@dillense
Copy link
Contributor

dillense commented Jul 19, 2022

Hi Jesse,
Thanks for your support to update this plugin build. As far as I understand, you are trying to make it work with Java 11. I see that CI is reporting a code quality check failure:

Error in error step, with arguments Static analysis quality gates not passed; halting early.

So, I cloned your fork and I'm working on the issue:

[ERROR] High: Boxing/unboxing to parse a primitive org.ow2.clif.jenkins.ClifPublisher.getDouble(String) [org.ow2.clif.jenkins.ClifPublisher] At ClifPublisher.java:[line 370] DM_BOXED_PRIMITIVE_FOR_PARSING

@dillense
Copy link
Contributor

Fix pulled to master branch in upstream repo.

@jglick
Copy link
Member Author

jglick commented Jul 19, 2022

you are trying to make it work with Java 11.

Yes, that would be one benefit.

Fix pulled to master branch in upstream repo.

b89e536, OK.

To be clear, this patch is untested.

<jenkins.version>2.138</jenkins.version>
<java.level>8</java.level>
<slf4jVersion>1.7.26</slf4jVersion>
<jenkins.version>2.277.4</jenkins.version>
Copy link
Member Author

Choose a reason for hiding this comment

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

(first LTS with new Spring Security)

<dependency>
<groupId>org.ow2.clif</groupId>
<artifactId>clif-api</artifactId>
<version>3.0.1</version>
<exclusions>
Copy link
Member Author

Choose a reason for hiding this comment

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

Generally means that runtime behavior is hard to predict. You can try

  • manual sanity checks
  • RealJenkinsRule
  • pluginFirstClassLoading (also dangerous)
  • using a different artifact that has fewer deps
  • forking a tool rather than trying to load it inside the controller’s JVM

@dillense
Copy link
Contributor

I'm going to test this.
Just one question: I understand the refreshing work about the POM, but what about modifications regarding @nonnull annotations? I see you are using an alternative not included in JDK, and I'm concerned about adding a new dependency. Is it mandatory, or does it come with significant benefits?

@jglick
Copy link
Member Author

jglick commented Jul 21, 2022

an alternative not included in JDK

Neither is the javax.** version. It was part of a JSR which was later abandoned.

Is it mandatory

Yes, newer POM and core versions do not include the javax.** variant of the annotation library, so you must switch.

@dillense
Copy link
Contributor

OK, thanks for your feedback. OK, let's switch to this implementation bound to the spotbugs tool.
A final observation: I see some warnings related to this initial one:

Downloading from maven-default-http-blocker: http://0.0.0.0/javax/servlet/servlet-api/maven-metadata.xml
[WARNING] Could not transfer metadata javax.servlet:servlet-api/maven-metadata.xml from/to maven-default-http-blocker (http://0.0.0.0/): transfer failed for http://0.0.0.0/javax/servlet/servlet-api/maven-metadata.xml

I tried to get some information about this issue but could not find a solution. Are you aware of this issue? Is it a problem or may we just live with it?

@jglick
Copy link
Member Author

jglick commented Jul 22, 2022

Not sure what is causing that warning exactly. Maven is seeing some repository definition, either in some transitive POM or in your local settings, which uses insecure http protocol, which it will no longer honor.

pom.xml Show resolved Hide resolved
@dillense dillense merged commit a1adde0 into jenkinsci:master Aug 8, 2022
@jglick jglick deleted the updates branch August 8, 2022 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants