Fix invalid POM caused by missing jackson-dataformat-yaml version #7969
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove likely-to-become-outdated copies of release scripts to ensure they are not run by accident from a branch (they should always be run from develop). Also remove CODEOWNERS to avoid the confusion of GitHub showing owner names like on develop, but codeowner reviews not actually being required (due to lack of branch protection or minimum review count on support branches)
As part of the Geode Release Process, the build number must be rolled forward so work can begin on the next release
…ernals (apache#7956) Replace reflection-based access to DirectByteBuffer private APIs with Unsafe field offset access, eliminating the need for --add-opens=java.base/java.nio=ALL-UNNAMED JVM flag. Key Changes: - Enhanced Unsafe wrapper with buffer field access methods * Added cached field offsets (BUFFER_ADDRESS_FIELD_OFFSET, BUFFER_CAPACITY_FIELD_OFFSET) * Added getBufferAddress/setBufferAddress methods * Added getBufferCapacity/setBufferCapacity methods * Field offset access does NOT require --add-opens flags - Refactored AddressableMemoryManager to eliminate reflection * Removed all reflection imports (Constructor, Method, InvocationTargetException) * Removed static volatile reflection caching fields * Reimplemented getDirectByteBufferAddress() using Unsafe.getBufferAddress() * Reimplemented createDirectByteBuffer() using field manipulation * Maintains zero-copy semantics by modifying buffer fields - Removed JAVA_NIO_OPEN flag from MemberJvmOptions * Deleted JAVA_NIO_OPEN constant and documentation * Removed flag from JAVA_11_OPTIONS list * Reduced required JVM flags from 5 to 4 Benefits: - Eliminates security audit findings for --add-opens usage - Improves Java module system compliance - Compatible with Java 17+ strong encapsulation (JEP 403) - Forward compatible with Java 21 - Simplifies deployment configuration - Better performance through cached field offsets - Enables GraalVM native image compilation This change is part of the broader initiative to eliminate all --add-opens and --add-exports flags from Apache Geode for full Java module system compliance.
…Internal Package (apache#7955) * refactor: Replace internal JDK DirectBuffer with public API solution Replace sun.nio.ch.DirectBuffer usage with BufferAttachmentTracker, using only public Java APIs (WeakHashMap and ByteBuffer). Changes: - Created BufferAttachmentTracker: WeakHashMap-based tracker for slice-to-original buffer mappings, replacing internal DirectBuffer.attachment() access - Updated BufferPool: Modified slice creation to record mappings and simplified getPoolableBuffer() to use the tracker - Removed DirectBuffer wrapper: Deleted geode-unsafe DirectBuffer wrapper class - Updated MemberJvmOptions: Removed SUN_NIO_CH_EXPORT from required JVM options - Added comprehensive unit tests: BufferAttachmentTrackerTest validates all tracker functionality Benefits: - Eliminates one JVM module export requirement - Uses only public Java APIs - Maintains functionality with automatic memory cleanup via WeakHashMap - Fully backward compatible Testing: - All BufferPool tests pass - New BufferAttachmentTracker tests pass - Compilation successful * Add comprehensive documentation to BufferAttachmentTracker - Add detailed PMD suppression justification explaining thread-safety - Document why ConcurrentHashMap is safe for concurrent access - Explain lock-free operations and atomic guarantees - Add 7-line comment block explaining mutable static field design choice * Apply spotless formatting to BufferAttachmentTrackerTest * fix: Correct buffer pooling to prevent capacity issues in NioEngine - Fixed acquirePredefinedFixedBuffer() to return full-capacity buffers instead of modifying buffer limits before return - Added BufferAttachmentTracker.removeTracking() in releaseBuffer() to properly clean up slice-to-original mappings - Created non-slicing buffer acquisition methods for NioPlainEngine and NioSslEngine which require reusable full-capacity buffers - Separated buffer acquisition into two use cases: * Single-use sliced buffers (2-param acquireDirectBuffer) * Reusable full-capacity buffers (3-param acquireDirectBuffer) This fixes IllegalArgumentException 'newLimit > capacity' errors in distributed tests by ensuring pooled buffers maintain proper capacity. * Fix IndexOutOfBoundsException in BufferAttachmentTracker Replace ConcurrentHashMap with synchronized IdentityHashMap to avoid ByteBuffer.equals() issues. ByteBuffer uses content-based equality which can throw IndexOutOfBoundsException when buffer state (position/limit) changes after being used as a map key. IdentityHashMap uses object identity (==) which is safe and appropriate for tracking buffer relationships.
…mar (apache#7942) * GEODE-10508: Fix ANTLR nondeterminism warnings in OQL grammar This commit resolves four nondeterminism warnings generated by ANTLR during the OQL grammar compilation process. These warnings indicated parser ambiguity that could lead to unpredictable parsing behavior. Problem Analysis: ----------------- 1. Lines 574 & 578 (projection rule): The parser could not distinguish between aggregateExpr and expr alternatives when encountering aggregate function keywords (sum, avg, min, max, count). These keywords are valid both as: - Aggregate function identifiers: sum(field) - Regular identifiers in expressions: sum as a field name Without lookahead, ANTLR could not deterministically choose which production rule to apply, resulting in nondeterminism warnings. 2. Lines 961 & 979 (aggregateExpr rule): Optional 'distinct' keyword created ambiguity in aggregate function parsing. The parser could not decide whether to: - Match the optional 'distinct' keyword, or - Skip it and proceed directly to the expression Both paths were valid, but ANTLR's default behavior doesn't specify preference, causing nondeterminism. Solution Implemented: -------------------- 1. Added syntactic predicates to projection rule (lines 574, 578): Predicate: (('sum'|'avg'|'min'|'max'|'count') TOK_LPAREN)=> This instructs the parser to look ahead and check if an aggregate keyword is followed by a left parenthesis. If true, it chooses aggregateExpr; otherwise, it chooses expr. This resolves the ambiguity by providing explicit lookahead logic. 2. Added greedy option to aggregateExpr rule (lines 961, 979): Option: options {greedy=true;} This tells the parser to greedily match the 'distinct' keyword whenever it appears, rather than being ambiguous about whether to match or skip. The greedy option eliminates the nondeterminism by establishing clear matching priority. 3. Updated test to use token constants (AbstractCompiledValueTestJUnitTest): Changed: hardcoded value 89 -> OQLLexerTokenTypes.LITERAL_or Rationale: Adding syntactic predicates changes ANTLR's token numbering in the generated lexer (LITERAL_or shifted from 89 to 94). Using the constant ensures test correctness regardless of future grammar changes. This is a best practice for maintaining test stability. Impact: ------- - Zero nondeterminism warnings from ANTLR grammar generation - No changes to OQL syntax or semantics (fully backward compatible) - No runtime behavior changes (modifications only affect parser generation) - All existing tests pass with updated token reference - Improved parser determinism and maintainability Technical Details: ----------------- - Syntactic predicates (=>) are standard ANTLR 2 feature for lookahead - Greedy option is standard ANTLR feature for optional subrule disambiguation - Token constant usage follows best practices for generated code references - Changes are compile-time only with no runtime performance impact Files Modified: -------------- - geode-core/src/main/antlr/org/apache/geode/cache/query/internal/parse/oql.g - geode-core/src/test/java/org/apache/geode/cache/query/internal/AbstractCompiledValueTestJUnitTest.java * GEODE-10508: Apply code formatting to test file Fix line length formatting for improved readability.
…e System Encapsulation (apache#7954) * Replace reflection-based UnsafeThreadLocal with WeakHashMap implementation - Removed reflection access to ThreadLocal/ThreadLocalMap internals - Implemented cross-thread value lookup using synchronized WeakHashMap - Removed requirement for --add-opens=java.base/java.lang=ALL-UNNAMED - WeakHashMap ensures terminated threads can be garbage collected - Maintains same API and functionality for deadlock detection - All existing tests pass without JVM flag changes This eliminates the fragile reflection-based approach that required special JVM flags and was vulnerable to Java module system changes. The new implementation is safer, more maintainable, and works across all Java versions without requiring internal access. * Remove --add-opens=java.base/java.lang from test configuration - Removed unnecessary JVM flag from geode-test.gradle line 185 - Flag no longer needed after UnsafeThreadLocal refactoring - Tests now run with same security constraints as production - All UnsafeThreadLocal and deadlock tests pass without the flag - Validates that refactoring truly eliminated reflection dependency
…ncy Information (apache#7961) * Correct license classification for Jakarta EE dependencies - Moved jakarta.servlet v6.0.0 and jakarta.transaction v2.0.1 from CDDL to EPL 2.0 section - These components use EPL 2.0 with GPL-2.0 + Classpath Exception, not CDDL 1.1 * GEODE-10511: Update istack-commons-runtime version from 4.0.1 to 4.1.1 - Aligns declared version with actual resolved version - Eliminates version conflict resolution between 4.0.1 and 4.1.1 - Makes DependencyConstraints.groovy consistent with LICENSE file - jaxb-core/jaxb-runtime 4.0.2 transitively requires 4.1.1 * GEODE-10511: Update test expectations for istack-commons-runtime 4.1.1 - Update geode-server-all dependency_classpath.txt - Update geode-assembly assembly_content.txt to remove 4.0.1 reference - Fixes integration test failures in both modules
…--add-opens Requirement (apache#7957) * GEODE-10522: Eliminate reflection in VMStats50 to remove --add-opens requirement Replace reflection-based access to platform MXBean methods with direct interface casting, eliminating the need for --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED JVM flag. Key Changes: - Replaced Method.invoke() with direct calls to com.sun.management interfaces - Removed setAccessible(true) calls that required module opening - Updated to use OperatingSystemMXBean and UnixOperatingSystemMXBean directly - Removed COM_SUN_MANAGEMENT_INTERNAL_OPEN flag from MemberJvmOptions - Removed unused ClassPathLoader import - Improved code clarity and type safety Benefits: - Completes Java Platform Module System (JPMS) compliance initiative - Eliminates last remaining --add-opens flag requirement - Improves security posture (no module violations) - Better performance (no reflection overhead) - Simpler, more maintainable code Testing: - All VMStats tests pass - Tested without module flags - Uses public, documented APIs from exported com.sun.management package This completes the module compliance initiative: - GEODE-10519: Eliminated java.base/java.lang opening - GEODE-10520: Eliminated sun.nio.ch export - GEODE-10521: Eliminated java.base/java.nio opening - GEODE-10522: Eliminated jdk.management/com.sun.management.internal opening (this commit) Apache Geode now requires ZERO module flags to run on Java 17+. * Apply code formatting to VMStats50 - Fix import ordering (move com.sun.management imports after java.util imports) - Remove trailing whitespace - Apply consistent formatting throughout * Address reviewer feedback: Add null check and improve error message - Add null check for platformOsBean before calling getAvailableProcessors() - Enhance error message to clarify impact on statistics vs core functionality - Both changes suggested by @sboorlagadda in PR review * Remove SUN_NIO_CH_EXPORT reference from JAVA_11_OPTIONS - Fix compilation error after merging GEODE-10520 changes - SUN_NIO_CH_EXPORT constant was removed but still referenced in list * Fix duplicate JAVA_NIO_OPEN and missing JAVA_LANG_OPEN - Remove duplicate JAVA_NIO_OPEN definition - Add missing JAVA_LANG_OPEN constant - Fix comment to correctly reference UnsafeThreadLocal for JAVA_LANG_OPEN
…nd Java 17 (apache#7953) * docs: Update documentation for Jakarta EE 10 and Java 17 - Update Java version format from 1.8.0_121 to 17.0.16 - Update all Geode module versions from 1.0.0 to 2.0.0 - Replace javax.transaction-api with jakarta.transaction-api 2.0.1 - Update dependency versions (slf4j 2.0.17, log4j 2.17.2, jgroups 3.6.20, fastutil 8.5.8) - Update config.yml: min_java_version='17', min_java_update='16' - Fix Java version template expressions across 20+ documentation files - Update WebLogic HTTP session management guide for Jakarta EE 10 - Update installation guides with Java 17 requirements Breaking Changes: - Minimum Java version now Java 17.0.16 (was Java 8u121) - Jakarta EE 10 required (was Java EE 8) - All javax.* packages replaced with jakarta.* Testing: - Verified peer-to-peer and client-server configurations - Documentation builds successfully - All quality checks passed (spotlessCheck, rat, checkPom, pmdMain) * docs: Address review feedback - fix version consistency and consolidate tc Server deprecation notes - Fix Tomcat version inconsistency: Changed CATALINA_HOME path from 10.1.49 to 10.1.30 to match example text - Consolidate duplicate tc Server removal messages into single Note for clarity - Improve documentation consistency and readability * Fix log file path to be consistent with server path
The geode-core module declares jackson-dataformat-yaml as a dependency without specifying a version, relying on DependencyConstraints.groovy to provide it. However, DependencyConstraints.groovy was missing the version constraint for com.fasterxml.jackson.dataformat.* artifacts. This caused the published geode-core-2.0.0.pom to have jackson-dataformat-yaml with no <version> tag, making the POM invalid according to Maven specification. Maven refuses to process ANY transitive dependencies from an invalid POM, which caused all dependencies (antlr, jopt-simple, micrometer-core, shiro-core, jakarta.transaction-api, geode-management, geode-deployment-legacy, rmiio) to not be pulled transitively. This fix adds the missing dependency constraint for jackson-dataformat-yaml, using jackson.version (2.17.0) to match other Jackson artifacts. Issue reported by Leon during 2.0.0.RC2 testing.
JinwooHwang
requested review from
sboorlagadda
and removed request for
sboorlagadda
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The published
geode-core-2.0.0.pomwas invalid, causing Maven to refuse processing ANY transitive dependencies. Users had to manually add all dependencies that should have been transitive.Maven Error:
Dependency Tree (broken):
Root Cause
geode-core/build.gradleline 207 declaresjackson-dataformat-yamlwithout a version:runtimeOnly('com.fasterxml.jackson.dataformat:jackson-dataformat-yaml')This relies on
DependencyConstraints.groovyto provide the version via the BOM. However,DependencyConstraints.groovydefined constraints for:com.fasterxml.jackson.core.*com.fasterxml.jackson.datatype.*com.fasterxml.jackson.dataformat.*(MISSING!)Result: The published POM had a dependency with no
<version>tag, making it invalid per Maven spec. Maven rejects invalid POMs and refuses to process their transitive dependencies.Solution
Added the missing dependency constraint to
DependencyConstraints.groovy:This adds
jackson-dataformat-yaml:2.17.0to thegeode-all-bomdependency management, ensuring the publishedgeode-corePOM is valid.Verification
After fix, dependency tree works correctly:
No invalid POM warnings. All transitive dependencies are pulled correctly.
Impact
This fixes the invalid POM that was blocking all transitive dependencies, including:
Users no longer need to manually declare these dependencies.
Testing
geode-all-bomcontainsjackson-dataformat-yaml:2.17.0geode-corePOM imports BOM in<dependencyManagement>Issue discovered by: Leon during 2.0.0.RC2 testing
For all changes, please confirm:
develop)?gradlew buildrun cleanly?