This is an automated email from the ASF dual-hosted git repository.

imbajin pushed a commit to branch feat/oink-community-content
in repository https://gitbox.apache.org/repos/asf/hugegraph-doc.git

commit dca964670b4fa4ddc245fb8b8679e3a054ebf8ba
Author: dark <[email protected]>
AuthorDate: Fri Sep 4 20:14:58 2026 +0800

    fix(community): make roster the atomic commit point
    
    - leave the last-good roster selected on every pre-commit failure\n- retain 
verified content-addressed assets as harmless candidates\n- downgrade orphan 
cleanup failures to explicit warnings\n- inject atomic roster-write failure in 
regression tests
---
 scripts/community_roster.md      |  4 +++
 scripts/community_roster.py      | 59 +++++++++++++++-------------------------
 scripts/test_community_roster.py | 15 ++++++++++
 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/scripts/community_roster.md b/scripts/community_roster.md
index 195e1729c..d317dbc72 100644
--- a/scripts/community_roster.md
+++ b/scripts/community_roster.md
@@ -35,3 +35,7 @@ python3 scripts/community_roster.py validate \
 ```
 
 The artifact option never invokes Hugo or downloads modules itself.
+
+Once the verified assets and roster are published, removal of an unreferenced
+old avatar is best effort. A cleanup failure emits an Actions warning but does
+not invalidate or roll back the complete new bundle.
diff --git a/scripts/community_roster.py b/scripts/community_roster.py
index 57ee72554..d7e05410f 100644
--- a/scripts/community_roster.py
+++ b/scripts/community_roster.py
@@ -478,8 +478,7 @@ def _validate_avatar_blob(name: str, raw: bytes) -> None:
 
 
 def _commit_bundle(candidate: dict, candidate_avatars: dict[str, bytes]) -> 
None:
-    """Install one bundle or restore the exact prior roster/avatar state."""
-    old_roster = ROSTER_PATH.read_bytes() if ROSTER_PATH.exists() else None
+    """Install verified immutable assets, then atomically publish the 
roster."""
     AVATAR_DIR.mkdir(parents=True, exist_ok=True)
     referenced = {
         pathlib.PurePosixPath(person["avatar"]).name
@@ -488,43 +487,29 @@ def _commit_bundle(candidate: dict, candidate_avatars: 
dict[str, bytes]) -> None
         if person.get("avatar")
     }
     existing = {path.name: path for path in AVATAR_DIR.glob("*.webp")}
-    installed: list[pathlib.Path] = []
-    replaced: dict[pathlib.Path, bytes] = {}
-    try:
-        for name, avatar in sorted(candidate_avatars.items()):
-            _validate_avatar_blob(name, avatar)
-            destination = AVATAR_DIR / name
-            if destination.exists() or destination.is_symlink():
-                previous = destination.read_bytes()
-                try:
-                    if destination.is_symlink():
-                        raise RosterError(f"candidate destination is a 
symlink: {destination}")
-                    _validate_avatar_blob(name, previous)
-                    continue
-                except RosterError:
-                    replaced[destination] = previous
-            staged = AVATAR_DIR / f".{name}.candidate"
-            try:
-                _copy_candidate(avatar, staged)
-                os.replace(staged, destination)
-            finally:
-                if staged.exists():
-                    _unlink(staged)
-            if destination not in replaced:
-                installed.append(destination)
-        raw = (json.dumps(candidate, indent=2, ensure_ascii=False) + 
"\n").encode()
-        _atomic_write(ROSTER_PATH, raw)
-    except Exception:
-        if old_roster is not None:
-            _atomic_write(ROSTER_PATH, old_roster)
-        for path, raw in replaced.items():
-            _atomic_write(path, raw)
-        for path in installed:
+    for name, avatar in sorted(candidate_avatars.items()):
+        _validate_avatar_blob(name, avatar)
+        destination = AVATAR_DIR / name
+        if destination.exists() or destination.is_symlink():
             try:
-                _unlink(path)
-            except OSError:
+                if destination.is_symlink():
+                    raise RosterError(f"candidate destination is a symlink: 
{destination}")
+                _validate_avatar_blob(name, destination.read_bytes())
+                continue
+            except RosterError:
                 pass
-        raise
+        staged = AVATAR_DIR / f".{name}.candidate"
+        try:
+            _copy_candidate(avatar, staged)
+            os.replace(staged, destination)
+        finally:
+            if staged.exists():
+                _unlink(staged)
+    raw = (json.dumps(candidate, indent=2, ensure_ascii=False) + "\n").encode()
+    # This atomic replace is the commit point. Failures before it leave the
+    # last-good roster selected; installed content-addressed assets are safe
+    # unreferenced candidates.
+    _atomic_write(ROSTER_PATH, raw)
     for name in sorted(set(existing) - referenced):
         try:
             _unlink(AVATAR_DIR / name)
diff --git a/scripts/test_community_roster.py b/scripts/test_community_roster.py
index 33a9e7315..4101c72a9 100644
--- a/scripts/test_community_roster.py
+++ b/scripts/test_community_roster.py
@@ -299,6 +299,21 @@ print(json.dumps([person["asf_id"] for person in 
result["roles"]["pmc"]]))
                     roster.refresh()
             self.assertEqual(b"last-good\n", roster_path.read_bytes())
 
+    def test_atomic_roster_write_failure_keeps_last_good_selected(self):
+        with tempfile.TemporaryDirectory(prefix="community-write-test-") as 
directory:
+            root = pathlib.Path(directory)
+            roster_path, avatar_dir = root / "roster.json", root / "avatars"
+            avatar_dir.mkdir()
+            roster_path.write_bytes(b"last-good\n")
+            candidate = {"roles": {"pmc": [{"avatar": 
"/img/community/avatars/new.webp"}], "committers": []}}
+            with mock.patch.object(roster, "ROSTER_PATH", roster_path), \
+                 mock.patch.object(roster, "AVATAR_DIR", avatar_dir), \
+                 mock.patch.object(roster, "_validate_avatar_blob"), \
+                 mock.patch.object(roster, "_atomic_write", 
side_effect=OSError("write failed")):
+                with self.assertRaisesRegex(OSError, "write failed"):
+                    roster._commit_bundle(candidate, {"new.webp": b"new"})
+            self.assertEqual(b"last-good\n", roster_path.read_bytes())
+
     def test_orphan_unlink_failure_is_a_successful_commit_warning(self):
         with tempfile.TemporaryDirectory(prefix="community-unlink-test-") as 
directory:
             root = pathlib.Path(directory)

Reply via email to