diff --git a/.github/bump_version.py b/.github/bump_version.py index 779a82e385..34aebfba93 100644 --- a/.github/bump_version.py +++ b/.github/bump_version.py @@ -2,6 +2,7 @@ import re import sys +import tomllib from pathlib import Path @@ -59,6 +60,46 @@ def update_file(path: Path, old_version: str, new_version: str): print(f" Updated {path}") +def get_project_name(pyproject_path: Path) -> str: + """The project name, normalised the way uv writes it in uv.lock.""" + name = tomllib.loads(pyproject_path.read_text())["project"]["name"] + return re.sub(r"[-_.]+", "-", name).lower() + + +def set_lock_version(text: str, package: str, new_version: str) -> str: + """Set the version of the project's own entry in uv.lock text. + + `uv sync --locked` and `uv lock --check` refuse a lock whose entry for + the project disagrees with pyproject.toml, so the two must move together. + Only that entry changes: a dependency can share the old version string. + The entry must be the editable project itself, as uv writes it. + """ + pattern = re.compile( + rf'^(\[\[package\]\]\nname = "{re.escape(package)}"\nversion = ")[^"\n]*(")$' + r'(?=\nsource = \{ editable = "\." \}$)', + re.MULTILINE, + ) + updated, count = pattern.subn(rf"\g<1>{new_version}\g<2>", text) + if count != 1: + raise ValueError(f"Expected one {package} entry in uv.lock, found {count}") + return updated + + +def update_lock(path: Path, package: str, new_version: str): + if not path.exists(): + print(f"{path} not found; it must move with pyproject.toml", file=sys.stderr) + sys.exit(1) + text = path.read_text() + try: + updated = set_lock_version(text, package, new_version) + except ValueError as error: + print(error, file=sys.stderr) + sys.exit(1) + if updated != text: + path.write_text(updated) + print(f" Updated {path}") + + def main(): root = Path(__file__).resolve().parent.parent pyproject = root / "pyproject.toml" @@ -70,6 +111,9 @@ def main(): print(f"Version: {current} -> {new} ({bump})") + # The lock first: if it is missing or has no entry for this package, stop + # before pyproject.toml changes. + update_lock(root / "uv.lock", get_project_name(pyproject), new) update_file(pyproject, current, new) diff --git a/.github/prepare_release.py b/.github/prepare_release.py index 3a4e42ff9c..74b72a62ba 100644 --- a/.github/prepare_release.py +++ b/.github/prepare_release.py @@ -9,9 +9,9 @@ Each attempt fetches main and starts from its tip. If that tip is already a version commit, or ``changelog.d`` has no fragments, there is nothing to release. Otherwise it bumps the version -(.github/bump_version.py), builds the changelog (towncrier), commits that as -"Update package version", checks the commit changes nothing else, and pushes -it to main. If main moved in the meantime the push is rejected; the next +(.github/bump_version.py, which also moves uv.lock's entry for this package), +builds the changelog (towncrier), commits that as "Update package version", +checks the commit changes nothing else, and pushes it to main. If main moved in the meantime the push is rejected; the next attempt starts again from the new tip, so the version and changelog always describe the commit they sit on. It never force-pushes. @@ -40,6 +40,7 @@ BOT_NAME = "github-actions[bot]" BOT_EMAIL = "41898282+github-actions[bot]@users.noreply.github.com" VERSION_LINE = re.compile(r'^version\s*=\s*"[0-9]+\.[0-9]+\.[0-9]+"$') +LOCK = "uv.lock" class ReleaseError(Exception): @@ -128,11 +129,63 @@ def version_text(version: tuple[int, int, int]) -> str: return ".".join(map(str, version)) +def project_lock_key(pyproject_text: str) -> tuple[str, dict]: + """The name and source uv.lock gives this package's own entry.""" + name = tomllib.loads(pyproject_text)["project"]["name"] + return re.sub(r"[-_.]+", "-", name).lower(), {"editable": "."} + + +def lock_problems(git: Git, base: str, head: str, version: str) -> list[str]: + """Problems with ``head``'s uv.lock, which may only move this package. + + The only change allowed is the version of this package's own entry (the + editable project), and only to ``version``. Everything else in the parsed + lock must be identical, so the commit cannot change a dependency. + """ + name, source = project_lock_key(git.out("show", f"{head}:pyproject.toml")) + try: + old = tomllib.loads(git.out("show", f"{base}:{LOCK}")) + new = tomllib.loads(git.out("show", f"{head}:{LOCK}")) + except tomllib.TOMLDecodeError as error: + return [f"{LOCK} is not valid TOML: {error}"] + own = [ + package + for package in new.get("package", []) + if package.get("name") == name and package.get("source") == source + ] + if len(own) != 1: + return [f"{LOCK} has {len(own)} entries for {name}, not one"] + problems = [] + if own[0].get("version") != version: + problems.append(f"{LOCK} gives {name} {own[0].get('version')}, not {version}") + own[0]["version"] = next( + ( + package.get("version") + for package in old.get("package", []) + if package.get("name") == name and package.get("source") == source + ), + None, + ) + if new != old: + problems.append(f"{LOCK} changes more than {name}'s version") + changed = [ + line[1:] + for line in git.out("diff", "--unified=0", base, head, "--", LOCK).splitlines() + if line[:1] in "+-" and not line.startswith(("+++", "---")) + ] + if len(changed) != 2 or not all(VERSION_LINE.match(line) for line in changed): + problems.append(f"{LOCK} changes more than one version line: {changed}") + return problems + + def check_release_diff(git: Git, base: str, head: str) -> None: """Fail unless ``head`` only versions ``base``. - The commit may change only the version line of pyproject.toml and - CHANGELOG.md, and delete changelog fragments; and the version must rise. + The commit may change only the version line of pyproject.toml, + CHANGELOG.md, and the version of this package's own entry in uv.lock + (to the new version), and delete changelog fragments; and the version + must rise. Version commits made before uv.lock moved with the version + leave it unchanged, and still pass. """ problems = [] for line in git.out( @@ -141,6 +194,14 @@ def check_release_diff(git: Git, base: str, head: str) -> None: status, _, path = line.partition("\t") if (status, path) in {("M", "pyproject.toml"), ("M", "CHANGELOG.md")}: continue + if (status, path) == ("M", LOCK): + problems += lock_problems( + git, + base, + head, + version_text(parse_version(git.out("show", f"{head}:pyproject.toml"))), + ) + continue if status == "D" and path.startswith("changelog.d/") and path.count("/") == 1: continue problems.append(f"{status} {path}") @@ -174,7 +235,10 @@ def make_version_commit(git: Git, python: str, towncrier: list[str]) -> str: [*towncrier, "build", "--yes", "--version", version], cwd=git.repo, check=True ) # Only what versioning changes; the guard below checks nothing else did. - git.run("add", "--all", "--", "pyproject.toml", "CHANGELOG.md", "changelog.d") + paths = ["pyproject.toml", "CHANGELOG.md", "changelog.d"] + if (git.repo / LOCK).exists(): + paths.append(LOCK) + git.run("add", "--all", "--", *paths) git.run("commit", "--no-verify", "-m", COMMIT_MESSAGE) return version diff --git a/changelog.d/uv-lock-version-in-step.fixed.md b/changelog.d/uv-lock-version-in-step.fixed.md new file mode 100644 index 0000000000..6fbc80d97e --- /dev/null +++ b/changelog.d/uv-lock-version-in-step.fixed.md @@ -0,0 +1 @@ +- The release version commit now moves uv.lock's entry for policyengine-uk with pyproject.toml, and its guard refuses any other change to uv.lock. diff --git a/policyengine_uk/tests/code_health/test_prepare_release.py b/policyengine_uk/tests/code_health/test_prepare_release.py index 671407e36b..0751571d97 100644 --- a/policyengine_uk/tests/code_health/test_prepare_release.py +++ b/policyengine_uk/tests/code_health/test_prepare_release.py @@ -12,13 +12,16 @@ 2. After a release, the remote branch's tip is the version commit, whose only parent is the tip the run fetched: history is extended, never rewritten. The commit deletes the fragments, adds them to the changelog and raises - the version, and changes nothing else. + the version, in pyproject.toml and in uv.lock's entry for the package, + and changes nothing else (a dependency sharing the old version keeps it). 3. Race: when another commit reaches main after the run fetched it, the run tries again on the new tip. The version and changelog then describe the new tip, and the other commit stays in history. 4. The guard refuses a version commit that changes another file, adds a - file, changes more of pyproject.toml than the version, or does not raise - the version. + file, changes more of pyproject.toml than the version, does not raise + the version, or changes uv.lock beyond moving the package's own entry to + the new version. A version commit that leaves uv.lock alone (as they + all did before it moved with the version) passes. 5. A push rejected for any reason other than main moving fails at once. 6. ``--github-output`` appends ``key=value`` lines and keeps what was there. 7. The tag script tags the given commit, not HEAD, with an annotated tag; @@ -99,6 +102,32 @@ """ +# The project's own entry, as uv writes it, and a dependency that happens to +# share its version. +LOCK = """version = 1 +revision = 3 +requires-python = ">=3.11" + +[[package]] +name = "demo" +version = "1.2.3" +source = { editable = "." } +dependencies = [ + { name = "lib" }, +] + +[[package]] +name = "lib" +version = "1.2.3" +source = { registry = "https://pypi.org/simple" } +""" + + +def lock_versions(repo, ref): + lock = tomllib.loads(git(repo, "show", f"{ref}:uv.lock")) + return {package["name"]: package["version"] for package in lock["package"]} + + @pytest.fixture(autouse=True) def git_env(monkeypatch): for name, value in GIT_ENV.items(): @@ -126,6 +155,7 @@ def remote(tmp_path): '[[tool.towncrier.type]]\ndirectory = "fixed"\nname = "Fixed"\n' ) (seed / "CHANGELOG.md").write_text("## [1.2.3]\n") + (seed / "uv.lock").write_text(LOCK) (seed / "changelog.d").mkdir() (seed / "changelog.d" / ".gitkeep").write_text("") (seed / "changelog.d" / "one.fixed.md").write_text("- Fixed one.\n") @@ -239,8 +269,10 @@ def test_a_release_extends_main_by_one_version_commit(remote, tmp_path, monkeypa "D\tchangelog.d/one.fixed.md", "M\tCHANGELOG.md", "M\tpyproject.toml", + "M\tuv.lock", ] assert version_at(remote, tip) == "1.2.4" + assert lock_versions(remote, tip) == {"demo": "1.2.4", "lib": "1.2.3"} assert "- Fixed one." in git(remote, "show", f"{tip}:CHANGELOG.md") # Reading the tip's bump_version.py leaves nothing behind in the checkout. assert git(work, "status", "--porcelain", "--untracked-files=all") == "" @@ -308,21 +340,50 @@ def test_a_rejected_push_that_is_not_a_race_fails_at_once(remote, tmp_path): assert len(attempts) == 1 -@pytest.mark.parametrize( - "fault", - ["another file", "a new file", "more of pyproject", "no version rise"], -) -def test_the_guard_refuses_more_than_versioning(remote, tmp_path, fault): - work = clone(remote, tmp_path / "work") +def version_by_hand(work, version="1.2.4", lock=None): + """Commit a version commit made by hand on HEAD; returns the base.""" base = git(work, "rev-parse", "HEAD") git(work, "rm", "-q", "changelog.d/one.fixed.md") (work / "CHANGELOG.md").write_text("## [1.2.4]\n- Fixed one.\n## [1.2.3]\n") pyproject = (work / "pyproject.toml").read_text() - version = "1.2.3" if fault == "no version rise" else "1.2.4" pyproject = pyproject.replace('version = "1.2.3"', f'version = "{version}"') - if fault == "more of pyproject": - pyproject = pyproject.replace("keep = 1", "keep = 2") (work / "pyproject.toml").write_text(pyproject) + if lock is not None: + (work / "uv.lock").write_text(lock) + return base + + +DEMO = 'name = "demo"\nversion = "1.2.3"' +LIB = 'name = "lib"\nversion = "1.2.3"' + + +@pytest.mark.parametrize( + "fault", + [ + "another file", + "a new file", + "more of pyproject", + "no version rise", + "a dependency in the lock", + "another version in the lock", + "more of the lock", + ], +) +def test_the_guard_refuses_more_than_versioning(remote, tmp_path, fault): + work = clone(remote, tmp_path / "work") + lock = LOCK.replace(DEMO, DEMO.replace("1.2.3", "1.2.4")) + if fault == "a dependency in the lock": + lock = lock.replace(LIB, LIB.replace("1.2.3", "1.2.4")) + if fault == "another version in the lock": + lock = LOCK.replace(DEMO, DEMO.replace("1.2.3", "1.2.5")) + if fault == "more of the lock": + lock = lock.replace('{ name = "lib" },\n', "") + base = version_by_hand( + work, "1.2.3" if fault == "no version rise" else "1.2.4", lock=lock + ) + if fault == "more of pyproject": + pyproject = (work / "pyproject.toml").read_text() + (work / "pyproject.toml").write_text(pyproject.replace("keep = 1", "keep = 2")) if fault == "another file": (work / "towncrier.py").write_text("# changed\n") if fault == "a new file": @@ -334,6 +395,19 @@ def test_the_guard_refuses_more_than_versioning(remote, tmp_path, fault): prepare.check_release_diff(prepare.Git(work), base, "HEAD") +@pytest.mark.parametrize("lock_moves", [True, False]) +def test_the_guard_accepts_the_lock_moving_with_the_version_or_not( + remote, tmp_path, lock_moves +): + work = clone(remote, tmp_path / "work") + lock = LOCK.replace(DEMO, DEMO.replace("1.2.3", "1.2.4")) if lock_moves else None + base = version_by_hand(work, lock=lock) + git(work, "add", "-A") + git(work, "commit", "-qm", "Update package version") + + prepare.check_release_diff(prepare.Git(work), base, "HEAD") + + def test_github_output_is_appended(remote, tmp_path, capsys, monkeypatch): work = clone(remote, tmp_path / "work") output = tmp_path / "github_output" diff --git a/policyengine_uk/tests/code_health/test_version_bump.py b/policyengine_uk/tests/code_health/test_version_bump.py new file mode 100644 index 0000000000..3689358af7 --- /dev/null +++ b/policyengine_uk/tests/code_health/test_version_bump.py @@ -0,0 +1,179 @@ +"""The version bump must keep uv.lock's entry for this package in step. + +``uv sync --locked`` and ``uv lock --check`` fail when uv.lock disagrees with +pyproject.toml. The lock records this package's own version, so a release that +bumps pyproject.toml without the lock leaves a lock that CI cannot check. +.github/bump_version.py therefore rewrites that one entry. These tests check +that the rewrite changes the package's own version and nothing else, including +dependencies that happen to share the old version string, both on the real +uv.lock and on generated ones. + +They read files from the repository checkout, so they skip when the package is +installed without one. +""" + +import importlib.util +import shutil +import subprocess +import sys +import tomllib +from pathlib import Path + +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +ROOT = Path(__file__).resolve().parents[3] +SCRIPT = ROOT / ".github" / "bump_version.py" +LOCK = ROOT / "uv.lock" +PYPROJECT = ROOT / "pyproject.toml" + +if not (SCRIPT.exists() and LOCK.exists() and PYPROJECT.exists()): + pytest.skip( + "the .github folder is not part of an installed package", + allow_module_level=True, + ) + +spec = importlib.util.spec_from_file_location("bump_version", SCRIPT) +bump_version = importlib.util.module_from_spec(spec) +spec.loader.exec_module(bump_version) + +PACKAGE = bump_version.get_project_name(PYPROJECT) + +versions = st.tuples(*[st.integers(0, 999)] * 3).map(lambda v: ".".join(map(str, v))) +# Names that share a prefix with this package, plus arbitrary ones. +other_names = st.one_of( + st.sampled_from([f"{PACKAGE}-data", f"{PACKAGE}2", "policyengine-core"]), + st.text("abcdefghijklmnopqrstuvwxyz0123456789-", min_size=1, max_size=12), +).filter(lambda name: name != PACKAGE) + + +def lock_versions(text: str) -> dict: + return { + package["name"]: package.get("version") + for package in tomllib.loads(text)["package"] + } + + +def make_lock(entries) -> str: + blocks = ['version = 1\nrevision = 3\nrequires-python = ">=3.11"\n'] + for name, version, editable in entries: + source = ( + '{ editable = "." }' + if editable + else '{ registry = "https://pypi.org/simple" }' + ) + blocks.append( + f'[[package]]\nname = "{name}"\nversion = "{version}"\nsource = {source}\n' + ) + return "\n".join(blocks) + + +def test_rewriting_the_real_lock_changes_only_this_package(): + text = LOCK.read_text() + updated = bump_version.set_lock_version(text, PACKAGE, "9999.0.0") + before, after = lock_versions(text), lock_versions(updated) + assert after[PACKAGE] == "9999.0.0" + assert {k: v for k, v in after.items() if k != PACKAGE} == { + k: v for k, v in before.items() if k != PACKAGE + } + changed = [ + (old, new) + for old, new in zip(text.splitlines(), updated.splitlines()) + if old != new + ] + assert len(changed) == 1 + assert len(text.splitlines()) == len(updated.splitlines()) + + +def test_a_dependency_sharing_the_old_version_is_left_alone(): + text = make_lock( + [ + ("numpy", "2.113.0", False), + (PACKAGE, "2.113.0", True), + ("pandas", "2.113.0", False), + ] + ) + updated = bump_version.set_lock_version(text, PACKAGE, "2.113.1") + assert lock_versions(updated) == { + "numpy": "2.113.0", + PACKAGE: "2.113.1", + "pandas": "2.113.0", + } + + +@pytest.mark.parametrize("copies", [0, 2]) +def test_a_missing_or_repeated_entry_is_an_error(copies): + text = make_lock([("numpy", "2.1.3", False)] + [(PACKAGE, "1.0.0", True)] * copies) + with pytest.raises(ValueError): + bump_version.set_lock_version(text, PACKAGE, "1.0.1") + + +# Generating unique names with a filter can be slow on a loaded runner; that +# says nothing about the rewrite, so don't fail on it. +@settings(suppress_health_check=[HealthCheck.too_slow], deadline=None) +@given( + st.lists(st.tuples(other_names, versions), max_size=6, unique_by=lambda e: e[0]), + st.integers(0, 6), + versions, + versions, +) +def test_rewrite_sets_only_this_package_and_is_idempotent( + others, position, old_version, new_version +): + entries = [(name, version, False) for name, version in others] + entries.insert(min(position, len(entries)), (PACKAGE, old_version, True)) + text = make_lock(entries) + + updated = bump_version.set_lock_version(text, PACKAGE, new_version) + + expected = dict(lock_versions(text), **{PACKAGE: new_version}) + assert lock_versions(updated) == expected + assert bump_version.set_lock_version(updated, PACKAGE, new_version) == updated + assert bump_version.set_lock_version(updated, PACKAGE, old_version) == text + + +def run_bump(tmp_path, lock_text): + """Run the real script on a scratch repository with a patch fragment.""" + (tmp_path / ".github").mkdir() + script = tmp_path / ".github" / "bump_version.py" + shutil.copy(SCRIPT, script) + (tmp_path / "pyproject.toml").write_text( + f'[project]\nname = "{PACKAGE}"\nversion = "1.2.3"\n' + ) + (tmp_path / "changelog.d").mkdir() + (tmp_path / "changelog.d" / "change.fixed.md").write_text("- A fix.\n") + if lock_text is not None: + (tmp_path / "uv.lock").write_text(lock_text) + return subprocess.run([sys.executable, str(script)], capture_output=True, text=True) + + +def test_main_moves_pyproject_and_lock_together(tmp_path): + # The lock's own entry starts stale, as on main before this change. + lock = make_lock([("numpy", "1.2.3", False), (PACKAGE, "1.2.0", True)]) + result = run_bump(tmp_path, lock) + assert result.returncode == 0, result.stderr + assert 'version = "1.2.4"' in (tmp_path / "pyproject.toml").read_text() + assert lock_versions((tmp_path / "uv.lock").read_text()) == { + "numpy": "1.2.3", + PACKAGE: "1.2.4", + } + + +@pytest.mark.parametrize( + "lock_text", + [None, make_lock([("numpy", "1.2.3", False)])], + ids=["no-lock", "no-entry"], +) +def test_main_stops_before_changing_anything(tmp_path, lock_text): + result = run_bump(tmp_path, lock_text) + assert result.returncode != 0 + assert 'version = "1.2.3"' in (tmp_path / "pyproject.toml").read_text() + if lock_text is not None: + assert (tmp_path / "uv.lock").read_text() == lock_text + + +def test_a_registry_entry_with_this_name_is_not_rewritten(): + text = make_lock([(PACKAGE, "1.0.0", False)]) + with pytest.raises(ValueError): + bump_version.set_lock_version(text, PACKAGE, "1.0.1")