Skip to content
Merged
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
8 changes: 8 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,11 @@ jobs:
# submit -> wait loop parks Adreno at ~1/3 clock), and that one-off
# uploads, VkFrameRing-style rings and the exemption comment stay quiet.
run: python3 scripts/tests/test_check_displayxr_app_vk_frame.py

- name: App-linter build-tree exclusion unit tests
# Hermetic (synthetic app trees + a throwaway git repo). Pins that every
# directory walk skips build output (.cxx/, .externalNativeBuild/, .gradle/,
# _deps/, build/) and git-ignored untracked files: a local Gradle build's
# .cxx/ holds FetchContent'd displayxr-common tests/ that tripped false
# INV-3.1 errors.
run: python3 scripts/tests/test_check_displayxr_app_skip_build_trees.py
63 changes: 54 additions & 9 deletions scripts/check_displayxr_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,21 @@
import argparse
import json
import re
import subprocess
import sys
import xml.etree.ElementTree as ET
from pathlib import Path

RULES_DOC = "docs/guides/displayxr-app-rules.md"

# Directories never linted: shared reference code, vendored headers, build output.
# The Android entries matter after a local Gradle build: AGP's externalNativeBuild
# tree (.cxx/, older AGP .externalNativeBuild/) holds FetchContent'd displayxr-common
# sources under _deps/, incl. its tests/, which tripped false INV-3.1 errors.
EXCLUDE_DIRS = {
"build", ".git", "third_party", "openxr_includes", "common",
"_package", "__pycache__", "node_modules", ".vs", "out", ".claude",
".cxx", ".externalNativeBuild", ".gradle", "_deps",
}
SOURCE_EXTS = {".c", ".cc", ".cpp", ".cxx", ".h", ".hh", ".hpp", ".m", ".mm"}
# JVM sources are linted for the Android rules only (§11) — the C/C++ checks
Expand Down Expand Up @@ -250,11 +255,51 @@ def strip_comments(text: str) -> str:
return re.sub(r"//[^\n]*", "", text)


_GIT_IGNORED_CACHE: dict = {}


def git_ignored_prefixes(root: Path) -> tuple:
"""Untracked-and-ignored paths under `root`, relative to it (directories end
in "/"), per the enclosing git repo's ignore rules. One `git ls-files` call
per root, cached. Empty when `root` is not in a git work tree or git is
unavailable — EXCLUDE_DIRS is then the only filter. Tracked files are never
reported, so a committed file inside an ignored directory is still linted."""
key = str(root)
if key not in _GIT_IGNORED_CACHE:
prefixes: tuple = ()
try:
out = subprocess.run(
["git", "-C", key, "ls-files", "-z", "--others", "--ignored",
"--exclude-standard", "--directory"],
capture_output=True, timeout=30, check=True).stdout
prefixes = tuple(e for e in out.decode("utf-8", "replace").split("\0") if e)
except (OSError, subprocess.SubprocessError):
pass
_GIT_IGNORED_CACHE[key] = prefixes
return _GIT_IGNORED_CACHE[key]


def is_excluded(path: Path, root: Path) -> bool:
"""True for files the linter must skip: anything under an EXCLUDE_DIRS
directory, or git-ignored (build trees, generated sources)."""
try:
relp = path.relative_to(root)
except ValueError:
return False
if any(part in EXCLUDE_DIRS for part in relp.parts[:-1]):
return True
ignored = git_ignored_prefixes(root)
if not ignored:
return False
posix = relp.as_posix()
return any(posix == e or (e.endswith("/") and posix.startswith(e)) for e in ignored)


def iter_source_files(root: Path):
for p in sorted(root.rglob("*")):
if not p.is_file() or p.suffix.lower() not in SOURCE_EXTS:
continue
if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1]):
if is_excluded(p, root):
continue
yield p

