mirror of
https://github.com/SCST-project/scst.git
synced 2026-10-22 06:02:33 +00:00
scripts: Validate analyzer diff bases as single commits
Passing the base directly to git diff allows an option such as --quiet to be interpreted as a flag. On a clean worktree it can succeed without selecting any sources, instead of rejecting the invalid revision. Resolve the base with rev-parse --verify --end-of-options and require a commit before building the diff command. Reuse the resulting object ID so options, ranges, tree objects and unknown revisions cannot turn an invalid request into an empty selection. Add a fixture covering rejected bases on a clean tree and a valid tag before and after a source change. Expose the resolver for future patch consumers as well.
This commit is contained in:
+2
-1
@@ -84,7 +84,8 @@ including staged new files. Missing and untracked files are omitted.
|
||||
|
||||
Diff selection compares a base with the working tree, including staged and
|
||||
unstaged changes. It selects whole current files, checks rename destinations,
|
||||
and skips deleted and untracked files.
|
||||
and skips deleted and untracked files. The base must resolve to one commit;
|
||||
options, ranges, tree objects and unknown revisions are rejected.
|
||||
|
||||
Explicit files may be untracked, but must exist and be within the same scope.
|
||||
They cannot be combined with a diff base. Both a path and its resolved symlink
|
||||
|
||||
@@ -40,9 +40,17 @@ def tracked_sources(root: Path) -> list[Path]:
|
||||
if in_scope(root / name, root) and (root / name).is_file()})
|
||||
|
||||
|
||||
def resolve_base(root: Path, base: str) -> str:
|
||||
"""Require one commit before passing a user-supplied base to Git diff."""
|
||||
return subprocess.check_output(
|
||||
["git", "-C", str(root), "rev-parse", "--verify", "--end-of-options",
|
||||
f"{base}^{{commit}}"], text=True).strip()
|
||||
|
||||
|
||||
def changed_sources(root: Path, base: str) -> list[Path]:
|
||||
revision = resolve_base(root, base)
|
||||
names = git_names(root, "diff", "--no-renames", "--name-only", "-z",
|
||||
"--diff-filter=d", base, "--", *git_pathspecs())
|
||||
"--diff-filter=d", revision, "--", *git_pathspecs())
|
||||
return sorted({root / name for name in names
|
||||
if in_scope(root / name, root) and (root / name).is_file()})
|
||||
|
||||
|
||||
@@ -89,6 +89,18 @@ class ScopeTests(unittest.TestCase):
|
||||
with self.assertRaises(subprocess.CalledProcessError):
|
||||
changed_sources(self.root, "missing-reference")
|
||||
|
||||
def test_diff_requires_one_commit_even_when_the_worktree_is_clean(self):
|
||||
source = self.source("scst/core.c")
|
||||
self.git("-c", "user.name=Test", "-c", "user.email=test@example.org",
|
||||
"-c", "commit.gpgsign=false", "commit", "-qm", "base")
|
||||
for base in ("--quiet", "--exit-code", "--cached", "HEAD..HEAD", "HEAD^{tree}"):
|
||||
with self.subTest(base=base), self.assertRaises(subprocess.CalledProcessError):
|
||||
changed_sources(self.root, base)
|
||||
self.git("tag", "baseline")
|
||||
self.assertEqual(changed_sources(self.root, "baseline"), [])
|
||||
source.write_text("int changed(void);\n")
|
||||
self.assertEqual(changed_sources(self.root, "baseline"), [source])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user