-
Notifications
You must be signed in to change notification settings - Fork 226
fix(script): 修复了选择模式和清理时Ctrl+C导致的崩溃(顺便让代码短了点 #674
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
审阅者指南此 PR 强化了 Linux 启动脚本在选择/清理过程中的 Ctrl+C 相关崩溃防护,简化了多处控制流,并提升了在 Linux 上运行 LLBot/PMHQ 时的进程处理、发行版特定安装以及交互式输入处理的健壮性。 文件级变更
Tips and commands与 Sourcery 交互
自定义你的使用体验访问你的 控制面板 来:
获取帮助Original review guide in EnglishReviewer's GuideThis PR hardens the Linux start script against Ctrl+C-related crashes during selection/cleanup, simplifies several control flows, and improves robustness of process handling, distro-specific setup, and interactive input handling when running LLBot/PMHQ on Linux. 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 个问题,并给出了一些整体性的反馈:
- 现在 Debian 的架构检测会将所有非 x86_64 的机器默认视为 arm64(
local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")),这会把之前在不支持架构上显式报错的行为改成静默地选择错误的架构;建议保留一个显式的case分支或在未知架构上保留报错路径。 - 在
install_arch中,用于克隆并构建 yay 的回退路径在执行完makepkg -si后,不再删除/tmp/yay_install,这与之前的版本行为不同,并可能在 /tmp 中留下过期的构建产物;建议恢复清理步骤。
给 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- The Debian architecture detection now defaults any non-x86_64 machine to arm64 (`local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")`), which changes the previous explicit error on unsupported architectures into a silent wrong-arch selection; consider keeping an explicit case statement or error path for unknown architectures.
- In `install_arch`, the fallback path that clones and builds yay no longer removes `/tmp/yay_install` after `makepkg -si`, which changes behavior from the previous version and can leave stale build artifacts in /tmp; consider restoring the cleanup step.
## Individual Comments
### Comment 1
<location> `script/start-linux.sh:157` </location>
<code_context>
- aarch64) ARCH="arm64" ;;
- *) error "不支持的架构: $MACHINE" ;;
- esac
+ local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")
if [ ! -f "/opt/QQ/qq" ] && confirm "未检测到 QQ,是否安装?"; then
</code_context>
<issue_to_address>
**issue:** Architecture detection now treats all non-x86_64 as arm64, which can mis-handle unsupported architectures.
The previous version explicitly errored on unsupported architectures. Now, anything not `x86_64` is treated as `arm64`, which will be wrong on e.g. `armv7l` or `riscv64` and will attempt to install an invalid package instead of failing fast. Consider reintroducing explicit validation, for example:
```bash
case "$MACHINE" in
x86_64) ARCH="amd64" ;;
aarch64) ARCH="arm64" ;;
*) error "不支持的架构: $MACHINE" ;;
esac
```
or keep the one-liner but guard it with a `case`/`if` that errors on unknown architectures.
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The Debian architecture detection now defaults any non-x86_64 machine to arm64 (
local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")), which changes the previous explicit error on unsupported architectures into a silent wrong-arch selection; consider keeping an explicit case statement or error path for unknown architectures. - In
install_arch, the fallback path that clones and builds yay no longer removes/tmp/yay_installaftermakepkg -si, which changes behavior from the previous version and can leave stale build artifacts in /tmp; consider restoring the cleanup step.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Debian architecture detection now defaults any non-x86_64 machine to arm64 (`local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")`), which changes the previous explicit error on unsupported architectures into a silent wrong-arch selection; consider keeping an explicit case statement or error path for unknown architectures.
- In `install_arch`, the fallback path that clones and builds yay no longer removes `/tmp/yay_install` after `makepkg -si`, which changes behavior from the previous version and can leave stale build artifacts in /tmp; consider restoring the cleanup step.
## Individual Comments
### Comment 1
<location> `script/start-linux.sh:157` </location>
<code_context>
- aarch64) ARCH="arm64" ;;
- *) error "不支持的架构: $MACHINE" ;;
- esac
+ local ARCH=$([ "$MACHINE" == "x86_64" ] && echo "amd64" || echo "arm64")
if [ ! -f "/opt/QQ/qq" ] && confirm "未检测到 QQ,是否安装?"; then
</code_context>
<issue_to_address>
**issue:** Architecture detection now treats all non-x86_64 as arm64, which can mis-handle unsupported architectures.
The previous version explicitly errored on unsupported architectures. Now, anything not `x86_64` is treated as `arm64`, which will be wrong on e.g. `armv7l` or `riscv64` and will attempt to install an invalid package instead of failing fast. Consider reintroducing explicit validation, for example:
```bash
case "$MACHINE" in
x86_64) ARCH="amd64" ;;
aarch64) ARCH="arm64" ;;
*) error "不支持的架构: $MACHINE" ;;
esac
```
or keep the one-liner but guard it with a `case`/`if` that errors on unknown architectures.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
fix issue #673 |
由 Sourcery 提供的摘要
改进 Linux 启动脚本,以更安全地处理 Ctrl+C,简化交互式提示,并精简 QQ/PMHQ/LLBot 的环境和依赖设置。
错误修复(Bug Fixes):
增强内容(Enhancements):
Original summary in English
Summary by Sourcery
Improve the Linux startup script to handle Ctrl+C more safely, simplify interactive prompts, and streamline environment and dependency setup for QQ/PMHQ/LLBot.
Bug Fixes:
Enhancements: