From 27284cac39b450577756cc9e6c82a611a1f3c251 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 13:42:06 +0100 Subject: [PATCH 1/9] Confirm commit to be tagged before tagging --- scripts-dev/release.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index 9d7c7c445fc0..f820dfd99fac 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -285,8 +285,8 @@ def tag(gh_token: Optional[str]): tag_name = f"v{current_version}" # Check we haven't released this version. - if tag_name in repo.tags: - raise click.ClickException(f"Tag {tag_name} already exists!\n") + # if tag_name in repo.tags: + # raise click.ClickException(f"Tag {tag_name} already exists!\n") # Get the appropriate changelogs and tag. changes = get_changes_for_version(current_version) @@ -295,6 +295,13 @@ def tag(gh_token: Optional[str]): if click.confirm("Edit text?", default=False): changes = click.edit(changes, require_save=False) + commit = repo.head.commit + click.echo( + f"{repo.head.ref} {commit} {commit.summary!r}\n" + f"by {commit.author} <{commit.author.email}>, " + f"committed at {commit.committed_datetime}" + ) + click.confirm(f"Tag this commit as {tag_name}?", default=False, abort=True) repo.create_tag(tag_name, message=changes, sign=True) if not click.confirm("Push tag to GitHub?", default=True): From 4a3669bdf1b1067cf8dce37a00e10b08d28738c1 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 15:40:05 +0100 Subject: [PATCH 2/9] Changelog --- changelog.d/12556.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12556.misc diff --git a/changelog.d/12556.misc b/changelog.d/12556.misc new file mode 100644 index 000000000000..dc245397fbcd --- /dev/null +++ b/changelog.d/12556.misc @@ -0,0 +1 @@ +Release script: confirm the commit to be tagged before tagging. From 0e3c017dbc64bfb24f17ab6c72da8bcea5a5b4b4 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 16:41:02 +0100 Subject: [PATCH 3/9] Undo commented bit that shouldn't have been, argh --- scripts-dev/release.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index f820dfd99fac..4700431b1627 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -285,8 +285,8 @@ def tag(gh_token: Optional[str]): tag_name = f"v{current_version}" # Check we haven't released this version. - # if tag_name in repo.tags: - # raise click.ClickException(f"Tag {tag_name} already exists!\n") + if tag_name in repo.tags: + raise click.ClickException(f"Tag {tag_name} already exists!\n") # Get the appropriate changelogs and tag. changes = get_changes_for_version(current_version) From 34105e3d1202ba5e0f7ee69dab2a440abb8b2dab Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 16:57:10 +0100 Subject: [PATCH 4/9] Instead, abort if we're on the wrong branch --- scripts-dev/release.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index 4700431b1627..34d907e6ebb5 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -167,9 +167,7 @@ def prepare(): assert not parsed_new_version.is_devrelease assert not parsed_new_version.is_postrelease - release_branch_name = ( - f"release-v{parsed_new_version.major}.{parsed_new_version.minor}" - ) + release_branch_name = get_release_branch_name(parsed_new_version) release_branch = find_ref(repo, release_branch_name) if release_branch: if release_branch.is_remote(): @@ -288,6 +286,15 @@ def tag(gh_token: Optional[str]): if tag_name in repo.tags: raise click.ClickException(f"Tag {tag_name} already exists!\n") + # Check we're on the right release branch + release_branch = get_release_branch_name(current_version) + if repo.active_branch.name != release_branch: + click.echo( + f"Need to be on the release branch ({release_branch}) before tagging. " + f"Currently on ({repo.active_branch.name})." + ) + click.get_current_context().abort() + # Get the appropriate changelogs and tag. changes = get_changes_for_version(current_version) @@ -295,13 +302,6 @@ def tag(gh_token: Optional[str]): if click.confirm("Edit text?", default=False): changes = click.edit(changes, require_save=False) - commit = repo.head.commit - click.echo( - f"{repo.head.ref} {commit} {commit.summary!r}\n" - f"by {commit.author} <{commit.author.email}>, " - f"committed at {commit.committed_datetime}" - ) - click.confirm(f"Tag this commit as {tag_name}?", default=False, abort=True) repo.create_tag(tag_name, message=changes, sign=True) if not click.confirm("Push tag to GitHub?", default=True): @@ -466,6 +466,10 @@ def get_package_version() -> version.Version: return version.Version(version_string) +def get_release_branch_name(version_number: version.Version) -> str: + return f"release-v{version_number.major}.{version_number.minor}" + + def find_ref(repo: git.Repo, ref_name: str) -> Optional[git.HEAD]: """Find the branch/ref, looking first locally then in the remote.""" if ref_name in repo.refs: From bc237191263a3ee25f9b67807336dfc74760c75e Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 18:24:42 +0100 Subject: [PATCH 5/9] factor out a `get_repo` function --- scripts-dev/release.py | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index 34d907e6ebb5..bdcefb31388e 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -87,13 +87,7 @@ def prepare(): """ # Make sure we're in a git repo. - try: - repo = git.Repo() - except git.InvalidGitRepositoryError: - raise click.ClickException("Not in Synapse repo.") - - if repo.is_dirty(): - raise click.ClickException("Uncommitted changes exist.") + repo = get_repo() click.secho("Updating git repo...") repo.remote().fetch() @@ -267,13 +261,7 @@ def tag(gh_token: Optional[str]): """Tags the release and generates a draft GitHub release""" # Make sure we're in a git repo. - try: - repo = git.Repo() - except git.InvalidGitRepositoryError: - raise click.ClickException("Not in Synapse repo.") - - if repo.is_dirty(): - raise click.ClickException("Uncommitted changes exist.") + repo = get_repo() click.secho("Updating git repo...") repo.remote().fetch() @@ -358,13 +346,7 @@ def publish(gh_token: str): """Publish release.""" # Make sure we're in a git repo. - try: - repo = git.Repo() - except git.InvalidGitRepositoryError: - raise click.ClickException("Not in Synapse repo.") - - if repo.is_dirty(): - raise click.ClickException("Uncommitted changes exist.") + repo = get_repo() current_version = get_package_version() tag_name = f"v{current_version}" @@ -470,6 +452,17 @@ def get_release_branch_name(version_number: version.Version) -> str: return f"release-v{version_number.major}.{version_number.minor}" +def get_repo() -> git.Repo: + """Get the project repo and check it's not got any uncommitted changes.""" + try: + repo = git.Repo() + except git.InvalidGitRepositoryError: + raise click.ClickException("Not in Synapse repo.") + if repo.is_dirty(): + raise click.ClickException("Uncommitted changes exist.") + return repo + + def find_ref(repo: git.Repo, ref_name: str) -> Optional[git.HEAD]: """Find the branch/ref, looking first locally then in the remote.""" if ref_name in repo.refs: From 6895e89739e755752615ad693c5c5ff3fc670fbd Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 18:39:05 +0100 Subject: [PATCH 6/9] Check we have the right tag checked out --- scripts-dev/release.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index bdcefb31388e..9f06899b921f 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -385,6 +385,13 @@ def upload(): current_version = get_package_version() tag_name = f"v{current_version}" + # Check we have the right tag checked out. + repo = get_repo() + tag = repo.tag(f"refs/tags/{tag_name}") + if repo.head.commit != tag.commit: + click.echo("Tag {tag_name} (tag.commit) is not currently checked out!") + click.get_current_context().abort() + pypi_asset_names = [ f"matrix_synapse-{current_version}-py3-none-any.whl", f"matrix-synapse-{current_version}.tar.gz", From f3d998542421ccba54477e593757f212510737de Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 26 Apr 2022 18:39:35 +0100 Subject: [PATCH 7/9] Clarify that `publish` only releases to GitHub --- scripts-dev/release.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index 9f06899b921f..efe5e5afe97a 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -343,7 +343,7 @@ def tag(gh_token: Optional[str]): @cli.command() @click.option("--gh-token", envvar=["GH_TOKEN", "GITHUB_TOKEN"], required=True) def publish(gh_token: str): - """Publish release.""" + """Publish release on GitHub.""" # Make sure we're in a git repo. repo = get_repo() @@ -351,7 +351,7 @@ def publish(gh_token: str): current_version = get_package_version() tag_name = f"v{current_version}" - if not click.confirm(f"Publish {tag_name}?", default=True): + if not click.confirm(f"Publish release {tag_name} on GitHub?", default=True): return # Publish the draft release From a6465623156731851d2568a39caf712d4d7d6b26 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 27 Apr 2022 12:19:23 +0100 Subject: [PATCH 8/9] Appease flake8 --- scripts-dev/release.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index efe5e5afe97a..b3f11935f26e 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -346,7 +346,7 @@ def publish(gh_token: str): """Publish release on GitHub.""" # Make sure we're in a git repo. - repo = get_repo() + get_repo() current_version = get_package_version() tag_name = f"v{current_version}" From 042dc354396290eb26eb62dabd1cf69bc83a29ff Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 27 Apr 2022 13:00:08 +0100 Subject: [PATCH 9/9] Better name for `get_repo` --- scripts-dev/release.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts-dev/release.py b/scripts-dev/release.py index b3f11935f26e..26e039ea99e6 100755 --- a/scripts-dev/release.py +++ b/scripts-dev/release.py @@ -87,7 +87,7 @@ def prepare(): """ # Make sure we're in a git repo. - repo = get_repo() + repo = get_repo_and_check_clean_checkout() click.secho("Updating git repo...") repo.remote().fetch() @@ -261,7 +261,7 @@ def tag(gh_token: Optional[str]): """Tags the release and generates a draft GitHub release""" # Make sure we're in a git repo. - repo = get_repo() + repo = get_repo_and_check_clean_checkout() click.secho("Updating git repo...") repo.remote().fetch() @@ -346,7 +346,7 @@ def publish(gh_token: str): """Publish release on GitHub.""" # Make sure we're in a git repo. - get_repo() + get_repo_and_check_clean_checkout() current_version = get_package_version() tag_name = f"v{current_version}" @@ -386,7 +386,7 @@ def upload(): tag_name = f"v{current_version}" # Check we have the right tag checked out. - repo = get_repo() + repo = get_repo_and_check_clean_checkout() tag = repo.tag(f"refs/tags/{tag_name}") if repo.head.commit != tag.commit: click.echo("Tag {tag_name} (tag.commit) is not currently checked out!") @@ -459,7 +459,7 @@ def get_release_branch_name(version_number: version.Version) -> str: return f"release-v{version_number.major}.{version_number.minor}" -def get_repo() -> git.Repo: +def get_repo_and_check_clean_checkout() -> git.Repo: """Get the project repo and check it's not got any uncommitted changes.""" try: repo = git.Repo()