Copilot commented on code in PR #1325:
URL:
https://github.com/apache/maven-javadoc-plugin/pull/1325#discussion_r3369867238
##########
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java:
##########
@@ -5765,8 +5779,14 @@ private static String getJavadocLink(MavenProject
project) {
}
String url = cleanUrl(project.getUrl());
+ String destDir = "apidocs"; // see AbstractJavadocMojo#destDir
+ final String pluginId =
"org.apache.maven.plugins:maven-javadoc-plugin";
+ String destDirConfigured = getPluginParameter(project, pluginId,
"destDir");
+ if (destDirConfigured != null) {
+ destDir = destDirConfigured;
+ }
- return url + "/apidocs";
+ return url + "/" + destDir;
Review Comment:
`getJavadocLink()` now tries to honor a configured `destDir`, but it only
reads plugin configuration from `project.build.plugins` / `pluginManagement`
via `getPluginParameter()`. If a project configures `destDir` in the
`<reporting>` section (common for `mvn site` / report use), this method will
ignore it and still produce `${project.url}/apidocs`, which can break detected
links when the output directory is customized.
##########
src/test/resources/unit/javadocjar-invalid-destdir/javadocjar-invalid-destdir-plugin-config.xml:
##########
@@ -0,0 +1,73 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0";
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <modelVersion>4.0.0</modelVersion>
+ <groupId>org.apache.maven.plugins.maven-javadoc-plugin.unit</groupId>
+ <artifactId>javadocjar-invalid-destdir</artifactId>
+ <packaging>jar</packaging>
+ <version>1.0-SNAPSHOT</version>
+ <inceptionYear>2006</inceptionYear>
+ <name>Maven Javadoc Plugin Javadoc Jar Invalid Destdir Test</name>
+ <url>http://maven.apache.org</url>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <configuration>
+
<destDir>${basedir}/target/test/unit/javadocjar-invalid-destdir/target/invalid</destDir>
+
<jarOutputDirectory>${basedir}/target/test/unit/javadocjar-invalid-destdir/target</jarOutputDirectory>
+
<outputDirectory>${basedir}/target/test/unit/javadocjar-invalid-destdir/target/site/apidocs</outputDirectory>
+
<javadocOptionsDir>${basedir}/target/test/unit/javadocjar-invalid-destdir/target/javadoc-bundle-options</javadocOptionsDir>
+ <finalName>javadocjar-invalid-destdir</finalName>
Review Comment:
In this test POM, `outputDirectory` is set to a path that already ends with
`/apidocs`. With the reintroduced `destDir` behavior, `outputDirectory` should
normally point at the *base* site directory (e.g. `.../target/site`) and
`destDir` controls the subdirectory under it. As written, the test is
configuring a base dir of `.../apidocs` and then also setting `destDir`, which
makes the output path harder to reason about and doesn’t match the pattern used
by the other unit test projects.
##########
src/test/java/org/apache/maven/plugins/javadoc/JavadocJarMojoTest.java:
##########
@@ -132,6 +132,24 @@ void testDefaultConfig(JavadocJarMojo mojo) throws
Exception {
assertThat(generatedFile).exists();
}
+ /**
+ * Test when the specified destDir parameter has an invalid value
+ *
+ * @throws Exception if any
+ */
+ @Test
+ @InjectMojo(goal = "jar", pom =
"javadocjar-invalid-destdir-plugin-config.xml")
+ @Basedir("/unit/javadocjar-invalid-destdir")
+ void testInvalidDestdir(JavadocJarMojo mojo) throws Exception {
+ mojo.execute();
+
+ // check if the javadoc jar file was generated
+ File generatedFile = new File(
+ getBasedir(),
+
"target/test/unit/javadocjar-invalid-destdir/target/javadocjar-invalid-destdir-javadoc.jar");
+ assertThat(generatedFile).doesNotExist();
+ }
Review Comment:
`testInvalidDestdir()` currently calls `mojo.execute()` directly. If the
plugin correctly rejects an invalid `destDir` (especially with
`failOnError=true` in the test POM), this test will fail due to the thrown
exception before reaching the file assertion. It’s more robust to assert the
expected exception *and* then verify the jar was not created.
##########
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java:
##########
@@ -1670,6 +1673,14 @@ public AbstractJavadocMojo(
@Parameter(property = "maven.javadoc.disableNoFonts", defaultValue =
"false")
private boolean disableNoFonts;
+ /**
+ * The name of the destination directory.
+ *
+ * @since 2.1
+ */
+ @Parameter(property = "destDir", defaultValue = "apidocs")
+ protected String destDir;
Review Comment:
`destDir` is used to build both filesystem paths and URL paths, but it is
currently accepted verbatim. Values containing path separators, `..`, or
absolute paths can lead to confusing output locations (and potentially writing
outside the intended output directory after path normalization). Since this PR
adds a test case for an "invalid" `destDir`, it looks like `destDir` is
intended to be a simple directory name and should be validated early with a
clear failure message.
##########
src/site/apt/examples/output-configuration.apt.vm:
##########
@@ -0,0 +1,60 @@
+ ------
+ Using Alternative Output Directory
+ ------
+ Vincent Siveton
+ ------
+ 2009-08-04
+ ------
+
+~~ Licensed to the Apache Software Foundation (ASF) under one
+~~ or more contributor license agreements. See the NOTICE file
+~~ distributed with this work for additional information
+~~ regarding copyright ownership. The ASF licenses this file
+~~ to you under the Apache License, Version 2.0 (the
+~~ "License"); you may not use this file except in compliance
+~~ with the License. You may obtain a copy of the License at
+~~
+~~ http://www.apache.org/licenses/LICENSE-2.0
+~~
+~~ Unless required by applicable law or agreed to in writing,
+~~ software distributed under the License is distributed on an
+~~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+~~ KIND, either express or implied. See the License for the
+~~ specific language governing permissions and limitations
+~~ under the License.
+
+~~ NOTE: For help with the syntax of this file, see:
+~~ http://maven.apache.org/doxia/references/apt-format.html
+
+Using Alternative Output Directory
+
+ To run the Javadocs reports in an other output directory, you need to
configure Javadoc Plugin as follow:
+
Review Comment:
There are a couple of grammar issues in the new documentation page (e.g. “in
an other” and “as follow”), which will show up verbatim on the published plugin
site.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
