Re: How to set default directories when developing a reporting plugin?
On 2023/10/22 02:38:07 Alexander Kriegisch wrote:
> Hello Michael, hello community.
>
> I have decided to actually change the plugin's behaviour the way I
> described:
>
> -- I overrode the default value of AbstractMavenReport.outputDirectory
> [1] by defining a setter method and adding a @Parameter annotation,
> specifying ${project.build.directory} as the base direcory for the
> report, because this is what I want to use when the mojo is
> executed directly. The abstract super class defaults to
> ${project.reporting.outputDirectory}, which is IMO suboptimal. In a
> site generation context, the report directory is set automatically
> anyway, because AbstractMavenReport.setReportOutputDirectory, as
> mentioned in my original message, sets both reportOutputDirectory
> and outputDirectory. BTW, is this "trick" to override an inherited
> property correct? Is there a canonical way to do that?
>
> -- Then, I introduced a new reportDirectory [2] property, whis is just
> a String, not a File. It specifies the subdirectory to be appended
> to outputDirectory [3]. This works in both use cases, stand-alone
> mojo execution and site generation.
>
> -- Hence, it is now possible to configure the base directory and the
> subdirectory name separately: The base directory is set in the
> identically named property outputDirectory, either in the plugin or
> in Maven Site, depending on the use case. The subdirectory is set
> in the new reportDirectory property. I.e., reports will end up in
> the following directories:
>** without configuration, stand-alone:
> target/aspectj-report
>** without configuration, site generation:
> target/site/aspectj-report
>** custom base directory, stand-alone: e.g.
> my-target/aspectj-report
>** custom base directory, site generation: e.g.
> target/my-site/aspectj-report
>** custom subdirectory, stand-alone: e.g.
> target/my-aspectj-report
>** custom subdirectory, site generation: e.g.
> target/site/my-aspectj-report
>** custom base + subdirectory, stand-alone: e.g.
> my-target/my-aspectj-report
>** custom base + subdirectory, site generation: e.g.
> target/my-site/my-aspectj-report
> Notably, the custom base directory for site generation should work
> for all plugins playing nicely, e.g. Javadoc or (now after my
> change) AspectJ. See the integration tests [4] for examples, namely
> CreateReport* test cases for stand-alone and CreateSite* ones for
> site generation use cases. For each test case, compare the
> differences in Maven Site and AspexctJ Maven settings and the
> corresponding checks in verify.groovy.
>
> [1]
> https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L90-L106
> [2]
> https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L57-L67
> [3]
> https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L255
> [4] https://github.com/dev-aspectj/aspectj-maven-plugin/tree/main/src/it
Just checked these. They look reasonable. I need to check MJAVADOC whether it
is reasonable or not. Let's continue with the open PRs.
-
To unsubscribe, e-mail: users-unsubscr...@maven.apache.org
For additional commands, e-mail: users-h...@maven.apache.org
Re: How to set default directories when developing a reporting plugin?
Michael,
first of all thanks for your conceptual thoughts and efforts to improve
both code and documentation.
> Let me know what you think!
I fully agree that the notion that both the output directory and the
report output directory are meant to be *base* directories. I disagree,
however, with that they should default to the same base directory. Hence
https://github.com/apache/maven-reporting-impl/pull/26. I am more in the
Maven Javadoc "fan club" in this case.
Regards
--
Alexander Kriegisch
https://scrum-master.de
Michael Osipov schrieb am 23.10.2023 00:53 (GMT +07:00):
> Alexander,
>
> I have now read the code of all our reporting plugins,
> maven-reporting-impl and maven-site-plugin.
>
> Here is my understanding how I expect to work which it is not at the
> moment and there is a flaw in the system were both MJXR and MJAVADOC
> apply a hack and I will explain why they do.
>
> We have
>
> standalone execution:
>> @Parameter(defaultValue = "${project.reporting.outputDirectory}",
>> readonly
>> = true, required = true)
>> protected File outputDirectory;
>
> and
>
> site execution:
>> @Parameter(property = "siteOutputDirectory", defaultValue =
>> "${project.reporting.outputDirectory}")
>> protected File outputDirectory;
>
> both point to root directory where all reports put there files in, from
> the simplest single page one, to multipage to those which are external
> and operate in subdirectories. Everything below that output directory is
> fixed and completely subject to the plugin.
>
> Both o.a.m.reporting.AbstractMavenReport.getReportOutputDirectory() and
> o.a.m.reporting.AbstractMavenReport.setReportOutputDirectory(File) exist
> for the sole purpose to accomondate both execution styles. One value is
> exected through DI (standalone) the other one set (through site).
> Therefore, in both cases #getReportOutputDirectory() should be used to
> get the right value to write to.
> In any case #setReportOutputDirectory(File) does *not* contain a
> directory for the reporting plugin only, but for the entire generated
> tree of plugins, regardless of 'mvn site' /or/ 'mvn pmd:pmd
> checkstyle:checkstyle'.
>
> Now, o.a.m.reporting.MavenReport.getOutputName() has bad documentation
> because it is actually a relative output *path* to the generated item
> with the base/root path given by
> o.a.m.reporting.MavenReport.getReportOutputDirectory(). Again, here is
> the name completely deceiving. Needs doc update as well.
>
> You might wonder, what to do now with:
>> @Override
>> public String getOutputName() {
>> return "xref/index";
>> }
>
> and
>
>> @Override
>> public void setReportOutputDirectory(File reportOutputDirectory) {
>> if ((reportOutputDirectory != null)
>> &&
>> (!reportOutputDirectory.getAbsolutePath().endsWith("xref")))
>> {
>> this.destDir = new File(reportOutputDirectory,
>> "xref").getAbsolutePath();
>> } else {
>> this.destDir = reportOutputDirectory.getAbsolutePath();
>> }
>> }
> ?
>
> Well, while I consider the former is correct, the latter is not:
> * Does one expect to pass *null* as reportOutputDirectory?
> * Why is *this.reportOutputDirectory* never updated?
> * It expects the report this live under *.../xref*, but ignores
>> /**
>> * Folder where the Xref files will be copied to.
>> */
>> @Parameter(defaultValue = "${project.reporting.outputDirectory}/xref")
>> private String destDir;
> * It mixes the the notion of a *pluginReportOutputDirectory* which is
> passed to the external report generator as a root/base or a multi sink
> if you want to generate multiple reports under one dir, but have the
> starting point at *foo/index* (output name) and all resides under *foo/*.
> MJAVADOC does the same trick although it has replaces the literal 'xref'
> with a field, but the issue remains the same.
> Whether the path component(s) between the root and *index* should be
> flexible, is upto the plugin (e.g, xref, xref-test, apidocs, etc.), but
> from my personal PoV changing it does not make:
> * In site execution this has to be carried along
> * If you need to change it otherwise you can rename/move it with an ant
> task or a subprocessor.
>
> What I will do not is to create a few issues and PRs to improve the
> situation and create a PR for JXR on top of 'doxia-2.0.0' how I consider
> it should be. Some of the method names are unfortunately, but I am
> afraid it is too hard to change them given they have too widespread use.
>
> I guess after all, I will need to perform another round of releases with
> those PRs before I can start new reporting plugin releases. I think what
> you have raised was long overdue and absoltuely legit.
>
> Let me know what you think!
>
> @Hervé, I would also appreciate to hear your opinion on this!
>
> Michael
>
>
RE: How to set default directories when developing a reporting plugin?
Alexander,
I have now read the code of all our reporting plugins,
maven-reporting-impl and maven-site-plugin.
Here is my understanding how I expect to work which it is not at the
moment and there is a flaw in the system were both MJXR and MJAVADOC
apply a hack and I will explain why they do.
We have
standalone execution:
@Parameter(defaultValue = "${project.reporting.outputDirectory}", readonly
= true, required = true)
protected File outputDirectory;
and
site execution:
@Parameter(property = "siteOutputDirectory", defaultValue =
"${project.reporting.outputDirectory}")
protected File outputDirectory;
both point to root directory where all reports put there files in, from
the simplest single page one, to multipage to those which are external
and operate in subdirectories. Everything below that output directory is
fixed and completely subject to the plugin.
Both o.a.m.reporting.AbstractMavenReport.getReportOutputDirectory() and
o.a.m.reporting.AbstractMavenReport.setReportOutputDirectory(File) exist
for the sole purpose to accomondate both execution styles. One value is
exected through DI (standalone) the other one set (through site).
Therefore, in both cases #getReportOutputDirectory() should be used to
get the right value to write to.
In any case #setReportOutputDirectory(File) does *not* contain a
directory for the reporting plugin only, but for the entire generated
tree of plugins, regardless of 'mvn site' /or/ 'mvn pmd:pmd
checkstyle:checkstyle'.
Now, o.a.m.reporting.MavenReport.getOutputName() has bad documentation
because it is actually a relative output *path* to the generated item
with the base/root path given by
o.a.m.reporting.MavenReport.getReportOutputDirectory(). Again, here is
the name completely deceiving. Needs doc update as well.
You might wonder, what to do now with:
@Override
public String getOutputName() {
return "xref/index";
}
and
@Override
public void setReportOutputDirectory(File reportOutputDirectory) {
if ((reportOutputDirectory != null)
&& (!reportOutputDirectory.getAbsolutePath().endsWith("xref")))
{
this.destDir = new File(reportOutputDirectory,
"xref").getAbsolutePath();
} else {
this.destDir = reportOutputDirectory.getAbsolutePath();
}
}
?
Well, while I consider the former is correct, the latter is not:
* Does one expect to pass *null* as reportOutputDirectory?
* Why is *this.reportOutputDirectory* never updated?
* It expects the report this live under *.../xref*, but ignores
/**
* Folder where the Xref files will be copied to.
*/
@Parameter(defaultValue = "${project.reporting.outputDirectory}/xref")
private String destDir;
* It mixes the the notion of a *pluginReportOutputDirectory* which is
passed to the external report generator as a root/base or a multi sink
if you want to generate multiple reports under one dir, but have the
starting point at *foo/index* (output name) and all resides under *foo/*.
MJAVADOC does the same trick although it has replaces the literal 'xref'
with a field, but the issue remains the same.
Whether the path component(s) between the root and *index* should be
flexible, is upto the plugin (e.g, xref, xref-test, apidocs, etc.), but
from my personal PoV changing it does not make:
* In site execution this has to be carried along
* If you need to change it otherwise you can rename/move it with an ant
task or a subprocessor.
What I will do not is to create a few issues and PRs to improve the
situation and create a PR for JXR on top of 'doxia-2.0.0' how I consider
it should be. Some of the method names are unfortunately, but I am
afraid it is too hard to change them given they have too widespread use.
I guess after all, I will need to perform another round of releases with
those PRs before I can start new reporting plugin releases. I think what
you have raised was long overdue and absoltuely legit.
Let me know what you think!
@Hervé, I would also appreciate to hear your opinion on this!
Michael
-
To unsubscribe, e-mail: users-unsubscr...@maven.apache.org
For additional commands, e-mail: users-h...@maven.apache.org
Re: How to set default directories when developing a reporting plugin?
Hello Michael, hello community.
I have decided to actually change the plugin's behaviour the way I
described:
-- I overrode the default value of AbstractMavenReport.outputDirectory
[1] by defining a setter method and adding a @Parameter annotation,
specifying ${project.build.directory} as the base direcory for the
report, because this is what I want to use when the mojo is
executed directly. The abstract super class defaults to
${project.reporting.outputDirectory}, which is IMO suboptimal. In a
site generation context, the report directory is set automatically
anyway, because AbstractMavenReport.setReportOutputDirectory, as
mentioned in my original message, sets both reportOutputDirectory
and outputDirectory. BTW, is this "trick" to override an inherited
property correct? Is there a canonical way to do that?
-- Then, I introduced a new reportDirectory [2] property, whis is just
a String, not a File. It specifies the subdirectory to be appended
to outputDirectory [3]. This works in both use cases, stand-alone
mojo execution and site generation.
-- Hence, it is now possible to configure the base directory and the
subdirectory name separately: The base directory is set in the
identically named property outputDirectory, either in the plugin or
in Maven Site, depending on the use case. The subdirectory is set
in the new reportDirectory property. I.e., reports will end up in
the following directories:
** without configuration, stand-alone:
target/aspectj-report
** without configuration, site generation:
target/site/aspectj-report
** custom base directory, stand-alone: e.g.
my-target/aspectj-report
** custom base directory, site generation: e.g.
target/my-site/aspectj-report
** custom subdirectory, stand-alone: e.g.
target/my-aspectj-report
** custom subdirectory, site generation: e.g.
target/site/my-aspectj-report
** custom base + subdirectory, stand-alone: e.g.
my-target/my-aspectj-report
** custom base + subdirectory, site generation: e.g.
target/my-site/my-aspectj-report
Notably, the custom base directory for site generation should work
for all plugins playing nicely, e.g. Javadoc or (now after my
change) AspectJ. See the integration tests [4] for examples, namely
CreateReport* test cases for stand-alone and CreateSite* ones for
site generation use cases. For each test case, compare the
differences in Maven Site and AspexctJ Maven settings and the
corresponding checks in verify.groovy.
[1]
https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L90-L106
[2]
https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L57-L67
[3]
https://github.com/dev-aspectj/aspectj-maven-plugin/blob/9f2d0e97ed963c2b69a7fdf731a4c919a6bb56ef/src/main/java/org/codehaus/mojo/aspectj/AjcReportMojo.java#L255
[4] https://github.com/dev-aspectj/aspectj-maven-plugin/tree/main/src/it
--
Alexander Kriegisch
https://scrum-master.de
Michael Osipov schrieb am 22.10.2023 01:17 (GMT +07:00):
>
>
> On 2023/10/20 02:08:25 Alexander Kriegisch wrote:
>> Hello.
>>
>> I am trying to improve a reporting mojo in a Maven plugin. The mojo
>> extends org.apache.maven.reporting.AbstractMavenReport. I recently
>> upgraded it to Doxia 2.0 - thanks again to Hervé B. and Michael O. for
>> their support - and cleaned out some cruft, removing redundant super
>> class fields and methods) in the mojo code. So far, so good.
>>
>> My question is about the different behaviour for configuring the report
>> output directory when starting the mojo directly as a Maven goal
>> compared to it being started during Maven site generation. The Javadoc
>> is helpful and explains:
>>
>>
>>
>> public abstract class AbstractMavenReport extends AbstractMojo
>> implements MavenMultiPageReport
>> {
>> /**
>>* The output directory for the report. Note that this parameter is
>>* only evaluated if the goal is run directly from the command line.
>>* If the goal is run indirectly as part of a site generation, the
>>* output directory configured in the Maven Site Plugin is used
>>* instead.
>>*/
>> @Parameter(defaultValue = "${project.reporting.outputDirectory}",
>> readonly = true, required = true)
>> protected File outputDirectory;
>>
>>
>>
>> I.e., if the user sets the directory as a plugin option, she is fine
>> when using the plugin directly. If she uses it as a reporting plugin for
>> site generation, she needs to set
RE: How to set default directories when developing a reporting plugin?
On 2023/10/20 02:08:25 Alexander Kriegisch wrote:
Hello.
I am trying to improve a reporting mojo in a Maven plugin. The mojo
extends org.apache.maven.reporting.AbstractMavenReport. I recently
upgraded it to Doxia 2.0 - thanks again to Hervé B. and Michael O. for
their support - and cleaned out some cruft, removing redundant super
class fields and methods) in the mojo code. So far, so good.
My question is about the different behaviour for configuring the report
output directory when starting the mojo directly as a Maven goal
compared to it being started during Maven site generation. The Javadoc
is helpful and explains:
public abstract class AbstractMavenReport extends AbstractMojo
implements MavenMultiPageReport
{
/**
* The output directory for the report. Note that this parameter is
* only evaluated if the goal is run directly from the command line.
* If the goal is run indirectly as part of a site generation, the
* output directory configured in the Maven Site Plugin is used
* instead.
*/
@Parameter(defaultValue = "${project.reporting.outputDirectory}",
readonly = true, required = true)
protected File outputDirectory;
I.e., if the user sets the directory as a plugin option, she is fine
when using the plugin directly. If she uses it as a reporting plugin for
site generation, she needs to set 'outputDirectory' in Maven Site
Plugin. But actually, that is not the right approach, because my
understanding is that 'outputDirectory' should rather set a base
directory for *all* reporting plugins, if the default target/site is not
OK. In that case, any plugin-specific output directory ought to be
interpreted as a subdirectory of the general reporting
'outputDirectory', not simply be ignored. I saw that Maven Javadoc
Plugin has some logic that seems to handle that, using a separate
'destDir' property to recalculate the output directory in a reporting
context. From class JavadocReport:
/**
* @param theDestDir the destination directory
*/
public void setDestDir(String theDestDir) {
this.destDir = theDestDir;
updateReportOutputDirectory(reportOutputDirectory, theDestDir);
}
private void updateReportOutputDirectory(File reportOutputDirectory,
String destDir)
{
if (reportOutputDirectory != null
&& destDir != null
&& !reportOutputDirectory.getAbsolutePath().endsWith(destDir))
{
this.reportOutputDirectory =
new File(reportOutputDirectory, destDir);
} else {
this.reportOutputDirectory =
reportOutputDirectory;
}
}
Different Maven plugins seem to handle this situation differently. Is
this the suggested, canonical approach? What do you think is a user's
expectation for multi-page report mojos extending AbstractMavenReport?
Certainly not that three conflicting reporting plugins have to argue
about which one can use Maven Site's 'outputDirectory' proprty for its
private purposes, leaving the other reporting plugins falsely
configured. Do all reporting plugins need to provide a separate
'destDir' or 'reportingDir' property for that use case? WDYT?
Sorry for asking a long-winded question, but I think it is important for
me to provide some context for the benefit of everyone trying to answer.
This is, indeed, a good question. I have stumbled about similar during
Doxia 2.0.0 work, but didn't dare to touch it because I didn't want to
open too many construction sites. Let me go through to get a better
understanding.
Regarding Javadoc Plugin: I don't know whether is the perfect external
reporting plugin because it does not inherit from AbstractMavenReport.
M
Re: How to set default directories when developing a reporting plugin?
One more thing: In class AbstractMavenReport, field
'reportOutputDirectory' is private (not protected), i.e. it cannot be
updated independently by extending mojos. Using the setter method also
updates 'outputDirectory' simultaneously. The latter is a protected
field, BTW. Of course, the setter method could be overridden or the
overriding class could somehow cater to the sacred knowledge of the side
effect in 'setReportOutputDirectory'. Interestingly, Maven Javadoc
Plugin's class JavadocReport simply defines its own private
'reportOutputDirectory' field, not extending AbstractMavenReport at all.
I would, however, rather stick with the approach to use the abstract
base class and extend it the right way, based on expert suggestions
here.
--
Alexander Kriegisch
https://scrum-master.de
Alexander Kriegisch schrieb am 20.10.2023 09:08 (GMT +07:00):
> Hello.
>
> I am trying to improve a reporting mojo in a Maven plugin. The mojo
> extends org.apache.maven.reporting.AbstractMavenReport. I recently
> upgraded it to Doxia 2.0 - thanks again to Hervé B. and Michael O. for
> their support - and cleaned out some cruft, removing redundant super
> class fields and methods) in the mojo code. So far, so good.
>
> My question is about the different behaviour for configuring the report
> output directory when starting the mojo directly as a Maven goal
> compared to it being started during Maven site generation. The Javadoc
> is helpful and explains:
>
>
>
> public abstract class AbstractMavenReport extends AbstractMojo
> implements MavenMultiPageReport
> {
> /**
>* The output directory for the report. Note that this parameter is
>* only evaluated if the goal is run directly from the command line.
>* If the goal is run indirectly as part of a site generation, the
>* output directory configured in the Maven Site Plugin is used
>* instead.
>*/
> @Parameter(defaultValue = "${project.reporting.outputDirectory}",
> readonly = true, required = true)
> protected File outputDirectory;
>
>
>
> I.e., if the user sets the directory as a plugin option, she is fine
> when using the plugin directly. If she uses it as a reporting plugin for
> site generation, she needs to set 'outputDirectory' in Maven Site
> Plugin. But actually, that is not the right approach, because my
> understanding is that 'outputDirectory' should rather set a base
> directory for *all* reporting plugins, if the default target/site is not
> OK. In that case, any plugin-specific output directory ought to be
> interpreted as a subdirectory of the general reporting
> 'outputDirectory', not simply be ignored. I saw that Maven Javadoc
> Plugin has some logic that seems to handle that, using a separate
> 'destDir' property to recalculate the output directory in a reporting
> context. From class JavadocReport:
>
>
>
> /**
> * @param theDestDir the destination directory
> */
> public void setDestDir(String theDestDir) {
> this.destDir = theDestDir;
> updateReportOutputDirectory(reportOutputDirectory, theDestDir);
> }
>
> private void updateReportOutputDirectory(File reportOutputDirectory,
> String destDir)
> {
> if (reportOutputDirectory != null
>&& destDir != null
>&& !reportOutputDirectory.getAbsolutePath().endsWith(destDir))
> {
> this.reportOutputDirectory =
> new File(reportOutputDirectory, destDir);
> } else {
> this.reportOutputDirectory =
> reportOutputDirectory;
> }
> }
>
>
>
> Different Maven plugins seem to handle this situation differently. Is
> this the suggested, canonical approach? What do you think is a user's
> expectation for multi-page report mojos extending AbstractMavenReport?
> Certainly not that three conflicting reporting plugins have to argue
> about which one can use Maven Site's 'outputDirectory' proprty for its
> private purposes, leaving the other reporting plugins falsely
> configured. Do all reporting plugins need to provide a separate
> 'destDir' or 'reportingDir' property for that use case? WDYT?
>
> Sorry for asking a long-winded question, but I think it is important for
> me to provide some context for the benefit of everyone trying to answer.
>
> Thanks in advance. Cheers
-
To unsubscribe, e-mail: users-unsubscr...@maven.apache.org
For additional commands, e-mail: users-h...@maven.apache.org