Expand Down Expand Up @@ -567,7 +612,7 @@ def check_mcp_pairing(root: Path, findings: list):
# Manifest ids.
manifest_ids = {}
for m in root.rglob("*.displayxr.json"):
if any(part in EXCLUDE_DIRS for part in m.relative_to(root).parts[:-1]):
if is_excluded(m, root):
continue
try:
data = json.loads(m.read_text(encoding="utf-8"))
Expand All @@ -590,7 +635,7 @@ def check_mcp_pairing(root: Path, findings: list):
for p in sorted(root.rglob("*")):
if p.suffix.lower() not in SOURCE_EXTS or not p.is_file():
continue
if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1]):
if is_excluded(p, root):
continue
try:
text = p.read_text(encoding="utf-8", errors="replace")
Expand Down Expand Up @@ -635,7 +680,7 @@ def check_manual_tracking_event(root: Path, findings: list):
for p in sorted(root.rglob("*")):
if p.suffix.lower() not in SOURCE_EXTS or not p.is_file():
continue
if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1]):
if is_excluded(p, root):
continue
try:
text = strip_comments(p.read_text(encoding="utf-8", errors="replace"))
Expand Down Expand Up @@ -749,7 +794,7 @@ def check_views_change_realloc(root: Path, findings: list):
for p in sorted(root.rglob("*")):
if p.suffix.lower() not in SOURCE_EXTS or not p.is_file():
continue
if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1]):
if is_excluded(p, root):
continue
try:
text = strip_comments(p.read_text(encoding="utf-8", errors="replace"))
Expand Down Expand Up @@ -929,7 +974,7 @@ def check_vk_frame_discipline(root: Path, findings: list):
for path in sorted(root.rglob("*")):
if not path.is_file() or path.suffix.lower() not in VK_DISCIPLINE_EXTS:
continue
if any(part in EXCLUDE_DIRS for part in path.relative_to(root).parts[:-1]):
if is_excluded(path, root):
continue
try:
raw = path.read_text(encoding="utf-8", errors="replace")
Expand Down Expand Up @@ -1119,7 +1164,7 @@ def iter_android_sources(root: Path):
for p in sorted(root.rglob("*")):
if not p.is_file() or p.suffix.lower() not in exts:
continue
if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1]):
if is_excluded(p, root):
continue
yield p

Expand Down Expand Up @@ -1279,7 +1324,7 @@ def check_android_manifest(mpath: Path, root: Path, findings: list) -> None:
def check_android(root: Path, findings: list) -> None:
"""§11 — Android pixel-exactness. No-ops entirely for non-Android apps."""
manifests = [m for m in sorted(root.rglob("AndroidManifest.xml"))
if not any(part in EXCLUDE_DIRS for part in m.relative_to(root).parts[:-1])]
if not is_excluded(m, root)]
if not manifests:
return # Not an Android app — every check below is Android-only.

Expand Down Expand Up @@ -1332,7 +1377,7 @@ def check_android(root: Path, findings: list) -> None:

