Write directly to relevant notes file in changelog.py (#19303)

This updates `changelog.py` to write the (non-internal) changes to the
relevant release notes file directly, rather than requiring a human to
copy-paste it in. For now, the file is just mutated, without committing.
The internal changes are still printed for the human to deal with.

For instance, `pants run src/python/pants_release/changelog.py --
--prior 2.18.0.dev1 --new 2.18.0.dev2` adds a new section to
`src/python/pants/notes/2.18.x.md`:

```diff
diff --git a/src/python/pants/notes/2.18.x.md b/src/python/pants/notes/2.18.x.md
index e648a4525c..d6668a24b1 100644
--- a/src/python/pants/notes/2.18.x.md
+++ b/src/python/pants/notes/2.18.x.md
@@ -1,5 +1,35 @@
 # 2.18.x Release Series
 
+## 2.18.0.dev2 (Jun 14, 2023)
+
+### New Features
+
+* Include complete platforms for FaaS environments for more reliable building ([#19253](#19253))
+
+* Add experimental support for Rustfmt ([#18842](#18842))
+
+* Helm deployment chart field ([#19234](#19234))
+
+### Plugin API Changes
+
+* Replace `include_special_cased_deps` flag with `should_traverse_deps_predicate` ([#19272](#19272))
+
+### Bug Fixes
+
+* Raise an error if isort can't read a config file ([#19294](#19294))
+
+* Improve handling of additional files in Helm unit tests ([#19263](#19263))
+
+* Add taplo to the release ([#19258](#19258))
+
+* Handle `from foo import *` wildcard imports in Rust dep inference parser ([#19249](#19249))
+
+* Support usage of `scala_artifact` addresses in `scalac_plugin` targets ([#19205](#19205))
+
+### Performance
+
+* `scandir` returns `Stat`s relative to its directory. ([#19246](#19246))
+
 ## 2.18.0.dev1 (Jun 02, 2023)
 
 ### New Features
```

This also moves it into the new `pants_release` root, adds some basic
tests, and has it fetch the relevant branch directly, rather than
prompting the user to do so. It also pulls out a `git.py` support module
with helpers for exec-ing `git` as an external process.

The commits are individually reviewable.

This is a step towards automating more of the "start release" process,
per #19279. After this
core refactoring of the functionality, I think the next step is to
combine it with the CONTRIBUTORS update and version bumping, and then
after that string it all together on CI so that starting a release is
fully automated.

docs/markdown/Contributions/releases/release-process.md

@@ -87,9 +87,9 @@ Update the release page in `src/python/pants/notes` for this release series, e.g
 
 Run `git fetch --all --tags` to be sure you have the latest release tags available locally.
 
-From the `main` branch, run `./pants run build-support/bin/changelog.py -- --prior 2.9.0.dev0 --new 2.9.0.dev1` with the relevant versions.
+From the `main` branch, run `./pants run src/python/pants_release/changelog.py -- --prior 2.9.0.dev0 --new 2.9.0.dev1` with the relevant versions.
 
-This will generate the sections to copy into the release notes. Delete any empty sections. Do not paste the `Internal` section into the notes file. Instead, paste into a comment on the prep PR.
+This will update the release notes file, for you to tweak and commit. The script also prints an `Internal` section to paste into a comment on the prep PR.
 
 You are encouraged to fix typos and tweak change descriptions for clarity to users. Ensure that there is exactly one blank line between descriptions, headers etc.
 

build-support/bin/changelog.py → src/python/pants_release/changelog.py

@@ -8,16 +8,15 @@
 import datetime
 import logging
 import re
-import subprocess
 import sys
 from collections import defaultdict
 from dataclasses import dataclass
 from enum import Enum
-from textwrap import dedent
+from pathlib import Path
 
 import requests
 from packaging.version import Version
-from pants_release.common import die
+from pants_release.git import git, git_fetch
 
 logger = logging.getLogger(__name__)
 
@@ -27,47 +26,43 @@ def create_parser() -> argparse.ArgumentParser:
     parser.add_argument(
         "--prior",
         required=True,
-        type=str,
+        type=Version,
         help="The version of the prior release, e.g. `2.0.0.dev0` or `2.0.0rc1`.",
     )
     parser.add_argument(
         "--new",
         required=True,
-        type=str,
+        type=Version,
         help="The version for the new release, e.g. `2.0.0.dev1` or `2.0.0rc2`.",
     )
     return parser
 
 
-def determine_release_branch(new_version_str: str) -> str:
-    new_version = Version(new_version_str)
-    # Use the main branch for all dev releases, and for the first alpha (which creates a stable branch).
-    use_main_branch = new_version.is_devrelease or (
-        new_version.pre
-        and "a0" == "".join(str(p) for p in new_version.pre)
-        and new_version.micro == 0
-    )
-    release_branch = "main" if use_main_branch else f"{new_version.major}.{new_version.minor}.x"
-    branch_confirmation = input(
-        f"Have you recently pulled from upstream on the branch `{release_branch}`? "
-        "This is needed to ensure the changelog is exhaustive. [Y/n]"
-    )
-    if branch_confirmation and branch_confirmation.lower() != "y":
-        die(f"Please checkout to the branch `{release_branch}` and pull from upstream. ")
-    return release_branch
+@dataclass(frozen=True)
+class ReleaseInfo:
+    version: Version
+    slug: str
+    branch: str
+
+    @staticmethod
+    def determine(new_version: Version) -> ReleaseInfo:
+        slug = f"{new_version.major}.{new_version.minor}.x"
+        # Use the main branch for all dev releases, and for the first alpha (which creates a stable branch).
+        use_main_branch = new_version.is_devrelease or (
+            new_version.pre
+            and "a0" == "".join(str(p) for p in new_version.pre)
+            and new_version.micro == 0
+        )
+        branch = "main" if use_main_branch else slug
+        return ReleaseInfo(version=new_version, slug=slug, branch=branch)
+
+    def notes_file_name(self) -> Path:
+        return Path(f"src/python/pants/notes/{self.slug}.md")
 
 
-def relevant_shas(prior: str, release_branch: str) -> list[str]:
+def relevant_shas(prior: Version, release_ref: str) -> list[str]:
     prior_tag = f"release_{prior}"
-    return (
-        subprocess.run(
-            ["git", "log", "--format=format:%H", release_branch, f"^{prior_tag}"],
-            check=True,
-            stdout=subprocess.PIPE,
-        )
-        .stdout.decode()
-        .splitlines()
-    )
+    return git("log", "--format=format:%H", release_ref, f"^{prior_tag}").splitlines()
 
 
 class Category(Enum):
@@ -131,15 +126,7 @@ def complete_categorization(category: Category | str) -> Category | None:
 
 
 def prepare_sha(sha: str) -> Entry:
-    subject = (
-        subprocess.run(
-            ["git", "log", "-1", "--format=format:%s", sha],
-            check=True,
-            stdout=subprocess.PIPE,
-        )
-        .stdout.decode()
-        .strip()
-    )
+    subject = git("log", "-1", "--format=format:%s", sha)
     pr_num_match = re.search(r"\(#(\d{4,5})\)\s*$", subject)
     if not pr_num_match:
         return Entry(category=None, text=f"* {subject}")
@@ -150,11 +137,13 @@ def prepare_sha(sha: str) -> Entry:
     return Entry(category=category, text=f"* {subject_with_url}")
 
 
-def instructions(new_version: str, entries: list[Entry]) -> str:
-    date = datetime.date.today().strftime("%b %d, %Y")
-    version_components = new_version.split(".", maxsplit=4)
-    major, minor = version_components[0], version_components[1]
+@dataclass(frozen=True)
+class Formatted:
+    external: str
+    internal: str
 
+
+def format_notes(release_info: ReleaseInfo, entries: list[Entry], date: datetime.date) -> Formatted:
     entries_by_category = defaultdict(list)
     for entry in entries:
         entries_by_category[entry.category].append(entry.text)
@@ -165,42 +154,65 @@ def format_entries(category: Category | None) -> str:
         lines = "\n\n".join(entries)
         if not entries:
             return ""
-        return f"\n### {heading}\n\n{lines}\n"
-
-    return dedent(
-        f"""\
-        Copy the below headers into `src/python/pants/notes/{major}.{minor}.x.md`. Then, put each
-        external-facing commit into the relevant category. Commits that are internal-only (i.e.,
-        that are only of interest to Pants developers and have no user-facing implications) should
-        be pasted into a PR comment for review, not the release notes.
-
-        You can tweak descriptions to be more descriptive or to fix typos, and you can reorder
-        based on relative importance to end users. Delete any unused headers.
-
-        ---------------------------------------------------------------------
-
-        ## {new_version} ({date})
-        {{new_features}}{{user_api_changes}}{{plugin_api_changes}}{{bugfixes}}{{performance}}{{documentation}}{{internal}}
-        --------------------------------------------------------------------
-        {{uncategorized}}
-        """
-    ).format(
-        new_features=format_entries(Category.NewFeatures),
-        user_api_changes=format_entries(Category.UserAPIChanges),
-        plugin_api_changes=format_entries(Category.PluginAPIChanges),
-        bugfixes=format_entries(Category.BugFixes),
-        performance=format_entries(Category.Performance),
-        documentation=format_entries(Category.Documentation),
-        internal=format_entries(Category.Internal),
-        uncategorized=format_entries(None),
+        return f"### {heading}\n\n{lines}"
+
+    external_categories = [
+        Category.NewFeatures,
+        Category.UserAPIChanges,
+        Category.PluginAPIChanges,
+        Category.BugFixes,
+        Category.Performance,
+        Category.Documentation,
+        # ensure uncategorized entries appear
+        None,
+    ]
+
+    external = "\n\n".join(
+        [
+            f"## {release_info.version} ({date:%b %d, %Y})",
+            *(
+                formatted
+                for category in external_categories
+                if (formatted := format_entries(category))
+            ),
+        ]
     )
+    internal = format_entries(Category.Internal)
+
+    return Formatted(external=external, internal=internal)
+
+
+def splice(existing_contents: str, new_section: str) -> str:
+    # Find the first `## 2.minor...` heading, to be able to insert immediately before it, or the end
+    # of file, if not such section exists
+    try:
+        index = existing_contents.index("\n## 2.")
+    except ValueError:
+        index = len(existing_contents)
+    return "".join([existing_contents[:index], "\n", new_section, "\n", existing_contents[index:]])
+
+
+def splice_into_file(release_info: ReleaseInfo, formatted: Formatted) -> None:
+    file_name = release_info.notes_file_name()
+    try:
+        existing_contents = file_name.read_text()
+    except FileNotFoundError:
+        # default content if the file doesn't exist yet
+        existing_contents = f"# {release_info.slug} Release Series\n"
+
+    file_name.write_text(splice(existing_contents, formatted.external))
 
 
 def main() -> None:
     args = create_parser().parse_args()
-    release_branch = determine_release_branch(args.new)
-    entries = [prepare_sha(sha) for sha in relevant_shas(args.prior, release_branch)]
-    print(instructions(args.new, entries))
+    release_info = ReleaseInfo.determine(args.new)
+    branch_sha = git_fetch(release_info.branch)
+    date = datetime.date.today()
+    entries = [prepare_sha(sha) for sha in relevant_shas(args.prior, branch_sha)]
+
+    formatted = format_notes(release_info, entries, date)
+    splice_into_file(release_info, formatted)
+    print(f"\nCommit {release_info.notes_file_name()} and create a PR.\n\n{formatted.internal}")
 
 
 if __name__ == "__main__":

src/python/pants_release/changelog_test.py

@@ -0,0 +1,105 @@
+# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from __future__ import annotations
+
+import datetime
+
+import pytest
+from packaging.version import Version
+from pants_release.changelog import Category, Entry, ReleaseInfo, format_notes, splice
+
+
+@pytest.mark.parametrize(
+    ("raw_version", "slug", "branch"),
+    [
+        ("2.0.0.dev0", "2.0.x", "main"),
+        ("2.0.0.dev1", "2.0.x", "main"),
+        ("2.0.0a0", "2.0.x", "main"),
+        ("2.0.0a1", "2.0.x", "2.0.x"),
+        ("2.0.0rc0", "2.0.x", "2.0.x"),
+        ("2.0.0rc1", "2.0.x", "2.0.x"),
+        ("2.0.0", "2.0.x", "2.0.x"),
+        ("2.0.1a0", "2.0.x", "2.0.x"),
+        ("2.1234.5678.dev0", "2.1234.x", "main"),
+        ("2.1234.5678.a0", "2.1234.x", "2.1234.x"),
+        ("2.1234.5678.a1", "2.1234.x", "2.1234.x"),
+        ("2.1234.5678rc0", "2.1234.x", "2.1234.x"),
+        ("2.1234.5678", "2.1234.x", "2.1234.x"),
+    ],
+)
+def test_releaseinfo_determine(raw_version: str, slug: str, branch: str) -> None:
+    version = Version(raw_version)
+    expected = ReleaseInfo(version=version, slug=slug, branch=branch)
+
+    computed = ReleaseInfo.determine(version)
+    assert computed == expected
+
+
+@pytest.mark.parametrize("category", [*(c for c in Category if c is not Category.Internal), None])
+def test_format_notes_external(category: None | Category) -> None:
+    release_info = ReleaseInfo(version=Version("2.1234.0.dev0"), slug="2.1234.x", branch="main")
+    entries = [Entry(category=category, text="some entry")]
+    date = datetime.date(9999, 8, 7)
+    heading = "Uncategorized" if category is None else category.heading()
+
+    formatted = format_notes(release_info, entries, date)
+
+    assert formatted.internal == ""
+    # we're testing the exact formatting, so no softwrap/dedent:
+    assert (
+        formatted.external
+        == f"""\
+## 2.1234.0.dev0 (Aug 07, 9999)
+
+### {heading}
+
+some entry"""
+    )
+
+
+def test_format_notes_internal() -> None:
+    release_info = ReleaseInfo(version=Version("2.1234.0.dev0"), slug="2.1234.x", branch="main")
+    entries = [Entry(category=Category.Internal, text="some entry")]
+    date = datetime.date(9999, 8, 7)
+
+    formatted = format_notes(release_info, entries, date)
+
+    assert formatted.external == "## 2.1234.0.dev0 (Aug 07, 9999)"
+    # we're testing the exact formatting, so no softwrap/dedent:
+    assert (
+        formatted.internal
+        == """\
+### Internal (put these in a PR comment for review, not the release notes)
+
+some entry"""
+    )
+
+
+@pytest.mark.parametrize(
+    ("existing_contents", "expected"),
+    [
+        pytest.param(
+            "# 2.1234.x Release series\n",
+            "# 2.1234.x Release series\n\nNEW SECTION\n",
+            id="defaults to end of file",
+        ),
+        pytest.param(
+            "# 2.1234.x Release series\n\n## 2.1234.5678rc9\nEXISTING1## 2.1234.0.dev0\nEXISTING2",
+            "# 2.1234.x Release series\n\nNEW SECTION\n\n## 2.1234.5678rc9\nEXISTING1## 2.1234.0.dev0\nEXISTING2",
+            id="finds the first release-like section",
+        ),
+        pytest.param(
+            "# 2.1234.x Release series\n\n## What's new\n\n---\n\n## 2.1234.5678rc9\nEXISTING1",
+            "# 2.1234.x Release series\n\n## What's new\n\n---\n\nNEW SECTION\n\n## 2.1234.5678rc9\nEXISTING1",
+            id="ignores 'What's new'",
+        ),
+        pytest.param(
+            "# 2.1234.x Release series\n\n### 2.1234\nEXISTING\n",
+            "# 2.1234.x Release series\n\n### 2.1234\nEXISTING\n\nNEW SECTION\n",
+            id="ignores unexpected heading depth",
+        ),
+    ],
+)
+def test_splice(existing_contents: str, expected: str) -> None:
+    assert splice(existing_contents, "NEW SECTION") == expected

src/python/pants_release/common.py

@@ -19,7 +19,6 @@
 """
 from __future__ import annotations
 
-import subprocess
 import time
 from typing import NoReturn
 
@@ -48,17 +47,3 @@ def elapsed_time() -> tuple[int, int]:
     now = time.time()
     elapsed_seconds = int(now - _SCRIPT_START_TIME)
     return elapsed_seconds // 60, elapsed_seconds % 60
-
-
-def git_merge_base() -> str:
-    get_tracking_branch = [
-        "git",
-        "rev-parse",
-        "--symbolic-full-name",
-        "--abbrev-ref",
-        "HEAD@{upstream}",
-    ]
-    process = subprocess.run(
-        get_tracking_branch, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8"
-    )
-    return str(process.stdout.rstrip()) if process.stdout else "main"