8000 Allow to not use --module-version for the Java compiler by pzygielo · Pull Request #331 · apache/maven-compiler-plugin · GitHub
[go: up one dir, main page]
Skip to content

Allow to not use --module-version for the Java compiler #331

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
Merged
merged 1 commit into from
Jun 20, 2025
Merged

Allow to not use --module-version for the Java compiler #331

merged 1 commit into from
Jun 20, 2025

Conversation

pzygielo
Copy link
Contributor

@pzygielo pzygielo marked this pull request as ready for review May 26, 2025 12:27
@desruisseaux
Copy link
Contributor

Do we need to add a new useModuleVersion plugin configuration option? Couldn't the users set the existing moduleVersion configuration option to an empty value instead if they do not want module version? It would still require a change in the code, but would avoid increasing the number of plugin configuration options.

@pzygielo
Copy link
Contributor Author

Couldn't the users set the existing moduleVersion configuration option to an empty value instead if they do not want module version?

Let me explore that option. As long as the same goal is achieved I'm not attached to new parameter.

@pzygielo
Copy link
Contributor Author

Do we need to add a new useModuleVersion plugin configuration option? Couldn't the users set the existing moduleVersion configuration option to an empty value

Unclear how to preserve compatibility with current default of project.version (since 3.14). Using special value not conforming to ModuleDescriptor.Version could be considered, but I don't like that idea very much.

Thus I prefer proposed solution, but I'm open to see it being solved differently.

@desruisseaux
Copy link
Contributor

No special value, just allow users to specify <moduleVersion/>. The value is set to an empty or blank string, and the code just needs to test if (!moduleVersion.isEmpty()) { add the option }.

@pzygielo
Copy link
Contributor Author

With <moduleVersion/> project.version seems to be injected. So !moduleVersion.isEmpty would always be true unless default was removed.

@pzygielo
Copy link
Contributor Author

Then, I don't know how to differentiate the case of explicit empty or blank string set with <moduleVersion/> from the case this is not set at all, for the project.version to be used.

@desruisseaux
Copy link
Contributor

It may be a question for @gnodet: we have a configuration option declared that way:

@Parameter(property = "maven.compiler.moduleVersion", defaultValue = "${project.version}")
private String moduleVersion;

Do you know if it is possible for a user to explicitly set an empty value? Apparently, using <moduleVersion/> causes Maven to fallback on defaultValue. Is there a way to said "do not use the default"? (other than adding a new useMavenVersion plugin configuration option).

@slawekjaranowski
Copy link
Member

we can also use a special value like skip for moduleVersion parameter ... but I'm not sure if will be a clear way
so new parameter is ok for me

@slawekjaranowski slawekjaranowski added the enhancement New feature or request label Jun 6, 2025
@pzygielo
Copy link
Contributor Author
pzygielo commented Jun 6, 2025

I'm also not sure about special value.

But perhaps new option, if it's accepted, could have better name than I proposed here.

@slawekjaranowski
Copy link
Member

I'm also not sure about special value.

But perhaps new option, if it's accepted, could have better name than I proposed here.

for me name is ok 😄

@desruisseaux
Copy link
Contributor

Sorry for being silent (I have a meeting to prepare next week). It seems to me that the cleanest solution would be to put an empty value <modul 8000 eVersion>. I haven't investigated yet why it doesn't work. I thought that maybe, if we cannot rely on the value injected automatically by Maven in annotated field, maybe we can find this information in Model. I haven't checked yet if this is feasible.

@slawekjaranowski
Copy link
Member

Here is s reason:

so I would not to do a magic in plugin for it

@desruisseaux
Copy link
Contributor

Thanks for the link. Alternatively, in the following declaration (added in commit 09dce4e):

@Parameter(property = "maven.compiler.moduleVersion", defaultValue = "${project.version}")
private String moduleVersion;

What about removing the default value completely, and let users specify it explicitly? The rational given in MCOMPILER-579 (that ${project.version} is not a valid module version when the version ends with -SNAPSHOT) seems quite convincing.

@slawekjaranowski
Copy link
Member

Thanks for the link. Alternatively, in the following declaration (added in commit 09dce4e):

@Parameter(property = "maven.compiler.moduleVersion", defaultValue = "${project.version}")
private String moduleVersion;

What about removing the default value completely, and let users specify it explicitly? The rational given in MCOMPILER-579 (that ${project.version} is not a valid module version when the version ends with -SNAPSHOT) seems quite convincing.

when we remove default value we will break a current behavior

8000
@pzygielo
Copy link
Contributor Author
pzygielo commented Jun 7, 2025

The rational given in MCOMPILER-579

There is something missing or misunderstood.

(that ${project.version} is not a valid module version when the version ends with -SNAPSHOT)

is completely valid module version when it ends with anything, as long as it follows ModuleDescriptor.Version spec (starts with the number, which the other example in issue HEAD-SNAPSHOT does not).

@pzygielo
Copy link
Contributor Author
pzygielo commented Jun 7, 2025 •
edited
Loading

For the

when we remove default value we will break a current behavior

additionally consider, that using the project.version was not introduced in

It just enabled module version to be modified.
So the behaviour of not specifying explicit module version and having it anyway is the old, old feature (2ba3f26).

@desruisseaux
Copy link
Contributor

(...snip...) is completely valid module version when it ends with anything,

Ah, okay. I do not remember having tested.

@pzygielo
Copy link
Contributor Author

@desruisseaux Do you have any other comments?

@desruisseaux
Copy link
Contributor

No other comment. I'm still interested in making possible to set an empty value instead of adding a new parameter, but I may explore this possibility in version 4. I should probably learn more about the relationship between Eclipse SISU and Maven 4 (for eclipse-sisu#167).

@pzygielo
Copy link
Contributor Author

Then, this could be merged as proposed, right?

@desruisseaux
Copy link
Contributor

Yes, sorry for being so slow. For merges on the 3.x branch, I tend to leave it @slawekjaranowski or any other who is familiar with the long history of this plugin and its interaction with Maven 3 (I'm more familiar with Maven 4 code base than Maven 3).

@slawekjaranowski slawekjaranowski merged commit 164bad4 into apache:maven-compiler-plugin-3.x Jun 20, 2025
88 checks passed
@github-actions github-actions bot added this to the 3.14.1 milestone Jun 20, 2025
@pzygielo pzygielo deleted the nmv branch June 20, 2025 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slawekjaranowski slawekjaranowski slawekjaranowski approved these changes

Assignees

@slawekjaranowski slawekjaranowski

Labels
enhancement New feature or request
Projects
None yet
Milestone
3.14.1
Development

Successfully merging this pull request may close these issues.
3 participants
@pzygielo @desruisseaux @slawekjaranowski
0