Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions tests/tools/test_file_ops_native_tool_arg.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
"""Tests for ``ShellFileOperations._escape_native_tool_arg`` and the
ripgrep command construction on Windows.

Background
----------
On Windows, ``_escape_shell_arg`` rewrites drive paths to the Git Bash
``/c/...`` form — required for MSYS tools (grep, find, test, bash
builtins). Native binaries are different: Hermes runs bash with
``MSYS_NO_PATHCONV=1``/``MSYS2_ARG_CONV_EXCL=*`` (see
``tools.environments.local._apply_windows_msys_bash_env_defaults``), so
argv reaches them verbatim, and a native ripgrep build (e.g. the WinGet
MSVC package) cannot resolve ``/c/...``:

rg: /c/Users/x: IO error for operation on /c/Users/x:
The system cannot find the path specified. (os error 3)

Every explicit-path content/file search failed in that configuration;
only the default relative root (``.``) worked. The fix feeds native
tools the ``C:/...`` form via ``_escape_native_tool_arg``.

These tests fake Windows on POSIX CI by patching ``_IS_WINDOWS``.
"""

from unittest.mock import patch

from tools.environments import local as local_mod
from tools.file_operations import ShellFileOperations


def _make_ops() -> ShellFileOperations:
"""Bare instance for command-building tests (no terminal_env needed)."""
return ShellFileOperations.__new__(ShellFileOperations)


class TestEscapeNativeToolArgWindows:
def test_msys_path_converted(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
assert _make_ops()._escape_native_tool_arg("/e/HermesWork/proj") == "'E:/HermesWork/proj'"

def test_backslash_native_converted(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
assert _make_ops()._escape_native_tool_arg(r"E:\HermesWork\proj") == "'E:/HermesWork/proj'"

def test_forward_slash_native_idempotent(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
assert _make_ops()._escape_native_tool_arg("E:/HermesWork/proj") == "'E:/HermesWork/proj'"

def test_relative_and_plain_posix_untouched(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
ops = _make_ops()
assert ops._escape_native_tool_arg(".") == "'.'"
assert ops._escape_native_tool_arg("/home/user/proj") == "'/home/user/proj'"

def test_single_quote_escaped(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
assert _make_ops()._escape_native_tool_arg("E:/it's/x") == "'E:/it'\"'\"'s/x'"

def test_empty_passthrough(self):
with patch.object(local_mod, "_IS_WINDOWS", True):
assert _make_ops()._escape_native_tool_arg("") == ""


class TestEscapeNativeToolArgNonWindows:
def test_noop_beyond_quoting(self):
with patch.object(local_mod, "_IS_WINDOWS", False):
ops = _make_ops()
assert ops._escape_native_tool_arg("/e/HermesWork/proj") == "'/e/HermesWork/proj'"
assert ops._escape_native_tool_arg(r"E:\HermesWork\proj") == r"'E:\HermesWork\proj'"


class _ExecResult:
exit_code = 0
stdout = ""
stderr = ""


class TestRipgrepGetsNativePath:
"""The path argument handed to rg must be in native form on Windows."""

def _capture(self, method, *args):
ops = _make_ops()
ops._has_command = lambda _cmd: True
captured = []
ops._exec = lambda cmd, timeout=None: captured.append(cmd) or _ExecResult()
with patch.object(local_mod, "_IS_WINDOWS", True):
method(ops, *args)
return captured

def test_search_with_rg(self):
captured = self._capture(
ShellFileOperations._search_with_rg,
"pattern", r"E:\HermesWork\proj", None, 50, 0, "files_only", 0,
)
assert captured, "rg was never invoked"
assert "'E:/HermesWork/proj'" in captured[0]

def test_search_files_rg(self):
captured = self._capture(
ShellFileOperations._search_files_rg,
"*foo*", r"E:\HermesWork\proj", 50, 0,
)
assert captured, "rg was never invoked"
assert "'E:/HermesWork/proj'" in captured[0]
29 changes: 25 additions & 4 deletions tools/file_operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,27 @@ def _escape_shell_arg(self, arg: str) -> str:
# Use single quotes and escape any single quotes in the string
return "'" + arg.replace("'", "'\"'\"'") + "'"

def _escape_native_tool_arg(self, path: str) -> str:
"""Escape a path argument for a native Windows binary.

``_escape_shell_arg`` rewrites drive paths to the Git Bash
``/c/...`` form required by MSYS tools (grep, find, test, bash
builtins). Native binaries are different: Hermes runs bash with
``MSYS_NO_PATHCONV=1``/``MSYS2_ARG_CONV_EXCL=*``, so argv reaches
them verbatim, and a native build (e.g. MSVC ripgrep from WinGet)
cannot resolve ``/c/...`` — every explicit-path search fails with
``IO error ... The system cannot find the path specified``.
Feed native tools the ``C:/...`` form instead. Both native and
MSYS builds accept it; off Windows this is a no-op beyond quoting.
"""
if not path:
return path
from tools.environments.local import _IS_WINDOWS, _msys_to_windows_path

if _IS_WINDOWS:
path = _msys_to_windows_path(path).replace("\\", "/")
return "'" + path.replace("'", "'\"'\"'") + "'"

def _atomic_write(self, path: str, content: str) -> "ExecuteResult":
"""Write ``content`` to ``path`` atomically via temp-file + rename.

Expand Down Expand Up @@ -2444,7 +2465,7 @@ def _search_files_rg(self, pattern: str, path: str, limit: int, offset: int) ->
# Try mtime-sorted first (rg 13+); fall back to unsorted if not supported.
cmd_sorted = (
f"rg --files --sortr=modified -g {self._escape_shell_arg(glob_pattern)} "
f"{self._escape_shell_arg(path)} 2>/dev/null "
f"{self._escape_native_tool_arg(path)} 2>/dev/null "
f"| head -n {fetch_limit}"
)
result = self._exec(cmd_sorted, timeout=60)
Expand All @@ -2455,7 +2476,7 @@ def _search_files_rg(self, pattern: str, path: str, limit: int, offset: int) ->
# --sortr may have failed on older rg; retry without it.
cmd_plain = (
f"rg --files -g {self._escape_shell_arg(glob_pattern)} "
f"{self._escape_shell_arg(path)} 2>/dev/null "
f"{self._escape_native_tool_arg(path)} 2>/dev/null "
f"| head -n {fetch_limit}"
)
result = self._exec(cmd_plain, timeout=60)
Expand Down Expand Up @@ -2539,8 +2560,8 @@ def _search_with_rg(self, pattern: str, path: str, file_glob: Optional[str],

# Add pattern and path
cmd_parts.append(self._escape_shell_arg(pattern))
cmd_parts.append(self._escape_shell_arg(path))
cmd_parts.append(self._escape_native_tool_arg(path))

# Fetch extra rows so we can report the true total before slicing.
# For context mode, rg emits separator lines ("--") between groups,
# so we grab generously and filter in Python.
Expand Down