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

Detach JavaMail #6165

Merged
merged 14 commits into from
Jan 18, 2022
Merged

Detach JavaMail #6165

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
32f1fac
Do not use `javax.mail` for `doCheckAdminAddress`
jglick Jan 6, 2022
15b1fdd
Detach JavaMail
basil Jan 6, 2022
1348ffb
Merge remote-tracking branch 'origin/master' into javamail
basil Jan 7, 2022
72bc903
Revert "Bump `git-changelist-maven-extension` from 1.2 to 1.3 (#6132)"
basil Jan 7, 2022
21e2f37
Revert "Revert "Bump `git-changelist-maven-extension` from 1.2 to 1.3…
basil Jan 8, 2022
b87684e
Bump releases
basil Jan 8, 2022
64aeabb
Fix split plugin cycles
basil Jan 8, 2022
5103131
Merge remote-tracking branch 'origin/master' into javamail
basil Jan 8, 2022
e6cb360
Bump releases
basil Jan 8, 2022
aa974a6
Bump versions
basil Jan 8, 2022
faf50aa
Bump versions
basil Jan 9, 2022
8f99ae4
Update core/src/main/resources/jenkins/split-plugins.txt
basil Jan 10, 2022
ed7b370
Update core/src/main/resources/jenkins/split-plugins.txt
basil Jan 11, 2022
770dd8f
Merge remote-tracking branch 'origin/master' into javamail
basil Jan 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions bom/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,11 +319,6 @@ THE SOFTWARE.
<artifactId>symbol-annotation</artifactId>
<version>1.1</version>
</dependency>
<dependency>
<groupId>com.sun.mail</groupId>
<artifactId>jakarta.mail</artifactId>
<version>1.6.5</version>
basil marked this conversation as resolved.
Show resolved Hide resolved
</dependency>

<!--XStream-->
<dependency>
Expand Down
4 changes: 0 additions & 4 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,6 @@ THE SOFTWARE.
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</dependency>
<dependency>
<groupId>com.sun.mail</groupId>
<artifactId>jakarta.mail</artifactId>
</dependency>
<dependency>
<groupId>jaxen</groupId>
<artifactId>jaxen</artifactId>
Expand Down
10 changes: 4 additions & 6 deletions core/src/main/java/jenkins/model/JenkinsLocationConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
import java.lang.reflect.Method;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.mail.internet.AddressException;
import javax.mail.internet.InternetAddress;
import javax.servlet.ServletContext;
import jenkins.util.SystemProperties;
import jenkins.util.UrlHelper;
Expand Down Expand Up @@ -210,11 +208,11 @@ public FormValidation doCheckUrl(@QueryParameter String value) {
}

