Skip to content

Commit

Permalink
Clean up output directory management
Browse files Browse the repository at this point in the history 
@kriegaex
  • Loading branch information
@michael-o
michael-o committed Oct 22, 2023
1 parent e7b43f3 commit ae81a34
Show file tree
Hide file tree
Showing 16 changed files with 32 additions and 73 deletions.
6 changes: 4 additions & 2 deletions maven-jxr-plugin/src/it/JXR-100_parameterlink/verify.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
assert new File( basedir, 'target/site/xref/com/mycompany/app/Foo.html' ).exists()
File file = new File( basedir, 'target/site/xref/com/mycompany/app/Foo.html' );

assert 4 == new File( basedir, '/target/site/xref/com/mycompany/app/Foo.html' ).text.count( '<a name="App" href="../../../com/mycompany/app/App.html#App">App</a>' )
assert file.exists()

assert 4 == file.text.count( '<a name="App" href="../../../com/mycompany/app/App.html#App">App</a>' )
47 changes: 24 additions & 23 deletions maven-jxr-plugin/src/main/java/org/apache/maven/plugin/jxr/AbstractJxrReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,23 +194,23 @@ && hasSources(currentFile)) {
}

/**
* Creates the Xref for the Java files found in the given source directory and puts them in the given destination
* Creates the Xref for the Java files found in the given source directory and puts them in the given output
* directory.
*
* @param locale The user locale to use for the Xref generation
* @param destinationDirectory The output directory
* @param outputDirectory The output directory
* @param sourceDirs The source directories
* @throws java.io.IOException
* @throws org.apache.maven.jxr.JxrException
*/
private void createXref(Locale locale, String destinationDirectory, List<String> sourceDirs)
private void createXref(Locale locale, String outputDirectory, List<String> sourceDirs)
throws IOException, JxrException {
FileManager fileManager = new FileManager();
PackageManager packageManager = new PackageManager(fileManager);
JavaCodeTransform codeTransform = new JavaCodeTransform(packageManager, fileManager);

JXR jxr = new JXR(packageManager, codeTransform);
jxr.setDest(Paths.get(destinationDirectory));
jxr.setDest(Paths.get(outputDirectory));
jxr.setInputEncoding(getInputEncoding());
jxr.setLocale(locale);
jxr.setOutputEncoding(getOutputEncoding());
Expand All @@ -234,7 +234,7 @@ private void createXref(Locale locale, String destinationDirectory, List<String>
}

// and finally copy the stylesheet
copyRequiredResources(destinationDirectory);
copyRequiredResources(outputDirectory);
}

