You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "hgschmie (via GitHub)" <gi...@apache.org> on 2023/08/21 20:56:15 UTC
[GitHub] [maven-javadoc-plugin] hgschmie opened a new pull request, #227: [MJAVADOC-769] fix for transitive filename based modules
hgschmie opened a new pull request, #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227
When a project depends on an artifact with a manifest entry for
Automatic-Module-Name which in turn depends on an artifact that uses the
filename to determine the module name, it will move the former onto the
module path and patch the latter into the main artifact. However, now
the direct dependency on the module path can no longer access the
classes that have been patched only into the main module and javadoc
generation fails.
As the JDK only differentiates between modules with a module descriptor
and "everything else" (modules with an automatic module entry in the
manifest and modules with file name based names), the javadoc plugin
should do the same.
This patch changes the treatment of dependencies with an
Automatic-Module-Name to match dependencies that use a filename based
module name. The plugin now patches all of those dependencies into the
main module and the build succeeds.
Includes an integration test.
Following this checklist to help us incorporate your
contribution quickly and easily:
- [X] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MJAVADOC) filed
for the change (usually before you start working on it). Trivial changes like typos do not
require a JIRA issue. Your pull request should address just this issue, without
pulling in other changes.
- [X] Each commit in the pull request should have a meaningful subject line and body.
- [X] Format the pull request title like `[MJAVADOC-XXX] - Fixes bug in ApproximateQuantiles`,
where you replace `MJAVADOC-XXX` with the appropriate JIRA issue. Best practice
is to use the JIRA issue title in the pull request title and in the first line of the
commit message.
- [X] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [X] Run `mvn clean verify -Prun-its` to make sure basic checks pass. A more thorough check will
be performed on your pull request automatically.
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
you have to acknowledge this by using the following check-box.
- [X] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MJAVADOC-769] fix for transitive filename based modules [maven-javadoc-plugin]
Posted by "jira-importer (via GitHub)" <gi...@apache.org>.
jira-importer commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-2957401504
Resolve #1151
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1316473897
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
Review Comment:
Please consider to use goals:
- javadoc and test-javadoc
we not need jars with javadocs
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
Review Comment:
Please consider to use goals:
- javadoc and test-javadoc
we not need jars with javadocs
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1316473897
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
Review Comment:
Please consider to use goals:
- javadoc-no-fork and test-javadoc-no-fork
we not need jars with javadocs
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie merged pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie merged PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1316473035
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,170 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <version>3.3.0</version>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>test-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>test-jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
Review Comment:
This execution should be before javadoc - it can have impact on test
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1314450414
##########
src/it/projects/MJAVADOC-769/src/test/java/mavenbugs/mjavadoc769/RightTest.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed 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.
+ */
+package mavenbugs.mjavadoc769;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+
+import org.junit.jupiter.api.Test;
+
+public class RightTest {
+
+ @Test
+ public void testFoo() throws Exception {
Review Comment:
yes. Otherwise, the scopes of the various dependencies in the project would not make sense, someone would show up saying "you are doing it wrong, you need to change you scope to bla bla bla" and we would not discuss the actual problem but just why dependency scopes need to be different and then everything works.
This is a *real world* example that highlights why this change makes sense. I did not artificially construct this but literally lifted it 1:1 from Jdbi.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1709047944
thanks for checking this. Do you have the exact error that you saw? Is it an open source project that I can check out?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1709311461
This is the slightly longer (and a bit more winded) explanation of what that patch does:
consider a project that spits out a module. This can be a defined module or an automatic module; it does not actually matter. What is important is that the maven tool chain considers it something where it will use the module path.
let's consider this dependency graph: `project --> automatic module with manifest entry --> automatic module with file name based module id` (If that is too abstract for you, you can call those `jdbi3-guice -> guice-5.1.0 -> javax.inject-1`).
- jdbi3-guice is a defined module (it contains a module-info.java file)
- guice-5.1.0 is an automatic module that contains an entry in `META-INF/MANIFEST.MF`
- javax.inject-1 is an automatic module that has no JPMS information, so it is treated as a filename based module.
To run code that uses these three modules, it needs to do `--module-path jdbi3-guice.jar:guice-5.1.0.jar:javax.inject-1.jar`. This will work fine. The automatic modules open all their packages to the module path; there are no overlaps and the jdbi3-guice module can consume all the pieces that it needs.
When the maven toolchain is trying to set up javadoc generation, it will inspect all three modules and come to the following conclusion:
- jdbi3-guice --> `ModuleNameSource.MODULEDESCRIPTOR`
- guice-5.1.0 --> `ModuleNameSource.MANIFEST`
- javax.inject-1 --> `ModuleNameSource.FILENAME`
(this is buried deep in the plexus-java component)
Now, the javadoc plugin sees this and creates module and classpath:
```
--add-modules ALL-MODULE-PATH
--module-path 'jdbi/guice/target/jdbi3-guice-3.41.0.jar:m2repo/guice-5.1.0.jar'
--patch-module org.jdbi.v3.guice='m2repo/javax.inject-1.jar'
```
because it treats the `ModuleNameSource.MANIFEST` *different* from `ModuleNameSource.FILENAME`
So, everything cool? No. What happens now is that a class in the main module (`InternalBindingProvider`) implements a class in the guice module (`com.google.inject.Provider`) which in turn *extends* a class from javax.inject-1 (`javax.inject.Provider`). But the javax.inject-1 jar has *only been patched into the jdbi3-guice component*. So javadoc can not find it when the guice class says "I extend that" and that leads to:
```
InternalImportBindingBuilder.java:88: error: cannot access Provider
static final class InternalBindingProvider<T> implements Provider<T> {
^
class file for javax.inject.Provider not found
1 error
```
so the right fix is one of two things:
- move all dependencies of the main artifact onto the module path. This sounds nice in theory but reality is that this only works for defined modules (those that have a module-info.java). Many of the other ones (especially the ones that simply tack an automatic module name into the jar after compile time) have dependencies with overlapping packages (e.g. jsr305 as the biggest offender) and short of inspecting every jar and keeping a list of exports, this option leads to more problems than actual solutions. It is, however, *the right solution* but, unless the rest of the tooling starts to differentiate between defined modules and automatic modules, not a viable solution.
- patch *all* the automatic modules that are dependencies for the main artifact into the main artifact. IAW, don't differentiate between the two types of automatic modules (manifest and filename based).
And that is what this patch does: It removes the differentiation between manifest and filename based dependencies.
A more complete fix would do this:
- plot the complete dependency graph
- start from the root. Any dependency that is a defined module "infects" its full downstream tree. All of those dependencies need to go onto the module path
- look at the remainder of the graph. Anything here needs to be patched into the main module.
E.g.
```
artifact --+--- defined module a --+-- defined module b-- +-- automatic module 1 --+ automatic module 2
|
+--- automatic module 4 --+-- automatic module 5
|
+--- automatic module 6 --+-- defined module c -+-- automatic module 5
```
- defined module a goes onto the module path. As does defined module b, automatic module 1 and automatic module 2
- defined module c goes onto the module path. This infects automatic module 5.
- automatic module 4 gets patched into the main module. its dependency has been forced onto the module path
- automatic module 6 gets patched into the main module.
So the result is
```
--module-path defined module a:defined module b:defined module c:automatic module 1:automatic module 2:automatic module 5
-- patch-module main.module=automatic module 4:automatic module 6
```
Or in the case above, the chain `main artifact --> guice (automatic module) -> javax.inject (automatic module)` leads to both guice and javax.inject being patched into main artifact.
If guice ever wises up and provides a module descriptor, it would be this scenario: `main artifact --> guice (defined module) -> javax.inject (automatic module)`
Which would force guice onto the module path and its dependency (javax.inject) as well. That works as well as patching both. What does not work is move one of them onto the module path and patch the other one into the main module.
I *would* implement that, except that AbstractJavadocMojo.java is a brittle 6,100 lines (!) abomination littered with special cases, exceptions and dependencies on other components of unclear function. Also most mojos in the plugin share functionality here, all the way to the aggregation jar target. I did not dare to touch this and so I did the minimal invasive patch.
No, seriously, this thing needs to be refactored before any serious work can be done. Or better yet, thrown away and start over (maven-javadoc-next-plugin anyone?)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1314315492
##########
src/it/projects/MJAVADOC-769/src/main/java/mavenbugs/mjavadoc769/InternalImportBindingBuilder.java:
##########
@@ -0,0 +1,116 @@
+package mavenbugs.mjavadoc769;
+
+/*
+ * Licensed 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.
+ */
+
+import java.lang.annotation.Annotation;
+
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Provider;
+import com.google.inject.Scope;
+import com.google.inject.TypeLiteral;
+import com.google.inject.binder.LinkedBindingBuilder;
+import com.google.inject.binder.ScopedBindingBuilder;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Import binding builder.
+ */
+public final class InternalImportBindingBuilder<T> implements ScopedBindingBuilder {
Review Comment:
Do we need implementation for testing java docs ...?
Maybe methods with java docs and empty body will be ok.
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
Review Comment:
Does such jar and test jar build for current project are used by javadoc-p in the same module?
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
Review Comment:
We build test-jar with tests classes - but java doc is executed only for main java code.
##########
src/it/projects/MJAVADOC-769/src/test/java/mavenbugs/mjavadoc769/RightTest.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Licensed 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.
+ */
+package mavenbugs.mjavadoc769;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+
+import org.junit.jupiter.api.Test;
+
+public class RightTest {
+
+ @Test
+ public void testFoo() throws Exception {
Review Comment:
Similar - do we need a unit test here ...?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1709048842
> I did a test, in test project execute
>
> `mvn clean javadoc:3.5.0:javadoc` - build pass `mvn clean package javadoc:3.5.0:javadoc` - build failed
>
> so I assume that existing project jar with `Automatic-Module-Name` in target directory can cause or trigger a problem when I remove `Automatic-Module-Name` from test project - problem not occurs
>
> Of course your fix resolve such issue
>
> disclosure - I'm starting a journey with JPMS 😄
I will hold off merging until I understand what fails here.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1707405320
I did a test, in test project execute
`mvn clean javadoc:3.5.0:javadoc` - build pass
`mvn clean package javadoc:3.5.0:javadoc` - build failed
so I assume that existing project jar with `Automatic-Module-Name` in target directory can cause or trigger a problem
when I remove `Automatic-Module-Name` from test project - problem not occurs
Of course your fix resolve such issue
disclosure - I'm starting a journey with JPMS 😄
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] slawekjaranowski commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1316470527
##########
src/it/projects/MJAVADOC-769/src/main/java/mavenbugs/mjavadoc769/InternalImportBindingBuilder.java:
##########
@@ -0,0 +1,116 @@
+package mavenbugs.mjavadoc769;
+
+/*
+ * Licensed 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.
+ */
+
+import java.lang.annotation.Annotation;
+
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Provider;
+import com.google.inject.Scope;
+import com.google.inject.TypeLiteral;
+import com.google.inject.binder.LinkedBindingBuilder;
+import com.google.inject.binder.ScopedBindingBuilder;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Import binding builder.
+ */
+public final class InternalImportBindingBuilder<T> implements ScopedBindingBuilder {
Review Comment:
test classes can be simplified - but ok
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#issuecomment-1709250741
I am not sure that the behavior that you are observing is related to this PR.
With the current release (3.5.0):
When running "clean javadoc:javadoc", the javadoc plugin will consider your project to not use modules (the whole "automatic module name" thing is broken/iffy. JPMS itself only considers two different things: modules (which have a module-info.java) and "everything else". Maven tries to differentiate between modules, "things that have an automatic module name set" and "things that use filename based module names" which is IMHO wrong).
Anyway, in my project, if I use "clean javadoc:javadoc", this is what gets executed:
```
-classpath <... all the jars ...>
-encoding 'UTF-8'
-public -quiet --release 11
-sourcepath <... all the source folders ...>
... all the other stuff
```
however, when I run "clean package javadoc:javadoc", the javadoc plugin will see the jar which now contains a META-INF/MANIFEST.MF with an automatic module name and consider this project to be modularized (which is IMHO wrong but this is for another day to discuss) and executes
```
--add-modules ALL-MODULE-PATH
--module-path <the project jar>:<... all the jars that are either real modules or have an automatic module name...>
--patch-module <module-identifier>=<... all the source folders ...>:<... all the jars that are filename based modules ...>
... all the other stuff ...
```
In your case, the second one succeeds and the first one fails.
What the patch does is that only "real" modules (the ones with a module descriptor) end up on the module path and everything else ends up being patched. e.g. for my project, this is how javadoc is called without the patch:
```
--add-modules ALL-MODULE-PATH
--module-path '/Users/henning/.m2/repository/com/google/errorprone/error_prone_annotations/2.21.1/error_prone_annotations-2.21.1.jar: \
/Users/henning/.m2/repository/io/leangen/geantyref/geantyref/1.3.14/geantyref-1.3.14.jar: \
/Users/henning/.m2/repository/jakarta/annotation/jakarta.annotation-api/2.1.1/jakarta.annotation-api-2.1.1.jar: \
/Users/henning/.m2/repository/org/antlr/antlr4-runtime/4.13.0/antlr4-runtime-4.13.0.jar: \
/Users/henning/.m2/repository/org/checkerframework/checker-qual/3.37.0/checker-qual-3.37.0.jar: \
/Users/henning/.m2/repository/org/immutables/value/2.9.3/value-2.9.3.jar: \
/Users/henning/.m2/repository/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar: \
/Users/henning/code/jdbi/core/target/jdbi3-core-3.41.1-SNAPSHOT.jar'
--patch-module org.jdbi.v3.core='/Users/henning/code/jdbi/core/src/main/java: \
/Users/henning/code/jdbi/core/target/generated-sources/antlr4: \
/Users/henning/code/jdbi/core/target/generated-sources/annotations: \
/Users/henning/.m2/repository/org/inferred/freebuilder/2.8.0/freebuilder-2.8.0.jar'
```
and with the patch:
```
--add-modules ALL-MODULE-PATH
--module-path '/Users/henning/code/jdbi/core/target/jdbi3-core-3.41.1-SNAPSHOT.jar: \
/Users/henning/.m2/repository/jakarta/annotation/jakarta.annotation-api/2.1.1/jakarta.annotation-api-2.1.1.jar'
--patch-module org.jdbi.v3.core='/Users/henning/code/jdbi/core/src/main/java: \
/Users/henning/code/jdbi/core/target/generated-sources/antlr4: \
/Users/henning/code/jdbi/core/target/generated-sources/annotations: \
/Users/henning/.m2/repository/com/google/errorprone/error_prone_annotations/2.21.1/error_prone_annotations-2.21.1.jar: \
/Users/henning/.m2/repository/io/leangen/geantyref/geantyref/1.3.14/geantyref-1.3.14.jar: \
/Users/henning/.m2/repository/org/antlr/antlr4-runtime/4.13.0/antlr4-runtime-4.13.0.jar: \
/Users/henning/.m2/repository/org/checkerframework/checker-qual/3.37.0/checker-qual-3.37.0.jar: \
/Users/henning/.m2/repository/org/immutables/value/2.9.3/value-2.9.3.jar: \
/Users/henning/.m2/repository/org/inferred/freebuilder/2.8.0/freebuilder-2.8.0.jar: \
/Users/henning/.m2/repository/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar'
```
which may or may not make a difference in your case. But I am fairly certain that the failure that you observe with `clean javadoc:javadoc` vs. `clean package javadoc:javadoc` is caused by one not using modules at all and the other one is using the module path.
If you run the javadoc process with the `<debug>` flag set to true and share your `options` file from the `target/apidocs` folder, I can take a look and say more. Also, the actual error message would be helpful.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1314451026
##########
src/it/projects/MJAVADOC-769/pom.xml:
##########
@@ -0,0 +1,169 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+~ Licensed 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 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <groupId>org.apache.maven.plugins.javadoc.it</groupId>
+ <artifactId>mjavadoc769</artifactId>
+ <version>1.0-SNAPSHOT</version>
+ <packaging>jar</packaging>
+
+ <url>https://issues.apache.org/jira/browse/MJAVADOC-769</url>
+
+ <properties>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <maven.compiler.source>9</maven.compiler.source>
+ <maven.compiler.target>9</maven.compiler.target>
+ <moduleName>mavenbugs.mjavadoc769</moduleName>
+ </properties>
+
+ <dependencyManagement>
+ <dependencies>
+ <dependency>
+ <groupId>org.junit</groupId>
+ <artifactId>junit-bom</artifactId>
+ <version>5.10.0</version>
+ <type>pom</type>
+ <scope>import</scope>
+ </dependency>
+ </dependencies>
+ </dependencyManagement>
+
+ <dependencies>
+ <dependency>
+ <groupId>com.google.inject</groupId>
+ <artifactId>guice</artifactId>
+ <version>5.1.0</version>
+ </dependency>
+
+ <dependency>
+ <groupId>com.google.guava</groupId>
+ <artifactId>guava</artifactId>
+ <version>31.1-jre</version>
+ </dependency>
+
+ <dependency>
+ <groupId>jakarta.inject</groupId>
+ <artifactId>jakarta.inject-api</artifactId>
+ <version>2.0.1.MR</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>javax.inject</groupId>
+ <artifactId>javax.inject</artifactId>
+ <version>1</version>
+ <scope>provided</scope>
+ <optional>true</optional>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-params</artifactId>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+
+ <build>
+ <pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>@project.version@</version>
+ <configuration>
+ <outputDirectory>${project.build.directory}/apidocs
+ </outputDirectory>
+ <release>11</release>
+ <nodeprecated>false</nodeprecated>
+ <author>false</author>
+ <nohelp>true</nohelp>
+ </configuration>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-jar-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>default-jar</id>
+ <!-- add module name to main artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ <execution>
+ <id>test-jar</id>
+ <!-- add module name to test artifact -->
+ <configuration>
+ <archive>
+ <manifestEntries
+ combine.children="append">
+ <Automatic-Module-Name>
+ ${moduleName}.tests
+ </Automatic-Module-Name>
+ </manifestEntries>
+ </archive>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </pluginManagement>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>javadoc-jar</id>
+ <phase>package</phase>
+ <goals>
+ <goal>jar</goal>
Review Comment:
that is irrelevant. The tests are only executed to illustrate why the scopes in the pom make sense. The dependency scopes in the pom is what exhibits the problem that this patch fixes.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-javadoc-plugin] hgschmie commented on a diff in pull request #227: [MJAVADOC-769] fix for transitive filename based modules
Posted by "hgschmie (via GitHub)" <gi...@apache.org>.
hgschmie commented on code in PR #227:
URL: https://github.com/apache/maven-javadoc-plugin/pull/227#discussion_r1314455927
##########
src/it/projects/MJAVADOC-769/src/main/java/mavenbugs/mjavadoc769/InternalImportBindingBuilder.java:
##########
@@ -0,0 +1,116 @@
+package mavenbugs.mjavadoc769;
+
+/*
+ * Licensed 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.
+ */
+
+import java.lang.annotation.Annotation;
+
+import com.google.inject.Inject;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Provider;
+import com.google.inject.Scope;
+import com.google.inject.TypeLiteral;
+import com.google.inject.binder.LinkedBindingBuilder;
+import com.google.inject.binder.ScopedBindingBuilder;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
+
+/**
+ * Import binding builder.
+ */
+public final class InternalImportBindingBuilder<T> implements ScopedBindingBuilder {
Review Comment:
I start changing the code, the dependencies will change which is the whole point of the integration test. Then I spend an hour to write a new, synthetic test, when I can simply lift a selfcontainted class that I fully wrote myself and can donate to the project under apache license out of an apache licensed code base.
Does it matter? We can keep critiquing but the point is that this specific class (which is apache licensed so no problem ) exhibits the problem and that is why it is here. This is an integration test to lock down a specific behavior, not an exercise in code efficiency).
You keep asking for "where are the tests". Here is a real world example of a test that exhibits a specific problem with a fix.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org