From 7d7629f6239892d9fa000ab8ede8d61650a48813 Mon Sep 17 00:00:00 2001 From: Alex Rubinsteyn Date: Thu, 1 Oct 2026 23:31:28 -0400 Subject: [PATCH] Guard deployment of merged releases and tag successful uploads --- AGENTS.md | 4 +- CHANGELOG.md | 9 ++ CONTRIBUTING.md | 1 + RELEASING.md | 50 +++---- deploy.sh | 79 ++++++++++- docs/development_audit.md | 2 +- tests/test_deploy_script.py | 252 ++++++++++++++++++++++++++++++++++++ varcode/version.py | 2 +- 8 files changed, 366 insertions(+), 33 deletions(-) create mode 100644 tests/test_deploy_script.py diff --git a/AGENTS.md b/AGENTS.md index 2fa6aa61..6624effe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,7 +7,7 @@ Guide for coding agents working in this repo. Read this before touching code. ## Golden Rules 1. **Never commit to `main`.** Always `git checkout -b ` before editing. Land via PR. -2. **Every PR bumps the version.** Even doc-only PRs — at minimum a patch bump. `deploy.sh ` handles the bump + commit + push. +2. **Every PR bumps the version.** Even doc-only PRs — at minimum a patch bump in `varcode/version.py` with release notes. Include the bump in the PR; `deploy.sh [version]` publishes the merged version and checks any supplied version matches. 3. **"Done" means merged AND deployed to PyPI** — never stop at merge. After a PR merges, run `./deploy.sh` from a clean main. Skipping deploy = task not done. 4. **File problems as issues, don't silently work around them.** If you hit a bug here or in a sibling openvax/pirl-unc repo, open a GitHub issue on the correct repo and link it from the PR. 5. **After a PR ships, look for the next block of work.** Read open issues across the relevant openvax repos, group by dependency + urgency. Prefer *foundational* changes that unblock multiple downstream improvements; otherwise chain the smallest independent improvements. @@ -29,7 +29,7 @@ Before telling the user a change is "complete": - `./develop.sh` — editable install (dev mode) - `./lint.sh` — ruff check - `./test.sh` — pytest (with coverage where configured) -- `./deploy.sh [version]` — lint → test → optional version bump → build → twine upload → tag → push +- `./deploy.sh [--dry-run] [version]` — clean synchronized main/master + unused-tag checks → lint → test → build → twine check → upload → tag → push. Dry runs stop before upload; version changes always go through a PR. ## Code Style diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ff19c07..1314dca3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Change Log +## [v10.11.3](https://github.com/openvax/varcode/tree/v10.11.3) (2026-10-01) + +- Guard deployment with a clean main/master checkout synchronized with live + origin, matching version, unused tag, lint/tests and distribution checks + (#414). Publish the merged version and push its tag only after upload; + `--dry-run` validates and builds without publishing or changing Git history. +- Preserve each attempt's release artifacts for publication verification and + recovery. Keep version bumps in PRs and align the release instructions. + ## [v10.11.2](https://github.com/openvax/varcode/tree/v10.11.2) (2026-10-01) - Pin bare GRCh38 references during pytest to the existing Ensembl 81 fixture diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 85ba3689..39e6a878 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -23,6 +23,7 @@ python3 -m venv .venv source .venv/bin/activate python -m pip install -e '.[rna,test-data]' pytest pytest-cov pytest-xdist ruff python -m pip install mkdocs mkdocs-material 'mkdocstrings[python]' +python -m pip install build twine ``` `rna` installs the optional BAM reader; `test-data` installs the pinned diff --git a/RELEASING.md b/RELEASING.md index 4bd4a003..cb0cb517 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -23,35 +23,41 @@ with build/upload credentials configured for PyPI. Use a clean checkout: git switch main git pull --ff-only origin main git status --porcelain -python -c 'from varcode.version import __version__; print(__version__)' +python -c 'import runpy; print(runpy.run_path("varcode/version.py")["__version__"])' ``` -Confirm that the working tree is clean, the branch is `main`, and the imported -version is the merged release. Then run: +The script requires a clean `main` or `master` whose HEAD matches the live +branch on `origin`. The release tag must be absent both locally and remotely. +Then run: ```bash ./deploy.sh ``` -The current script runs lint and tests, installs/upgrades `build` and `twine`, -recreates `dist/`, builds distributions, and uploads them to PyPI. It does -**not** check the branch or working tree, accept a version argument, bump the -version, create a tag, or push commits. Those missing safeguards are tracked -in [#414](https://github.com/openvax/varcode/issues/414); enforce the clean-main -requirement yourself until the script implements it. If a step fails, fix its -root cause and verify the publication state before retrying an upload. +Use `./deploy.sh X.Y.Z` to also assert the already-merged version. -After a successful upload, verify the exact version on -[PyPI](https://pypi.org/project/varcode/), compare its wheel/sdist SHA256 hashes -with the files in `dist/`, and smoke-test the published package in an isolated -environment. Tag the same merged commit with `v` and push that tag: +`deploy.sh` runs lint and tests, builds a wheel and source distribution in a +new directory under `dist/`, and runs `twine check --strict` on that exact pair. +It rechecks the checkout and remote refs before uploading, then tags the +original merged commit with `v` and pushes only that tag. Version +changes belong in the PR; a different version argument fails without editing +or committing files. Activate the development environment first; the script +uses `python` and the installed `build` and `twine` tools. -```bash -# Replace X.Y.Z with the verified published version. -git tag vX.Y.Z -git push origin vX.Y.Z -``` +`./deploy.sh --dry-run [version]` performs the same guards, lint, tests, build, +and distribution checks, then stops before uploading or changing Git history. +It needs read access to `origin` and leaves its build artifacts under `dist/`. -If the tag already exists, verify its target instead of moving it. Check the -main-branch CI and documentation deployment, then review open issues for the -next dependency or correctness blocker. +After a successful upload, verify the exact version on +[PyPI](https://pypi.org/project/varcode/), compare its wheel/sdist SHA256 hashes +with the files in the printed artifact directory, and smoke-test the published +package in an isolated environment. Verify that the pushed tag targets the +merged commit. Check the main-branch CI and documentation deployment, then +review open issues for the next dependency or correctness blocker. + +If publication fails, keep the printed artifact directory and inspect PyPI +before retrying. Upload only missing files from that original pair after +verifying the published hashes; rebuilding may produce different bytes. +Finish tagging the original commit and pushing the tag once both files are +verified. If only the tag push failed, verify the local tag's target and retry +`git push origin refs/tags/vX.Y.Z`. Never move an existing release tag. diff --git a/deploy.sh b/deploy.sh index c1896d9e..2428d905 100755 --- a/deploy.sh +++ b/deploy.sh @@ -1,7 +1,72 @@ -./lint.sh && \ -./test.sh && \ -python3 -m pip install --upgrade build && \ -python3 -m pip install --upgrade twine && \ -rm -rf dist && \ -python3 -m build && \ -python3 -m twine upload dist/* +#!/usr/bin/env bash +set -euo pipefail + +usage() { + echo "Usage: ./deploy.sh [--dry-run] [version]" + echo "Publish the merged version; an optional version must match varcode/version.py." +} + +fail() { echo "deploy.sh: $*" >&2; exit 1; } + +dry_run=0 +expected_version="" +while [[ $# -gt 0 ]]; do + case "$1" in + --dry-run) dry_run=1 ;; + -h|--help) usage; exit 0 ;; + -*) fail "Unknown option: $1" ;; + *) + [[ -z "$expected_version" ]] || fail "Expected at most one version." + expected_version="${1#v}" + [[ -n "$expected_version" ]] || fail "Expected a release version." + ;; + esac + shift +done + +cd "$(dirname "$0")" +branch="$(git symbolic-ref --quiet --short HEAD)" || fail "Deploy from main or master, not a detached HEAD." +[[ "$branch" == main || "$branch" == master ]] || fail "Deploys require main or master (current: $branch)." +commit="$(git rev-parse HEAD)" +version="$(python -c 'import runpy; print(runpy.run_path("varcode/version.py")["__version__"])')" +[[ "$version" =~ ^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$ ]] || fail "Invalid release version: $version" +[[ -z "$expected_version" || "$expected_version" == "$version" ]] || fail "Requested $expected_version, but the merged version is $version. Bump versions in a PR." +tag="v$version" + +# Check live origin refs, not a possibly stale remote-tracking branch. +check_release() { + local status remote_branch remote_tag + status="$(git status --porcelain --untracked-files=all)" + [[ -z "$status" ]] || fail "Working tree is not clean." + [[ "$(git symbolic-ref --quiet --short HEAD)" == "$branch" && "$(git rev-parse HEAD)" == "$commit" ]] || fail "Checkout changed during release." + remote_branch="$(git ls-remote --exit-code origin "refs/heads/$branch")" || fail "Cannot read origin/$branch." + [[ "${remote_branch%%$'\t'*}" == "$commit" ]] || fail "HEAD must match live origin/$branch; pull the merged release first." + if git show-ref --verify --quiet "refs/tags/$tag"; then + fail "Tag $tag already exists locally. Verify the publication state before retrying." + fi + remote_tag="$(git ls-remote origin "refs/tags/$tag")" || fail "Cannot check release tags on origin." + [[ -z "$remote_tag" ]] || fail "Tag $tag already exists on origin." +} + +check_release +./lint.sh +./test.sh + +# Keep each attempt's original bytes, including after a failed/partial upload. +mkdir -p dist +artifact_dir="$(mktemp -d "dist/$tag.XXXXXX")" +echo "Building $tag at $commit; artifacts: $artifact_dir" +python -m build --outdir "$artifact_dir" +distributions=("$artifact_dir/varcode-$version-py3-none-any.whl" "$artifact_dir/varcode-$version.tar.gz") +python -m twine check --strict "${distributions[@]}" +check_release + +if [[ "$dry_run" -eq 1 ]]; then + echo "Dry run passed for $tag; no upload, commit, tag, or push. Artifacts: $artifact_dir" + exit 0 +fi + +python -m twine upload --non-interactive --disable-progress-bar "${distributions[@]}" +git tag "$tag" "$commit" +git push origin "refs/tags/$tag" +echo "Deployed varcode $version: https://pypi.org/project/varcode/$version/" diff --git a/docs/development_audit.md b/docs/development_audit.md index 4f5807f7..d36f474c 100644 --- a/docs/development_audit.md +++ b/docs/development_audit.md @@ -38,7 +38,7 @@ domain-specific reviews and independent scientific oracles. |---|---|---| | High | VCF export derives header sample order from a set but writes row values in dictionary order. A two-row reproduction assigns tumor GT/DP to normal and emits `44:0/1` under `GT:DP`. Differing sample sets also discard sample data; headers and allele-indexed metadata lack a complete preservation contract. | Filed [#502](https://github.com/openvax/varcode/issues/502); resolved in 10.1.3 with explicit [export guarantees and limits](csv.md#vcf-export). | | High | The phase cap returns an all-cis placeholder that the consumer classifies as a precise effect. The existing CFTR pair yields cis/trans alternatives at cap 8 but only `p.S159T` at cap 1, with no molecular evidence resolving phase. Four unphased germline alleles exceed the default cap. | Filed [#503](https://github.com/openvax/varcode/issues/503); documented in [germline annotation](germline.md). | -| High | `deploy.sh` lacks the branch/clean-tree guards, version handling, tagging, and pushing described in AGENTS.md. It can upload from the wrong checkout if invoked without independent checks. | Existing [#414](https://github.com/openvax/varcode/issues/414); release instructions now describe the actual script and required manual checks. | +| High | `deploy.sh` lacked the branch/clean-tree guards, version handling, tagging, and pushing described in AGENTS.md. | Resolved in 10.11.3 ([#414](https://github.com/openvax/varcode/issues/414)): guarded deployment of the merged version, dry runs, distribution checks and automatic tagging; version bumps remain in PRs. | | Medium | The test cache probe accesses `EnsemblRelease.db`, which can download missing reference data while supposedly checking local installation. | Resolved [#493](https://github.com/openvax/varcode/issues/493) in 10.11.2 by pinning bare GRCh38 test references to the existing Ensembl 81 fixture and removing the cache probe. | | Medium | Some tests cannot detect failures, including the fast-path self-comparison and protocol/attribute surveys. VCF round-trip checks omitted ALT and complete record counts. | Existing [#479](https://github.com/openvax/varcode/issues/479); this change strengthens the VCF checks. The broader test review remains open. | | Medium | Reference mismatch fields contain transcript-oriented bases but the message labels them as genome bases; minus-strand errors can be read backwards. | Existing [#434](https://github.com/openvax/varcode/issues/434); troubleshooting now explains the current orientation. | diff --git a/tests/test_deploy_script.py b/tests/test_deploy_script.py new file mode 100644 index 00000000..306c0b61 --- /dev/null +++ b/tests/test_deploy_script.py @@ -0,0 +1,252 @@ +"""Exercise deployment with real local Git repos and offline build/upload stubs.""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import sys + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +VERSION = "1.2.3" +TAG = "v" + VERSION + + +def git(repo, *args): + return subprocess.run( + ["git", "-C", str(repo), *args], check=True, capture_output=True, + text=True).stdout.strip() + + +@pytest.fixture +def release(tmp_path, monkeypatch): + repo = tmp_path / "release repo" + repo.mkdir() + git(repo, "init", "-b", "main") + git(repo, "config", "user.name", "Release Test") + git(repo, "config", "user.email", "release@example.invalid") + git(repo, "config", "commit.gpgsign", "false") + git(repo, "config", "tag.gpgsign", "false") + shutil.copy(ROOT / "deploy.sh", repo / "deploy.sh") + (repo / "varcode").mkdir() + (repo / "varcode/version.py").write_text('__version__ = "1.2.3"\n') + (repo / ".gitignore").write_text("dist/\n__pycache__/\n") + for step in ("lint", "test"): + script = repo / (step + ".sh") + script.write_text( + '#!/bin/sh\n' + 'echo %s >> "$RELEASE_LOG"\n' + '[ "${FAIL_STEP:-}" != %s ]\n' % (step, step)) + script.chmod(0o755) + git(repo, "add", ".") + git(repo, "commit", "-m", "Release fixture") + origin = tmp_path / "origin.git" + git(tmp_path, "init", "--bare", str(origin)) + git(repo, "remote", "add", "origin", str(origin)) + git(repo, "push", "-u", "origin", "main") + hook = origin / "hooks/pre-receive" + hook.write_text( + '#!/bin/sh\ncat >> "$PUSH_LOG"\n[ "${FAIL_STEP:-}" != push ]\n') + hook.chmod(0o755) + + # Only Python package build/upload commands are faked. Version reading and + # all Git operations use the real executables; no network service is used. + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + python = bin_dir / "python" + python.write_text("#!" + sys.executable + "\n" + ''' +import json +import os +from pathlib import Path +import runpy +import subprocess +import sys + +args = sys.argv[1:] +if args[:1] == ["-c"]: + os.execv(sys.executable, [sys.executable, *args]) +if args[:2] == ["-m", "build"]: + step = "build" +else: + assert args[:2] == ["-m", "twine"], args + step = args[2] +with open(os.environ["RELEASE_LOG"], "a") as log: + log.write(step + "\\n") +with open(os.environ["RELEASE_ARGS"], "a") as log: + log.write(json.dumps(args) + "\\n") +if os.environ.get("FAIL_STEP") == step: + sys.exit(17) +if step == "build": + output = Path(args[args.index("--outdir") + 1]) + version = runpy.run_path("varcode/version.py")["__version__"] + for suffix in ("-py3-none-any.whl", ".tar.gz"): + (output / ("varcode-" + version + suffix)).write_bytes(b"original bytes") + if os.environ.get("BUILD_HOOK"): + subprocess.run(["bash", os.environ["BUILD_HOOK"]], check=True) +else: + paths = [Path(arg) for arg in args[3:] if not arg.startswith("--")] + assert len(paths) == 2 and all(path.is_file() for path in paths), args + if step == "upload": + assert not subprocess.check_output(["git", "tag", "--list", "v1.2.3"]) +''') + python.chmod(0o755) + (bin_dir / "python3").symlink_to(python) + monkeypatch.setenv("PATH", str(bin_dir) + os.pathsep + os.environ["PATH"]) + monkeypatch.setenv("RELEASE_LOG", str(tmp_path / "commands")) + monkeypatch.setenv("RELEASE_ARGS", str(tmp_path / "arguments")) + monkeypatch.setenv("PUSH_LOG", str(tmp_path / "pushes")) + monkeypatch.delenv("FAIL_STEP", raising=False) + monkeypatch.delenv("BUILD_HOOK", raising=False) + return repo + + +def deploy(repo, *args): + # Running from outside the repository must still release the script's repo. + return subprocess.run( + ["bash", str(repo / "deploy.sh"), *args], cwd=repo.parent, + capture_output=True, text=True) + + +def steps(repo): + log = repo.parent / "commands" + return log.read_text().splitlines() if log.exists() else [] + + +@pytest.mark.parametrize("branch", ["main", "master"]) +@pytest.mark.parametrize("args", [[], [VERSION], [TAG]]) +def test_publish_tags_only_the_merged_commit(release, branch, args): + if branch == "master": + git(release, "branch", "-m", "master") + git(release, "push", "-u", "origin", "master") + (release.parent / "pushes").unlink() + before = git(release, "rev-parse", "HEAD") + result = deploy(release, *args) + assert result.returncode == 0, result.stderr + assert steps(release) == ["lint", "test", "build", "check", "upload"] + assert git(release, "rev-parse", "HEAD", TAG).splitlines() == [before, before] + assert git(release, "ls-remote", "origin", "refs/tags/" + TAG).startswith(before) + assert (release.parent / "pushes").read_text().split()[2:] == ["refs/tags/" + TAG] + assert not git(release, "status", "--porcelain") + files = list((release / "dist").glob("*/*")) + assert {path.name for path in files} == { + "varcode-1.2.3-py3-none-any.whl", "varcode-1.2.3.tar.gz"} + commands = [json.loads(line) for line in + (release.parent / "arguments").read_text().splitlines()] + assert commands[1][:4] == ["-m", "twine", "check", "--strict"] + assert set(commands[2][5:]) == {str(path.relative_to(release)) for path in files} + + +def test_dry_run_builds_without_publishing_or_changing_refs(release): + before = git(release, "show-ref") + result = deploy(release, "--dry-run", VERSION) + assert result.returncode == 0, result.stderr + assert steps(release) == ["lint", "test", "build", "check"] + assert git(release, "show-ref") == before + assert not (release.parent / "pushes").exists() + assert not git(release, "status", "--porcelain") + + +@pytest.mark.parametrize("args", [ + ["1.2.4"], ["--dry-run", "1.2.4"], ["--unknown"], + [VERSION, "1.2.4"], [""], ["v"], ["01.2.3"], +]) +def test_bad_arguments_stop_before_gates_and_never_bump(release, args): + before = git(release, "rev-parse", "HEAD") + result = deploy(release, *args) + assert result.returncode != 0 + assert not steps(release) + assert git(release, "rev-parse", "HEAD") == before + assert not git(release, "status", "--porcelain") + assert not (release.parent / "pushes").exists() + + +@pytest.mark.parametrize("state", ["topic", "detached", "dirty", "staged", "untracked"]) +def test_invalid_checkout_stops_before_gates(release, state): + if state == "topic": + git(release, "checkout", "-b", "topic") + elif state == "detached": + git(release, "checkout", "--detach") + elif state == "untracked": + (release / "new-file").touch() + else: + (release / ".gitignore").write_text("# edited\n") + if state == "staged": + git(release, "add", ".gitignore") + result = deploy(release) + assert result.returncode != 0 + assert ("main or master" if state in ("topic", "detached") else "not clean") in result.stderr + assert not steps(release) + + +@pytest.mark.parametrize("state", ["ahead", "behind", "unreachable", "missing"]) +def test_requires_live_origin_branch(release, state): + if state in ("ahead", "behind"): + git(release, "commit", "--allow-empty", "-m", "Unreleased change") + if state == "behind": + git(release, "push", "origin", "main") + git(release, "reset", "--hard", "HEAD~") + # Even a stale remote-tracking ref matching HEAD must not pass. + git(release, "update-ref", "refs/remotes/origin/main", "HEAD") + elif state == "unreachable": + git(release, "remote", "set-url", "origin", str(release / "missing")) + else: + git(release.parent / "origin.git", "update-ref", "-d", "refs/heads/main") + result = deploy(release) + assert result.returncode != 0 + assert "origin/main" in result.stderr + assert not steps(release) + + +@pytest.mark.parametrize("remote", [False, True]) +def test_existing_tags_are_never_overwritten(release, remote): + git(release, "tag", TAG) + if remote: + git(release, "push", "origin", "refs/tags/" + TAG) + git(release, "tag", "-d", TAG) + result = deploy(release) + assert result.returncode != 0 + assert "already exists" in result.stderr + assert not steps(release) + + +@pytest.mark.parametrize("step", ["lint", "test", "build", "check", "upload", "push"]) +def test_failed_step_stops_release_and_preserves_artifacts(release, monkeypatch, step): + monkeypatch.setenv("FAIL_STEP", step) + result = deploy(release) + assert result.returncode != 0 + pipeline = ["lint", "test", "build", "check", "upload", "push"] + assert steps(release) == pipeline[:min(pipeline.index(step) + 1, 5)] + assert bool(git(release, "tag", "--list", TAG)) == (step == "push") + assert not git(release, "ls-remote", "origin", "refs/tags/" + TAG) + if step in ("check", "upload", "push"): + files = list((release / "dist").glob("*/*")) + assert len(files) == 2 + assert all(path.read_bytes() == b"original bytes" for path in files) + + +@pytest.mark.parametrize("command", [ + "touch untracked", + "git checkout -b topic", + "git commit --allow-empty -m changed", + "git tag v1.2.3", + "git push origin HEAD:refs/tags/v1.2.3", + 'git --git-dir="$PWD/../origin.git" update-ref -d refs/heads/main', +]) +def test_changes_during_build_block_upload(release, monkeypatch, command): + hook = release.parent / "during-build.sh" + hook.write_text(command + "\n") + monkeypatch.setenv("BUILD_HOOK", str(hook)) + result = deploy(release) + assert result.returncode != 0 + assert steps(release) == ["lint", "test", "build", "check"] + + +def test_each_attempt_preserves_earlier_artifacts(release): + for _ in range(2): + result = deploy(release, "--dry-run") + assert result.returncode == 0, result.stderr + assert len(list((release / "dist").iterdir())) == 2 + assert len(list((release / "dist").glob("*/*"))) == 4 diff --git a/varcode/version.py b/varcode/version.py index ef8b8d83..49db6570 100644 --- a/varcode/version.py +++ b/varcode/version.py @@ -1 +1 @@ -__version__ = "10.11.2" +__version__ = "10.11.3"