Add Java version compatibility checks for Gradle, Android and Maven projects - #328
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughChangesThe package manager detects Java, Gradle, and Maven versions, validates Java compatibility before plugin execution, extracts Maven Java requirements from Java and build-tool compatibility
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PackageManager
participant JavaRuntime
participant BuildTool
participant MavenPlugin
PackageManager->>JavaRuntime: run java -version
JavaRuntime-->>PackageManager: Java major version
PackageManager->>BuildTool: read wrapper and project versions
BuildTool-->>PackageManager: compatibility requirements
PackageManager->>PackageManager: validate Java compatibility
MavenPlugin->>PackageManager: initialize superclass
PackageManager-->>MavenPlugin: success or failure
PackageManager->>BuildTool: execute compatible task
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
woocheol-lge
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fosslight_dependency/_package_manager.py (1)
161-226: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winScope the Java compatibility checks to the managers that use them.
PackageManager.run_plugin()calls_get_java_version()before branching bypackage_manager_name, so the base class also blocks any subclass that defines its ownrun_plugin()— including Maven. Add the Java check inside the relevant branch, or extract/reuse a separate check fromMaven.run_plugin()so Maven Java compatibility can be enforced and non-Java package managers like npm/pip are not aborted whenjavais absent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_dependency/_package_manager.py` around lines 161 - 226, The unconditional _get_java_version() call at the start of PackageManager.run_plugin() incorrectly affects non-Java managers and can interfere with subclass overrides. Move Java version retrieval and compatibility validation into the Gradle/Android and Maven-specific branches, preserving Maven’s Java compatibility enforcement while allowing npm/pip and other non-Java managers to proceed without Java installed.
🧹 Nitpick comments (1)
src/fosslight_dependency/_package_manager.py (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate Java-major-version parsing logic.
_get_java_versionandget_java_version_from_pomboth re-implement the same "normalize raw version string to a Java major int" logic (the1.xhandling plus int-parsing fallbacks). Extract a shared helper to avoid drift between the two copies.
src/fosslight_dependency/_package_manager.py#L91-146: replace the inline normalization block (Lines 120-137) with a call to a new shared helper, e.g._parse_java_major_version(raw_value).src/fosslight_dependency/_package_manager.py#L778-828: replace the per-candidate normalization block (Lines 804-820) with the same shared helper.♻️ Proposed shared helper
def _parse_java_major_version(raw_value): text = str(raw_value).strip() if text.startswith('1.'): parts = text.split('.') if len(parts) >= 2: try: return int(parts[1]) except ValueError: pass try: return int(text) except ValueError: try: return int(text.split('.')[0]) except ValueError: return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_dependency/_package_manager.py` at line 1, Extract the duplicated Java-version normalization into a shared helper named _parse_java_major_version, preserving the existing 1.x handling and integer-parsing fallbacks. Replace the inline normalization logic in _get_java_version and each candidate-normalization block in get_java_version_from_pom with calls to this helper, keeping their surrounding selection and fallback behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 155-159: Update _run_command and _run_command_output to pass the
configured subprocess timeout, ensuring external commands can raise
subprocess.TimeoutExpired and cannot hang indefinitely. Apply the same timeout
to the direct subprocess.run invocation in get_maven_version, while preserving
existing command execution and timeout handling in _get_java_version,
_run_gradle_plugin_task, and _run_fosslight_all_deps_task.
- Around line 168-199: Update the Gradle compatibility branching in the
package-manager Java check so Gradle 8.5+ requires Java 17 or higher without
applying the outdated maximum Java 21 cap. Preserve the Gradle 7.3–8.4 Java
11–17 and older-version Java 8–11 constraints, and revise requirement_text to
accurately describe the open-ended Gradle 8.5+ requirement.
---
Outside diff comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 161-226: The unconditional _get_java_version() call at the start
of PackageManager.run_plugin() incorrectly affects non-Java managers and can
interfere with subclass overrides. Move Java version retrieval and compatibility
validation into the Gradle/Android and Maven-specific branches, preserving
Maven’s Java compatibility enforcement while allowing npm/pip and other non-Java
managers to proceed without Java installed.
---
Nitpick comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Line 1: Extract the duplicated Java-version normalization into a shared helper
named _parse_java_major_version, preserving the existing 1.x handling and
integer-parsing fallbacks. Replace the inline normalization logic in
_get_java_version and each candidate-normalization block in
get_java_version_from_pom with calls to this helper, keeping their surrounding
selection and fallback behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8593aae8-4dab-4350-a432-5858e372ab99
📒 Files selected for processing (1)
src/fosslight_dependency/_package_manager.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/fosslight_dependency/_package_manager.py (1)
779-828: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winParse
pom.xmlinstead of regex-scanning it.
get_java_version_from_pom()scans the rawpom.xmltext and can pick up<maven.compiler.release>17</maven.compiler.release>from inside an XML comment, leading to an incorrect Java requirement. Usexml.etree.ElementTree(comments are excluded from the default parsed tree) and only search active/default Maven properties/profiles as needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/fosslight_dependency/_package_manager.py` around lines 779 - 828, Update get_java_version_from_pom to parse pom.xml with xml.etree.ElementTree instead of regex-scanning raw text, so commented-out properties are ignored. Traverse active/default Maven properties and profiles as needed, extract the existing compiler-release/source/target and java-version keys, preserve current normalization and maximum-candidate behavior, and return None for missing or invalid XML.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 91-106: Update _get_java_version to catch OSError when invoking
_run_command, covering permission and other execution failures in addition to
FileNotFoundError. Preserve the existing graceful failure behavior by logging
the Java command error and returning False.
- Around line 805-823: Extract the duplicated Java-version parsing logic into a
shared _normalize_java_major function. Replace the inline normalization block in
get_java_version_from_pom and the equivalent logic in _get_java_version with
calls to this helper, preserving the existing handling of 1.x and modern version
formats.
---
Nitpick comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 779-828: Update get_java_version_from_pom to parse pom.xml with
xml.etree.ElementTree instead of regex-scanning raw text, so commented-out
properties are ignored. Traverse active/default Maven properties and profiles as
needed, extract the existing compiler-release/source/target and java-version
keys, preserve current normalization and maximum-candidate behavior, and return
None for missing or invalid XML.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f531bb99-a28e-408a-89b3-e9b842698d78
📒 Files selected for processing (1)
src/fosslight_dependency/_package_manager.py
dd-jy
reviewed
Jul 27, 2026
dd-jy
requested changes
Jul 27, 2026
…, and Maven projects Signed-off-by: woocheol <jayden6659@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/fosslight_dependency/_package_manager.py`:
- Around line 145-151: Update Maven.run_plugin so it checks self.input_file_name
and reuses the existing Maven report before calling _get_java_version; only
perform Java validation when Maven execution is required. Preserve the existing
behavior for Gradle and Android and return the reused report result without
requiring Java.
- Around line 806-825: Extend get_java_version_from_pom() to inspect
maven-compiler-plugin configuration for source and target values in addition to
top-level properties. Resolve Maven property references such as ${version.java}
against the POM properties before passing values to _normalize_java_major, while
preserving the existing candidate-selection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cb96d26b-b2c3-4a4c-94a1-c5a70dc29e38
📒 Files selected for processing (2)
src/fosslight_dependency/_package_manager.pysrc/fosslight_dependency/package_manager/Maven.py
dd-jy
approved these changes
Jul 28, 2026
dd-jy
approved these changes
Jul 28, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary by CodeRabbit