Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-39134: [Java] Create module info compiler plugin #39135

Merged
merged 10 commits into from
Dec 12, 2023

Conversation

jduo
Copy link
Member

@jduo jduo commented Dec 7, 2023

Rationale for this change

Create and integrate a maven plugin for compiling module-info.java files without using maven-compiler-plugin
and release 9+. This is necessary for supporting Java 8 unsafe code, which has different than unsafe code
in JDK 9.

What changes are included in this PR?

  • Add the module-info-compiler-maven-plugin and utilize it in the codebase.
  • Exclude module-info.java files from maven-compiler-plugin.

Are these changes tested?

Yes, they generate valid module-info.class files in JARs and test JARs.

Are there any user-facing changes?

No

@jduo jduo requested a review from lidavidm as a code owner December 7, 2023 22:39
Copy link

github-actions bot commented Dec 7, 2023

⚠️ GitHub issue #39134 has been automatically assigned in GitHub to PR creator.

@jduo jduo force-pushed the create-module-info-compiler branch from a26e026 to 1e1c045 Compare December 7, 2023 22:43
</dependencies>

<build>
<pluginManagement><!-- lock down plugins versions to avoid using Maven defaults (may be moved to parent pom) -->
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want to avoid the defaults/are these inconsistent with the global config?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was generated by the maven plugin archetype I used.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Dec 8, 2023
@jduo jduo force-pushed the create-module-info-compiler branch from 1e1c045 to 0aa8605 Compare December 11, 2023 19:51
jduo added 10 commits December 11, 2023 13:00
Note that some configuration from the arrow root POM
is copied to the maven plugins module because it cannot
be inherited without creating a cyclic depdendency.
Reports a used dependency as unused only on Windows 2022 CI runs
It is not correctly getting skipped.
This won't build with the JDK 8 target
Newer versions of the checkstyle plugin do support module-info.java files
but require rewriting the checkstyle rules file.
Add a test goal to the module-info-compiler plugin.
Rename goals to be consistent with maven-compiler-plugin
@jduo jduo force-pushed the create-module-info-compiler branch from 0aa8605 to dea90f9 Compare December 11, 2023 21:01
@lidavidm lidavidm merged commit d0e74d7 into apache:main Dec 12, 2023
14 checks passed
@lidavidm lidavidm removed the awaiting merge Awaiting merge label Dec 12, 2023
@lidavidm
Copy link
Member

@github-actions crossbow submit java

Copy link

Revision: dea90f9

Submitted crossbow builds: ursacomputing/crossbow @ actions-0fe7a83d3b

Task Status
java-jars GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit d0e74d7.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 6 possible false positives for unstable benchmarks that are known to sometimes produce them.

@kou kou changed the title GH-39134: Create module info compiler plugin GH-39134: [Java] Create module info compiler plugin Dec 12, 2023
mapleFU pushed a commit to mapleFU/arrow that referenced this pull request Dec 13, 2023
### Rationale for this change
Create and integrate a maven plugin for compiling module-info.java files without using maven-compiler-plugin
and release 9+. This is necessary for supporting Java 8 unsafe code, which has different than unsafe code
in JDK 9.

### What changes are included in this PR?
- Add the module-info-compiler-maven-plugin and utilize it in the codebase.
- Exclude module-info.java files from maven-compiler-plugin.

### Are these changes tested?
Yes, they generate valid module-info.class files in JARs and test JARs.

### Are there any user-facing changes?
No
* Closes: apache#39134

Authored-by: James Duong <[email protected]>
Signed-off-by: David Li <[email protected]>
@danepitkin
Copy link
Member

dependabot is trying to update some of the dependencies added here. Should we ignore them? How do we determine the right support matrix for dependencies?

#39202
#39201

@kou
Copy link
Member

kou commented Dec 14, 2023

@jduo It seems that this broke our documentation CI job. Could you check it?

https://github.com/apache/arrow/actions/runs/7182308293/job/19558525786#step:7:13088

