Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion morpc/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
__version__ = "0.7.0"
__version__ = "0.7.1"

import logging
logger = logging.getLogger(__name__)
Expand Down
109 changes: 107 additions & 2 deletions morpc/frictionless/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,98 @@ def _batch_assets(assets, baseLength, limit=30000):
return batches


def create_release(resources, owner, repo, tag, title=None, notes=None, overrideNotes=None, assets=None, dryRun=False):
def _git(args, cwd):
"""Run a git command in cwd and return (returncode, stdout stripped of trailing whitespace)."""
import subprocess

result = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True)
return result.returncode, result.stdout.strip()


def _check_worktree_synced(dir, dryRun=False):
"""Check that dir's repository is clean and in sync with its remote, raising if it is not.

`gh release create` with no --target cuts the tag at the default branch's HEAD *on GitHub*. Work
that has not reached GitHub is therefore not in the release, even though the descriptors uploaded
as assets were built from it. The common way to hit this is to run a notebook end to end: the
rebuilt descriptors are still only in the working tree when the release cell runs, so the tag
lands on the previous build's commit.

Three states produce that outcome, and all three are checked: uncommitted changes, commits that
have not been pushed, and a current branch that is not the one the tag will be cut on.

The ahead/behind comparison is made against the last-fetched state of the remote ref. Nothing is
fetched here, so a remote commit made since the last fetch is not seen. That does not affect the
case this guards against, which is local work that has not gone out.

Parameters
----------
dir : str or PathLike
A directory inside the repository to check. A falsy value means the working directory.
dryRun : bool
Optional. If True, log a warning instead of raising. A dry run is what one does mid-build
with a dirty tree, so failing it would make it useless. Defaults to False.

Raises
------
RuntimeError
If the repository is dirty, out of sync, or not a repository at all, and dryRun is False.
"""
import shutil

dir = dir or "."

def fail(message):
if dryRun:
logger.warning("{} Not raising because this is a dry run.".format(message))
return True
logger.error(message)
raise RuntimeError(message)

if shutil.which("git") is None:
return fail("git is required to check that the release will include your work but was not found on PATH.")

code, _ = _git(["rev-parse", "--show-toplevel"], dir)
if code != 0:
return fail("{} is not inside a git repository, so there is no way to check that the release will include your work.".format(dir))

code, status = _git(["status", "--porcelain"], dir)
if code != 0:
return fail("Could not read the status of the repository at {}.".format(dir))
if status:
paths = "\n".join(" {}".format(line) for line in status.splitlines())
return fail("The working tree has uncommitted changes. The release tag is cut at the branch head on GitHub, so these would not be in the release:\n{}\nCommit and push them, then create the release.".format(paths))

code, upstream = _git(["rev-parse", "--abbrev-ref", "--symbolic-full-name", "@{u}"], dir)
if code != 0:
return fail("The current branch has no upstream branch, so there is no way to check that your commits have reached GitHub. Push it first.")

code, counts = _git(["rev-list", "--left-right", "--count", "{}...HEAD".format(upstream)], dir)
if code != 0:
return fail("Could not compare the current branch with {}.".format(upstream))
behind, ahead = (int(count) for count in counts.split())
if ahead:
return fail("The current branch is {} commit(s) ahead of {}. The release tag is cut at the branch head on GitHub, so those commits would not be in the release. Push them, then create the release.".format(ahead, upstream))
if behind:
return fail("The current branch is {} commit(s) behind {}. The release would be cut at commits you have not seen. Pull, re-run the build, then create the release.".format(behind, upstream))

# The remote's own HEAD names the branch the tag will be cut on. Many clones never set it, in
# which case the branch cannot be checked and the two checks above still stand on their own.
remote = upstream.split("/")[0]
code, defaultRef = _git(["symbolic-ref", "--short", "refs/remotes/{}/HEAD".format(remote)], dir)
if code != 0:
logger.warning("{}/HEAD is not set locally, so the current branch cannot be checked against the default branch. Run `git remote set-head {} -a` to set it.".format(remote, remote))
return

defaultBranch = defaultRef.split("/", 1)[1]
code, branch = _git(["rev-parse", "--abbrev-ref", "HEAD"], dir)
if code != 0:
return fail("Could not determine the current branch of the repository at {}.".format(dir))
if branch != defaultBranch:
return fail("The current branch is {}, but the release tag is cut at {}, the default branch on GitHub. Merge {} into {} and push, then create the release.".format(branch, defaultBranch, branch, defaultBranch))


def create_release(resources, owner, repo, tag, title=None, notes=None, overrideNotes=None, assets=None, allowDirty=False, dryRun=False):
"""Create a GitHub release from a set of published resource descriptors.

This is the multi-resource counterpart to the hand-rolled `gh release create` step a workflow
Expand All @@ -354,6 +445,10 @@ def create_release(resources, owner, repo, tag, title=None, notes=None, override
in memory may have no known descriptor path, and the descriptor file is itself an asset that must
be uploaded. Use prepare_release() to publish descriptors first; pass the same paths here.

The tag is cut at the default branch's head on GitHub, not at the local HEAD, so the repository
must be clean and pushed before the release is created. That is checked first, and nothing is
published if the check fails -- see _check_worktree_synced().

The release is created without assets, which are then uploaded in batches small enough to fit
within the maximum command line length. If an upload fails, the release and its tag are deleted so
that the release can be retried once the cause is fixed, rather than left partially populated.
Expand Down Expand Up @@ -381,10 +476,15 @@ def create_release(resources, owner, repo, tag, title=None, notes=None, override
assets : list of str
Optional. Extra asset paths to upload alongside the ones derived from resources, e.g. a data
package descriptor.
allowDirty : bool
Optional. If True, skip the check that the repository is clean and in sync with its remote.
The release may then be cut at a commit that does not contain the data it describes. Defaults
to False.
dryRun : bool
Optional. If True, run every preflight check except the tag-existence check, log the resolved
asset list and notes, and return without creating a release or calling `gh` to check the tag.
Defaults to False.
The repository check logs a warning rather than raising, since a dry run is what one does
mid-build with a dirty tree. Defaults to False.

Returns
-------
Expand All @@ -402,6 +502,11 @@ def create_release(resources, owner, repo, tag, title=None, notes=None, override
if isinstance(resources, str):
resources = [resources]

if allowDirty:
logger.warning("allowDirty is set. Not checking whether the release will include the data it describes.")
else:
_check_worktree_synced(os.path.dirname(resources[0]), dryRun=dryRun)

descriptors = []
assetPaths = []
for resourcePath in resources:
Expand Down
29 changes: 29 additions & 0 deletions reference/dev_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,32 @@ command line length on the paths alone and fail the same way (WinError 206).
release exists with only some assets. The rollback narrows that window but cannot close
it: if the delete also fails, a partial release and its tag are left behind and must be
removed by hand before retrying.

## 2026-09-14 — Check the repository is clean and pushed before creating a release

Branch: `feat/release-worktree-guard`

`gh release create` with no `--target` cuts the tag at the default branch's HEAD *on
GitHub*, not at the local HEAD. Running a notebook end to end therefore produced a
release whose tag predated the build it describes: the rebuilt descriptors, the HTML
export and the metadata were still only in the working tree when the release cell ran.

- Added `_check_worktree_synced(dir, dryRun=False)`, called first in `create_release`.
It raises if the directory is not in a repository, if the working tree is dirty
(untracked-but-not-ignored files included), if the branch has no upstream, if it is
ahead of or behind that upstream, or if it is not the remote's default branch. The
dirty-tree message lists the offending paths.
- `dryRun=True` downgrades all of these to a warning, since a dry run is what one does
mid-build with a dirty tree.
- Added an `allowDirty` parameter as an escape hatch. It warns when set.
- Nothing is fetched, so the ahead/behind comparison is against the last-fetched state
of the remote ref. That does not affect the case this guards against, which is local
work that has not gone out.
- If `<remote>/HEAD` is not set locally (many clones never set it) the default-branch
check is skipped with a warning naming `git remote set-head`; the other checks stand.

**Consequence for workflow repos:** the HTML export must now run *before* the release
cell and be committed with everything else, so the rendered run will not show the output
of the release cell itself. Repos that track files rewritten during a run — a `*.log`
that is not gitignored, notably `morpc-parcels-standardize` — cannot cut a release at
all until those are ignored.
191 changes: 190 additions & 1 deletion tests/test_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,20 @@
resolve_data_path,
write_resource,
)
from morpc.frictionless.release import _batch_assets
from morpc.frictionless import release
from morpc.frictionless.release import _batch_assets, _check_worktree_synced


@pytest.fixture(autouse=True)
def _skip_worktree_check(monkeypatch):
"""Neutralize create_release()'s repository preflight for the tests that are not about it.

Those tests build resources in tmp_path, which is not a repository, so the preflight would fail
them all for a reason none of them is testing. It is exercised directly by its own tests below,
which call _check_worktree_synced through the name imported above -- that name is bound to the
real function and is unaffected by patching the module attribute here.
"""
monkeypatch.setattr(release, "_check_worktree_synced", lambda *args, **kwargs: None)


ASSET_URL = "https://github.com/morpc/morpc-parcels-standardize/releases/download/v2026.7.22/data.csv"
Expand Down Expand Up @@ -1258,3 +1271,179 @@ def test_load_data_local_resource_is_unaffected(tmp_path):
)
data, resource, schema = load_data(str(resourcePath))
assert data["id"].tolist() == [1, 2]


# --- _check_worktree_synced ---

def _run_git(*args, cwd):
subprocess.run(["git", *args], cwd=str(cwd), check=True, capture_output=True)


def _make_repo(tmp_path, name="repo"):
"""A repo with one commit, pushed to a bare remote whose HEAD names the default branch."""
remote = tmp_path / "{}.git".format(name)
repo = tmp_path / name
subprocess.run(["git", "init", "--bare", "-b", "main", str(remote)], check=True, capture_output=True)
subprocess.run(["git", "init", "-b", "main", str(repo)], check=True, capture_output=True)
_run_git("config", "user.email", "test@example.com", cwd=repo)
_run_git("config", "user.name", "Test", cwd=repo)
(repo / "README.md").write_text("hello\n")
_run_git("add", "-A", cwd=repo)
_run_git("commit", "-m", "initial", cwd=repo)
_run_git("remote", "add", "origin", str(remote), cwd=repo)
_run_git("push", "-u", "origin", "main", cwd=repo)
_run_git("remote", "set-head", "origin", "-a", cwd=repo)
return repo


def test_check_worktree_synced_passes_when_clean_and_pushed(tmp_path):
repo = _make_repo(tmp_path)

assert _check_worktree_synced(str(repo)) is None


def test_check_worktree_synced_modified_file_raises_and_names_it(tmp_path):
repo = _make_repo(tmp_path)
(repo / "README.md").write_text("changed\n")

with pytest.raises(RuntimeError, match="README.md"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_untracked_file_raises(tmp_path):
repo = _make_repo(tmp_path)
(repo / "data.resource.yaml").write_text("name: data\n")

with pytest.raises(RuntimeError, match="data.resource.yaml"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_ignored_file_is_not_dirty(tmp_path):
repo = _make_repo(tmp_path)
(repo / ".gitignore").write_text("*.log\n")
_run_git("add", "-A", cwd=repo)
_run_git("commit", "-m", "ignore logs", cwd=repo)
_run_git("push", cwd=repo)
(repo / "run.log").write_text("a log line\n")

assert _check_worktree_synced(str(repo)) is None


def test_check_worktree_synced_dirty_tree_only_warns_on_a_dry_run(tmp_path, caplog):
repo = _make_repo(tmp_path)
(repo / "README.md").write_text("changed\n")

with caplog.at_level("WARNING"):
assert _check_worktree_synced(str(repo), dryRun=True) is True
assert "README.md" in caplog.text


def test_check_worktree_synced_unpushed_commit_raises(tmp_path):
repo = _make_repo(tmp_path)
(repo / "README.md").write_text("changed\n")
_run_git("commit", "-am", "a commit that never left", cwd=repo)

with pytest.raises(RuntimeError, match="ahead"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_behind_the_remote_raises(tmp_path):
repo = _make_repo(tmp_path)
other = tmp_path / "other"
subprocess.run(["git", "clone", str(tmp_path / "repo.git"), str(other)], check=True, capture_output=True)
_run_git("config", "user.email", "test@example.com", cwd=other)
_run_git("config", "user.name", "Test", cwd=other)
(other / "README.md").write_text("someone else's change\n")
_run_git("commit", "-am", "upstream moved", cwd=other)
_run_git("push", cwd=other)
_run_git("fetch", cwd=repo)

with pytest.raises(RuntimeError, match="behind"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_branch_without_an_upstream_raises(tmp_path):
repo = _make_repo(tmp_path)
_run_git("checkout", "-b", "feature", cwd=repo)

with pytest.raises(RuntimeError, match="no upstream"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_non_default_branch_raises(tmp_path):
repo = _make_repo(tmp_path)
_run_git("checkout", "-b", "feature", cwd=repo)
_run_git("push", "-u", "origin", "feature", cwd=repo)

with pytest.raises(RuntimeError, match="default branch"):
_check_worktree_synced(str(repo))


def test_check_worktree_synced_skips_the_branch_check_when_remote_head_is_unset(tmp_path, caplog):
repo = _make_repo(tmp_path)
_run_git("symbolic-ref", "-d", "refs/remotes/origin/HEAD", cwd=repo)
_run_git("checkout", "-b", "feature", cwd=repo)
_run_git("push", "-u", "origin", "feature", cwd=repo)

with caplog.at_level("WARNING"):
assert _check_worktree_synced(str(repo)) is None
assert "origin/HEAD is not set" in caplog.text


def test_check_worktree_synced_outside_a_repository_raises(tmp_path):
with pytest.raises(RuntimeError, match="not inside a git repository"):
_check_worktree_synced(str(tmp_path))


def test_check_worktree_synced_missing_git_raises(tmp_path, monkeypatch):
monkeypatch.setattr(shutil, "which", lambda name: None)

with pytest.raises(RuntimeError, match="git is required"):
_check_worktree_synced(str(tmp_path))


def test_create_release_dirty_worktree_raises_before_any_gh_call(tmp_path, monkeypatch):
# Opt out of the autouse stub: this test is about the preflight running inside create_release.
monkeypatch.setattr(release, "_check_worktree_synced", _check_worktree_synced)
monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/gh" if name == "gh" else "/usr/bin/git")
repo = _make_repo(tmp_path)
_build_data(repo)
resourcePath = repo / "data.resource.yaml"
create_resource("data.csv", resourcePath=str(resourcePath), ignoreSchema=True, name="parcels", writeResource=True)

calls = []
realRun = subprocess.run

def _fake_run(args, **kwargs):
calls.append(args)
# The preflight shells out to git for real; only gh is faked.
if args[0] == "git":
return realRun(args, **kwargs)
return _FakeCompleted(1)

monkeypatch.setattr(subprocess, "run", _fake_run)

with pytest.raises(RuntimeError, match="uncommitted changes"):
create_release([str(resourcePath)], "morpc", "repo", "v2026.7.22")

assert not [args for args in calls if args[0] == "gh"]


def test_create_release_allow_dirty_skips_the_check(tmp_path, monkeypatch):
monkeypatch.setattr(release, "_check_worktree_synced", _check_worktree_synced)
monkeypatch.setattr(shutil, "which", lambda name: "/usr/bin/gh")
_build_data(tmp_path)
resourcePath = tmp_path / "data.resource.yaml"
create_resource("data.csv", resourcePath=str(resourcePath), ignoreSchema=True, name="parcels", writeResource=True)

calls = []

def _fake_run(args, **kwargs):
calls.append(args)
return _FakeCompleted(1)

monkeypatch.setattr(subprocess, "run", _fake_run)

create_release([str(resourcePath)], "morpc", "repo", "v2026.7.22", allowDirty=True)

assert calls[0][:3] == ["gh", "release", "view"]
Loading