Skip to content

Commit dbaae2c

Browse files
malaverdiereclaude
andcommitted
ci: do not narrow the matrix when grammar submodules are missing
CI caught the exact under-selection this design exists to prevent. The reference graph is read out of the upstream grammars, so with submodules uninitialised the edges simply are not there: assert {'c', 'cpp'} >= {'c', 'cpp', 'solidity'} tree-sitter-solidity is what records that solidity reads tree-sitter-c, so without it checked out a tree-sitter-c bump would silently skip solidity. - languages-for-paths now refuses to narrow when any tree-sitter-* dir has no content, and says which one. Dotfiles do not count as content: a deinitialised submodule can retain a .git file and would otherwise look populated. - The enumerate job checks the grammars out so narrowing actually applies. Done by hand rather than via checkout's submodules: input, because gosu and requirements use SSH URLs that need the HTTPS rewrite first. - Tests now assert both modes: the graph edges when submodules are present, and the fall-back-to-everything behaviour when they are not. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7a71f4d commit dbaae2c

3 files changed

Lines changed: 64 additions & 3 deletions

File tree

.github/workflows/test-languages.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ jobs:
3434
# and `event_name == 'pull_request' && 0 || 1` would silently be 1 --
3535
# 0 is falsy in a GitHub Actions expression.
3636
fetch-depth: 0
37+
# The reference graph lives inside the upstream grammars, so without them
38+
# languages-for-paths cannot narrow and falls back to the full matrix.
39+
# Done by hand rather than via checkout's submodules: input because the
40+
# gosu and requirements submodules use SSH URLs, unusable in keyless CI.
41+
- name: Check out grammars (for the reference graph)
42+
run: |
43+
set -euo pipefail
44+
git config --global url."https://github.com/".insteadOf "git@github.com:"
45+
git submodule update --init --depth 1 --jobs 8
3746
- id: list
3847
env:
3948
BASE_SHA: ${{ github.event.pull_request.base.sha }}

lang/scripts/languages-for-paths

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,21 @@ def all_languages(src: Path) -> set[str]:
8080
return {p.name.removeprefix("semgrep-") for p in src.glob("semgrep-*") if p.is_dir()}
8181

8282

83+
def unpopulated_grammar_dirs(src: Path) -> list[str]:
84+
"""Upstream dirs with no checked-out content (submodule not initialised).
85+
86+
Edges live inside the upstream grammars, so an uninitialised submodule makes
87+
the graph silently incomplete: without tree-sitter-solidity checked out,
88+
nothing records that it reads tree-sitter-c, and a tree-sitter-c bump would
89+
not test solidity. Callers must treat this as "cannot narrow".
90+
"""
91+
def has_content(d: Path) -> bool:
92+
# Dotfiles only (a stray .git) does not count: nothing to read edges from.
93+
return any(p.name[0] != "." for p in d.iterdir())
94+
95+
return sorted(d.name for d in src.glob("tree-sitter-*") if d.is_dir() and not has_content(d))
96+
97+
8398
def main() -> int:
8499
"""Print each language needing a test run on its own line."""
85100
argparse.ArgumentParser(
@@ -91,10 +106,19 @@ def main() -> int:
91106
paths = [line.strip() for line in sys.stdin if line.strip()]
92107
changed_dirs = {d for p in paths if (d := grammar_dir_of(p))}
93108
outside = [p for p in paths if grammar_dir_of(p) is None]
109+
unpopulated = unpopulated_grammar_dirs(src)
110+
111+
if not paths:
112+
why = "no changed paths given"
113+
elif outside:
114+
why = f"shared path {outside[0]!r}"
115+
elif unpopulated:
116+
why = f"{len(unpopulated)} uninitialised submodule(s), e.g. {unpopulated[0]}"
117+
else:
118+
why = None
94119

95-
if outside or not paths:
96-
why = "no changed paths given" if not paths else f"shared path {outside[0]!r}"
97-
print(f"languages-for-paths: testing everything ({why})", file=sys.stderr)
120+
if why:
121+
print(f"languages-for-paths: selecting everything ({why})", file=sys.stderr)
98122
languages = all_languages(src)
99123
else:
100124
languages = affected_languages(changed_dirs, src) & all_languages(src)

lang/test_languages_for_paths.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,25 @@
55
import subprocess
66
from pathlib import Path
77

8+
import pytest
9+
810
LANG_DIR = Path(__file__).resolve().parent
911
SCRIPT = LANG_DIR / "scripts" / "languages-for-paths"
1012
SRC = "lang/semgrep-grammars/src"
1113

14+
# The reference graph is read out of the upstream grammars, so narrowing only
15+
# works when the submodules are checked out. Without them the script must fall
16+
# back to the full matrix -- asserted by test_uninitialised_submodules_* below.
17+
GRAMMARS = LANG_DIR / "semgrep-grammars" / "src"
18+
SUBMODULES_PRESENT = not [
19+
d
20+
for d in GRAMMARS.glob("tree-sitter-*")
21+
if d.is_dir() and not any(p.name[0] != "." for p in d.iterdir())
22+
]
23+
needs_submodules = pytest.mark.skipif(
24+
not SUBMODULES_PRESENT, reason="grammar submodules not initialised"
25+
)
26+
1227

1328
def run(*paths: str) -> list[str]:
1429
proc = subprocess.run(
@@ -23,21 +38,34 @@ def all_languages() -> list[str]:
2338
).stdout.split()
2439

2540

41+
@needs_submodules
2642
def test_language_own_dir_narrows_to_that_language():
2743
assert run(f"{SRC}/semgrep-lua/grammar.js") == ["lua"]
2844

2945

46+
@needs_submodules
3047
def test_follows_references_between_grammars():
3148
"""tree-sitter-c is read by cpp and solidity, not just c."""
3249
assert set(run(f"{SRC}/tree-sitter-c/src/parser.c")) >= {"c", "cpp", "solidity"}
3350
assert set(run(f"{SRC}/tree-sitter-c-sharp/grammar.js")) >= {"c-sharp", "c-sharp-pro"}
3451

3552

53+
@needs_submodules
3654
def test_upstream_dir_without_a_language_of_its_own():
3755
"""tree-sitter-xml has no semgrep-xml; it is only reachable via html."""
3856
assert run(f"{SRC}/tree-sitter-xml/grammar.js") == ["html"]
3957

4058

59+
@pytest.mark.skipif(SUBMODULES_PRESENT, reason="submodules are initialised here")
60+
def test_uninitialised_submodules_select_everything():
61+
"""Without the upstream grammars the graph is incomplete, so do not narrow.
62+
63+
This is the dangerous case: tree-sitter-solidity records that it reads
64+
tree-sitter-c, so with it absent a tree-sitter-c bump would skip solidity.
65+
"""
66+
assert run(f"{SRC}/tree-sitter-c/src/parser.c") == all_languages()
67+
68+
4169
def test_shared_path_tests_everything():
4270
"""Anything outside the grammar dirs can affect every language."""
4371
for shared in ["core/src/x.ml", "lang/Makefile", "lang/scripts/list-languages", ".github/x.yml"]:

0 commit comments

Comments
 (0)