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

Update javax.mail to 1.6.5 #4660

Merged
merged 1 commit into from
Apr 30, 2020
Merged

Conversation

jvz
Copy link
Member

@jvz jvz commented Apr 16, 2020

This has changed maven coordinates to the jakarta project, though the
code is binary compatible still.

Proposed changelog entries

  • Developer: Update javax.mail to jakarta.mail 1.6.5

Proposed upgrade guidelines

N/A

Submitter checklist

  • (If applicable) Jira issue is well described
  • Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
    • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
  • Appropriate autotests or explanation to why this change has no tests
  • For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

  • There are at least 2 approvals for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title and/or Proposed changelog entries are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
  • If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

This has changed maven coordinates to the jakarta project, though the
code is binary compatible still.
@jvz jvz requested review from jeffret-b, daniel-beck and a team April 16, 2020 19:00
</dependency>
<dependency>
<groupId>org.jvnet.hudson</groupId>
<artifactId>activation</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

why is this removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. I'm not even sure what was patched originally, but it's a transitive dependency of jakarta.mail already.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

do we need to run PCT or full ATH on this?

edit: I guess if binary compat tests pass, it should be fine

@timja
Copy link
Member

timja commented Apr 16, 2020

We probably need a change log entry for this? in case it causes issues

@jvz
Copy link
Member Author

jvz commented Apr 16, 2020

do we need to run PCT or full ATH on this?

I ran PCT of this using a branch from stable-2.222 with no issues. It looks like ATH is automatically run for PRs in this repo.

As for the javax.activation thing, this looks like the fork, but it has no history: https://github.com/kohsuke/javax.activation

Copy link
Contributor

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

Seems good, if it's sufficiently tested. I haven't testing it any myself.

@timja
Copy link
Member

timja commented Apr 16, 2020

Only ath smoke tests are run on PR not the full set afaik

@res0nance res0nance added the developer Changes which impact plugin developers label Apr 18, 2020
@res0nance
Copy link
Contributor

Full changelog here

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Be careful with Jakarta code for the activation lib, there are/were known issues with module handling there due to the groupId changes. Before we merge that, an investigation is needed. Maybe a manual testing on Java 11 also

@jvz
Copy link
Member Author

jvz commented Apr 20, 2020

Thanks, I was wondering why that was patched! If this release is wonky, we'll need to create another update for a forked version of that.

@jeffret-b jeffret-b added the on-hold This pull request depends on another event/release, and it cannot be merged right now label Apr 21, 2020
@jvz
Copy link
Member Author

jvz commented Apr 23, 2020

I tested this out locally on Java 11 using Gmail as my SMTP host (had to disable a security setting that marks Jenkins as an "insecure" client to try it out), worked fine. Only log messages I'm seeing are related to a removal of javax.annotation stuff.

@jvz jvz removed the on-hold This pull request depends on another event/release, and it cannot be merged right now label Apr 23, 2020
@jvz jvz requested a review from oleg-nenashev April 23, 2020 16:33
@jvz
Copy link
Member Author

jvz commented Apr 29, 2020

Test failures are unrelated it looks like. Infrastructural.

@jeffret-b jeffret-b added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Apr 29, 2020
@jeffret-b
Copy link
Contributor

jeffret-b commented Apr 29, 2020

This will be ready for release after the customary waiting period of 24 hours. However, we may want to wait until after the release of 2.235 based upon the current LTS selection discussion.

@timja
Copy link
Member

timja commented Apr 29, 2020

This will be ready for release after the customary waiting period of 24 hours. However, we may want to wait until after the release of 2.235 based upon the current LTS selection discussion.

I agree, unless any objection jvz can we skip this from the current weekly as I assume there's no rush on this?

@jvz
Copy link
Member Author

jvz commented Apr 29, 2020

There are some security hardening updates included in newer javamail releases which would be great to get into an LTS.

@timja
Copy link
Member

timja commented Apr 29, 2020

Sure, easy enough to back out if any issues then

@jeffret-b
Copy link
Contributor

Matt raises a good point so I retract my earlier comment about waiting.

@jeffret-b jeffret-b merged commit abb1310 into jenkinsci:master Apr 30, 2020
@jvz jvz deleted the javaxmail-update branch May 4, 2020 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Changes which impact plugin developers ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants