[maven-4.0.x] Fix mvnup tool issues #7934-#7938 (#9311) #10915
Merged
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
* Fix mvnup tool issues apache#7934-apache#7938 This commit addresses five critical issues with the Maven upgrade tool (mvnup): **Issue apache#7934: External parent preservation** - Fixed InferenceStrategy to detect external parents vs reactor parents - External parents now preserve required groupId, artifactId, and version elements - Prevents 'parent.groupId is missing' errors when using META-POMs **Issue apache#7935: .mvn directory creation for all model versions** - Modified AbstractUpgradeGoal to create .mvn directory for both 4.0.0 and 4.1.0 - Helps Maven find project root and avoids root directory warnings **Issue apache#7936: Downgrade error handling** - Changed ModelUpgradeStrategy to fail with errors instead of warnings for invalid downgrades - Tool now properly exits with error code when attempting 4.1.0 → 4.0.0 downgrade **Issue apache#7937: Unicode icon fallback** - Enhanced UpgradeContext to detect Unicode support using terminal.stdoutEncoding() - Falls back to ASCII characters ([OK], [ERROR], etc.) when Unicode not supported - Improves compatibility with Windows CMD/PowerShell and other terminals **Issue apache#7938: Child version removal in 4.1.0** - Enhanced inference logic to properly remove matching child versions in subprojects - Ensures child project versions are removed when they match parent version **Testing:** - Updated existing tests to reflect correct behavior - Added new tests for downgrade handling and Unicode detection - All mvnup tests passing with comprehensive coverage Fixes apache#7934, apache#7935, apache#7936, apache#7937, apache#7938 * Improve parent reactor detection logic - Replace heuristic-based approach with direct GAV matching against pomMap - More accurate detection of external vs reactor parents - Added comprehensive test coverage for both scenarios - Suggested by code review feedback * Refactor to use GAVUtils.extractGAVWithParentResolution - Replace manual GAV extraction with existing GAVUtils method - More robust handling of parent resolution and inheritance - Leverages existing tested utility for GAV extraction - Cleaner and more maintainable code * Address review feedback: Implement ConsoleIcon enum with charset detection This commit addresses all review feedback from @elharo and @Bukama: **Icon Management Improvements:** - Created ConsoleIcon enum with Unicode/ASCII fallback pairs - Moved all icon logic into the enum for consistency and reusability - Implemented ConsoleIcon.getIcon(Terminal) for clean integration **Unicode Detection Enhancements:** - Use Charset.newEncoder().canEncode(char) for accurate character testing - Use terminal.encoding() instead of deprecated stdoutEncoding() - Simplified fallback to Charset.defaultCharset() - Removed complex heuristics and exception handling **Code Quality Improvements:** - UpgradeContext methods are now clean one-liners - Better separation of concerns between icon rendering and context management - Comprehensive test coverage for all scenarios - Future-proof design for adding new icons **Review Comments Addressed:** - @elharo: 'maybe this should be a field that can be shared?' → Icons now shared via enum - @Bukama: 'maybe an enum with icon and fallback text?' → Implemented with Terminal integration - @elharo: 'There are others that also support Unicode, e.g. UCS-2., UCS-4' → Now tests actual encoding capability - @elharo: 'Please be specific about exceptions' → No longer needed with simplified approach - @elharo: 'toLowerCase(Locale.ENGLISH)' → No longer needed with canEncode() approach **Technical Benefits:** - More accurate Unicode detection per character - Works with any charset automatically - Cleaner and more maintainable code - Better test coverage and reliability (cherry picked from commit adf2c49)
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.
Backport
This will backport the following commits from
mastertomaven-4.0.x:Questions ?
Please refer to the Backport tool documentation