Upgrade Jetty from 9.4.49.v20220914 to 10.0.12 #409
Conversation
basil
added
dependencies
| @@ -1,15 +1,16 @@ | |||
| # https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates | |||
|
|
|||
| --- | |||
There was a problem hiding this comment.
Or just delete the comment since the link is dead anyway!
|
|
||
| ## Updating Jetty | ||
| `hpi:run` mojo is a variant of `jetty:run` mojo, and because of the way plugin descriptor is generated, this module copies some code from Jetty Maven plugin, specifically `AbstractJettyMojo.java` and `ConsoleScanner.java`. | ||
|
|
||
| To keep upstream tracking easier, pristine copies of these files are copied into `incoming-x.y` branch, then package renamed. This version specific incoming branch is then "theirs" merged into the `incoming` branch, which acts as the upstream tracking branch. | ||
|
|
||
| This branch is then merged into `master` via `git merge -X ignore-space-at-eol incoming`. See diff between `incoming` and `master` on these files to see the exact local patches. |
There was a problem hiding this comment.
No longer relevant, as we no longer copy code from Jetty Maven plugin.
| <exclusions> | ||
| <exclusion> | ||
| <groupId>asm</groupId> | ||
| <artifactId>asm</artifactId> | ||
| </exclusion> | ||
| </exclusions> |
There was a problem hiding this comment.
Was pulling in an ancient version of ASM 3, which is not compatible with Jetty Maven plugin.
| <exclusions> | ||
| <exclusion> | ||
| <groupId>jakarta.annotation</groupId> | ||
| <artifactId>jakarta.annotation-api</artifactId> | ||
| </exclusion> | ||
| </exclusions> |
There was a problem hiding this comment.
Was pulling in a Jakarta import version of this API which conflicts with that of Jetty.
| <exclude>org.codehaus.plexus:plexus-archiver</exclude> | ||
| <exclude>org.codehaus.plexus:plexus-container-default</exclude> | ||
| <exclude>org.ow2.asm:asm</exclude> |
There was a problem hiding this comment.
Miscellaneous cleanup: realized that this was unnecessary while updating this list.
| getWebAppConfig().setWar(webAppFile.getCanonicalPath()); | ||
| super.configureWebApp(); |
There was a problem hiding this comment.
This has to be done after setting the WAR rather than before in order to avoid tripping an assertion.
| @@ -540,6 +534,7 @@ public void configureWebApplication() throws Exception { | |||
| userStore.addUser("bob", new Password("bob"), new String[] {"user", "male"}); | |||
| userStore.addUser("charlie", new Password("charlie"), new String[] {"user", "male"}); | |||
| getWebAppConfig().getSecurityHandler().setLoginService(hashLoginService); | |||
| finishConfigurationBeforeStart(); | |||
There was a problem hiding this comment.
The super class no longer provides two distinct hooks here, but to keep the diff small I retained the existing split between two methods and made the first vector into the second.
| @@ -605,93 +600,30 @@ private String loadVersion(InputStream is) throws IOException { | |||
| } | |||
|
|
|||
| @Override | |||
| public void configureScanner() throws MojoExecutionException { | |||
| public void startScanner() throws Exception { | |||
There was a problem hiding this comment.
Adapting the hack to changes in the superclass's calling conventions. See the comment for explanation.
| // Use a bigger buffer, as Stapler traces can get pretty large on deeply nested URLs. | ||
| hc.setResponseHeaderSize(12 * 1024); | ||
|
|
||
| super.startScanner(); | ||
| } |
There was a problem hiding this comment.
Was copypasta from the superclass with no customizations, except for the call to generateHpl() which was dead code anyway.
| protected boolean isPackagingSupported() { | ||
| if (!supportedPackagings.contains("hpi")) { | ||
| List<String> newSupportedPackagings = new ArrayList<>(supportedPackagings); | ||
| newSupportedPackagings.add("hpi"); | ||
| supportedPackagings = List.copyOf(newSupportedPackagings); | ||
| } | ||
| return super.isPackagingSupported(); | ||
| } |
There was a problem hiding this comment.
I considered forcing consumers to add
<supportedPackagings>
<supportedPackaging>hpi</supportedPackaging>
</supportedPackagings>but felt it would be smoother if the default included hpi. I didn't remove war but I don't feel strongly about this.
| @@ -1,15 +1,16 @@ | |||
| # https://help.github.com/github/administering-a-repository/configuration-options-for-dependency-updates | |||
|
|
|||
| --- | |||
There was a problem hiding this comment.
Or just delete the comment since the link is dead anyway!
| @Parameter(property = "port", defaultValue = "8080") | ||
| protected int defaultPort; |
There was a problem hiding this comment.
I think we would prefer to keep the static port anyway, since this is for interactive use and having a stable root URL across runs is desirable.
Now that the HPI plugin requires Java 11, we can move to a Jetty 10 based implementation To test this I ran
mvn hpi:runbefore and after this change, ensuring that Jenkins came up successfully in both cases and could be reloaded by pressing the enter key.