Skip to content

Conversation

@desruisseaux
Copy link
Contributor

@desruisseaux desruisseaux commented Oct 25, 2025

The specification that we wrote in maven.mdo said about the <targetPath> element:

Specifies an explicit target path, overriding the default value. The path is relative to the ${project.build.outputDirectory} directory, which is typically target/classes in a Java project.

However, the implementation in the DefaultSourceRoot(Session session, Path baseDir, Source source) constructor resolves against baseDir, which is not even inside target.

Furthermore, the specification is incomplete. The default directory should be target/classes when the scope is "main", but should be target/test-classes when the scope is "test". The current specification said nothing about the fact that the default value depends on the scope.

This pull request is divided in 4 commits. The first commit is only trivial javadoc fixes. The second commit refactors the DefaultSourceRoot implementation as a Java record, but without significant changes to its behaviour. The benefit of using record is to reduce the boilerplate code (the number of lines is reduced by about one third), and it also forces us to be more rigorous since all constructions must pass by the canonical constructor. The third commit is the one that actually fixes the issue described above. The last commit proposes a new method in the Project interface.

A dot is already automatically added by Javadoc, so these trailing
dots were causing dots to appear twice in the generated Javadoc.
… the code and forces

us to be more consistent since all constructions must pass by the canonical constructor.
This refactored class should have the same behavior as the previous class (no new feature).
…cation in `maven.mdo`

requires that we resolve against `${project.build.outputDirectory}`, which is not `baseDir`.
Also modify the specification for resolving against `${project.build.testOutputDirectory}`
if the scope is test and `${project.build.directory}` is the scope is neither main or test.
@desruisseaux desruisseaux force-pushed the fix/relative-target-path branch from cb433be to 410c4d0 Compare October 25, 2025 16:54
@desruisseaux desruisseaux requested a review from gnodet October 25, 2025 18:09
…d to repeat the same code in the plugins.

The `SourceRoot.targetPath(Project)` method become simpler, delegating most of the work to the new method.
Copy link
Contributor

@gnodet gnodet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @desruisseaux Do we need to backport that to 4.0.0 ? The modifications in the API looks binary compatible, so this looks ok.

@gnodet gnodet added bug Something isn't working backport-to-4.0.x labels Oct 26, 2025
@desruisseaux
Copy link
Contributor Author

Yes, it was my plan to prepare a backport if this pull request is approved.

@desruisseaux desruisseaux merged commit c6b008b into apache:master Oct 26, 2025
73 of 76 checks passed
@desruisseaux desruisseaux deleted the fix/relative-target-path branch October 26, 2025 17:22
@github-actions github-actions bot added this to the 4.1.0 milestone Oct 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-4.0.x bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants