Skip to content

修复(shell):在使用前验证 bash 在 Windows 上的可用性#228

Closed
Mcy0618 wants to merge 0 commit intoHKUDS:mainfrom
Mcy0618:main
Closed

修复(shell):在使用前验证 bash 在 Windows 上的可用性#228
Mcy0618 wants to merge 0 commit intoHKUDS:mainfrom
Mcy0618:main

Conversation

@Mcy0618
Copy link
Copy Markdown
Contributor

@Mcy0618 Mcy0618 commented May 4, 2026

Summary

WSL bash 跳过、Git Bash 优先、cmd兜底、bash-isms 适配、超时/异常处理、引号内误判防护。

Validation

验证通过

Notes

Copilot AI review requested due to automatic review settings May 4, 2026 02:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

这个 PR 旨在改进 Windows 上的 shell 选择逻辑,在实际执行前先验证 bash 是否可用,并在不可用时回退到 PowerShell 或 cmd.exe,以避免误选不可工作的 WSL bash.exe。它属于 openharness.utils.shell 这条公共命令执行路径上的兼容性修复,因此会影响工具执行、任务运行、hook、cron 等多个子系统的命令启动行为。

Changes:

  • shell.py 中新增 bash 可用性探测,并调整 Windows 下的 shell 选择优先级。
  • 为 PowerShell 增加简单的命令适配逻辑,尝试处理部分 bash 风格语法。
  • test_shell.py 中补充 Windows 回退路径、bash 探测逻辑和辅助函数的测试覆盖。

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
src/openharness/utils/shell.py 更新 Windows shell 解析逻辑,新增 bash 探测与 PowerShell 适配辅助函数。
tests/test_utils/test_shell.py 为新的 Windows 分支、适配函数和 bash 探测行为补充回归测试。

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/openharness/utils/shell.py Outdated
Comment on lines +30 to +33
powershell = shutil.which("pwsh") or shutil.which("powershell")
if powershell:
return [powershell, "-NoLogo", "-NoProfile", "-Command", command]
adapted = _adapt_for_powershell(command)
return [powershell, "-NoLogo", "-NoProfile", "-Command", adapted]
Comment on lines 27 to +28
bash = shutil.which("bash")
if bash:
if bash and _bash_is_usable(bash):
Comment thread src/openharness/utils/shell.py Outdated
Comment on lines +154 to +160
Simple commands (just calling an executable) pass through directly.
Commands with bash-isms (&&, ||, export, source) are wrapped in cmd /c
via PowerShell single-quoted string for maximum compatibility.
"""
if _contains_bash_isms(command):
escaped = command.replace("'", "''")
return f"cmd /d /s /c '{escaped}'"
Comment thread src/openharness/utils/shell.py Outdated
Comment on lines +168 to +179
bash_patterns = [
" && ",
"|| ",
" export ",
"\nexport ",
" source ",
"\nsource ",
]
if any(p in stripped for p in bash_patterns):
return True
# Also check for export/source at the very start of the command
if stripped.startswith("export ") or stripped.startswith("source "):
Comment thread tests/test_utils/test_shell.py Outdated

def test_bash_is_usable_returns_true_for_functional_bash(monkeypatch):
"""Functional bash returns True."""
class _FakeResult:
"""WSL bash without distro returns non-zero → False."""
class _FakeResult:
returncode = 1

@tjb-tech
Copy link
Copy Markdown
Collaborator

tjb-tech commented May 6, 2026

Thanks for the PR. I reviewed it against the latest main branch. I took the low-risk core fix and pushed it to main in commit d248975: on Windows, resolve_shell_command() now validates a discovered bash by actually running bash -lc "exit 0"; if that fails, OpenHarness skips it and falls back to PowerShell/cmd. This addresses the WSL bash.exe exists-but-unusable case.

I intentionally did not take the PowerShell command-adaptation part from the PR because it attempts to infer bash syntax and rewrite commands through cmd /c, which could change command semantics in subtle ways. I also skipped the unrelated/stale provider changes in the branch because provider config has moved on since then.

Covered by tests in tests/test_utils/test_shell.py; full local pytest and CI are green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants