Conversation
📝 WalkthroughWalkthroughGradle wrapper resolution now returns absolute paths for ChangesGradle wrapper resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 1002-1007: Update the wrapper-selection logic in the surrounding
function to choose the platform-specific filename first, then validate that
selected path exists before returning it. Preserve the Windows/non-Windows
choices in cmd_gradle and ensure a missing wrapper is not returned as a valid
executable path.
- Around line 1002-1007: Update get_gradle_cmd() to accept input_dir and resolve
gradlew or gradlew.bat relative to that directory, then pass input_dir from
collect_gradle_download_urls(). Preserve the existing Windows selection and
absolute-path behavior while ensuring wrapper checks and subprocess execution
use the same project directory.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6b5f20df-7750-48f1-a33a-bed9ac6ef081
📒 Files selected for processing (1)
src/fosslight_dependency/_package_manager.py
| # Absolute path: the command is run through subprocess with shell=False, and | ||
| # on Windows CreateProcess cannot resolve a bare '.bat' name ([WinError 2]). | ||
| if platform.system() == const.WINDOWS: | ||
| cmd_gradle = "gradlew.bat" | ||
| cmd_gradle = os.path.abspath("gradlew.bat") | ||
| else: | ||
| cmd_gradle = "./gradlew" | ||
| cmd_gradle = os.path.abspath("gradlew") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the selected wrapper before returning it.
The surrounding condition accepts either wrapper, but each branch returns only the platform-specific filename. If that filename is absent, the function returns an absolute path to a missing file.
Select the platform-specific filename first, then check that file exists.
Proposed fix
- if os.path.isfile('gradlew') or os.path.isfile('gradlew.bat'):
+ wrapper_name = "gradlew.bat" if platform.system() == const.WINDOWS else "gradlew"
+ if os.path.isfile(wrapper_name):
if platform.system() == const.WINDOWS:
- cmd_gradle = os.path.abspath("gradlew.bat")
+ cmd_gradle = os.path.abspath(wrapper_name)
else:
- cmd_gradle = os.path.abspath("gradlew")
+ cmd_gradle = os.path.abspath(wrapper_name)🤖 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 1002 - 1007,
Update the wrapper-selection logic in the surrounding function to choose the
platform-specific filename first, then validate that selected path exists before
returning it. Preserve the Windows/non-Windows choices in cmd_gradle and ensure
a missing wrapper is not returned as a valid executable path.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect every caller to provide or establish the directory used for resolution.
rg -nP '\bget_gradle_cmd\s*\(' --glob '*.py'
rg -n -C 5 'collect_gradle_download_urls|cwd=input_dir' src/fosslight_dependency/_package_manager.pyRepository: fosslight/fosslight_dependency_scanner
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked files matching _package_manager.py:\n'
git ls-files | rg '(^|/)package_manager\.py$|_package_manager\.py$|fosslight_dependency' || true
printf '\nSearch get_gradle_cmd in tracked files:\n'
rg -n '\bget_gradle_cmd\s*\(' . || true
printf '\nTarget file existence and relevant lines if present:\n'
if [ -f src/fosslight_dependency/_package_manager.py ]; then
wc -l src/fosslight_dependency/_package_manager.py
sed -n '960,1040p' src/fosslight_dependency/_package_manager.py | nl -ba -v960
else
echo 'src/fosslight_dependency/_package_manager.py not found'
fiRepository: fosslight/fosslight_dependency_scanner
Length of output: 1853
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Relevant _package_manager.py sections:'
sed -n '970,1035p' src/fosslight_dependency/_package_manager.py
echo
echo 'Search gradle command calls and cwd usage in tracked Python files:'
rg -n -C 4 'get_gradle_cmd|cwd=|subprocess\.run|collect_gradle_download_urls' src/fosslight_dependency --glob '*.py' || trueRepository: fosslight/fosslight_dependency_scanner
Length of output: 20734
Resolve the Gradle wrapper from input_dir.
get_gradle_cmd() checks and resolves gradlew/gradlew.bat against the process current directory, while collect_gradle_download_urls(input_dir, ...) runs the command with cwd=input_dir. If these directories differ, the wrapper may be found in a different project or selected via a different relative path. Pass input_dir to get_gradle_cmd() and construct the absolute path from that directory.
🤖 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 1002 - 1007,
Update get_gradle_cmd() to accept input_dir and resolve gradlew or gradlew.bat
relative to that directory, then pass input_dir from
collect_gradle_download_urls(). Preserve the existing Windows selection and
absolute-path behavior while ensuring wrapper checks and subprocess execution
use the same project directory.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
2 participants
The wrapper command is passed to subprocess as a list with shell=False.
On Windows, CreateProcess cannot resolve a bare '.bat' name, so returning
'gradlew.bat' made every Gradle/Android analysis fail with [WinError 2].
Return os.path.abspath() from both _resolve_wrapper_command() and
get_gradle_cmd(). This keeps shell=False, so there are no quoting or shell
injection concerns, and an absolute path is equally valid on POSIX
(ensure_executable() only stats and chmods the path).
Summary by CodeRabbit