fix: handle pip install execution in frozen runtime#4985
Merged
Soulter merged 2 commits intoAstrBotDevs:masterfrom Feb 9, 2026
Merged
fix: handle pip install execution in frozen runtime#4985Soulter merged 2 commits intoAstrBotDevs:masterfrom
Soulter merged 2 commits intoAstrBotDevs:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并留下了一些总体反馈:
- 在
_get_pip_subprocess_executable中,依赖'python' in Path(candidate).name.lower()这种方式,可能会在 frozen/重命名的环境中遗漏有效的解释器;在返回None之前,可以考虑使用更健壮的检查方式(例如检查文件是否存在且可执行,或者回退到shutil.which('python'))。 - 当
_get_pip_subprocess_executable返回None时,当前会静默地回退到进程内模式;在这一分支里增加一条调试日志,会有助于理解为什么在某些环境中没有使用子进程模式。
供 AI Agent 使用的提示
Please address the comments from this code review:
## Overall Comments
- In `_get_pip_subprocess_executable`, relying on `'python' in Path(candidate).name.lower()` may miss valid interpreters in frozen/renamed environments; consider a more robust check (e.g., checking file existence and executability or falling back to `shutil.which('python')`) before returning `None`.
- When `_get_pip_subprocess_executable` returns `None`, it silently falls back to in-process mode; adding a debug log in that branch would make it easier to understand why subprocess mode is not used in certain environments.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:49-50` </location>
<code_context>
+
+
def _get_pip_main():
try:
from pip._internal.cli.main import main as pip_main
</code_context>
<issue_to_address>
**suggestion:** Catching only FileNotFoundError may miss other subprocess launch failures like PermissionError.
If the chosen `subprocess_executable` exists but isn’t actually runnable (wrong permissions, it’s a directory, other OS issues), `asyncio.create_subprocess_exec` will raise `PermissionError`/`OSError` and bypass the in‑process fallback. Consider catching `OSError` (or `Exception` if you prefer) and then falling back to `_run_pip_in_process`, optionally logging the exception for debugging.
Suggested implementation:
```python
except OSError as exc:
logger.warning(
"Failed to launch pip subprocess (%r). Falling back to in-process pip: %s",
subprocess_executable,
exc,
)
```
If the existing `except` block around `asyncio.create_subprocess_exec` contains different logging or additional logic, you should:
1. Replace the entire `except FileNotFoundError:` block (including its body) with the `except OSError as exc:` block above.
2. Ensure that the logic which falls back to `_run_pip_in_process` is still triggered when the subprocess launch fails (typically by leaving `result_code` as `None` so the later fallback code executes).
3. If `logger` is not available in this scope or uses a different name, adjust the logging call accordingly.
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进之后的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
_get_pip_subprocess_executable, relying on'python' in Path(candidate).name.lower()may miss valid interpreters in frozen/renamed environments; consider a more robust check (e.g., checking file existence and executability or falling back toshutil.which('python')) before returningNone. - When
_get_pip_subprocess_executablereturnsNone, it silently falls back to in-process mode; adding a debug log in that branch would make it easier to understand why subprocess mode is not used in certain environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_get_pip_subprocess_executable`, relying on `'python' in Path(candidate).name.lower()` may miss valid interpreters in frozen/renamed environments; consider a more robust check (e.g., checking file existence and executability or falling back to `shutil.which('python')`) before returning `None`.
- When `_get_pip_subprocess_executable` returns `None`, it silently falls back to in-process mode; adding a debug log in that branch would make it easier to understand why subprocess mode is not used in certain environments.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:49-50` </location>
<code_context>
+
+
def _get_pip_main():
try:
from pip._internal.cli.main import main as pip_main
</code_context>
<issue_to_address>
**suggestion:** Catching only FileNotFoundError may miss other subprocess launch failures like PermissionError.
If the chosen `subprocess_executable` exists but isn’t actually runnable (wrong permissions, it’s a directory, other OS issues), `asyncio.create_subprocess_exec` will raise `PermissionError`/`OSError` and bypass the in‑process fallback. Consider catching `OSError` (or `Exception` if you prefer) and then falling back to `_run_pip_in_process`, optionally logging the exception for debugging.
Suggested implementation:
```python
except OSError as exc:
logger.warning(
"Failed to launch pip subprocess (%r). Falling back to in-process pip: %s",
subprocess_executable,
exc,
)
```
If the existing `except` block around `asyncio.create_subprocess_exec` contains different logging or additional logic, you should:
1. Replace the entire `except FileNotFoundError:` block (including its body) with the `except OSError as exc:` block above.
2. Ensure that the logic which falls back to `_run_pip_in_process` is still triggered when the subprocess launch fails (typically by leaving `result_code` as `None` so the later fallback code executes).
3. If `logger` is not available in this scope or uses a different name, adjust the logging call accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Collaborator
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我在这里给了一些整体性的反馈:
- 考虑对
_get_pip_subprocess_executable()的返回结果做缓存(例如通过functools.lru_cache),这样在每次执行安装调用时就不会反复解析并检查同一批候选可执行文件。 - 既然子进程的执行现在不再依赖
_is_frozen_runtime(),请再确认一下_is_frozen_runtime()在这个模块的其他地方是否仍然被使用;如果没有使用,就可以移除以避免死代码。
提供给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- Consider memoizing the result of `_get_pip_subprocess_executable()` (e.g., via `functools.lru_cache`) so you don't repeatedly resolve and check the same candidate executables on every install call.
- Now that subprocess execution no longer depends on `_is_frozen_runtime()`, double-check whether `_is_frozen_runtime()` is still used elsewhere in this module; if not, it can be removed to avoid dead code.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- Consider memoizing the result of
_get_pip_subprocess_executable()(e.g., viafunctools.lru_cache) so you don't repeatedly resolve and check the same candidate executables on every install call. - Now that subprocess execution no longer depends on
_is_frozen_runtime(), double-check whether_is_frozen_runtime()is still used elsewhere in this module; if not, it can be removed to avoid dead code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider memoizing the result of `_get_pip_subprocess_executable()` (e.g., via `functools.lru_cache`) so you don't repeatedly resolve and check the same candidate executables on every install call.
- Now that subprocess execution no longer depends on `_is_frozen_runtime()`, double-check whether `_is_frozen_runtime()` is still used elsewhere in this module; if not, it can be removed to avoid dead code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Soulter
approved these changes
Feb 9, 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
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.
Modifications / 改动点
修复桌面客户端,安装python包失败的问题
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
通过优先使用检测到的 Python 可执行文件以子进程方式调用 pip,并在必要时回退到进程内 pip 调用的方式,提高基于 pip 的软件包安装的可靠性。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Improve reliability of pip-based package installation by preferring a subprocess invocation with a detected Python executable and falling back to in-process pip when necessary.
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
通过优先使用检测到的 Python 可执行文件以子进程方式调用 pip,并在必要时回退到进程内 pip 调用的方式,提高基于 pip 的软件包安装的可靠性。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Improve reliability of pip-based package installation by preferring a subprocess invocation with a detected Python executable and falling back to in-process pip when necessary.
Bug Fixes:
Enhancements: