-
Notifications
You must be signed in to change notification settings - Fork 226
feat(script): 优化了脚本对各种Ctrl+C退出的处理能力 #670
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
审阅者指南重构 Linux 启动脚本,以添加健壮的基于信号的清理机制(可选保留 QQ 相关进程)、显式的 sudo 校验、更健壮的依赖/QQ 安装流程,同时改进模式选择的交互体验,以及对显示/X11 的处理。 新的信号驱动清理流程与 QQ 保留流程的时序图sequenceDiagram
actor User
participant ShellScript
participant PMHQProcess
participant QQProcess
participant XServer
User->>ShellScript: run start-linux.sh
ShellScript->>ShellScript: trap cleanup on SIGINT SIGTERM EXIT
ShellScript->>PMHQProcess: start pmhq with sub_cmd (&)
ShellScript->>ShellScript: store PMHQ_PID
ShellScript->>ShellScript: wait PMHQ_PID
User-->>ShellScript: press Ctrl+C (SIGINT)
ShellScript->>ShellScript: invoke cleanup
alt cleanup not already running
ShellScript->>ShellScript: set CLEANING=1
ShellScript->>PMHQProcess: SIGSTOP process group (-PMHQ_PID)
ShellScript->>User: prompt on /dev/tty
User-->>ShellScript: choice Y/n or timeout
alt user keeps QQ (choice n/N)
ShellScript->>PMHQProcess: SIGCONT process group
ShellScript->>User: log QQ and related processes detached
ShellScript->>ShellScript: exit 0
else user closes QQ (default or Y)
ShellScript->>PMHQProcess: SIGCONT process group
ShellScript->>PMHQProcess: SIGTERM process group
ShellScript->>QQProcess: pkill -15 QQ binaries
loop wait for clean exit
ShellScript->>PMHQProcess: poll with kill -0
ShellScript->>QQProcess: pgrep QQ binaries
end
alt processes still remain
ShellScript->>PMHQProcess: SIGKILL process group
ShellScript->>QQProcess: pkill -9 QQ binaries
end
alt non-arch and not using xvfb
ShellScript->>XServer: xhost -local:username
end
ShellScript->>User: log cleanup complete
ShellScript->>ShellScript: exit 0
end
else cleanup already running
ShellScript->>ShellScript: return immediately
end
start-linux.sh 中针对发行版安装和 sudo 校验的新流程图flowchart TD
Start[Start script] --> DetectDistro[Detect distro via pacman/apt]
DetectDistro -->|pacman| SetArch[Set DISTRO=arch]
DetectDistro -->|apt| SetDebian[Set DISTRO=debian]
DetectDistro -->|other| ErrUnsupported[error 当前只支持 apt 或 pacman 包管理器]
SetArch --> InstallArch
SetDebian --> InstallDebian
subgraph ArchInstall[install_arch]
InstallArch --> CheckSudoArch[check_sudo sudo -v]
CheckSudoArch -->|fail| ErrSudoArch[error Sudo 验证失败]
CheckSudoArch -->|ok| PacmanDeps[pacman install base deps]
PacmanDeps --> QQCheckArch{QQ binary exists?}
QQCheckArch -->|yes| ArchDone[return]
QQCheckArch -->|no| AskAUR[confirm install QQ via AUR]
AskAUR -->|no| ArchDone
AskAUR -->|yes| YayCheck{yay installed?}
YayCheck -->|yes| YayInstallQQ[yay -S qq-nt etc]
YayInstallQQ --> ArchDone
YayCheck -->|no| TryPacmanYay[pacman -S yay]
TryPacmanYay -->|success| YayInstallQQ
TryPacmanYay -->|fail| BuildYay[git clone yay; makepkg]
BuildYay -->|fail| ErrYayBuild[error yay 编译失败]
BuildYay -->|success| YayInstallQQ
end
subgraph DebianInstall[install_debian]
InstallDebian --> CheckSudoDebian[check_sudo sudo -v]
CheckSudoDebian -->|fail| ErrSudoDebian[error Sudo 验证失败]
CheckSudoDebian -->|ok| QQCheckDeb{QQ binary exists?}
QQCheckDeb -->|yes| CheckTools[check ffmpeg/xvfb]
QQCheckDeb -->|no| AskQQDeb[confirm install QQ]
AskQQDeb -->|no| CheckTools
AskQQDeb -->|yes| AptUpdate[apt-get update; install wget]
AptUpdate -->|fail| ErrAptUpdate[error apt update 或 wget 安装失败]
AptUpdate -->|ok| DownloadDeb[wget QQ deb]
DownloadDeb -->|fail| ErrQQDownload[error QQ 安装包下载失败]
DownloadDeb -->|ok| DetectLib[choose libasound2 or libasound2t64]
DetectLib --> InstallQQDeb[apt install QQ deb and deps]
InstallQQDeb -->|fail| ErrQQDeps[error QQ 依赖安装失败]
InstallQQDeb -->|ok| RemoveDeb[rm deb]
RemoveDeb --> CheckTools
CheckTools --> MissingPkgs{any ffmpeg/xvfb missing?}
MissingPkgs -->|no| DebianDone[return]
MissingPkgs -->|yes| AptInstallTools[apt-get install missing]
AptInstallTools -->|fail| ErrTools[error 工具安装失败]
AptInstallTools -->|ok| DebianDone
end
ArchDone --> PostInstall
DebianDone --> PostInstall
PostInstall[chmod and chown llbot; continue to mode selection] --> End[End install phase]
文件级变更
使用技巧和常用命令与 Sourcery 交互
自定义你的使用体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors the Linux startup script to add robust signal-driven cleanup with optional QQ process preservation, explicit sudo validation, and more resilient dependency/QQ installation flows, while improving mode selection UX and display/X11 handling. Sequence diagram for the new signal-driven cleanup and QQ preservation flowsequenceDiagram
actor User
participant ShellScript
participant PMHQProcess
participant QQProcess
participant XServer
User->>ShellScript: run start-linux.sh
ShellScript->>ShellScript: trap cleanup on SIGINT SIGTERM EXIT
ShellScript->>PMHQProcess: start pmhq with sub_cmd (&)
ShellScript->>ShellScript: store PMHQ_PID
ShellScript->>ShellScript: wait PMHQ_PID
User-->>ShellScript: press Ctrl+C (SIGINT)
ShellScript->>ShellScript: invoke cleanup
alt cleanup not already running
ShellScript->>ShellScript: set CLEANING=1
ShellScript->>PMHQProcess: SIGSTOP process group (-PMHQ_PID)
ShellScript->>User: prompt on /dev/tty
User-->>ShellScript: choice Y/n or timeout
alt user keeps QQ (choice n/N)
ShellScript->>PMHQProcess: SIGCONT process group
ShellScript->>User: log QQ and related processes detached
ShellScript->>ShellScript: exit 0
else user closes QQ (default or Y)
ShellScript->>PMHQProcess: SIGCONT process group
ShellScript->>PMHQProcess: SIGTERM process group
ShellScript->>QQProcess: pkill -15 QQ binaries
loop wait for clean exit
ShellScript->>PMHQProcess: poll with kill -0
ShellScript->>QQProcess: pgrep QQ binaries
end
alt processes still remain
ShellScript->>PMHQProcess: SIGKILL process group
ShellScript->>QQProcess: pkill -9 QQ binaries
end
alt non-arch and not using xvfb
ShellScript->>XServer: xhost -local:username
end
ShellScript->>User: log cleanup complete
ShellScript->>ShellScript: exit 0
end
else cleanup already running
ShellScript->>ShellScript: return immediately
end
Flow diagram for distro-specific install and sudo validation in start-linux.shflowchart TD
Start[Start script] --> DetectDistro[Detect distro via pacman/apt]
DetectDistro -->|pacman| SetArch[Set DISTRO=arch]
DetectDistro -->|apt| SetDebian[Set DISTRO=debian]
DetectDistro -->|other| ErrUnsupported[error 当前只支持 apt 或 pacman 包管理器]
SetArch --> InstallArch
SetDebian --> InstallDebian
subgraph ArchInstall[install_arch]
InstallArch --> CheckSudoArch[check_sudo sudo -v]
CheckSudoArch -->|fail| ErrSudoArch[error Sudo 验证失败]
CheckSudoArch -->|ok| PacmanDeps[pacman install base deps]
PacmanDeps --> QQCheckArch{QQ binary exists?}
QQCheckArch -->|yes| ArchDone[return]
QQCheckArch -->|no| AskAUR[confirm install QQ via AUR]
AskAUR -->|no| ArchDone
AskAUR -->|yes| YayCheck{yay installed?}
YayCheck -->|yes| YayInstallQQ[yay -S qq-nt etc]
YayInstallQQ --> ArchDone
YayCheck -->|no| TryPacmanYay[pacman -S yay]
TryPacmanYay -->|success| YayInstallQQ
TryPacmanYay -->|fail| BuildYay[git clone yay; makepkg]
BuildYay -->|fail| ErrYayBuild[error yay 编译失败]
BuildYay -->|success| YayInstallQQ
end
subgraph DebianInstall[install_debian]
InstallDebian --> CheckSudoDebian[check_sudo sudo -v]
CheckSudoDebian -->|fail| ErrSudoDebian[error Sudo 验证失败]
CheckSudoDebian -->|ok| QQCheckDeb{QQ binary exists?}
QQCheckDeb -->|yes| CheckTools[check ffmpeg/xvfb]
QQCheckDeb -->|no| AskQQDeb[confirm install QQ]
AskQQDeb -->|no| CheckTools
AskQQDeb -->|yes| AptUpdate[apt-get update; install wget]
AptUpdate -->|fail| ErrAptUpdate[error apt update 或 wget 安装失败]
AptUpdate -->|ok| DownloadDeb[wget QQ deb]
DownloadDeb -->|fail| ErrQQDownload[error QQ 安装包下载失败]
DownloadDeb -->|ok| DetectLib[choose libasound2 or libasound2t64]
DetectLib --> InstallQQDeb[apt install QQ deb and deps]
InstallQQDeb -->|fail| ErrQQDeps[error QQ 依赖安装失败]
InstallQQDeb -->|ok| RemoveDeb[rm deb]
RemoveDeb --> CheckTools
CheckTools --> MissingPkgs{any ffmpeg/xvfb missing?}
MissingPkgs -->|no| DebianDone[return]
MissingPkgs -->|yes| AptInstallTools[apt-get install missing]
AptInstallTools -->|fail| ErrTools[error 工具安装失败]
AptInstallTools -->|ok| DebianDone
end
ArchDone --> PostInstall
DebianDone --> PostInstall
PostInstall[chmod and chown llbot; continue to mode selection] --> End[End install phase]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - 我发现了 1 个问题,并给出了一些高层次的反馈:
- 使用
trap cleanup ... EXIT时,交互式的 QQ 关闭提示会在脚本正常结束时也出现(而不仅仅是 Ctrl+C/SIGTERM);如果你只想处理异常退出,建议从 trap 中去掉EXIT,或者在cleanup中根据信号/来源进行分支处理。 - 在
install_debian中,command -v xvfb会一直失败,因为实际的可执行文件是Xvfb/xvfb-run,所以脚本会反复误以为缺少 xvfb;建议改为检查xvfb-run或Xvfb。 cleanup中的read ... < /dev/tty在非交互式或重定向场景(如 cron/systemd)下可能会失败或意外阻塞;你可能需要先用 TTY 检查([ -t 0 ]),在没有终端时跳过交互式提示。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `trap cleanup ... EXIT` means the interactive QQ shutdown prompt will also appear on normal script completion (not just Ctrl+C/SIGTERM); if you only want to handle abrupt exits, consider dropping `EXIT` from the trap or branching in `cleanup` based on the signal/source.
- In `install_debian`, `command -v xvfb` will always fail because the binary is `Xvfb`/`xvfb-run`, so the script will repeatedly think xvfb is missing; consider checking for `xvfb-run` or `Xvfb` instead.
- The `read ... < /dev/tty` in `cleanup` can fail or block unexpectedly in non-interactive or redirected environments (e.g. cron/systemd); you may want to guard this with a TTY check (`[ -t 0 ]`) and skip the interactive prompt when no terminal is available.
## Individual Comments
### Comment 1
<location> `script/start-linux.sh:97` </location>
<code_context>
+ exit 0
+}
+
+trap cleanup SIGINT SIGTERM EXIT
+
confirm() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using EXIT in the trap makes cleanup run (and prompt) even on normal termination and masks the original exit status.
This means the interactive prompt in `cleanup` can run even on clean or non-interactive exits, and the final `exit 0` will override any earlier non-zero status, hiding failures.
Consider either:
- Moving non-interactive cleanup (e.g. `xhost -local`, process cleanup) into a separate EXIT handler that preserves `$?`, and
- Restricting the interactive "是否关闭 QQ" prompt to SIGINT/SIGTERM only, or
- Removing EXIT from the trap if you don’t want interactive behavior on normal exit.
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据这些反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Using
trap cleanup ... EXITmeans the interactive QQ shutdown prompt will also appear on normal script completion (not just Ctrl+C/SIGTERM); if you only want to handle abrupt exits, consider droppingEXITfrom the trap or branching incleanupbased on the signal/source. - In
install_debian,command -v xvfbwill always fail because the binary isXvfb/xvfb-run, so the script will repeatedly think xvfb is missing; consider checking forxvfb-runorXvfbinstead. - The
read ... < /dev/ttyincleanupcan fail or block unexpectedly in non-interactive or redirected environments (e.g. cron/systemd); you may want to guard this with a TTY check ([ -t 0 ]) and skip the interactive prompt when no terminal is available.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `trap cleanup ... EXIT` means the interactive QQ shutdown prompt will also appear on normal script completion (not just Ctrl+C/SIGTERM); if you only want to handle abrupt exits, consider dropping `EXIT` from the trap or branching in `cleanup` based on the signal/source.
- In `install_debian`, `command -v xvfb` will always fail because the binary is `Xvfb`/`xvfb-run`, so the script will repeatedly think xvfb is missing; consider checking for `xvfb-run` or `Xvfb` instead.
- The `read ... < /dev/tty` in `cleanup` can fail or block unexpectedly in non-interactive or redirected environments (e.g. cron/systemd); you may want to guard this with a TTY check (`[ -t 0 ]`) and skip the interactive prompt when no terminal is available.
## Individual Comments
### Comment 1
<location> `script/start-linux.sh:97` </location>
<code_context>
+ exit 0
+}
+
+trap cleanup SIGINT SIGTERM EXIT
+
confirm() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Using EXIT in the trap makes cleanup run (and prompt) even on normal termination and masks the original exit status.
This means the interactive prompt in `cleanup` can run even on clean or non-interactive exits, and the final `exit 0` will override any earlier non-zero status, hiding failures.
Consider either:
- Moving non-interactive cleanup (e.g. `xhost -local`, process cleanup) into a separate EXIT handler that preserves `$?`, and
- Restricting the interactive "是否关闭 QQ" prompt to SIGINT/SIGTERM only, or
- Removing EXIT from the trap if you don’t want interactive behavior on normal exit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
优化进程退出逻辑,支持在退出时交互式选择是否保留 QQ 进程(有个BUG解决的很粗暴)
增强错误处理,增加 sudo 权限校验
Summary by Sourcery
改进 Linux 启动脚本在进程生命周期管理和交互式关闭行为方面的健壮性。
新功能:
增强:
Original summary in English
Summary by Sourcery
Improve the Linux startup script’s robustness around process lifecycle management and interactive shutdown behavior.
New Features:
Enhancements: