Fix archlinux-java execution and order#102
Conversation
- Moved the archlinux-java fix command before detect_java to ensure detection uses the fixed state. - Replaced direct sudo call with run_as_root to follow codebase conventions. - Added redirection to /dev/null and allowed failure with || : to prevent script interruption. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request reorders the JDK detection block in tools/server-start.sh to execute after the Arch Linux Java fix, and replaces the use of sudo with run_as_root for the archlinux-java fix command. Feedback suggests checking if the script is already running as root before attempting the fix, to prevent unexpected password prompts for non-root users.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if has archlinux-java; then | ||
| sudo archlinux-java fix &>/dev/null || : | ||
| run_as_root archlinux-java fix &>/dev/null || : | ||
| fi |
There was a problem hiding this comment.
Using run_as_root here can cause the script to block and prompt the user for a sudo password in interactive shells, which is unexpected and disruptive for a Minecraft server startup script. Since fixing system-wide Java configurations (archlinux-java fix) requires root privileges and is a system administration task, it should only be attempted if the script is already running as root (e.g., $EUID -eq 0). This avoids prompting non-root users for passwords.
| if has archlinux-java; then | |
| sudo archlinux-java fix &>/dev/null || : | |
| run_as_root archlinux-java fix &>/dev/null || : | |
| fi | |
| if [[ $EUID -eq 0 ]] && has archlinux-java; then | |
| archlinux-java fix &>/dev/null || : | |
| fi |
Code Review SummaryStatus: No New Issues Found | Recommendation: Merge The incremental diff from commit 62a7205 to HEAD contains only shfmt formatting fixes across 11 files (function declaration spacing, redirection alignment). No functional changes were made. The previous WARNING on Files Reviewed (10 files)
Previous Review Summaries (2 snapshots, latest commit 62a7205)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 62a7205)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Previous review (commit 03684f7)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (1 file)
Reviewed by laguna-m.1-20260312:free · Input: 2M · Output: 20.2K · Cached: 859.4K |
- Moved the archlinux-java fix command before detect_java in tools/server-start.sh. - Replaced direct sudo call with run_as_root. - Fixed shell formatting in multiple scripts to satisfy CI lint checks. - Removed accidental junk files. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
- Moved the archlinux-java fix command before detect_java in tools/server-start.sh. - Replaced direct sudo call with run_as_root. - Fixed shell formatting (shfmt) across all tools/ scripts and minecraft/config/versions.sh to satisfy CI lint checks. - Specific fixes include adding space in function definitions and removing spaces after redirection operators. Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves Java startup reliability on Arch Linux by running archlinux-java fix before JDK detection and standardizing privileged execution via run_as_root. It also applies consistent Bash function formatting across multiple tooling scripts.
Changes:
- Run
archlinux-java fixbeforedetect_javaintools/server-start.sh, usingrun_as_root. - Normalize function declarations and some redirection spacing across
tools/*.shandminecraft/config/versions.sh. - Minor consistency improvement opportunity in
tools/mod-updates.shby switching hardcodedsudotorun_as_root(flagged in review comments).
Reviewed changes
Copilot reviewed 1 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/world-optimize.sh | Function formatting/spacing normalization. |
| tools/watchdog.sh | Function formatting normalization. |
| tools/systemd-service.sh | Function formatting normalization. |
| tools/server-start.sh | Run archlinux-java fix (via run_as_root) before Java detection. |
| tools/prepare.sh | Function formatting normalization. |
| tools/monitor.sh | Function formatting normalization. |
| tools/mod-updates.sh | Function formatting normalization; still contains hardcoded sudo calls (commented). |
| tools/mc-client.sh | Function formatting normalization. |
| tools/logrotate.sh | Function formatting/spacing normalization. |
| tools/common.sh | Function formatting normalization; review flags rg dependency usage (commented). |
| tools/backup.sh | Function formatting normalization. |
| minecraft/config/versions.sh | Function formatting normalization. |
| send_command(){ | ||
| send_command() { | ||
| local cmd="$1" session_name="minecraft" | ||
| if command -v screen &>/dev/null && screen -list | rg -q "$session_name" &>/dev/null; then |
| [[ -d world ]] && sudo chown -R "$(id -un):$(id -gn)" world &>/dev/null || : | ||
| sudo chmod -R 755 ./*.sh &>/dev/null || : |
| check_server_port() { | ||
| local port="${1:-25565}" | ||
| if has nc; then | ||
| nc -z localhost "$port" &>/dev/null |
Improved the
archlinux-javafix implementation intools/server-start.sh. The fix command is now executed before JDK detection, ensuring thatdetect_javaworks with the corrected system state. Additionally, the code now uses therun_as_rootutility instead of a hardcodedsudocall, adhering to the project's coding standards.PR created automatically by Jules for task 3997987096099316359 started by @Ven0m0