[ https://issues.apache.org/jira/browse/MJAVADOC-775?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17795975#comment-17795975 ]
Alexander Kriegisch edited comment on MJAVADOC-775 at 12/13/23 2:01 AM: ------------------------------------------------------------------------ {quote}Bug triage and maintenance is important. {quote} [~elharo], then why did you not triage, comparing the situations in the two issues? ๐ This is *not* a duplicate, I just checked, adding some debug statements to the javadoc plugin, and finally realised that # there are two situations in which {{<tagletpath>my/path</tagletpath>}} can appear in the plugin configuration - namely (top level) option {{tagletpath}} and {{{}taglets{}}}/{{{}taglet{}}}/{{{}tagletpath{}}}) - and # therefore, my issue description falsely linked to the former (mea culpa, I just fixed that), while, as you would have seen if you had inspected and run my reproducer, I am using the latter. MJAVADOC-783 is about path concatenation when consolidating those two plus {{{}tagletartifacts{}}}, creating a single taglet path for the javadoc CLI option {{-tagletpath}} from all 3 sources. 783 is about an omitted path separator and exists independently of the problem described here. I managed to separately reproduce and fix both issues idependently of each other. The fix for MJAVADOC-783 (as suggested similarly by [~robjg]) is: {code:none} --- a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java +++ b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java @@ -2956,7 +2956,7 @@ public abstract class AbstractJavadocMojo extends AbstractMojo { path.append(StringUtils.join(pathParts.iterator(), File.pathSeparator)); if (tagletpath != null && !tagletpath.isEmpty()) { - path.append(JavadocUtil.unifyPathSeparator(tagletpath)); + path.append(path.length() > 0 ? File.pathSeparator : "").append(JavadocUtil.unifyPathSeparator(tagletpath)); } return path.toString(); {code} The fix for this issue MJAVADOC-775 is something similar to: {code:none} --- a/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java +++ b/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java @@ -136,7 +136,8 @@ public class JavadocUtil { Path directory = projectBasedir.resolve(dir); - if (Files.isDirectory(directory)) { + if (Files.isDirectory(directory) + || directory.getFileName().toString().endsWith(".jar")) { pruned.add(directory.toAbsolutePath()); } } {code} [~elharo], maybe the check for ".jar" at the end of the path name is too naive. Maybe we need to consider other packaging types, too, or maybe we should not remove non-directory entries at all and just verify that the given paths actually exist, not matter if they are directories or files. WDYT? And would you like one PR fixing both issues or one per issue? Sorry, I just had a small time box to debug this, hoping that actually a Maven Javadoc committer would do it. So, other than the external reproducer project linked to, I have not created any unit or integration tests for this. I could open two PR drafts and your could commit on top of them. Now that the root causes of both issues are clear, it should be easy to create regression tests for them. was (Author: kriegaex): {quote}Bug triage and maintenance is important.{quote} [~elharo], then why did you not triage, comparing the situations in the two issues? ๐ This is *not* a duplicate, I just checked, adding some debug statements to the javadoc plugin, and finally realised that # there are two situations in which {{<tagletpath>my/path</tagletpath>}} can appear in the plugin configuration - namely (top level) option {{tagletpath}} and {{taglets}}/{{taglet}}/{{tagletpath}}) - and # therefore, my issue description falsely linked to the former, while, as you would have seen if you had inspected and run my reproducer, I am using the latter. MJAVADOC-783 is about path concatenation when consolidating those two plus {{tagletartifacts}}, creating a single taglet path for the javadoc CLI option {{-tagletpath}} from all 3 sources. 783 is about an omitted path separator and exists independently of the problem described here. I managed to separately reproduce and fix both issues idependently of each other. The fix for MJAVADOC-783 (as suggested similarly by [~robjg]) is: {code:none} --- a/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java +++ b/src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java @@ -2956,7 +2956,7 @@ public abstract class AbstractJavadocMojo extends AbstractMojo { path.append(StringUtils.join(pathParts.iterator(), File.pathSeparator)); if (tagletpath != null && !tagletpath.isEmpty()) { - path.append(JavadocUtil.unifyPathSeparator(tagletpath)); + path.append(path.length() > 0 ? File.pathSeparator : "").append(JavadocUtil.unifyPathSeparator(tagletpath)); } return path.toString(); {code} The fix for this issue MJAVADOC-775 is something similar to: {code:none} --- a/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java +++ b/src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java @@ -136,7 +136,8 @@ public class JavadocUtil { Path directory = projectBasedir.resolve(dir); - if (Files.isDirectory(directory)) { + if (Files.isDirectory(directory) + || directory.getFileName().toString().endsWith(".jar")) { pruned.add(directory.toAbsolutePath()); } } {code} [~elharo], maybe the check for ".jar" at the end of the path name is too naive. Maybe we need to consider other packaging types, too, or maybe we should not remove non-directory entries at all and just verify that the given paths actually exist, not matter if they are directories or files. WDYT? And would you like one PR fixing both issues or one per issue? Sorry, I just had a small time box to debug this, hoping that actually a Maven Javadoc committer would do it. So, other than the external reproducer project linked to, I have not created any unit or integration tests for this. I could open two PR drafts and your could commit on top of them. Now that the root causes of both issues are clear, it should be easy to create regression tests for them. > Option 'tagletpath' not working, completely ignored > --------------------------------------------------- > > Key: MJAVADOC-775 > URL: https://issues.apache.org/jira/browse/MJAVADOC-775 > Project: Maven Javadoc Plugin > Issue Type: Bug > Components: javadoc > Affects Versions: 3.6.0 > Reporter: Alexander Kriegisch > Priority: Major > > Theย [{{tagletpath}} > property|https://maven.apache.org/plugins/maven-javadoc-plugin/apidocs/org/apache/maven/plugins/javadoc/options/Taglet.html#getTagletpath()] > used in a [{{taglets}}/{{taglet}} > element|https://maven.apache.org/plugins/maven-javadoc-plugin/javadoc-mojo.html#taglets] > seems to be completely ignored, despite being documented. It looks as if > this never worked before. It is also utterly untested. There is an old bug > MJAVADOC-231, but whatever that was meant to fix, it seems to be unrelated to > this one. If I have a local JAR unavailable on Maven Central sitting in a > libraries folder, I cannot use it in any other way than to contrive to put a > copy into my local Maven repository and use {{tagletArtifact}} instead of > {{taglets}}/{{taglet}}/{{tagletpath}}. > I noticed this, when trying to help someone asking a question on the [Maven > users mailing > list|https://lists.apache.org/thread/zv25z3hx9gbmbt6wwhby6cy0t5lr542l]. In > his [sample repository|https://github.com/jkesselm/xalan-java-mvn], an effort > to convert the Xalan Ant Build to Maven, the commit to check out is is > [6daf2890|https://github.com/jkesselm/xalan-java-mvn/tree/6daf2890bc03b13cd741b963d33897d61dcf4e5f]. > Just run {{mvn -DskipTests=true site}}. -- This message was sent by Atlassian Jira (v8.20.10#820010)