Error:  Failed to execute goal org.apache.maven.plugins:maven-site-plugin:3.3:site (default-site) on project arrow-maven-plugins: Execution default-site of goal org.apache.maven.plugins:maven-site-plugin:3.3:site failed: A required class was missing while executing org.apache.maven.plugins:maven-site-plugin:3.3:site: org/apache/maven/doxia/siterenderer/DocumentContent
Error:  -----------------------------------------------------
Error:  realm =    plugin>org.apache.maven.plugins:maven-site-plugin:3.3
Error:  strategy = org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy
Error:  urls[0] = file:/root/.m2/repository/org/apache/maven/plugins/maven-site-plugin/3.3/maven-site-plugin-3.3.jar
Error:  urls[1] = file:/root/.m2/repository/org/apache/maven/reporting/maven-reporting-exec/1.1/maven-reporting-exec-1.1.jar
Error:  urls[2] = file:/root/.m2/repository/org/apache/maven/reporting/maven-reporting-api/3.0/maven-reporting-api-3.0.jar
Error:  urls[3] = file:/root/.m2/repository/org/apache/maven/shared/maven-shared-utils/0.3/maven-shared-utils-0.3.jar
Error:  urls[4] = file:/root/.m2/repository/com/google/code/findbugs/jsr305/2.0.1/jsr305-2.0.1.jar
Error:  urls[5] = file:/root/.m2/repository/org/codehaus/plexus/plexus-component-annotations/1.5.5/plexus-component-annotations-1.5.5.jar
Error:  urls[6] = file:/root/.m2/repository/org/sonatype/aether/aether-util/1.7/aether-util-1.7.jar
Error:  urls[7] = file:/root/.m2/repository/org/eclipse/aether/aether-util/0.9.0.M2/aether-util-0.9.0.M2.jar
Error:  urls[8] = file:/root/.m2/repository/org/sonatype/sisu/sisu-inject-bean/1.4.2/sisu-inject-bean-1.4.2.jar
Error:  urls[9] = file:/root/.m2/repository/org/sonatype/sisu/sisu-guice/2.1.7/sisu-guice-2.1.7-noaop.jar
Error:  urls[10] = file:/root/.m2/repository/org/codehaus/plexus/plexus-interpolation/1.14/plexus-interpolation-1.14.jar
Error:  urls[11] = file:/root/.m2/repository/org/sonatype/plexus/plexus-sec-dispatcher/1.3/plexus-sec-dispatcher-1.3.jar
Error:  urls[12] = file:/root/.m2/repository/org/sonatype/plexus/plexus-cipher/1.4/plexus-cipher-1.4.jar
Error:  urls[13] = file:/root/.m2/repository/org/apache/maven/maven-archiver/2.4.2/maven-archiver-2.4.2.jar
Error:  urls[14] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-sink-api/1.4/doxia-sink-api-1.4.jar
Error:  urls[15] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-logging-api/1.4/doxia-logging-api-1.4.jar
Error:  urls[16] = file:/root/.m2/repository/junit/junit/3.8.1/junit-3.8.1.jar
Error:  urls[17] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-core/1.4/doxia-core-1.4.jar
Error:  urls[18] = file:/root/.m2/repository/xerces/xercesImpl/2.9.1/xercesImpl-2.9.1.jar
Error:  urls[19] = file:/root/.m2/repository/xml-apis/xml-apis/1.3.04/xml-apis-1.3.04.jar
Error:  urls[20] = file:/root/.m2/repository/org/apache/httpcomponents/httpclient/4.0.2/httpclient-4.0.2.jar
Error:  urls[21] = file:/root/.m2/repository/commons-logging/commons-logging/1.1.1/commons-logging-1.1.1.jar
Error:  urls[22] = file:/root/.m2/repository/commons-codec/commons-codec/1.3/commons-codec-1.3.jar
Error:  urls[23] = file:/root/.m2/repository/org/apache/httpcomponents/httpcore/4.0.1/httpcore-4.0.1.jar
Error:  urls[24] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-module-xhtml/1.4/doxia-module-xhtml-1.4.jar
Error:  urls[25] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-module-apt/1.4/doxia-module-apt-1.4.jar
Error:  urls[26] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-module-xdoc/1.4/doxia-module-xdoc-1.4.jar
Error:  urls[27] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-module-fml/1.4/doxia-module-fml-1.4.jar
Error:  urls[28] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-module-markdown/1.4/doxia-module-markdown-1.4.jar
Error:  urls[29] = file:/root/.m2/repository/org/pegdown/pegdown/1.2.1/pegdown-1.2.1.jar
Error:  urls[30] = file:/root/.m2/repository/org/parboiled/parboiled-java/1.1.4/parboiled-java-1.1.4.jar
Error:  urls[31] = file:/root/.m2/repository/org/parboiled/parboiled-core/1.1.4/parboiled-core-1.1.4.jar
Error:  urls[32] = file:/root/.m2/repository/org/ow2/asm/asm/4.1/asm-4.1.jar
Error:  urls[33] = file:/root/.m2/repository/org/ow2/asm/asm-tree/4.1/asm-tree-4.1.jar
Error:  urls[34] = file:/root/.m2/repository/org/ow2/asm/asm-analysis/4.1/asm-analysis-4.1.jar
Error:  urls[35] = file:/root/.m2/repository/org/ow2/asm/asm-util/4.1/asm-util-4.1.jar
Error:  urls[36] = file:/root/.m2/repository/javax/servlet/servlet-api/2.5/servlet-api-2.5.jar
Error:  urls[37] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-decoration-model/1.4/doxia-decoration-model-1.4.jar
Error:  urls[38] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-site-renderer/1.4/doxia-site-renderer-1.4.jar
Error:  urls[39] = file:/root/.m2/repository/org/apache/velocity/velocity-tools/2.0/velocity-tools-2.0.jar
Error:  urls[40] = file:/root/.m2/repository/commons-beanutils/commons-beanutils/1.7.0/commons-beanutils-1.7.0.jar
Error:  urls[41] = file:/root/.m2/repository/commons-digester/commons-digester/1.8/commons-digester-1.8.jar
Error:  urls[42] = file:/root/.m2/repository/commons-chain/commons-chain/1.1/commons-chain-1.1.jar
Error:  urls[43] = file:/root/.m2/repository/commons-validator/commons-validator/1.3.1/commons-validator-1.3.1.jar
Error:  urls[44] = file:/root/.m2/repository/dom4j/dom4j/1.1/dom4j-1.1.jar
Error:  urls[45] = file:/root/.m2/repository/sslext/sslext/1.2-0/sslext-1.2-0.jar
Error:  urls[46] = file:/root/.m2/repository/org/apache/struts/struts-core/1.3.8/struts-core-1.3.8.jar
Error:  urls[47] = file:/root/.m2/repository/antlr/antlr/2.7.2/antlr-2.7.2.jar
Error:  urls[48] = file:/root/.m2/repository/org/apache/struts/struts-taglib/1.3.8/struts-taglib-1.3.8.jar
Error:  urls[49] = file:/root/.m2/repository/org/apache/struts/struts-tiles/1.3.8/struts-tiles-1.3.8.jar
Error:  urls[50] = file:/root/.m2/repository/commons-collections/commons-collections/3.2.1/commons-collections-3.2.1.jar
Error:  urls[51] = file:/root/.m2/repository/org/apache/maven/doxia/doxia-integration-tools/1.5/doxia-integration-tools-1.5.jar
Error:  urls[52] = file:/root/.m2/repository/org/codehaus/plexus/plexus-archiver/1.0/plexus-archiver-1.0.jar
Error:  urls[53] = file:/root/.m2/repository/org/codehaus/plexus/plexus-io/1.0/plexus-io-1.0.jar
Error:  urls[54] = file:/root/.m2/repository/org/codehaus/plexus/plexus-i18n/1.0-beta-7/plexus-i18n-1.0-beta-7.jar
Error:  urls[55] = file:/root/.m2/repository/org/apache/velocity/velocity/1.5/velocity-1.5.jar
Error:  urls[56] = file:/root/.m2/repository/oro/oro/2.0.8/oro-2.0.8.jar
Error:  urls[57] = file:/root/.m2/repository/org/codehaus/plexus/plexus-velocity/1.1.8/plexus-velocity-1.1.8.jar
Error:  urls[58] = file:/root/.m2/repository/org/codehaus/plexus/plexus-utils/1.5.10/plexus-utils-1.5.10.jar
Error:  urls[59] = file:/root/.m2/repository/org/mortbay/jetty/jetty/6.1.25/jetty-6.1.25.jar
Error:  urls[60] = file:/root/.m2/repository/org/mortbay/jetty/servlet-api/2.5-20081211/servlet-api-2.5-20081211.jar
Error:  urls[61] = file:/root/.m2/repository/org/mortbay/jetty/jetty-util/6.1.25/jetty-util-6.1.25.jar
Error:  urls[62] = file:/root/.m2/repository/commons-lang/commons-lang/2.5/commons-lang-2.5.jar
Error:  urls[63] = file:/root/.m2/repository/commons-io/commons-io/1.4/commons-io-1.4.jar
Error:  Number of foreign imports: 1
Error:  import: Entry[import  from realm ClassRealm[maven.api, parent: null]]
Error:  
Error:  -----------------------------------------------------: org.apache.maven.doxia.siterenderer.DocumentContent