/**
Expand Down Expand Up @@ -285,28 +285,28 @@ private String getBottomText() {
}

/**
* Copy some required resources (like the stylesheet) to the given directory
* Copy some required resources (like the stylesheet) to the given target directory
*
* @param dir the directory to copy the resources to
* @param targetDirectory the directory to copy the resources to
*/
private void copyRequiredResources(String dir) {
private void copyRequiredResources(String targetDirectory) {
if (stylesheet != null && !stylesheet.isEmpty()) {
File stylesheetFile = new File(stylesheet);
File destStylesheetFile = new File(dir, "stylesheet.css");
File targetStylesheetFile = new File(targetDirectory, "stylesheet.css");

try {
if (stylesheetFile.isAbsolute()) {
FileUtils.copyFile(stylesheetFile, destStylesheetFile);
FileUtils.copyFile(stylesheetFile, targetStylesheetFile);
} else {
URL stylesheetUrl = this.getClass().getClassLoader().getResource(stylesheet);
FileUtils.copyURLToFile(stylesheetUrl, destStylesheetFile);
FileUtils.copyURLToFile(stylesheetUrl, targetStylesheetFile);
}
} catch (IOException e) {
getLog().warn("An error occured while copying the stylesheet to the target directory", e);
}
} else {
if (javadocTemplatesVersion.isAtLeast("1.8")) {
copyResources(dir, "jdk8/", "stylesheet.css");
copyResources(targetDirectory, "jdk8/", "stylesheet.css");
} else if (javadocTemplatesVersion.isAtLeast("1.7")) {
String[] jdk7Resources = {
"stylesheet.css",
Expand All @@ -315,31 +315,31 @@ private void copyRequiredResources(String dir) {
"resources/titlebar.gif",
"resources/titlebar_end.gif"
};
copyResources(dir, "jdk7/", jdk7Resources);
copyResources(targetDirectory, "jdk7/", jdk7Resources);
} else if (javadocTemplatesVersion.isAtLeast("1.6")) {
copyResources(dir, "jdk6/", "stylesheet.css");
copyResources(targetDirectory, "jdk6/", "stylesheet.css");
} else if (javadocTemplatesVersion.isAtLeast("1.4")) {
copyResources(dir, "jdk4/", "stylesheet.css");
copyResources(targetDirectory, "jdk4/", "stylesheet.css");
} else {
// Fallback to the original stylesheet
copyResources(dir, "", "stylesheet.css");
copyResources(targetDirectory, "", "stylesheet.css");
}
}
}

/**
* Copy styles and related resources to the given directory
*
* @param dir the directory to copy the resources to
* @param targetDirectory the target directory to copy the resources to
* @param sourceDirectory resources subdirectory to copy from
* @param files names of files to copy
*/
private void copyResources(String dir, String sourceDirectory, String... files) {
private void copyResources(String targetDirectory, String sourceDirectory, String... files) {
try {
for (String file : files) {
URL resourceUrl = this.getClass().getClassLoader().getResource(sourceDirectory + file);
File destResourceFile = new File(dir, file);
FileUtils.copyURLToFile(resourceUrl, destResourceFile);
File targetResourceFile = new File(targetDirectory, file);
FileUtils.copyURLToFile(resourceUrl, targetResourceFile);
}
} catch (IOException e) {
getLog().warn("An error occured while copying the resource to the target directory", e);
Expand Down Expand Up @@ -403,7 +403,7 @@ protected void executeReport(Locale locale) throws MavenReportException {
setJavadocTemplatesVersion();

try {
createXref(locale, getDestinationDirectory(), sourceDirs);
createXref(locale, getPluginReportOutputDirectory(), sourceDirs);
} catch (JxrException | IOException e) {
throw new MavenReportException("Error while generating the HTML source code of the project.", e);
}
Expand Down Expand Up @@ -533,11 +533,12 @@ private Path getJavadocLocation() throws IOException {
}

/**
* Abstract method that returns the target directory where the generated JXR reports will be put.
* Abstract method that returns the plugin report output directory where the generated JXR reports will be put
* beneath {@link #getReportOutputDirectory()}.
*
* @return a String that contains the target directory name
*/
protected abstract String getDestinationDirectory();
protected abstract String getPluginReportOutputDirectory();

/**
* Abstract method that returns the specified source directories that will be included in the JXR report generation.
Expand Down
20 changes: 2 additions & 18 deletions maven-jxr-plugin/src/main/java/org/apache/maven/plugin/jxr/JxrReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,15 @@ public class JxrReport extends AbstractJxrReport {
@Parameter
private String sourcePath;

/**
* Directory where the Xref files will be copied to.
*/
@Parameter(defaultValue = "${project.reporting.outputDirectory}/xref")
private String destDir;

/**
* Directory where Javadoc is generated for this project.
*/
@Parameter(defaultValue = "${project.reporting.outputDirectory}/apidocs")
private File javadocDir;

@Override
protected String getDestinationDirectory() {
return destDir;
protected String getPluginReportOutputDirectory() {
return getReportOutputDirectory().getAbsolutePath() + "/xref";
}

@Override
Expand Down Expand Up @@ -129,14 +123,4 @@ public String getOutputName() {
protected File getJavadocDir() {
return javadocDir;
}

@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();
}
}
}
20 changes: 2 additions & 18 deletions maven-jxr-plugin/src/main/java/org/apache/maven/plugin/jxr/JxrTestReport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,6 @@ public class JxrTestReport extends AbstractJxrReport {
@Parameter(defaultValue = "${project.testCompileSourceRoots}", required = true, readonly = true)
private List<String> sourceDirs;

/**
* Directory where the Xref files will be copied to.
*/
@Parameter(defaultValue = "${project.reporting.outputDirectory}/xref-test")
private String destDir;

/**
* Directory where Test Javadoc is generated for this project.
*/
Expand Down Expand Up @@ -88,8 +82,8 @@ protected List<String> getSourceRoots(MavenProject project) {
}

@Override
protected String getDestinationDirectory() {
return destDir;
protected String getPluginReportOutputDirectory() {
return getReportOutputDirectory().getAbsolutePath() + "/xref-test";
}

@Override
Expand All @@ -111,14 +105,4 @@ public String getOutputName() {
protected File getJavadocDir() {
return testJavadocDir;
}

@Override
public void setReportOutputDirectory(File reportOutputDirectory) {
if ((reportOutputDirectory != null)
&& (!reportOutputDirectory.getAbsolutePath().endsWith("xref-test"))) {
this.destDir = new File(reportOutputDirectory, "xref-test").getAbsolutePath();
} else {
this.destDir = reportOutputDirectory.getAbsolutePath();
}
}
}
1 change: 0 additions & 1 deletion maven-jxr-plugin/src/test/resources/unit/aggregate-test/aggregate-test-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ under the License.
<value>${basedir}/src/test/resources/unit/aggregate-test/submodule1</value>
<value>${basedir}/src/test/resources/unit/aggregate-test/submodule2</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/aggregate-test/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/aggregate-test/target/site/apidocs</javadocDir>
<linkJavadoc>false</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...n/src/test/resources/unit/default-configuration/default-configuration-plugin-config-4.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/4/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/4/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...n/src/test/resources/unit/default-configuration/default-configuration-plugin-config-6.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/6/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/6/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...n/src/test/resources/unit/default-configuration/default-configuration-plugin-config-7.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/7/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/7/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...n/src/test/resources/unit/default-configuration/default-configuration-plugin-config-8.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/8/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/8/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...gin/src/test/resources/unit/default-configuration/default-configuration-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...jxr-plugin/src/test/resources/unit/default-configuration/exception-test-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/default-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/default-configuration/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/default-configuration/target/site/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...gin/src/test/resources/unit/exclude-configuration/exclude-configuration-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/exclude-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/exclude-configuration/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/exclude-configuration/target/site/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...gin/src/test/resources/unit/include-configuration/include-configuration-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/include-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/include-configuration/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/include-configuration/target/site/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...-jxr-plugin/src/test/resources/unit/nojavadocdir-test/nojavadocdir-test-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/nojavadocdir-test</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/nojavadocdir-test/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/nojavadocdir-test/target/site/apidocs</javadocDir>
<linkJavadoc>true</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion .../resources/unit/nojavadoclink-configuration/nojavadoclink-configuration-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/nojavadoclink-configuration</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/nojavadoclink-configuration/target/site/xref</destDir>
<javadocDir>${basedir}/target/test/unit/nojavadoclink-configuration/target/site/apidocs</javadocDir>
<linkJavadoc>false</linkJavadoc>
<bottom>Copyright 2006 Apache Foundation</bottom>
Expand Down
1 change: 0 additions & 1 deletion ...xr-plugin/src/test/resources/unit/testsourcedir-test/testsourcedir-test-plugin-config.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ under the License.
<sourceDirs>
<value>${basedir}/src/test/resources/unit/testsourcedir-test/src/test/java</value>
</sourceDirs>
<destDir>${basedir}/target/test/unit/testsourcedir-test/target/site/xref-test</destDir>
<bottom>Copyright 2006 Apache Foundation</bottom>
<javadocVersion>1.4</javadocVersion>
<stylesheet>stylesheet.css</stylesheet>
Expand Down

3 comments on commit ae81a34

@michael-o
Copy link
Member Author

Choose a reason for hiding this comment

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

@kriegaex This is what I expect as clean, not hacks or workarounds. Reusing the super class.

@kriegaex
Copy link

@kriegaex kriegaex commented on ae81a34 Oct 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@michael-o, thanks for the showcase. If you look at the snippets from AJ Maven that I shared on the mailing list (ML), you will see that this is pretty much the same thing I also did, i.e. we have a similar vision of what "clean" means.

There is one important difference, though: In my plugin, I overrode the default for the output directory to be ${project.build.directory} instead of the abstract base class's ${project.reporting.outputDirectory}, because the latter is simply wrong IMO. For stand-alone mojo execution, a plugin should not mess with Maven Site's default directory. Imagine a situation in which each module should get its own, self-consistent report when called stand-alone, but the site should contain an aggregate with a different structure or maybe no report from that plugin at all. The default would then pollute the site directory. This is why on the ML I asked you, if overriding a @Parameter annotation on a base class field by another @Parameter annotation on a setter in a subclass it is the right or canonical way to implement such a default override.

BTW, like I also said before, Maven Javadoc Plugin, even though it does not use the abstract base class, implements the default correctly: build dir for stand-alone, report dir in site generation context.

@michael-o
Copy link
Member Author

@michael-o michael-o commented on ae81a34 Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is why on the ML I asked you, if overriding a @Parameter annotation on a base class field by another @Parameter annotation on a setter in a subclass it is the right or canonical way to implement such a default override.

This, unfortunately I don't now from the top of my head, but if your UTs and ITs do not fail then, I guess it is fine. I am not aware of a canonical way.

Please sign in to comment.