Fixes #12900 - JPMS and environments. #12910
Conversation
sbordet
commented
Mar 14, 2025
sbordet
commented
- Environments are now built using a ModuleLayer, that contains all the jars related to that environment.
- Introduced websocket-core-[client|server].mod to avoid that the WebSocket core jars will be present in both the server ModuleLayer and the environment ModuleLayer, causing JPMS errors.
- Now server.mod specifies --add-modules ALL-DEFAULT,ALL-MODULE-PATH. ALL-DEFAULT is necessary to expose JDK modules to the environment ModuleLayer. ALL-MODULE-PATH was previously hardcoded in the code, now it's externalized.
- Removed most of the [jpms] directives in *.mod files, as they are either covered by ALL-DEFAULT, or not necessary.
- EnvironmentBuilder now uses the ModuleLayer APIs to create an environment ModuleLayer child of the server ModuleLayer (the latter created by the JVM at startup, containing all the server/core jars).
* Environments are now built using a ModuleLayer, that contains all the jars related to that environment. * Introduced websocket-core-[client|server].mod to avoid that the WebSocket core jars will be present in both the server ModuleLayer and the environment ModuleLayer, causing JPMS errors. * Now server.mod specifies --add-modules ALL-DEFAULT,ALL-MODULE-PATH. ALL-DEFAULT is necessary to expose JDK modules to the environment ModuleLayer. ALL-MODULE-PATH was previously hardcoded in the code, now it's externalized. * Removed most of the [jpms] directives in *.mod files, as they are either covered by ALL-DEFAULT, or not necessary. * EnvironmentBuilder now uses the ModuleLayer APIs to create an environment ModuleLayer child of the server ModuleLayer (the latter created by the JVM at startup, containing all the server/core jars). Signed-off-by: Simone Bordet <[email protected]>
…leLayers hierarchy to get the module-path of each layer, as well as navigate the ClassLoader hierarchy. Removed now unnecessary part that was splitting system property "jdk.module.path". Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/JPMSOpts.java
Outdated
Show resolved
Hide resolved
| environment = Environment.get(envName); | ||
| if (environment == null) | ||
| envBuilder = new EnvironmentBuilder(envName); |
There was a problem hiding this comment.
This code will work so that an already known environment will switch subsequent args back to acting on the main environment. I don't think we want that. Maybe throw?
There was a problem hiding this comment.
Not sure I understand?
An already known environment will be retrieved by Environment.get but it will have that given name, so it would act on that environment, not the "main" environment (not sure what you refer to here by "main": you mean the current environment, or the core (i.e. not eeN) environment?).
A command line --env ee10 --add-modules foo --env ee11 --add-modules bar --env ee10 --add-modules baz would be badly formatted, I don't think we want to support that.
There was a problem hiding this comment.
Exactly. We should throw. Currently the second --env ee10 will reset the environment builder to null and then continue.
There was a problem hiding this comment.
Currently the second --env ee10 will reset the environment bullet to null and then continue.
Yes. And if a --add-modules (or the others explicitly mentioned in the cases) is specified, it will throw NPE.
Otherwise, if it's an XML or properties file, will be applied to the right environment.
Seems to me we already do the right thing.
There was a problem hiding this comment.
Nope. There are paths through that don't throw. Just throw if an environment is already defined
There was a problem hiding this comment.
@gregw I have added a test case, and are now throwing if the environment already exists.
Signed-off-by: Simone Bordet <[email protected]>
...ty-websocket/jetty-websocket-jetty-client/src/main/config/modules/websocket-jetty-client.mod
Show resolved
Hide resolved
jetty-core/jetty-xml/src/main/java/org/eclipse/jetty/xml/EnvironmentBuilder.java
Show resolved
Hide resolved
Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