def scan_manifests(root: Path, findings: list):
manifests = [p for p in root.rglob("*.displayxr.json")
if not any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts[:-1])]
if not is_excluded(p, root)]
if not manifests:
findings.append(Finding(WARN, "INV-9.1", str(root), 1,
"No *.displayxr.json found — the app won't appear in any workspace launcher.",
Expand Down
155 changes: 155 additions & 0 deletions scripts/tests/test_check_displayxr_app_skip_build_trees.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
#!/usr/bin/env python3
"""Unit tests for the build-tree / git-ignore exclusion in
scripts/check_displayxr_app.py.

After a local Gradle build, AGP's externalNativeBuild output (.cxx/) holds the
FetchContent'd displayxr-common sources under _deps/, incl. its tests/. The
linter used to walk into it and report false INV-3.1 errors against those test
files (seen on test_apps/handle/cube_handle_vk_android, 2026-10-11). Every
directory walk now goes through is_excluded(): a fixed list of build-output
directory names, plus anything the enclosing git repo ignores.

Hermetic: each case writes a tiny synthetic app tree to a temp dir and lints it.

python3 scripts/tests/test_check_displayxr_app_skip_build_trees.py
"""
from __future__ import annotations

import shutil
import subprocess
import sys
import tempfile
import unittest
from pathlib import Path

sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
import check_displayxr_app as lint # noqa: E402

# Reads the active mode's view count but never opts into PRIMARY_MULTIVIEW_DXR:
# the shape INV-3.1 flags as an ERROR.
OFFENDING = """
void pick_mode(XrSession s)
{
uint32_t renderingModeCount = 0;
xrEnumerateDisplayRenderingModesDXR(s, 0, &renderingModeCount, nullptr);
}
"""

# A clean N-view app source (opts in, so INV-3.1 is silent for it).
CLEAN = """
void begin(XrSession s)
{
XrViewConfigurationType t = XR_VIEW_CONFIGURATION_TYPE_PRIMARY_MULTIVIEW_DXR;
}
"""

# The vk-discipline walker is a separate loop; pin it too.
VK_DRAIN = """
void RenderFrame()
{
vkCmdDraw(cmd, 3, 1, 0, 0);
vkQueueSubmit(q, 1, &si, VK_NULL_HANDLE);
vkQueueWaitIdle(q);
}
"""

BUILD_TREE_PATHS = [
".cxx/Debug/4x5y6z/arm64-v8a/_deps/displayxr_common-src/tests/test_views.cpp",
".externalNativeBuild/cmake/debug/arm64-v8a/offending.cpp",
".gradle/8.7/offending.cpp",
"_deps/displayxr_common-src/tests/offending.cpp",
"build/intermediates/offending.cpp",
]


def write_tree(root: Path, files: dict[str, str]) -> None:
for rel, text in files.items():
p = root / rel
p.parent.mkdir(parents=True, exist_ok=True)
p.write_text(text)


def inv31(root: Path) -> list:
findings: list = []
lint.scan_sources(root, findings)
return [f for f in findings if f.rule == "INV-3.1"]


def git(root: Path, *args: str) -> None:
subprocess.run(["git", "-C", str(root), *args], check=True, capture_output=True)


class SkipBuildTrees(unittest.TestCase):
def setUp(self):
self._tmp = tempfile.TemporaryDirectory()
self.root = Path(self._tmp.name).resolve()

def tearDown(self):
self._tmp.cleanup()

def test_fixture_trips_inv31_when_in_source_tree(self):
# Control: the offending source IS flagged where a real app source lives,
# so the exclusion tests below are not passing vacuously.
write_tree(self.root, {"src/main/cpp/offending.cpp": OFFENDING})
got = inv31(self.root)
self.assertEqual(len(got), 1, got)
self.assertEqual(got[0].level, lint.ERROR)

def test_build_trees_not_scanned(self):
for path in BUILD_TREE_PATHS:
with self.subTest(path=path):
with tempfile.TemporaryDirectory() as d:
root = Path(d).resolve()
write_tree(root, {"src/main/cpp/main.cpp": "int main() { return 0; }\n", path: OFFENDING})
self.assertEqual(inv31(root), [])

def test_build_tree_does_not_mask_real_opt_in(self):
# A build tree's sources must not count for the app either way: a stray
# PRIMARY_MULTIVIEW_DXR in .cxx/ must not silence a real app-side error.
write_tree(self.root, {
"src/main/cpp/main.cpp": OFFENDING,
".cxx/Debug/x/_deps/displayxr_common-src/common/views.cpp": CLEAN,
})
self.assertEqual(len(inv31(self.root)), 1)

def test_vk_walker_skips_build_trees(self):
write_tree(self.root, {".cxx/Debug/x/_deps/foo-src/render.cpp": VK_DRAIN})
findings: list = []
lint.check_vk_frame_discipline(self.root, findings)
self.assertEqual(findings, [])

def test_no_git_repo_falls_back_to_fixed_list(self):
if shutil.which("git"):
probe = subprocess.run(["git", "-C", str(self.root), "rev-parse", "--is-inside-work-tree"],
capture_output=True)
if probe.returncode == 0:
self.skipTest("temp dir is inside a git work tree on this machine")
self.assertEqual(lint.git_ignored_prefixes(self.root), ())

@unittest.skipUnless(shutil.which("git"), "git not installed")
def test_git_ignored_paths_not_scanned(self):
git(self.root, "init", "-q")
write_tree(self.root, {
".gitignore": "generated/\n*.gen.cpp\n",
# Neutral, NOT CLEAN: an opt-in anywhere silences INV-3.1 app-wide.
"src/main/cpp/main.cpp": "int main() { return 0; }\n",
"generated/nested/offending.cpp": OFFENDING,
"src/main/cpp/stub.gen.cpp": OFFENDING,
})
self.assertEqual(inv31(self.root), [])

@unittest.skipUnless(shutil.which("git"), "git not installed")
def test_tracked_file_in_ignored_dir_still_scanned(self):
# git-ignore only hides UNTRACKED files: a committed source inside an
# ignored directory is part of the app and must stay linted.
git(self.root, "init", "-q")
write_tree(self.root, {
".gitignore": "generated/\n",
"generated/offending.cpp": OFFENDING,
})
git(self.root, "add", "-f", "generated/offending.cpp")
self.assertEqual(len(inv31(self.root)), 1)


if __name__ == "__main__":
unittest.main(verbosity=2)
Loading