Skip to content

Conversation

@deniskuzZ
Copy link
Member

@deniskuzZ deniskuzZ commented Nov 22, 2025

What changes were proposed in this pull request?

Upgrade Hive's parent pom (org.apache:apache) version to pick up newer versions of compiler and javadoc plugins

Why are the changes needed?

Post jdk21 upgrade fixes

Does this PR introduce any user-facing change?

No

How was this patch tested?

Locally + Jenkins

@deniskuzZ deniskuzZ force-pushed the compile_javadoc_plugins branch from 4ff9b74 to b8af6bb Compare November 22, 2025 20:08
@deniskuzZ deniskuzZ force-pushed the compile_javadoc_plugins branch 2 times, most recently from 613d5f5 to 07d5458 Compare November 22, 2025 21:05
@deniskuzZ deniskuzZ changed the title Compile javadoc plugins Upgrade Compiler & Javadoc plugins in HMS Nov 22, 2025
Copy link
Contributor

@difin difin left a comment

Choose a reason for hiding this comment

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

+1

LGTM, but storage-api seems to miss these changes too.

@deniskuzZ deniskuzZ force-pushed the compile_javadoc_plugins branch from 07d5458 to b33690e Compare November 22, 2025 23:22
@deniskuzZ deniskuzZ changed the title Upgrade Compiler & Javadoc plugins in HMS Upgrade compiler and javadoc plugins Nov 22, 2025
@deniskuzZ deniskuzZ changed the title Upgrade compiler and javadoc plugins HIVE-29336: Upgrade compiler and javadoc plugins Nov 22, 2025
@zhangbutao
Copy link
Contributor

zhangbutao commented Nov 23, 2025

I tried this PR with cmd:
mvn clean install javadoc:javadoc javadoc:aggregate -Pjavadoc -DskipTests

But not found any site/apidocs directory.
find . -type d -path "*/site/apidocs" 2>/dev/null

If building branch-4.1, you can find site/apidocs in these directories:
./hive-branch4.1/hive$ find . -type d -path "*/site/apidocs" 2>/dev/null

./serde/target/site/apidocs
./jdbc/target/site/apidocs
./classification/target/site/apidocs
./common/target/site/apidocs
./udf/target/site/apidocs
./metastore/target/site/apidocs
./ql/target/site/apidocs
./cli/target/site/apidocs
./llap-ext-client/target/site/apidocs
./llap-tez/target/site/apidocs
./hplsql/target/site/apidocs
./druid-handler/target/site/apidocs
./jdbc-handler/target/site/apidocs
./contrib/target/site/apidocs
./hbase-handler/target/site/apidocs
./kafka-handler/target/site/apidocs
./service/target/site/apidocs
./vector-code-gen/target/site/apidocs
./service-rpc/target/site/apidocs
./parser/target/site/apidocs
./beeline/target/site/apidocs
./llap-client/target/site/apidocs
./iceberg/iceberg-catalog/target/site/apidocs
./iceberg/iceberg-handler/target/site/apidocs
./iceberg/patched-iceberg-core/target/site/apidocs
./iceberg/patched-iceberg-api/target/site/apidocs
./iceberg/iceberg-shading/target/site/apidocs
./hcatalog/webhcat/svr/target/site/apidocs
./hcatalog/webhcat/java-client/target/site/apidocs
./hcatalog/hcatalog-pig-adapter/target/site/apidocs
./hcatalog/core/target/site/apidocs
./hcatalog/server-extensions/target/site/apidocs
./storage-api/target/site/apidocs
./streaming/target/site/apidocs
./kudu-handler/target/site/apidocs
./testutils/target/site/apidocs
./llap-server/target/site/apidocs
./accumulo-handler/target/site/apidocs
./shims/0.23/target/site/apidocs
./shims/common/target/site/apidocs
./shims/aggregator/target/site/apidocs
./llap-common/target/site/apidocs
./standalone-metastore/metastore-rest-catalog/target/site/apidocs
./standalone-metastore/metastore-tools/tools-common/target/site/apidocs
./standalone-metastore/metastore-tools/metastore-benchmarks/target/site/apidocs
./standalone-metastore/metastore-server/target/site/apidocs
./standalone-metastore/metastore-common/target/site/apidocs
./target/site/apidocs

The ./target/site/apidocs directory is the final aggregated output we need.

@deniskuzZ
Copy link
Member Author

deniskuzZ commented Nov 23, 2025

I tried this PR with cmd: mvn clean install javadoc:javadoc javadoc:aggregate -Pjavadoc -DskipTests

But not found any site/apidocs directory. find . -type d -path "*/site/apidocs" 2>/dev/null

