Skip to content

fix(grep): keep the glob attached to --include so MSYS cannot expand it - #78

Merged
shauryagangrade merged 2 commits into
shauryagangrade:mainfrom
dchaudhari7177:fix/grep-include-glob-msys-expansion
Aug 14, 2026
Merged

fix(grep): keep the glob attached to --include so MSYS cannot expand it#78
shauryagangrade merged 2 commits into
shauryagangrade:mainfrom
dchaudhari7177:fix/grep-include-glob-msys-expansion

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

Problem

grep passed the filter as two arguments:

cmd = [grep_bin, *flags, "--include", glob, "-e", pattern, path]

On Windows the grep found on PATH is normally Git for Windows' MSYS build, and its runtime glob-expands a bare * argument against the current directory before grep parses it. The filter then names whatever file sorted first in the cwd, so a search of any other directory silently reports No matches.

This isn't an edge case — with Git installed, shutil.which("grep") finds it, so every grep call takes this path. The tool returns "no matches" for everything.

It's also why tests/test_tools.py::test_grep and ::test_grep_ignore_case fail on a Windows checkout of main:

assert 'needle in hay' in "No matches for 'needle' in C:\...\test_grep0."

Reproduction

cwd holding one unrelated file, searching a temp tree containing two matches:

--include *        rc=1  matches=0     <- expansion hits the cwd, not the target
--include=*        rc=0  matches=2
MSYS=noglob + sep  rc=0  matches=2
omitted            rc=0  matches=2

Fix

One character — attach the glob to the flag, --include=<glob>. MSYS doesn't expand a token containing =, and the two spellings are equivalent to GNU grep everywhere else.

I preferred this to the alternatives: omitting --include when glob == "*" fixes only the default and leaves *.py mangled, and setting MSYS=noglob in the environment is a bigger hammer that affects the whole child process.

Filtering is unaffected:

--include=*        -> ['a.txt', 'b.py', 'c.md']
--include=*.py     -> ['b.py']
--include=*.txt    -> ['a.txt']

Tests

The two existing greps now pass. Three added:

  • test_grep_passes_include_as_one_argument and test_grep_include_defaults_to_everything assert the command shape, not the result. This is deliberate: without the MSYS runtime both spellings behave identically, so on a Linux runner a revert to the two-argument form would otherwise stay green.
  • test_grep_filters_by_glob checks end-to-end that a glob still selects files.

Against main (stashing only gcode/tools.py): 5 failed, 17 passed. With the fix, the full suite is 57 passed — up from 52 passed / 2 failed. ruff check clean.

Found while looking at #63 (differential tests, _grep_python vs system grep) — that issue is a good idea independently, since this class of divergence is exactly what it would catch. Happy to send it separately.

grep passed the filter as two arguments, ["--include", glob]. On Windows the
grep found on PATH is normally Git for Windows' MSYS build, and its runtime
glob-expands a bare "*" argument against the *current* directory before grep
parses it. The filter then names whatever file sorted first in the cwd, so a
search of any other directory silently reports "No matches".

That is the whole tool failing, not an edge case: with Git installed,
shutil.which("grep") finds it, so every grep call takes this path.

Reproduced directly -- cwd holding one unrelated file, searching a temp tree
that contains two matches:

    --include *      rc=1  matches=0     <- expansion hits the cwd
    --include=*      rc=0  matches=2
    omitted          rc=0  matches=2

Attaching the glob to the flag hides it from that expansion, and is
equivalent everywhere else. Filtering is unaffected: --include=*.py still
selects only .py files.

This is why tests/test_tools.py::test_grep and ::test_grep_ignore_case fail
on a Windows checkout of main.

Tests: the two existing greps now pass; three added. Two assert the command
shape rather than the result, because both spellings behave identically
without the MSYS runtime -- on a Linux runner a revert would otherwise stay
green. The third checks end-to-end that a glob still filters.
@shauryagangrade
shauryagangrade merged commit cd5011d into shauryagangrade:main Aug 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants