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.

Sign up for GitHub

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

Jump to bottom

Use spotbugs from bom #152

Merged
merged 3 commits into from
Oct 13, 2020
Merged

Use spotbugs from bom #152

merged 3 commits into from
Oct 13, 2020

Conversation

timja
Copy link
Member

@timja timja commented Oct 8, 2020 •
edited
Loading

$ mvn dependency:tree | grep spotbugs
[INFO] +- com.github.spotbugs:spotbugs-annotations:jar:4.0.3:provided (optional)

One of the PRs required for jenkinsci/bom#322

Pulls in jenkinsci/jenkins#4689 which bumps spotbugs to 4.x

First LTS with it in is 2.249.1

Tested with bootstrap api plugin, no upper bounds issues

@timja
Use spotbugs from bom
ae77082 
$ mvn dependency:tree | grep spotbugs
[INFO] +- com.github.spotbugs:spotbugs-annotations:jar:4.0.3:provided (optional)
jglick
jglick approved these changes Oct 8, 2020
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Note that findsecbugs is standard in the parent POM now.

uhafner
uhafner reviewed Oct 9, 2020
View reviewed changes
pom.xml Outdated
@@ -22,15 +22,14 @@
</description>

<properties>
<jenkins.baseline>2.204</jenkins.baseline>
<jenkins.version>${jenkins.baseline}.6</jenkins.version>
<jenkins.baseline>2.249</jenkins.baseline>
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be too new? From our dev documentation:

At the moment, the Jenkins releases 2.222.1 and 2.235.1 make good core dependencies unless there are specific reasons, like new features, to choose a different release.

Copy link
Member Author

@timja timja Oct 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

alternatively we would need to downgrade spotbugs to 3.x (by pulling from the bom on an older line) but that has the negative side affect of using 3.x which has the edu nonnull annotations deprecated.

but that might be a better option?

unless there are specific reasons, like new features, to choose a different release.

^^

Copy link
Member

Choose a reason for hiding this comment

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

IMHO this counts as a “specific reason”.

Copy link
Member

Choose a reason for hiding this comment

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

Well SpotBugs adds no functionality - it is a dev lib to improve static analysis. It should never be a reason to lose a lot of users by using the latest LTS as baseline.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've downgraded the baseline, and also raised a PR against core to update it's version to the latest spotbugs: jenkinsci/jenkins#4982

Copy link
Member

@uhafner uhafner Oct 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

When I test the new pom in one of my plugins I get a lot of enforcer errors now:

[INFO] Adding ignore: **/ModuleUtils*
[WARNING] Rule 5: org.apache.maven.plugins.enforcer.RequireUpperBoundDeps failed with message:
Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for com.github.spotbugs:spotbugs-annotations:3.1.11 paths to dependency are:
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-com.github.spotbugs:spotbugs-annotations:3.1.11
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:plugin-util-api:1.2.5
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.1.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:font-awesome-api:5.14.0-1
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.0.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:bootstrap4-api:4.5.2-1
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.1.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:jquery3-api:3.5.1-1
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.0.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:echarts-api:4.9.0-1
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.1.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:data-tables-api:1.10.21-2
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.0.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:plugin-util-api:1.2.5
    +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.1.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:plugin-util-api:1.2.5
    +-edu.hm.hafner:codingstyle:1.4.0
      +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.1.2
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:bootstrap4-api:4.5.2-1
    +-io.jenkins.plugins:popper-api:1.16.0-6
      +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.0.0
and
+-io.jenkins.plugins:forensics-api:0.8.0-SNAPSHOT
  +-io.jenkins.plugins:echarts-api:4.9.0-1
    +-edu.hm.hafner:echarts-build-trends:2.0.0
      +-com.github.spotbugs:spotbugs-annotations:3.1.11 (managed) <-- com.github.spotbugs:spotbugs-annotations:4.0.2
]

Copy link
Member Author

Choose a reason for hiding this comment

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

It’s because of cross dependencies on a lot of them as far as I can tell

Once this is released then it needs to go to the more meta ones first like plugin util api and it should sort itself out

@timja timja requested a review from uhafner October 9, 2020 20:08
@uhafner uhafner merged commit 358685c into jenkinsci:master Oct 13, 2020
@timja timja deleted the use-spotbugs-from-bom branch October 13, 2020 20:22
@timja
Copy link
Member Author

timja commented Oct 13, 2020

feel free to ping me on any dependabot PRs that fail on this.

@uhafner uhafner added the dependencies Update of dependencies label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jglick jglick jglick approved these changes

@uhafner uhafner Awaiting requested review from uhafner

Assignees
No one assigned
Labels
dependencies Update of dependencies
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timja @jglick @uhafner