From fb3e6b588242a8a0b6dfa410d60a9f70f905dbed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:04:05 +0000 Subject: [PATCH 1/2] Initial plan From f915b82f7d7f02fa78617736d8260a4ded2f7f3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 11 Sep 2026 11:13:03 +0000 Subject: [PATCH 2/2] Implement canonical/filename changelog area mapping Co-authored-by: phlax <454682+phlax@users.noreply.github.com> --- .../base/utils/abstract/project/changelog.py | 16 ++++- .../tests/test_abstract_project_changelogs.py | 2 + py/envoy.code.check/README.rst | 13 ++++ .../envoy/code/check/abstract/changelog.py | 49 +++++++++++-- .../tests/test_abstract_changelog.py | 69 +++++++++++++++++-- 5 files changed, 136 insertions(+), 13 deletions(-) diff --git a/py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py b/py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py index 730d9ae991..06afa3864e 100644 --- a/py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py +++ b/py/envoy.base.utils/envoy/base/utils/abstract/project/changelog.py @@ -32,6 +32,8 @@ CHANGELOG_ENTRY_GLOB = "*/*.rst" CHANGELOG_CONFIG_PATH = "changelogs/changelogs.yaml" ENTRY_SEPARATOR = "__" +AREA_SEPARATOR = "/" +AREA_FILENAME_SEPARATOR = "~" CHANGELOG_SUMMARY_PATH = "changelogs/summary.md" CHANGELOG_ENTRY_URL_TPL = ( "https://raw.githubusercontent.com/envoyproxy/envoy/" @@ -53,6 +55,14 @@ YAML_CHANGELOGS_VERSION = "1.23" +def area_from_filename(stem_area: str) -> str: + return stem_area.replace(AREA_FILENAME_SEPARATOR, AREA_SEPARATOR) + + +def area_to_filename(area: str) -> str: + return area.replace(AREA_SEPARATOR, AREA_FILENAME_SEPARATOR) + + class LegacyChangelog: # Parser for changelogs < 1.23.0 @@ -138,9 +148,11 @@ def data_from_entry_map( f"Invalid entry filename " f"(expected exactly one '{ENTRY_SEPARATOR}'): " f"{entry_path}") - area, _slug = path.stem.split(ENTRY_SEPARATOR, 1) + stem_area, _slug = path.stem.split(ENTRY_SEPARATOR, 1) change = typing.Change(text) - entry: typing.ChangeDict = dict(area=area, change=change) + entry: typing.ChangeDict = dict( + area=area_from_filename(stem_area), + change=change) sections.setdefault(path.parent.name, []).append(entry) return cast( typing.ChangelogDict, diff --git a/py/envoy.base.utils/tests/test_abstract_project_changelogs.py b/py/envoy.base.utils/tests/test_abstract_project_changelogs.py index 25c22f4289..f90e5082a9 100644 --- a/py/envoy.base.utils/tests/test_abstract_project_changelogs.py +++ b/py/envoy.base.utils/tests/test_abstract_project_changelogs.py @@ -1419,6 +1419,7 @@ def test_abstract_changelog_get_data(iters, patches, raises): def test_abstract_changelog_data_from_entry_map_happy_path(): entries = { "new_features/grpc__cool.rst": "New feature.\n", + "new_features/dns~cares__preserve-qid.rst": "Preserve qid.\n", "bug_fixes/oauth2__foo_fix.rst": "Fixed oauth2.\n", "bug_fixes/jwt__bar_fix.rst": "Fixed jwt.\n"} @@ -1430,6 +1431,7 @@ def test_abstract_changelog_data_from_entry_map_happy_path(): dict(area="jwt", change="Fixed jwt.\n"), dict(area="oauth2", change="Fixed oauth2.\n")], new_features=[ + dict(area="dns/cares", change="Preserve qid.\n"), dict(area="grpc", change="New feature.\n")])) diff --git a/py/envoy.code.check/README.rst b/py/envoy.code.check/README.rst index f9757c1ece..46052374a3 100644 --- a/py/envoy.code.check/README.rst +++ b/py/envoy.code.check/README.rst @@ -39,6 +39,19 @@ Run only against files changed since a given ref: See ``envoy.code.check --help`` for the full set of options. +Changelog entry areas +--------------------- + +Changelog area IDs in ``changelogs/changelogs.yaml`` are canonical and may use +``/`` to represent hierarchy (for example ``dns/cares``). Because ``/`` cannot +be used in entry filenames, changelog entries encode ``/`` as ``~`` in +``changelogs/current/
/__.rst`` (for example +``dns~cares__preserve-qid.rst``). + +Area keys must match ``[a-z0-9_\\-/]+`` (so ``~`` is not valid in keys). Area +titles must not contain ``~``. If an area key contains ``/``, it must define an +explicit non-empty ``title``. + Links ----- diff --git a/py/envoy.code.check/envoy/code/check/abstract/changelog.py b/py/envoy.code.check/envoy/code/check/abstract/changelog.py index 88cf9d5f9b..90824e48eb 100644 --- a/py/envoy.code.check/envoy/code/check/abstract/changelog.py +++ b/py/envoy.code.check/envoy/code/check/abstract/changelog.py @@ -20,14 +20,36 @@ ) from envoy.code.check import abstract, interface +try: + from envoy.base.utils.abstract.project.changelog import ( + AREA_FILENAME_SEPARATOR, + AREA_SEPARATOR, + area_from_filename, + ) +except ImportError: + AREA_SEPARATOR = "/" + AREA_FILENAME_SEPARATOR = "~" + + def area_from_filename(stem_area: str) -> str: + return stem_area.replace(AREA_FILENAME_SEPARATOR, AREA_SEPARATOR) + MAX_VERSION_FOR_CHANGES_SECTION = "1.16" VALID_CHANGELOG_AREA_RE = re.compile(r"^[a-z0-9_\-/]+$") VALID_CHANGELOG_AREA_PATTERN = r"[a-z0-9_\-/]+" +VALID_CHANGELOG_ENTRY_AREA_RE = re.compile(r"^[a-z0-9_\-~]+$") +VALID_CHANGELOG_ENTRY_AREA_PATTERN = r"[a-z0-9_\-~]+" @abstracts.implementer(interface.IChangelogChangesChecker) class AChangelogChangesChecker(metaclass=abstracts.Abstraction): + """Changelog checker. + + Changelog area IDs are canonicalized with ``/`` for hierarchy in + ``changelogs/changelogs.yaml`` while changelog entry filenames + encode ``/`` as ``~`` in ``__.rst``. + """ + error_message = ( "{version}/{section}/{entry[area]}: " "{error}\n{entry[change]}") @@ -127,12 +149,21 @@ def check_entry_filename( f"{path}: Filename stem must contain exactly one " f"`{ENTRY_SEPARATOR}` separator " f"(expected `{ENTRY_SEPARATOR}`)") - area, slug = path.stem.split(ENTRY_SEPARATOR, 1) - if not area: + stem_area, slug = path.stem.split(ENTRY_SEPARATOR, 1) + if not stem_area: return f"{path}: Area part of filename is empty" + if not VALID_CHANGELOG_ENTRY_AREA_RE.match(stem_area): + return ( + f"{path}: Invalid area '{stem_area}' " + f"(must match {VALID_CHANGELOG_ENTRY_AREA_PATTERN})") + area = area_from_filename(stem_area) if self.areas and area not in self.areas: + area_display = ( + f"'{area}' (from filename '{stem_area}')" + if area != stem_area + else f"'{area}'") return ( - f"{path}: Invalid area '{area}'. " + f"{path}: Invalid area {area_display}. " f"Valid areas come from {self.config_path}") if not slug: return f"{path}: Slug part of filename is empty" @@ -144,13 +175,23 @@ def check_areas_file(self) -> tuple[str, ...]: title_areas: dict[str, list[str]] = {} errors = [] for area, area_data in self.areas.items(): - title = area_data["title"] + title = area_data.get("title", "") title_areas.setdefault(title, []).append(area) if not VALID_CHANGELOG_AREA_RE.match(area): errors.append( f"{self.config_path}: " f"Invalid area key '{area}' " f"(must match {VALID_CHANGELOG_AREA_PATTERN})") + if AREA_FILENAME_SEPARATOR in title: + errors.append( + f"{self.config_path}: " + "Title must not contain '~' " + "(it is only a filename escape for '/')") + if AREA_SEPARATOR in area and not title.strip(): + errors.append( + f"{self.config_path}: " + f"Area '{area}' contains '/' and must set an explicit " + "title") if not VALID_CHANGELOG_AREA_RE.match(title): errors.append( f"{self.config_path}: " diff --git a/py/envoy.code.check/tests/test_abstract_changelog.py b/py/envoy.code.check/tests/test_abstract_changelog.py index d09d9b19bd..5096a1aca6 100644 --- a/py/envoy.code.check/tests/test_abstract_changelog.py +++ b/py/envoy.code.check/tests/test_abstract_changelog.py @@ -890,6 +890,9 @@ def test_changeschecker_check_section_name( [( # valid "bug_fixes", "myarea__myslug", ".rst", None), + ( # valid encoded nested area + "bug_fixes", "dns~cares__myslug", ".rst", + None), ( # invalid section "weird_section", "area__slug", ".rst", ("weird_section", "Invalid section")), @@ -907,13 +910,17 @@ def test_changeschecker_check_section_name( ("Area", "empty")), ( # empty slug "bug_fixes", "area__", ".rst", - ("Slug", "empty"))]) + ("Slug", "empty")), + ( # invalid filename area chars + "bug_fixes", "bad.area__slug", ".rst", + (r"[a-z0-9_\-~]+", ))]) def test_changeschecker_check_entry_filename( section, stem, suffix, expected): changelog = DummyChangelogChangesChecker( {"bug_fixes": MagicMock()}, {"myarea": {"title": "myarea"}, - "area": {"title": "area"}}) + "area": {"title": "area"}, + "dns/cares": {"title": "dns/cares"}}) path = MagicMock() path.parent.name = section path.stem = stem @@ -935,13 +942,13 @@ def test_changeschecker_check_entry_filename_invalid_area(): {"some_other_area": {"title": "some_other_area"}}) path = MagicMock() path.parent.name = "bug_fixes" - path.stem = "myarea__myslug" + path.stem = "dns~cares__myslug" path.suffix = ".rst" result = changelog.check_entry_filename(path) assert result is not None - assert "Invalid area" in result + assert "Invalid area 'dns/cares' (from filename 'dns~cares')" in result assert "changelogs.yaml" in result @@ -955,6 +962,21 @@ def test_changeschecker_check_entry_filename_without_areas(): assert changelog.check_entry_filename(path) is None +def test_changeschecker_check_entry_filename_decoded_area_mismatch(): + changelog = DummyChangelogChangesChecker( + {"bug_fixes": MagicMock()}, + {"dns/cares": {"title": "dns/cares"}}) + path = MagicMock() + path.parent.name = "bug_fixes" + path.stem = "dns_cares__myslug" + path.suffix = ".rst" + + result = changelog.check_entry_filename(path) + + assert result is not None + assert "Invalid area 'dns_cares'" in result + + def test_changeschecker_check_areas_file_empty(): changelog = DummyChangelogChangesChecker("SECTIONS", {}) assert changelog.check_areas_file() == () @@ -992,17 +1014,45 @@ def test_changeschecker_check_areas_file_invalid_keys(): changelog = DummyChangelogChangesChecker( "SECTIONS", {"Bad": {"title": "bad"}, - "bad.dot": {"title": "also_bad"}}) + "dns~cares": {"title": "dns/cares"}}) assert ( changelog.check_areas_file() == ( "changelogs/changelogs.yaml: Invalid area key 'Bad' " "(must match [a-z0-9_\\-/]+)", - "changelogs/changelogs.yaml: Invalid area key 'bad.dot' " + "changelogs/changelogs.yaml: Invalid area key 'dns~cares' " "(must match [a-z0-9_\\-/]+)", )) +def test_changeschecker_check_areas_file_tilde_in_title(): + changelog = DummyChangelogChangesChecker( + "SECTIONS", + {"dns/cares": {"title": "dns~cares"}}) + assert ( + changelog.check_areas_file() + == ( + "changelogs/changelogs.yaml: Title must not contain '~' " + "(it is only a filename escape for '/')", + "changelogs/changelogs.yaml: Invalid title 'dns~cares' " + "for area 'dns/cares' (must match [a-z0-9_\\-/]+)", + )) + + +def test_changeschecker_check_areas_file_slash_key_requires_title(): + changelog = DummyChangelogChangesChecker( + "SECTIONS", + {"dns/cares": {"title": ""}}) + assert ( + changelog.check_areas_file() + == ( + "changelogs/changelogs.yaml: Area 'dns/cares' contains '/' " + "and must set an explicit title", + "changelogs/changelogs.yaml: Invalid title '' " + "for area 'dns/cares' (must match [a-z0-9_\\-/]+)", + )) + + def test_changeschecker_check_areas_file_valid(): changelog = DummyChangelogChangesChecker( "SECTIONS", @@ -1161,9 +1211,13 @@ async def test_changelogstatus_errors_invalid_area_from_entry_files(tmp_path): " title: Bug fixes\n" "areas:\n" " known:\n" - " title: known\n") + " title: known\n" + " dns/cares:\n" + " title: dns/cares\n") current_dir = tmp_path / "changelogs" / "current" (current_dir / "bug_fixes").mkdir(parents=True) + (current_dir / "bug_fixes" / "dns~cares__slug.rst").write_text( + "Valid nested area content.") (current_dir / "bug_fixes" / "unknown__slug.rst").write_text( "Valid content.") config = utils.from_yaml(config_path, utils.typing.ChangelogConfigDict) @@ -1192,3 +1246,4 @@ async def _execute(func, *args): result = await status.errors assert any("Invalid area 'unknown'" in error for error in result) + assert not any("dns/cares" in error for error in result)