If building branch-4.1, you can find site/apidocs in these directories: ./hive-branch4.1/hive$ find . -type d -path "*/site/apidocs" 2>/dev/null

./serde/target/site/apidocs
./jdbc/target/site/apidocs
./classification/target/site/apidocs
./common/target/site/apidocs
./udf/target/site/apidocs
./metastore/target/site/apidocs
./ql/target/site/apidocs
./cli/target/site/apidocs
./llap-ext-client/target/site/apidocs
./llap-tez/target/site/apidocs
./hplsql/target/site/apidocs
./druid-handler/target/site/apidocs
./jdbc-handler/target/site/apidocs
./contrib/target/site/apidocs
./hbase-handler/target/site/apidocs
./kafka-handler/target/site/apidocs
./service/target/site/apidocs
./vector-code-gen/target/site/apidocs
./service-rpc/target/site/apidocs
./parser/target/site/apidocs
./beeline/target/site/apidocs
./llap-client/target/site/apidocs
./iceberg/iceberg-catalog/target/site/apidocs
./iceberg/iceberg-handler/target/site/apidocs
./iceberg/patched-iceberg-core/target/site/apidocs
./iceberg/patched-iceberg-api/target/site/apidocs
./iceberg/iceberg-shading/target/site/apidocs
./hcatalog/webhcat/svr/target/site/apidocs
./hcatalog/webhcat/java-client/target/site/apidocs
./hcatalog/hcatalog-pig-adapter/target/site/apidocs
./hcatalog/core/target/site/apidocs
./hcatalog/server-extensions/target/site/apidocs
./storage-api/target/site/apidocs
./streaming/target/site/apidocs
./kudu-handler/target/site/apidocs
./testutils/target/site/apidocs
./llap-server/target/site/apidocs
./accumulo-handler/target/site/apidocs
./shims/0.23/target/site/apidocs
./shims/common/target/site/apidocs
./shims/aggregator/target/site/apidocs
./llap-common/target/site/apidocs
./standalone-metastore/metastore-rest-catalog/target/site/apidocs
./standalone-metastore/metastore-tools/tools-common/target/site/apidocs
./standalone-metastore/metastore-tools/metastore-benchmarks/target/site/apidocs
./standalone-metastore/metastore-server/target/site/apidocs
./standalone-metastore/metastore-common/target/site/apidocs
./target/site/apidocs

The ./target/site/apidocs directory is the final aggregated output we need.

after jdk upgrade to 21 javadoc & compiler plugins have issues. Hive root pom was upgraded, but not the rest of modules with apache parent
https://maven.apache.org/plugins/maven-javadoc-plugin/javadoc-mojo.html
new location: target/site/apidocs → target/reports/apidocs

@Aggarwal-Raghav
Copy link
Contributor

@deniskuzZ Based on the changes done in this PR, what I understand is that the goal is to sync the maven compiler and javadoc plugin in standalone-metastore and storage-api module. Which makes sense but why hardcoded version is used? IMO, it would be better to introduce variables in standalone-metastore/pom.xml and storage-api/pom.xml.

In order to verify if variables are getting resolved correctly mvn help:effective-pom can be ran in respective module.

@deniskuzZ
Copy link
Member Author

deniskuzZ commented Nov 23, 2025

@deniskuzZ Based on the changes done in this PR, what I understand is that the goal is to sync the maven compiler and javadoc plugin in standalone-metastore and storage-api module. Which makes sense but why hardcoded version is used? IMO, it would be better to introduce variables in standalone-metastore/pom.xml and storage-api/pom.xml.

In order to verify if variables are getting resolved correctly mvn help:effective-pom can be ran in respective module.

plugin versions are only used once in pluginManagement declaration. if you check the parent module (org.apache.apache), it's done the same way.
but maybe we should simply upgrade to https://github.com/apache/maven-apache-parent/blob/apache-35/pom.xml
let me try that

@deniskuzZ deniskuzZ force-pushed the compile_javadoc_plugins branch from b318418 to ddd8034 Compare November 23, 2025 23:21
<artifactId>mockito-inline</artifactId>
<version>${iceberg.mockito-core.version}</version>
</dependency>
<dependency>
Copy link
Member Author

@deniskuzZ deniskuzZ Nov 23, 2025

Choose a reason for hiding this comment

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

fixes sub-module compilation, until now compilation was failing within iceberg module

@sonarqubecloud
Copy link

<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<compilerArgs>
<arg>--add-opens</arg>
Copy link
Member Author

Choose a reason for hiding this comment

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

@akshat0395, why was it needed in the first place?

