Issue #9396 - fixes to resolve WebSocket JPMS warnings

[lachlan-roberts]

Issue #9396

• fix some JPMS warnings for websocket
• add testing for websocket with JPMS in DistributionTests
• Make WebSocketShutdownContainers publicly exported in module-info files.
• Optional dependencies for WebSocketShutdownContainer and JettyWebSocketClientConfiguration should be requires static transitive as they are in public signatures but also optional.

[lachlan-roberts]

I don't think the requires static transitive works. I'm getting the following error when running tests for jetty-ee9-websocket-client

Corrupted channel by directly writing to native stream in forked JVM 1. Stream 'java.lang.module.ResolutionException: Module org.eclipse.jetty.ee9.websocket.jetty.client does not read a module that exports org.eclipse.jetty.ee9.webapp'.

The issue is that we must export the package of JettyWebSocketClientConfiguration as we use it with the service provider, and because this is publicly exposed we need the requires transitive. However we also want to make it an optional dependency so we want the requires static. And so the addition of the static modifier is what is causing this error.

[gregw]

@janbartel help

[gregw]

i think this could just be a surefire issue. I might needs something like:

      <plugin>
        <artifactId>maven-surefire-plugin</artifactId>
        <configuration>
          <argLine>
            @{argLine} ${jetty.surefire.argLine}
            --add-modules org.eclipse.jetty.ee9.webapp
            --add-reads org.eclipse.jetty.ee9.websocket.jetty.client=org.eclipse.jetty.ee9.webapp
          </argLine>
        </configuration>
      </plugin>

[gregw]

@lachlan-roberts that fixed it, so I've pushed to this branch.
@janbartel do you think that looks right?

[janbartel]

You do not need the read line. That is only used where we cannot declare an outright requires in the module-info.java, eg for test-only dependencies. Your module-info.java has declared a requires static and that must be sufficient and should not need to be forcibly overridden in this way.

[gregw]

wot @janbartel said!

I think that was my original mistake and I went from "Try A" to "Try A and B" without actually doing "Try B".

[lachlan-roberts]

These --add-modules and --add-reads are removed by #9981.
I will merge these two together now simone is happy with other changes in this PR.

[gregw]

Look OK to me

[gregw]

@jmcc0nn3ll @joakime This has not actually been built in CI?

[jmcc0nn3ll]

@jmcc0nn3ll @joakime This has not actually been built in CI?

This needs to push a notification to the build servers, I updated DNS but perhaps there's something else that we need to do to fix that connection.