Harden release upload flow

Author: justin808

.github/workflows/ci.yml

@@ -27,7 +27,7 @@ jobs:
           python -m pip install --upgrade pip
           # django-rspack is a sibling package and is not published yet.
           python -m pip install "django-rspack @ git+https://github.com/shakacode/django-rspack.git@7b09b8baf8d23c822978fcb472672a811927f5e5"
-          python -m pip install "Django==${{ matrix.django-version }}" -e .[dev]
+          python -m pip install "Django==${{ matrix.django-version }}" -e '.[dev]'
 
       - name: Ruff
         run: ruff check .
@@ -57,7 +57,7 @@ jobs:
           python -m pip install --upgrade pip
           # django-rspack is a sibling package and is not published yet.
           python -m pip install "django-rspack @ git+https://github.com/shakacode/django-rspack.git@7b09b8baf8d23c822978fcb472672a811927f5e5"
-          python -m pip install -e .[dev]
+          python -m pip install -e '.[dev]'
 
       - name: Install example dependencies
         working-directory: example

.github/workflows/release.yml

@@ -42,7 +42,7 @@ jobs:
           python -m pip install --upgrade pip
           # django-rspack is a sibling package and is not published yet.
           python -m pip install "django-rspack @ git+https://github.com/shakacode/django-rspack.git@7b09b8baf8d23c822978fcb472672a811927f5e5"
-          python -m pip install "Django==${{ matrix.django-version }}" -e .[dev]
+          python -m pip install "Django==${{ matrix.django-version }}" -e '.[dev]'
 
       - name: Ruff
         run: ruff check .
@@ -74,7 +74,7 @@ jobs:
           python -m pip install --upgrade pip
           # django-rspack is a sibling package and is not published yet.
           python -m pip install "django-rspack @ git+https://github.com/shakacode/django-rspack.git@7b09b8baf8d23c822978fcb472672a811927f5e5"
-          python -m pip install -e .[dev]
+          python -m pip install -e '.[dev]'
 
       - name: Install example dependencies
         working-directory: example
@@ -116,7 +116,7 @@ jobs:
           from pathlib import Path
           import re
 
-          text = Path("src/react_on_django/__about__.py").read_text()
+          text = Path("src/react_on_django/__about__.py").read_text(encoding="utf-8")
           match = re.search(r'^__version__ = "([^"]+)"$', text, re.M)
           if not match:
               raise SystemExit("Could not find __version__ in src/react_on_django/__about__.py")
@@ -131,14 +131,7 @@ jobs:
           PACKAGE_VERSION: ${{ steps.version.outputs.version }}
           INPUT_VERSION: ${{ inputs.version }}
         run: |
-          expected_tag="v${PACKAGE_VERSION}"
-
-          if [[ "${GITHUB_EVENT_NAME}" == "push" && "${GITHUB_REF_NAME}" != "${expected_tag}" ]]; then
-            echo "Tag ${GITHUB_REF_NAME} does not match package version ${PACKAGE_VERSION}."
-            exit 1
-          fi
-
-          if [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" && -n "${INPUT_VERSION}" && "${INPUT_VERSION}" != "${PACKAGE_VERSION}" ]]; then
+          if [[ -n "${INPUT_VERSION}" && "${INPUT_VERSION}" != "${PACKAGE_VERSION}" ]]; then
             echo "Requested version ${INPUT_VERSION} does not match package version ${PACKAGE_VERSION}."
             exit 1
           fi

Makefile

@@ -1,9 +1,10 @@
 .PHONY: release release-dry-run
 
 PYTHON ?= python
+_truthy = $(filter 1 true yes y on TRUE YES Y ON,$(strip $(1)))
 
 release:
-	$(PYTHON) scripts/release.py $(if $(VERSION),--version $(VERSION),) $(if $(REPOSITORY),--repository $(REPOSITORY),) $(if $(YES),--yes,) $(if $(SKIP_CHECKS),--skip-checks,) $(if $(SKIP_PUSH),--skip-push,)
+	$(PYTHON) scripts/release.py $(if $(VERSION),--version $(VERSION),) $(if $(REPOSITORY),--repository $(REPOSITORY),) $(if $(call _truthy,$(YES)),--yes,) $(if $(call _truthy,$(SKIP_CHECKS)),--skip-checks,) $(if $(call _truthy,$(SKIP_PUSH)),--skip-push,)
 
 release-dry-run:
-	$(PYTHON) scripts/release.py --dry-run $(if $(VERSION),--version $(VERSION),) $(if $(REPOSITORY),--repository $(REPOSITORY),) $(if $(YES),--yes,) $(if $(SKIP_CHECKS),--skip-checks,)
+	$(PYTHON) scripts/release.py --dry-run $(if $(VERSION),--version $(VERSION),) $(if $(REPOSITORY),--repository $(REPOSITORY),) $(if $(call _truthy,$(YES)),--yes,) $(if $(call _truthy,$(SKIP_CHECKS)),--skip-checks,)

RELEASING.md

@@ -4,15 +4,15 @@ The maintainer flow is:
 
 1. Update `CHANGELOG.md` for the version you want to ship.
 2. Run `make release`.
-3. Confirm the version, branch, checks, upload target, and push step.
-4. If you chose `pypi` or `testpypi`, `twine` uploads locally.
-5. Otherwise, push the tag and use the GitHub workflow manually if you still
-   want CI-based publishing.
+3. Confirm the version, branch, checks, push step, and upload target.
+4. The command pushes the branch plus tag before any local `twine` upload.
+5. If you did not choose a local upload, use the GitHub workflow manually after
+   the tag is pushed if you still want CI-based publishing.
 
 `make release` is the Python equivalent of `rake release`. It validates the
 repo state, bumps `src/react_on_django/__about__.py` if needed, runs the
-release checks, creates the release commit, tags it, optionally uploads with
-`twine`, and then pushes the branch plus tag to GitHub.
+release checks, creates the release commit, tags it, pushes the branch plus tag
+to GitHub, and optionally uploads with `twine`.
 
 ## Before you run it
 
@@ -89,6 +89,13 @@ Dry run without commit, tag, or push:
 make release-dry-run VERSION=0.1.0a1
 ```
 
+Boolean Makefile flags are enabled by truthy values like `1`, `true`, `yes`,
+`y`, or `on`. For example:
+
+```bash
+make release-dry-run VERSION=0.1.0a1 YES=1 SKIP_CHECKS=true
+```
+
 The release command runs:
 
 - `ruff check .`
@@ -142,8 +149,12 @@ python -m pip install react-on-django==0.1.0a1
 4. Runs the release checks and local package build.
 5. Commits `CHANGELOG.md` and `src/react_on_django/__about__.py` if needed.
 6. Creates `vX.Y.Z` or `vX.Y.ZaN`.
-7. Optionally uploads `dist/*` with `twine upload --repository pypi|testpypi`.
-8. Pushes the current branch plus tags to `origin`.
+7. Pushes the current branch plus tags to `origin`.
+8. Optionally uploads `dist/*` with `twine upload --repository pypi|testpypi`.
+
+The command refuses `SKIP_PUSH=1 REPOSITORY=pypi` or
+`SKIP_PUSH=1 REPOSITORY=testpypi` because a package upload should not exist
+without the matching commit and tag on GitHub.
 
 ## Local upload setup
 

scripts/release.py

@@ -95,15 +95,15 @@ def version_sort_key(version: ParsedVersion) -> tuple[int, int, int, int, int]:
 
 
 def read_current_version() -> ParsedVersion:
-    text = VERSION_FILE.read_text()
+    text = VERSION_FILE.read_text(encoding="utf-8")
     match = re.search(r'^__version__ = "([^"]+)"$', text, re.M)
     if not match:
         raise ReleaseError(f"Could not find __version__ in {VERSION_FILE}")
     return parse_version(match.group(1))
 
 
 def update_version_file(target_version: ParsedVersion) -> None:
-    text = VERSION_FILE.read_text()
+    text = VERSION_FILE.read_text(encoding="utf-8")
     updated = re.sub(
         r'^__version__ = "([^"]+)"$',
         f'__version__ = "{target_version}"',
@@ -113,36 +113,34 @@ def update_version_file(target_version: ParsedVersion) -> None:
     )
     if updated == text:
         raise ReleaseError(f"Could not update version in {VERSION_FILE}")
-    VERSION_FILE.write_text(updated)
+    VERSION_FILE.write_text(updated, encoding="utf-8")
 
 
 @contextmanager
 def prepared_version(
     target_version: ParsedVersion, *, restore_after_success: bool
 ) -> Iterator[None]:
-    original = VERSION_FILE.read_text()
+    original = VERSION_FILE.read_text(encoding="utf-8")
     current_version = read_current_version()
     should_update = current_version != target_version
 
     if should_update:
         update_version_file(target_version)
 
+    success = False
     try:
         yield
-    except Exception:
-        if should_update:
-            VERSION_FILE.write_text(original)
-        raise
-    else:
-        if should_update and restore_after_success:
-            VERSION_FILE.write_text(original)
+        success = True
+    finally:
+        if should_update and (restore_after_success or not success):
+            VERSION_FILE.write_text(original, encoding="utf-8")
 
 
 def latest_changelog_version() -> ParsedVersion | None:
     if not CHANGELOG_FILE.exists():
         return None
 
-    for line in CHANGELOG_FILE.read_text().splitlines():
+    for line in CHANGELOG_FILE.read_text(encoding="utf-8").splitlines():
         match = CHANGELOG_HEADER_RE.match(line.strip())
         if not match:
             continue
@@ -159,7 +157,7 @@ def extract_changelog_section(version: ParsedVersion) -> str | None:
     if not CHANGELOG_FILE.exists():
         return None
 
-    lines = CHANGELOG_FILE.read_text().splitlines()
+    lines = CHANGELOG_FILE.read_text(encoding="utf-8").splitlines()
     collecting = False
     section_lines: list[str] = []
 
@@ -389,6 +387,11 @@ def resolve_upload_plan(
 ) -> UploadPlan:
     selected = parse_repository_name(repository)
     if dry_run:
+        if selected is not None:
+            print(
+                f"Warning: ignoring --repository {selected} because --dry-run does not upload.",
+                flush=True,
+            )
         return UploadPlan(repository=None, source="dry-run")
     if selected is not None:
         return UploadPlan(repository=selected, source="explicit option")
@@ -397,6 +400,14 @@ def resolve_upload_plan(
     return prompt_upload_repository()
 
 
+def validate_upload_push_plan(*, skip_push: bool, upload_plan: UploadPlan) -> None:
+    if skip_push and upload_plan.repository is not None:
+        raise ReleaseError(
+            "Cannot upload distributions with --skip-push. Push the release branch and "
+            "tag before uploading to pypi or testpypi."
+        )
+
+
 def upload_distributions(repository: str) -> None:
     artifacts = dist_artifacts()
     run(sys.executable, "-m", "twine", "upload", "--repository", repository, *artifacts)
@@ -488,6 +499,7 @@ def main(argv: list[str] | None = None) -> int:
             dry_run=args.dry_run,
             yes=args.yes,
         )
+        validate_upload_push_plan(skip_push=args.skip_push, upload_plan=upload_plan)
         print_release_summary(
             context,
             dry_run=args.dry_run,
@@ -516,10 +528,10 @@ def main(argv: list[str] | None = None) -> int:
 
             stage_and_commit_release_files(context.target_version)
             create_tag(context.target_version)
-            if upload_plan.repository is not None:
-                upload_distributions(upload_plan.repository)
             if not args.skip_push:
                 push_release(context.branch)
+            if upload_plan.repository is not None:
+                upload_distributions(upload_plan.repository)
         finally:
             clean_build_artifacts()
 

tests/test_release.py

@@ -1,5 +1,6 @@
 import importlib.util
 import sys
+from contextlib import contextmanager
 from pathlib import Path
 
 import pytest
@@ -118,7 +119,7 @@ def test_release_check_commands_sets_example_python_for_e2e():
     release = load_release_module()
 
     commands = list(release.release_check_commands(skip_checks=False))
-    example_test = commands[-1]
+    example_test = next(command for command in commands if command[0] == ["./bin/test-ci"])
 
     assert example_test[0] == ["./bin/test-ci"]
     assert example_test[1].name == "example"
@@ -151,6 +152,21 @@ def test_prepared_version_restores_after_pre_commit_failure(tmp_path, monkeypatc
     assert version_file.read_text() == '__version__ = "0.1.0"\n'
 
 
+def test_prepared_version_restores_after_keyboard_interrupt(tmp_path, monkeypatch):
+    release = load_release_module()
+    version_file = tmp_path / "__about__.py"
+    version_file.write_text('__version__ = "0.1.0"\n')
+    monkeypatch.setattr(release, "VERSION_FILE", version_file)
+
+    with pytest.raises(KeyboardInterrupt):
+        with release.prepared_version(
+            release.parse_version("0.1.0a1"), restore_after_success=False
+        ):
+            raise KeyboardInterrupt()
+
+    assert version_file.read_text() == '__version__ = "0.1.0"\n'
+
+
 def test_prepared_version_keeps_target_version_after_success(tmp_path, monkeypatch):
     release = load_release_module()
     version_file = tmp_path / "__about__.py"
@@ -200,3 +216,89 @@ def test_resolve_upload_plan_defaults_to_skip_when_yes_is_set():
 
     assert plan.repository is None
     assert plan.source == "non-interactive default"
+
+
+def test_resolve_upload_plan_warns_when_dry_run_ignores_repository(capsys):
+    release = load_release_module()
+
+    plan = release.resolve_upload_plan("testpypi", dry_run=True, yes=True)
+
+    assert plan.repository is None
+    assert plan.source == "dry-run"
+    assert (
+        "ignoring --repository testpypi because --dry-run does not upload"
+        in capsys.readouterr().out
+    )
+
+
+def test_validate_upload_push_plan_rejects_upload_without_push():
+    release = load_release_module()
+    plan = release.UploadPlan(repository="pypi", source="explicit option")
+
+    with pytest.raises(
+        release.ReleaseError,
+        match="Cannot upload distributions with --skip-push",
+    ):
+        release.validate_upload_push_plan(skip_push=True, upload_plan=plan)
+
+
+def test_main_pushes_release_refs_before_upload(monkeypatch):
+    release = load_release_module()
+    target = release.parse_version("0.1.0a1")
+    current = release.parse_version("0.1.0")
+    calls = []
+
+    @contextmanager
+    def fake_prepared_version(*args, **kwargs):
+        yield
+
+    monkeypatch.setattr(release, "read_current_version", lambda: current)
+    monkeypatch.setattr(
+        release,
+        "resolve_target_version",
+        lambda requested, current_version: target,
+    )
+    monkeypatch.setattr(
+        release,
+        "validate_repo_state",
+        lambda version: release.ReleaseContext(
+            branch="release-branch",
+            current_version=current,
+            target_version=version,
+            changelog_dirty=False,
+            version_dirty=False,
+        ),
+    )
+    monkeypatch.setattr(
+        release,
+        "resolve_upload_plan",
+        lambda repository, dry_run, yes: release.UploadPlan(
+            repository="testpypi", source="explicit option"
+        ),
+    )
+    monkeypatch.setattr(release, "print_release_summary", lambda *args, **kwargs: None)
+    monkeypatch.setattr(release, "prepared_version", fake_prepared_version)
+    monkeypatch.setattr(
+        release,
+        "run_release_checks",
+        lambda skip_checks: calls.append("checks"),
+    )
+    monkeypatch.setattr(release, "build_distributions", lambda: calls.append("build"))
+    monkeypatch.setattr(
+        release,
+        "stage_and_commit_release_files",
+        lambda target_version: calls.append("commit"),
+    )
+    monkeypatch.setattr(release, "create_tag", lambda target_version: calls.append("tag"))
+    monkeypatch.setattr(release, "push_release", lambda branch: calls.append("push"))
+    monkeypatch.setattr(
+        release,
+        "upload_distributions",
+        lambda repository: calls.append("upload"),
+    )
+    monkeypatch.setattr(release, "clean_build_artifacts", lambda: calls.append("clean"))
+
+    exit_code = release.main(["--version", "0.1.0a1", "--repository", "testpypi", "--yes"])
+
+    assert exit_code == 0
+    assert calls == ["checks", "build", "commit", "tag", "push", "upload", "clean"]