fix(cli): create .bat wrapper on Windows instead of POSIX shell script
On Windows, hermes profile create produced a #!/bin/sh script that the shell cannot execute. Now creates a .bat file with @echo off + %* on Windows, and keeps the POSIX shell script on macOS/Linux. Also fixes check_alias_collision to use 'where' instead of 'which' on Windows, and remove_wrapper_script to find .bat files. Fixes #34708
This commit is contained in:
liuhao1024
@ -329,16 +329,19 @@ def check_alias_collision(name: str) -> Optional[str]:
|
||||
|
||||
# Check existing commands in PATH
|
||||
wrapper_dir = _get_wrapper_dir()
|
||||
is_windows = sys.platform == "win32"
|
||||
try:
|
||||
result = subprocess.run(
|
||||
["which", canon], capture_output=True, text=True, timeout=5,
|
||||
["where" if is_windows else "which", canon],
|
||||
capture_output=True, text=True, timeout=5,
|
||||
)
|
||||
if result.returncode == 0:
|
||||
existing_path = result.stdout.strip()
|
||||
existing_path = result.stdout.strip().splitlines()[0]
|
||||
# Allow overwriting our own wrappers
|
||||
if existing_path == str(wrapper_dir / canon):
|
||||
expected = wrapper_dir / (f"{canon}.bat" if is_windows else canon)
|
||||
if existing_path == str(expected):
|
||||
try:
|
||||
content = (wrapper_dir / canon).read_text()
|
||||
content = expected.read_text()
|
||||
if "hermes -p" in content:
|
||||
return None # it's our wrapper, safe to overwrite
|
||||
except Exception:
|
||||
@ -359,6 +362,7 @@ def _is_wrapper_dir_in_path() -> bool:
|
||||
def create_wrapper_script(name: str) -> Optional[Path]:
|
||||
"""Create a shell wrapper script at ~/.local/bin/<name>.
|
||||
|
||||
On Windows, creates a ``.bat`` file instead of a POSIX shell script.
|
||||
Returns the path to the created wrapper, or None if creation failed.
|
||||
"""
|
||||
canon = normalize_profile_name(name)
|
||||
@ -369,28 +373,47 @@ def create_wrapper_script(name: str) -> Optional[Path]:
|
||||
print(f"⚠ Could not create {wrapper_dir}: {e}")
|
||||
return None
|
||||
|
||||
wrapper_path = wrapper_dir / canon
|
||||
try:
|
||||
wrapper_path.write_text(f'#!/bin/sh\nexec hermes -p {canon} "$@"\n')
|
||||
wrapper_path.chmod(wrapper_path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
|
||||
return wrapper_path
|
||||
except OSError as e:
|
||||
print(f"⚠ Could not create wrapper at {wrapper_path}: {e}")
|
||||
return None
|
||||
is_windows = sys.platform == "win32"
|
||||
if is_windows:
|
||||
wrapper_path = wrapper_dir / f"{canon}.bat"
|
||||
try:
|
||||
wrapper_path.write_text(f"@echo off\r\nhermes -p {canon} %*\r\n")
|
||||
return wrapper_path
|
||||
except OSError as e:
|
||||
print(f"⚠ Could not create wrapper at {wrapper_path}: {e}")
|
||||
return None
|
||||
else:
|
||||
wrapper_path = wrapper_dir / canon
|
||||
try:
|
||||
wrapper_path.write_text(f'#!/bin/sh\nexec hermes -p {canon} "$@"\n')
|
||||
wrapper_path.chmod(wrapper_path.stat().st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH)
|
||||
return wrapper_path
|
||||
except OSError as e:
|
||||
print(f"⚠ Could not create wrapper at {wrapper_path}: {e}")
|
||||
return None
|
||||
|
||||
|
||||
def remove_wrapper_script(name: str) -> bool:
|
||||
"""Remove the wrapper script for a profile. Returns True if removed."""
|
||||
wrapper_path = _get_wrapper_dir() / normalize_profile_name(name)
|
||||
if wrapper_path.exists():
|
||||
try:
|
||||
# Verify it's our wrapper before removing
|
||||
content = wrapper_path.read_text()
|
||||
if "hermes -p" in content:
|
||||
wrapper_path.unlink()
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
wrapper_dir = _get_wrapper_dir()
|
||||
canon = normalize_profile_name(name)
|
||||
is_windows = sys.platform == "win32"
|
||||
|
||||
# Check both the extensionless path (POSIX) and .bat (Windows)
|
||||
candidates = [wrapper_dir / canon]
|
||||
if is_windows:
|
||||
candidates.insert(0, wrapper_dir / f"{canon}.bat")
|
||||
|
||||
for wrapper_path in candidates:
|
||||
if wrapper_path.exists():
|
||||
try:
|
||||
# Verify it's our wrapper before removing
|
||||
content = wrapper_path.read_text()
|
||||
if "hermes -p" in content:
|
||||
wrapper_path.unlink()
|
||||
return True
|
||||
except Exception:
|
||||
pass
|
||||
return False
|
||||
|
||||
|
||||
|
||||
@ -600,6 +600,88 @@ class TestAliasCollision:
|
||||
assert result is not None
|
||||
assert "reserved" in result.lower()
|
||||
|
||||
def test_uses_where_on_windows(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "win32")
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(returncode=1, stdout="")
|
||||
check_alias_collision("mybot")
|
||||
call_args = mock_run.call_args[0][0]
|
||||
assert call_args[0] == "where"
|
||||
|
||||
def test_uses_which_on_posix(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "darwin")
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(returncode=1, stdout="")
|
||||
check_alias_collision("mybot")
|
||||
call_args = mock_run.call_args[0][0]
|
||||
assert call_args[0] == "which"
|
||||
|
||||
def test_windows_checks_bat_extension(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "win32")
|
||||
wrapper_dir = profile_env / ".local" / "bin"
|
||||
wrapper_dir.mkdir(parents=True, exist_ok=True)
|
||||
bat_path = wrapper_dir / "mybot.bat"
|
||||
bat_path.write_text("@echo off\r\nhermes -p mybot %*\r\n")
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(
|
||||
returncode=0, stdout=str(bat_path),
|
||||
)
|
||||
result = check_alias_collision("mybot")
|
||||
assert result is None # our own wrapper, safe to overwrite
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# TestWrapperScript
|
||||
# ===================================================================
|
||||
|
||||
class TestWrapperScript:
|
||||
"""Tests for create_wrapper_script() and remove_wrapper_script()."""
|
||||
|
||||
def test_creates_sh_on_posix(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "darwin")
|
||||
from hermes_cli.profiles import create_wrapper_script
|
||||
wrapper = create_wrapper_script("mybot")
|
||||
assert wrapper is not None
|
||||
assert wrapper.name == "mybot"
|
||||
content = wrapper.read_text()
|
||||
assert content.startswith("#!/bin/sh")
|
||||
assert "hermes -p mybot" in content
|
||||
|
||||
def test_creates_bat_on_windows(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "win32")
|
||||
from hermes_cli.profiles import create_wrapper_script
|
||||
wrapper = create_wrapper_script("mybot")
|
||||
assert wrapper is not None
|
||||
assert wrapper.name == "mybot.bat"
|
||||
content = wrapper.read_text()
|
||||
assert "@echo off" in content
|
||||
assert "hermes -p mybot" in content
|
||||
assert "%*" in content
|
||||
|
||||
def test_remove_finds_bat_on_windows(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "win32")
|
||||
from hermes_cli.profiles import create_wrapper_script, remove_wrapper_script
|
||||
wrapper = create_wrapper_script("mybot")
|
||||
assert wrapper is not None
|
||||
assert wrapper.exists()
|
||||
removed = remove_wrapper_script("mybot")
|
||||
assert removed is True
|
||||
assert not wrapper.exists()
|
||||
|
||||
def test_remove_finds_sh_on_posix(self, profile_env, monkeypatch):
|
||||
monkeypatch.setattr("sys.platform", "darwin")
|
||||
from hermes_cli.profiles import create_wrapper_script, remove_wrapper_script
|
||||
wrapper = create_wrapper_script("mybot")
|
||||
assert wrapper is not None
|
||||
assert wrapper.exists()
|
||||
removed = remove_wrapper_script("mybot")
|
||||
assert removed is True
|
||||
assert not wrapper.exists()
|
||||
|
||||
def test_remove_returns_false_when_absent(self, profile_env):
|
||||
from hermes_cli.profiles import remove_wrapper_script
|
||||
assert remove_wrapper_script("nonexistent") is False
|
||||
|
||||
|
||||
# ===================================================================
|
||||
# TestRenameProfile
|
||||
|
||||
Reference in New Issue
Block a user