Add option to run an application on either CLASSPATH or MODULEPATH

[abhinayagarwal]

Adds a new configuration to the plugin: runtimePath which accepts 3 values:

• MODULEPATH: Adds all dependencies to modulepath.
• CLASSPATH: Adds all dependencies to classpath (including JavaFX dependencies)
  DEFAULT: Switches to the default behaviour of the plugin

Fixes #72 and #91

[HGuillemet]

What is the status of this PR ? Could you merge it ?

[abhinayagarwal]

Hi @HGuillemet ,

Thanks for the reminder. I will have a look at this PR in the coming days and make sure its merged.

[johanvos]

No DEFAULT on RuntimePath:

 Failed to execute goal org.openjfx:javafx-maven-plugin:0.0.5-SNAPSHOT:run (default-cli) on project dlsample: Unable to parse configuration of mojo org.openjfx:javafx-maven-plugin:0.0.5-SNAPSHOT:run: Cannot convert 'DEFAULT' to Enum: No enum constant org.openjfx.model.RuntimePath.DEFAULT -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.openjfx:javafx-maven-plugin:0.0.5-SNAPSHOT:run (default-cli) on project dlsample: Unable to parse configuration of mojo org.openjfx:javafx-maven-plugin:0.0.5-SNAPSHOT:run: Cannot convert 'DEFAULT' to Enum

[abhinayagarwal]

Hi @johanvos

This issue has been fixed with the latest commit.

[abhinayagarwal]

Hi @HGuillemet ,

Jose and I are at a standstill about what should happen if the application contains a module-info.java descriptor file but the runtimePathOption CLASSPATH is set in the plugin configuration. There are 2 choices:

1. The plugin should ignore module-info descriptor and run the application as it would have in a non-modular world, keeping all the dependencies and classes on classpath
2. The module-info.java descriptor takes precedence and an error is thrown. For ex: "Applications with module descriptor can't run on classpath. You need to remove the descriptor file or the runtimeOption and try again"

For more information, please have a look at the conversation.

What are your thoughts for this case?

[betanzos]

Hi @HGuillemet ,

Jose and I are at a standstill about what should happen if the application contains a module-info.java descriptor file but the runtimePathOption CLASSPATH is set in the plugin configuration. There are 2 choices:

1. The plugin should ignore module-info descriptor and run the application as it would have in a non-modular world, keeping all the dependencies and classes on classpath
2. The module-info.java descriptor takes precedence and an error is thrown. For ex: "Applications with module descriptor can't run on classpath. You need to remove the descriptor file or the runtimeOption and try again"

For more information, please have a look at the conversation.

What are your thoughts for this case?

IMO, I prefer the option 1 adding a WARNING message informing it. Since the plugin behavior for runtimePathOption default value is to run the application in modulepath, setting up a different value is an explicit intentions declaration that tell us: hey!, I want to force to run this application on CLASSPATH.

Something really important is that when there are a module descriptor the compiler will ALWAYS use it. Because of this, the module descriptor must be properly configured. Otherwise we will have compilation errors.

[HGuillemet]

In fact applications with module descriptors can run on classpath, if they are launched in the old way.
And conversely the main class can be loaded from a non-modular jar (loaded in the unnamed module) but the whole application can contain modules.
I wonder if it won't be better and clearer to distinguish options that control how the classpath and modulepath are populated, and an option to choose between running the main class in a module (-m ...) or not (old way with the class name, or -jar), and which default depends on the presence of a module descriptor.

• if main class is run in a module, the default behavior to populate the module path using the module descriptor and putting the rest of dependencies in the classpath is the most common need. Putting all dependencies in module path can be useful. Putting all dependencies in the classpath doesn't make much sense and should probably be forbidden.
• if main class is run in the unamed module. the default behavior should be to put all dependencies in the class path and the module descriptor, if present, should be ignored. But putting all dependencies in the module path too can be useful.

Also, a jar could technically appear in both module and class path, it's the presence of a requires or --add-modules that determines if it's loaded as a named module or in the unnamed module.

So to answer your question: 1) is probably better, but I wonder if some configuration like:

<mainClassLaunchStyle>auto | module | classpath</mainClassLaunchStyle>
<classPath>auto | all-dependencies</classPath>
<modulePath>auto | all-dependencies</modulePath>

won't be clearer and allows more flexibility.

[abhinayagarwal]

Since the plugin behavior for runtimePathOption default value is to run the application in modulepath

runtimePathOption doesn't have a DEFAULT value any more. In case runtimePathOption is not added, the plugin decides the path on which dependencies needs to go depending on different factors. For example, any dependency which is already modularised or contains AutomaticModule name will be on modulepath. Rest will go on the classpath.

an option to choose between running the main class in a module (-m ...) or not (old way with the class name, or -jar), and which default depends on the presence of a module descriptor.

The plugin already does this, inherently, without giving an option to the user. In the presence of a module descriptor, -m is used. In its absence, its ignored.

With this PR adding <runtimePathOption>CLASSPATH</runtimePathOption> allows the user to ignore the module descriptor even when it is present. Therefore, -m is not used.

mainClassLaunchStyle	runtimePathOption	remarks
auto	NA	mainclass on module/classpath, depending on descriptor
module	MODULEPATH	Fails, if no module descriptor is present
classpath	CLASSPATH	Ignores module descriptor, if present

The only case, I can imagine, not fulfilled here is when the user wants all dependencies on modulepath, have a module descriptor, and yet run the mainclass from classpath. Adding 3 different configuration options to the plugin for one case is an overkill. @HGuillemet Am I missing any other case(s)?

Moreover, 3 different options causes too many non-valid cases to control if all of these options are used together. For example:

<classPath>all-dependencies</classPath>
<modulePath>all-dependencies</modulePath>

or

<mainClassLaunchStyle>module</mainClassLaunchStyle>
<classPath>all-dependencies</classPath>

[HGuillemet]

Since the plugin behavior for runtimePathOption default value is to run the application in modulepath

runtimePathOption doesn't have a DEFAULT value any more. In case runtimePathOption is not added, the plugin decides the path on which dependencies needs to go depending on different factors. For example, any dependency which is already modularised or contains AutomaticModule name will be on modulepath. Rest will go on the classpath.

I though you reconstructed the module graph (from the descriptors) and add to the module path the modules that will be brought into the graph by the requires. That was the main point of the MODULEPATH runtimePathOption : allow modules to be found when they are brought to the graph by --add-modules.

an option to choose between running the main class in a module (-m ...) or not (old way with the class name, or -jar), and which default depends on the presence of a module descriptor.

The plugin already does this, inherently, without giving an option to the user. In the presence of a module descriptor, -m is used. In its absence, its ignored.

Yes, that corresponds to the default of the suggested mainClassLaunchStyle option.

Moreover, 3 different options causes too many non-valid cases to control if all of these options are used together. For example:

<classPath>all-dependencies</classPath>
<modulePath>all-dependencies</modulePath>

Technically, this is valid.

<mainClassLaunchStyle>module</mainClassLaunchStyle>
<classPath>all-dependencies</classPath>

This too.

You might as well put all maven dependencies to both classpath and module path. That will cover all cases and won't cause troubles whatever the launch style is, but having huge command lines and maybe uselessly slowing down a bit the process of finding modules and classes. What really matters is what is brought into the module graph with -m, with requires and with --add-modules

I started to augment your table with all possible cases (presence of descriptor, launch style, class path and module path population), but that's quite complex and not fully determined as long as we don't know what the default/auto behaviour exactly is for each option.

Examples of what is not covered by the PR:

• the main class is in the unnamed module (classpath launch style) but some of the dependencies must be run as modules, so we need a module path
• the main class is in a jar without descriptor, but must be run as an automatic module (module launch style)

I don't know if giving fine-grain control to cover all what can be done with the command line is the way to go. The concerned use cases seem quite rare. But the standstill you were at is the sign that there might be some more natural configuration options that will cover more cases while still having a nice default behaviour that will make 99% of users happy.

[abhinayagarwal]

Hi @HGuillemet ,

I though you reconstructed the module graph (from the descriptors) and add to the module path the modules that will be brought into the graph by the requires

Previously, the module path was build using the descriptor if one was present. With this PR, if one uses runtimePathOption as MODULEPATH, we put everything on the module-path, irrespective of its presence in the module descriptor. If runtimePathOption is not provided and a module-descriptor is present, the modules are still added according to the descriptor.

You might as well put all maven dependencies to both classpath and module path

I won't do this :)

But the standstill you were at is the sign that there might be some more natural configuration options that will cover more cases while still having a nice default behaviour that will make 99% of users happy.

I am sure there will always be area of improvement. For this PR, my hunch is that we are at a good position and it should be merged as it is.

[kevinrushforth]

@abhinayagarwal If I understand correctly, it seems that the CLASSPATH option allows loading the JavaFX modules themselves on the CLASSPATH? If so, this is not a supported mode. As an FYI, I filed JDK-8256362 to emit a warning to that effect.

[abhinayagarwal]

@kevinrushforth Thanks for the information. I will add a warning for the same in the plugin.