public FormValidation doCheckAdminAddress(@QueryParameter String value) {
try {
new InternetAddress(value);
// TODO if equal to Messages.Mailer_Address_Not_Configured(), suggest configuring it with FormValidation.warning?
if (Util.fixNull(value).contains("@")) {
return FormValidation.ok();
} catch (AddressException e) {
return FormValidation.error(e.getMessage());
} else {
return FormValidation.error(Messages.JenkinsLocationConfiguration_does_not_look_like_an_email_address());
}
}

Expand Down
1 change: 1 addition & 0 deletions core/src/main/resources/jenkins/model/Messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ IdStrategy.CaseSensitive.DisplayName=Case sensitive
IdStrategy.CaseSensitiveEmailAddress.DisplayName=Case sensitive (email address)

Mailer.Address.Not.Configured=address not configured yet <nobody@nowhere>
JenkinsLocationConfiguration.does_not_look_like_an_email_address=Does not look like an email address
Mailer.Localhost.Error=Please set a valid host name, instead of localhost
Mailer.NotHttp.Error=The URL is invalid, please ensure you are using http:// or https:// with a valid domain.

Expand Down
2 changes: 2 additions & 0 deletions core/src/main/resources/jenkins/split-plugin-cycles.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,5 @@ junit jaxb
bouncycastle-api jaxb
command-launcher jaxb
jdk-tool jaxb

javax-activation-api javax-mail-api
3 changes: 3 additions & 0 deletions core/src/main/resources/jenkins/split-plugins.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ trilead-api 2.184 1.0.4

# JENKINS-64107
sshd 2.281 3.0.1

javax-activation-api 2.328 1.2.0-1
javax-mail-api 2.328 1.6.2-1
6 changes: 6 additions & 0 deletions test/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,12 @@ THE SOFTWARE.
<version>391.ve4a_38c1b_cf4b_</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>javax-mail-api</artifactId>
<version>1.6.2-1</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
Expand Down
12 changes: 12 additions & 0 deletions war/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,18 @@ THE SOFTWARE.
<version>1.0.4</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
<groupId>io.jenkins.plugins</groupId>
<artifactId>javax-activation-api</artifactId>
<version>1.2.0-1</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
<groupId>io.jenkins.plugins</groupId>
<artifactId>javax-mail-api</artifactId>
<version>1.6.2-1</version>
Copy link
Member

Choose a reason for hiding this comment

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

We used to bundle 1.6.5 and now we are detaching 1.6.2? https://github.com/jenkinsci/javax-mail-api-plugin/blob/javax-mail-api-1.6.2-1/pom.xml#L47 vs.

jenkins/bom/pom.xml

Line 325 in c31d930

<version>1.6.5</version>

BTW you may consider https://www.jenkins.io/doc/developer/publishing/releasing-cd/#pom-file-modifications having plugin version automatically track library version. Otherwise it is all too easy for Dependabot to let https://github.com/jenkinsci/javax-mail-api-plugin/blob/e8b4d80f40e47e76aad413366953d333b0975f97/pom.xml#L36-L47 get out of synch.

Copy link
Member Author

Choose a reason for hiding this comment

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

We used to bundle 1.6.5 and now we are detaching 1.6.2?

Correct. Really the root cause of this problem is that the Jakarta organization published a few releases under the original package names before switching to the Jakarta package names. I suppose we could release a 1.6.5 version of javax-mail-api that switches to the last Jakarta organization release prior to the Jakarta package rename. But then plugins wouldn't be able to depend on javax-mail-api and jakarta-mail-api concurrently without awkward Enforcer conflicts. Seemed simpler to avoid the issue by sticking to the last pre-Jakarta-organization release. I have not been able to find any meaningful differences between the last "pre-Jakarta-organization" release and the last "post-Jakarta-organization-change-pre-package-rename" release that would affect end users, much less any binary incompatibilities.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I did not follow much of this. My understanding was that 2.x uses the new package name while 1.6.x uses the old. What is the delta between 1.6.2 and 1.6.5?

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 a bit tough to explain, but 1.6.2 was the last release under the original groupId and artifactId (and used the javax. package namespace). 1.6.3, 1.6.4, and 1.6.5 were released under a new Jakarta groupId and artifactId (but still used the javax. package namespace). 2.0.0 was released under the same Jakarta groupId and artifactId (but used the jakarta. package namespace). So this makes things a bit awkward if you want to put both the old javax. classes and the new jakarta. packages on the same classpath. You can't easily do that with 1.6.3, 1.6.4, and 1.6.5 because they share the same groupId and artifactId as 2.0.0 (despite having different packages).

This doesn't matter much for production, but it does matter for plugin/BOM tests that might pull in e.g. both javax-mail-api and jakarta-mail-api as test-scoped dependencies. If I had used 1.6.5 in javax-mail-api that would have forced me to use the Jakarta groupId and artifactId, which would then conflict with the 2.0 version from jakarta-mail-api. If you want to have both sets of packages on the classpath at the same time, you basically need to stick to the original pre-Jakarta group ID and artifact ID, which also means being behind a few very small minor releases.

Just how small? See https://eclipse-ee4j.github.io/mail/docs/CHANGES.txt

Copy link
Member

Choose a reason for hiding this comment

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

OK, makes sense now, thanks for explanation!

<type>hpi</type>
</artifactItem>
</artifactItems>
<outputDirectory>${project.build.directory}/${project.build.finalName}/WEB-INF/detached-plugins</outputDirectory>
<stripVersion>true</stripVersion>
Expand Down