diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 924244569..8c1253272 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -80,8 +80,9 @@ vermin --no-tips -t=3.8- --violations install.py # install.py stays on P ``` `upstream` is this repository as a remote of your fork -(`git remote add upstream https://github.com/Abdess/retrobios`). A profile you -only edit is not held to `--strict`: its older refs are reviewed by hand. +(`git remote add upstream https://github.com/Abdess/retrobios`, then +`git fetch upstream`). A profile you only edit is not held to `--strict`: its +older refs are reviewed by hand. The full pipeline is not needed for a contribution: it rebuilds every pack in `dist/`, which is why maintainers run it before a release. diff --git a/scripts/generate_readme.py b/scripts/generate_readme.py index 01e3ac47d..6e1a30e62 100644 --- a/scripts/generate_readme.py +++ b/scripts/generate_readme.py @@ -986,8 +986,9 @@ vermin --no-tips -t=3.8- --violations install.py # install.py stays on P ``` `upstream` is this repository as a remote of your fork -(`git remote add upstream https://github.com/Abdess/retrobios`). A profile you -only edit is not held to `--strict`: its older refs are reviewed by hand. +(`git remote add upstream https://github.com/Abdess/retrobios`, then +`git fetch upstream`). A profile you only edit is not held to `--strict`: its +older refs are reviewed by hand. The full pipeline is not needed for a contribution: it rebuilds every pack in `dist/`, which is why maintainers run it before a release. diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index 3245ab3b0..ddde60eb1 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -4355,6 +4355,10 @@ def branch_profiles(emulators_dir: str, base: str, added_only: bool) -> list[str branch point are not counted, and ends at the working tree, so a profile not yet committed is checked too. A rename reads as an addition. """ + if subprocess.run(["git", "rev-parse", "--verify", "--quiet", base], + capture_output=True, check=False).returncode: + remote = base.split("/", 1)[0] if "/" in base else "upstream" + raise SystemExit(f"ERROR: {base} is not in this clone: run git fetch {remote} first") merge_base = _git_lines(["merge-base", base, "HEAD"])[0] paths = _git_lines([ "diff", "--name-only", "--no-renames", diff --git a/scripts/validate_pr.py b/scripts/validate_pr.py index 9d99637df..a7f3a0fc0 100644 --- a/scripts/validate_pr.py +++ b/scripts/validate_pr.py @@ -558,30 +558,35 @@ def _repo_key(url: str) -> str: def _project_main_line() -> str | None: - """The project's main line, fetched when no remote of this clone names it. + """The project's main line, fetched when this clone does not hold it. A fork's clone has origin alone. Once the commit is pushed to the fork's main, origin/main is HEAD and the diff against it was empty, so --changed - found nothing and exited 0. None leaves the upstream/ and origin/ lines - in charge: a remote of that name names the project, or the clone has no - remote at all and only its uncommitted files can be read. + found nothing and exited 0. A remote added as CONTRIBUTING says and never + fetched did the same: upstream/main did not exist and origin/main won. + None leaves the upstream/ and origin/ lines in charge when the clone has + no remote at all and only its uncommitted files can be read. """ remotes = _git_lines("remote") or [] if not remotes: return None project = {_repo_key(url) for url in PROJECT_URLS} + source = PROJECT_URLS[0] for remote in remotes: urls = _git_lines("remote", "get-url", "--all", remote) or [] if any(_repo_key(url) in project for url in urls): - return None if remote in ("upstream", "origin") else f"{remote}/main" + if _git_lines("rev-parse", "--verify", "--quiet", f"{remote}/main"): + return f"{remote}/main" + source = remote + break fetched = subprocess.run( - ["git", "fetch", "--quiet", "--no-tags", PROJECT_URLS[0], "main"], + ["git", "fetch", "--quiet", "--no-tags", source, "main"], capture_output=True, text=True, check=False, ) if fetched.returncode != 0: raise RuntimeError( - f"no remote of this clone is {PROJECT_URLS[0]}, and fetching its " - f"main failed: {fetched.stderr.strip()}" + f"this clone does not hold the main of {PROJECT_URLS[0]}, and " + f"fetching it from {source} failed: {fetched.stderr.strip()}" ) return "FETCH_HEAD" diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 0fef264af..b8998e541 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -5918,6 +5918,13 @@ class TestBranchProfiles(unittest.TestCase): ["draft", "new"], ) + def test_an_unfetched_base_says_to_fetch(self): + """`--base upstream/main` with upstream added and never fetched + stopped on git's "Not a valid object name" and no next step.""" + with self.assertRaises(SystemExit) as raised: + profile_sync.branch_profiles("emulators", "upstream/main", added_only=True) + self.assertIn("git fetch upstream", str(raised.exception)) + def test_the_flag_selects_those_profiles(self): args = profile_sync.build_parser().parse_args(["--changed", "--base", "main"]) profiles = {"kept": {}, "edited": {}, "new": {}, "draft": {}} diff --git a/tests/test_validate_pr_flow.py b/tests/test_validate_pr_flow.py index e5930f573..0cb7a8667 100644 --- a/tests/test_validate_pr_flow.py +++ b/tests/test_validate_pr_flow.py @@ -13,6 +13,7 @@ import subprocess import sys import tempfile import unittest +from unittest import mock from pathlib import Path import yaml @@ -146,5 +147,45 @@ class RunFromAnyDirectory(unittest.TestCase): self.assertIn("Already in the database at this path", messages) +class AnUnfetchedUpstreamIsFetched(unittest.TestCase): + """CONTRIBUTING has the project added as `upstream` with no fetch. The + remote named the project, so nothing was fetched, upstream/main did not + exist, origin/main was the pushed branch, and --changed found nothing.""" + + def _git(self, cwd: Path, *args: str) -> None: + subprocess.run(["git", *args], cwd=cwd, check=True, capture_output=True, text=True) + + def test_the_branch_file_is_found(self): + with tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") as tmp: + root = Path(tmp) + project = root / "Abdess" / "retrobios.git" + fork = root / "fork.git" + clone = root / "clone" + for bare in (project, fork): + bare.mkdir(parents=True) + self._git(bare, "init", "-q", "--bare", "-b", "main") + clone.mkdir() + self._git(clone, "init", "-q", "-b", "main") + self._git(clone, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "--allow-empty", "-m", "base") + self._git(clone, "push", "-q", str(project), "main") + (clone / "bios").mkdir() + (clone / "bios" / "new.bin").write_bytes(b"x") + self._git(clone, "add", "bios/new.bin") + self._git(clone, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-q", + "-m", "add") + self._git(clone, "remote", "add", "origin", str(fork)) + self._git(clone, "push", "-q", "origin", "main") + self._git(clone, "fetch", "-q", "origin") + self._git(clone, "remote", "add", "upstream", str(project)) + cwd = os.getcwd() + os.chdir(clone) + try: + with mock.patch.object(validate_pr, "PROJECT_URLS", (str(project),)): + self.assertEqual(validate_pr.get_changed_files(), ["bios/new.bin"]) + finally: + os.chdir(cwd) + + if __name__ == "__main__": unittest.main()