clayburn pushed a commit to clayburn/arrow that referenced this pull request Jan 23, 2024
### Rationale for this change
Create and integrate a maven plugin for compiling module-info.java files without using maven-compiler-plugin
and release 9+. This is necessary for supporting Java 8 unsafe code, which has different than unsafe code
in JDK 9.

### What changes are included in this PR?
- Add the module-info-compiler-maven-plugin and utilize it in the codebase.
- Exclude module-info.java files from maven-compiler-plugin.

### Are these changes tested?
Yes, they generate valid module-info.class files in JARs and test JARs.

### Are there any user-facing changes?
No
* Closes: apache#39134

Authored-by: James Duong <[email protected]>
Signed-off-by: David Li <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
### Rationale for this change
Create and integrate a maven plugin for compiling module-info.java files without using maven-compiler-plugin
and release 9+. This is necessary for supporting Java 8 unsafe code, which has different than unsafe code
in JDK 9.

### What changes are included in this PR?
- Add the module-info-compiler-maven-plugin and utilize it in the codebase.
- Exclude module-info.java files from maven-compiler-plugin.

### Are these changes tested?
Yes, they generate valid module-info.class files in JARs and test JARs.

### Are there any user-facing changes?
No
* Closes: apache#39134

Authored-by: James Duong <[email protected]>
Signed-off-by: David Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Java] (JPMS) Add a maven plugin for compiling modules while retaining JDK8 support
4 participants