From 44cffc5cb57b637c9dd37d58579692860d308eeb Mon Sep 17 00:00:00 2001 From: Loup-Garou911XD <90267658+Loup-Garou911XD@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:03:07 +0530 Subject: [PATCH] Make plugin metadata idempotent by comparing against the base branch auto_apply_plugin_metadata.py asked "what version is already released?" by reading the category manifest out of the PR's own working tree. Once ci-apply.yml has pushed its "[ci] apply-plugin-metadata-and-formatting" commit back to the PR branch, that tree already lists the version being added - so the check compared the new version against itself and raised "Version cant be lower or equal than the previous version." That fires on every re-run of PR Check: the one ci-apply.yml's own push triggers, and any run caused by a contributor pushing a follow-up commit. Published state now comes from the base branch instead, read with `git show $PLUGMAN_BASE_REF:` (ci-check.yml supplies the PR base sha; local runs fall back to origin/main, then to the working tree). The writer is idempotent to match: a version entry that is already stamped and still matches the plugin's md5sum is left alone rather than reset to null. An entry whose md5sum has drifted is still reset, which implements the TODO this replaces - a contributor can keep iterating on an unpublished version during review without bumping it every round. Bump enforcement against published versions is unchanged. Version comparison and ordering switch to the existing get_comparable_version_tuple_from_string; versioning_tools.semantic_to_str ordered 1.0.10 below 1.0.9 and silently truncated 3-digit components. --- .github/workflows/ci-check.yml | 7 ++ test/auto_apply_plugin_metadata.py | 142 ++++++++++++++++++++++------- 2 files changed, 115 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci-check.yml b/.github/workflows/ci-check.yml index 5eb96e3..ea4b8d8 100644 --- a/.github/workflows/ci-check.yml +++ b/.github/workflows/ci-check.yml @@ -79,7 +79,14 @@ jobs: < "${RUNNER_TEMP}/changed_py.z" fi + # PLUGMAN_BASE_REF tells the script which tree counts as "already + # published". Without it the script compares against the PR's own + # manifest, which ci-apply.yml has already written the new version into - + # so every re-run of this workflow (including the one ci-apply.yml's push + # triggers) would fail with "Version cant be lower or equal". - name: Apply Plugin Metadata (writes null version placeholders) + env: + PLUGMAN_BASE_REF: ${{ github.event.pull_request.base.sha }} run: | set -euo pipefail python test/auto_apply_plugin_metadata.py "$(cat "${RUNNER_TEMP}/changed_files.txt")" diff --git a/test/auto_apply_plugin_metadata.py b/test/auto_apply_plugin_metadata.py index 228e810..088e3cc 100644 --- a/test/auto_apply_plugin_metadata.py +++ b/test/auto_apply_plugin_metadata.py @@ -2,11 +2,20 @@ import sys import json import ast import os +import hashlib +import subprocess import get_latest -import versioning_tools +from auto_apply_version_metadata import get_comparable_version_tuple_from_string DEBUG = True +MANIFEST_PATHS = { + "minigames": "plugins/minigames.json", + "utilities": "plugins/utilities.json", + "maps": "plugins/maps.json", + "plugman": "index.json", +} + print("DOES THIS RUN AUTO APPLY PLUGIN METADATA?") @@ -15,23 +24,69 @@ def debug_print(*args, **kwargs): print(*args, **kwargs) -def get_latest_version(plugin_name, category) -> str: - filepaths = { - "minigames": "plugins/minigames.json", - "utilities": "plugins/utilities.json", - "maps": "plugins/maps.json", - "plugman": "index.json", - } +def version_key(version): + """Comparison/sort key for a version string, so 1.0.10 sorts above 1.0.9.""" + try: + return get_comparable_version_tuple_from_string(version) + except ValueError: + raise ValueError(f"Version {version!r} is not in x.y.z form.") + +def md5sum_of(path): + with open(path, "rb") as fin: + return hashlib.md5(fin.read()).hexdigest() + + +def get_latest_version(plugin_name, category) -> str: try: if category != "plugman": - return get_latest.get_latest_plugin_version(plugin_name, filepaths[category]) + return get_latest.get_latest_plugin_version(plugin_name, MANIFEST_PATHS[category]) return get_latest.get_latest_plugman_version() except Exception as e: raise e +def read_manifest_at(path, ref): + """Load a manifest as it exists at `ref`, or None if it can't be read there.""" + try: + blob = subprocess.run( + ["git", "show", f"{ref}:{path}"], + capture_output=True, + check=True, + ).stdout + return json.loads(blob) + except (OSError, subprocess.CalledProcessError, json.JSONDecodeError): + return None + + +def get_published_versions(plugin_name, category): + """Versions of `plugin_name` that are already published on the PR's base branch. + + The working tree cannot answer this. Once ci-apply.yml has pushed its + "[ci] apply-plugin-metadata-and-formatting" commit back to the PR branch, + the PR's own copy of the manifest already lists the version being added - + so comparing against the working tree rejects every re-run of PR Check, + including the one ci-apply.yml's own push triggers, and every re-run caused + by a contributor pushing a follow-up commit. + + PLUGMAN_BASE_REF is set by ci-check.yml to the PR's base sha. Local runs + fall back to origin/main, then to the working tree when there is no history + to consult at all. + """ + path = MANIFEST_PATHS[category] + manifest = None + for ref in (os.environ.get("PLUGMAN_BASE_REF"), "origin/main", "main"): + if ref: + manifest = read_manifest_at(path, ref) + if manifest is not None: + break + if manifest is None: + with open(path, "r") as file: + manifest = json.load(file) + return manifest["plugins"].get(plugin_name, {}).get("versions", {}) + + def update_plugman_json(version): with open("index.json", "r+") as file: data = json.load(file) @@ -45,39 +100,58 @@ def update_plugman_json(version): data["versions"] = dict(sorted(data["versions"].items(), reverse=True)) -def update_plugin_json(plugin_info, category): +def update_plugin_json(plugin_info, category, plugin_path): name = plugin_info["plugin_name"] + version = plugin_info["version"] + + # Ensure the version is always greater than the already PUBLISHED version - + # what is on the base branch, not what this PR's own tree happens to say. + published = get_published_versions(name, category) + if published: + latest_published = max(published, key=version_key) + if version_key(version) <= version_key(latest_published): + raise Exception( + "Version cant be lower or equal than the previous version. " + f"{name} {latest_published} is already published; bump the version " + f"in its plugman dict (currently {version})." + ) with open(f"plugins/{category}.json", "r+") as file: data = json.load(file) - try: - # Check if plugin is already in the json - plugin = data["plugins"][name] - plugman_version = int( - versioning_tools.semantic_to_str(get_latest_version(name, category)) - ) - current_version = int(versioning_tools.semantic_to_str(plugin_info["version"])) - # Ensure the version is always greater from the already released version - if current_version > plugman_version: - plugin["versions"][plugin_info["version"]] = None - # Ensure latest version appears first - plugin["versions"] = dict(sorted(plugin["versions"].items(), reverse=True)) - plugin["description"] = plugin_info["description"] - plugin["external_url"] = plugin_info["external_url"] - plugin["authors"] = plugin_info["authors"] - # In future - # We can clear the version metadata for if current_version = plugman_version for PRs - # So that its easier to update the plugin after review without changing version - elif current_version <= plugman_version: - raise Exception("Version cant be lower or equal than the previous version.") - except KeyError: - data["plugins"][name] = { + plugin = data["plugins"].get(name) + if plugin is None: + # New plugin. Key order here is the shape every other entry has. + plugin = data["plugins"][name] = { "description": plugin_info["description"], "external_url": plugin_info["external_url"], "authors": plugin_info["authors"], - "versions": {plugin_info["version"]: None}, + "versions": {}, } + versions = plugin["versions"] + stamped = versions.get(version) + # A null placeholder is only (re)written when there is something for + # auto_apply_version_metadata.py to stamp, which keeps re-runs over an + # already-processed tree a no-op - otherwise ci-apply.yml's push would + # trigger a PR Check that undoes the stamp, forever. + # + # A stamped entry whose md5sum no longer matches the file means the + # contributor pushed further edits under the same UNPUBLISHED version + # (the published case raised above). Reset it so the stamp is + # recomputed, rather than demanding a bump for every review iteration. + if version not in versions or ( + isinstance(stamped, dict) and stamped.get("md5sum") != md5sum_of(plugin_path) + ): + versions[version] = None + + # Ensure latest version appears first + plugin["versions"] = dict( + sorted(versions.items(), key=lambda item: version_key(item[0]), reverse=True) + ) + plugin["description"] = plugin_info["description"] + plugin["external_url"] = plugin_info["external_url"] + plugin["authors"] = plugin_info["authors"] + file.seek(0) json.dump(data, file, indent=2, ensure_ascii=False) # Ensure old content is removed @@ -132,7 +206,7 @@ def extract_plugman(plugins): ) result[kw.arg] = ast.literal_eval(kw.value) if category: - update_plugin_json(result, category=category) + update_plugin_json(result, category=category, plugin_path=plugin) else: update_plugman_json(result) # raise ValueError("Variable plugman not found in the file or has unsupported format.")