From 11cec0a2d0cd5aec6657697b2f500fc023d21671 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 18 Oct 2026 16:31:25 +0200 Subject: [PATCH] fix: read uncommitted BIOS files in --changed --- scripts/validate_pr.py | 83 +++++++++++++++++++--------------- tests/test_refresh_failures.py | 27 ++++++++++- 2 files changed, 72 insertions(+), 38 deletions(-) diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index c12646c3..9e433e50 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -333,44 +333,53 @@ def validate_file( return result -def get_changed_files() -> list[str]: - """BIOS files the branch changes against origin's main line. - - A base git cannot resolve is skipped; when none resolves the staged - changes are read and said so. A git that fails outright is an error, - never "nothing changed": a clone whose remote is not called origin - used to answer that no BIOS had changed while the branch committed one. - """ - compared = False - for base in ("main", "master", "v2"): - result = subprocess.run( - ["git", "diff", "--name-only", f"origin/{base}...HEAD"], - capture_output=True, - text=True, - check=False, - ) - if result.returncode != 0: - continue - compared = True - files = [f for f in result.stdout.strip().split("\n") if f.startswith("bios/")] - if files: - return files - if compared: - return [] - print( - "no origin/main, origin/master or origin/v2 to compare against; " - "reading the staged changes only", - file=sys.stderr, - ) - result = subprocess.run( - ["git", "diff", "--cached", "--name-only"], - capture_output=True, - text=True, - check=False, - ) +def _git_lines(*args: str) -> list[str] | None: + """Output lines of a git command, or None when git refuses it.""" + result = subprocess.run(["git", *args], capture_output=True, text=True, check=False) if result.returncode != 0: - raise RuntimeError(f"git diff --cached failed: {result.stderr.strip()}") - return [f for f in result.stdout.strip().split("\n") if f.startswith("bios/") and f] + return None + return [line for line in result.stdout.splitlines() if line] + + +def get_changed_files() -> list[str]: + """BIOS files this checkout changes against the project's main line. + + Committed changes are read against upstream/ first, then + origin/: on a fork whose main carries the commit, origin/main is + HEAD and the diff was empty. Uncommitted and untracked files count too, + since CONTRIBUTING has the file copied into bios/ before any commit. + A git that fails outright is an error, never "nothing changed". + """ + changed: list[str] = [] + compared = False + for remote in ("upstream", "origin"): + for base in ("main", "master", "v2"): + committed = _git_lines("diff", "--name-only", f"{remote}/{base}...HEAD") + if committed is None: + continue + compared = True + changed.extend(committed) + break + if compared and changed: + break + staged = _git_lines("diff", "--cached", "--name-only") + local = _git_lines("diff", "--name-only") + untracked = _git_lines("ls-files", "--others", "--exclude-standard", "--", "bios") + if staged is None and local is None and untracked is None: + raise RuntimeError("git cannot read this checkout") + for found in (staged, local, untracked): + changed.extend(found or []) + if not compared: + print( + "no upstream/ or origin/ main line to compare against; reading the " + "staged, modified and untracked files only", + file=sys.stderr, + ) + seen: set[str] = set() + return [ + f for f in changed + if f.startswith("bios/") and not (f in seen or seen.add(f)) and os.path.isfile(f) + ] def _files_to_validate( diff --git a/tests/test_refresh_failures.py b/tests/test_refresh_failures.py index ef589050..31c719e9 100644 --- a/tests/test_refresh_failures.py +++ b/tests/test_refresh_failures.py @@ -52,7 +52,32 @@ class GitThatCannotAnswer(unittest.TestCase): ) self.assertEqual(proc.returncode, 0, proc.stderr) self.assertIn("bios/x.bin", proc.stdout) - self.assertIn("staged changes only", proc.stderr) + self.assertIn("untracked files only", proc.stderr) + + +class ChangesNotYetCommittedAreRead(unittest.TestCase): + """--changed read only `git diff origin/main...HEAD`: a file copied into + bios/ before any commit, as CONTRIBUTING has it, or committed on a + fork's own main, answered "No changed BIOS files detected", rc 0.""" + + def test_an_untracked_file_is_read(self): + with tempfile.TemporaryDirectory() as tmp: + subprocess.run(["git", "init", "-q", tmp], check=True) + Path(tmp, "README").write_text("x") + subprocess.run(["git", "-C", tmp, "add", "README"], check=True) + subprocess.run(["git", "-C", tmp, "-c", "user.email=a@b", "-c", "user.name=a", + "commit", "-qm", "init"], check=True) + Path(tmp, "bios").mkdir() + Path(tmp, "bios", "new.bin").write_bytes(b"x") + proc = subprocess.run( + [sys.executable, "-c", ( + f"import sys; sys.path.insert(0, {str(REPO_ROOT / 'scripts')!r}); " + "import validate_pr; print(validate_pr.get_changed_files())" + )], + cwd=tmp, capture_output=True, text=True, timeout=60, check=False, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("bios/new.bin", proc.stdout) class RefreshFailuresAreFailures(unittest.TestCase):