[ 
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)

Reply via email to