<artifactId>maven-compiler-plugin</artifactId>
<configuration>
<compilerArgs>
<arg>--add-exports</arg>
Copy link
Member Author

Choose a reason for hiding this comment

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

@akshat0395, same here

@deniskuzZ deniskuzZ requested a review from zhangbutao November 25, 2025 16:45
@deniskuzZ
Copy link
Member Author

@Aggarwal-Raghav, could you please take another look?

Copy link
Contributor

@zhangbutao zhangbutao left a comment

Choose a reason for hiding this comment

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

+1 LGTM

Tested this PR with this cmd:
mvn clean install javadoc:javadoc javadoc:aggregate -Pjavadoc -DskipTests -Pdist

find . -type d -path "*/site/apidocs" 2>/dev/null
./serde/target/site/apidocs
./jdbc/target/site/apidocs
./classification/target/site/apidocs
./common/target/site/apidocs
./udf/target/site/apidocs
./metastore/target/site/apidocs
./ql/target/site/apidocs
./cli/target/site/apidocs
./llap-ext-client/target/site/apidocs
./llap-tez/target/site/apidocs
./hplsql/target/site/apidocs
./druid-handler/target/site/apidocs
./jdbc-handler/target/site/apidocs
./contrib/target/site/apidocs
./hbase-handler/target/site/apidocs
./kafka-handler/target/site/apidocs
./service/target/site/apidocs
./vector-code-gen/target/site/apidocs
./service-rpc/target/site/apidocs
./parser/target/site/apidocs
./beeline/target/site/apidocs
./llap-client/target/site/apidocs
./iceberg/iceberg-catalog/target/site/apidocs
./iceberg/iceberg-handler/target/site/apidocs
./iceberg/patched-iceberg-core/target/site/apidocs
./iceberg/patched-iceberg-api/target/site/apidocs
./iceberg/iceberg-shading/target/site/apidocs
./hcatalog/webhcat/svr/target/site/apidocs
./hcatalog/webhcat/java-client/target/site/apidocs
./hcatalog/hcatalog-pig-adapter/target/site/apidocs
./hcatalog/core/target/site/apidocs
./hcatalog/server-extensions/target/site/apidocs
./storage-api/target/site/apidocs
./streaming/target/site/apidocs
./kudu-handler/target/site/apidocs
./testutils/target/site/apidocs
./llap-server/target/site/apidocs
./accumulo-handler/target/site/apidocs
./shims/0.23/target/site/apidocs
./shims/common/target/site/apidocs
./shims/aggregator/target/site/apidocs
./llap-common/target/site/apidocs
./standalone-metastore/metastore-rest-catalog/target/site/apidocs
./standalone-metastore/metastore-tools/tools-common/target/site/apidocs
./standalone-metastore/metastore-tools/metastore-benchmarks/target/site/apidocs
./standalone-metastore/metastore-client/target/site/apidocs
./standalone-metastore/metastore-server/target/site/apidocs
./standalone-metastore/metastore-common/target/site/apidocs
./target/site/apidocs

@Aggarwal-Raghav
Copy link
Contributor

LGTM +1 (non-binding)

  1. Verified the mvn-compiler and javadoc plugin version in storage-api, standalone-metastore and parent pom.xml
  2. Observed the compilation error without explicit dependency of error_prone_annotations in iceberg/pom.xml and post adding the dependency build works ✅

@deniskuzZ deniskuzZ merged commit be14ad0 into apache:master Nov 26, 2025
2 checks passed
@deniskuzZ
Copy link
Member Author

Thank you for such a thorough review, @Aggarwal-Raghav, @zhangbutao, @difin!

@Aggarwal-Raghav
Copy link
Contributor

one benefit of this patch is that in standalone-metastore/metastore-rest-catalog/pom.xml which was using pom 23 version of surefire i.e. 2.22.0 has now in synced with 3.5.3. But we need to change the version tag to maven.surefire.plugin.version
for future.

    <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>${surefire.version}</version>
      </plugin>

Older effective pom
Screenshot 2025-11-26 at 11 23 18 PM

@deniskuzZ
Copy link
Member Author

one benefit of this patch is that in standalone-metastore/metastore-rest-catalog/pom.xml which was using pom 23 version of surefire i.e. 2.22.0 has now in synced with 3.5.3. But we need to change the version tag to maven.surefire.plugin.version for future.

    <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-surefire-plugin</artifactId>
        <version>${surefire.version}</version>
      </plugin>

Older effective pom Screenshot 2025-11-26 at 11 23 18 PM

@Aggarwal-Raghav, could you please create a ticket?

@Aggarwal-Raghav
Copy link
Contributor

@deniskuzZ , filed HIVE-29347

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.

5 participants