release freeze: restore only the <build> element on unfreeze

docs/PERCONA_OBS_TOOL.md

@@ -1469,9 +1469,12 @@ Flags:
    5. **Verify** — still inside the freeze window, polls (bounded by
       `--verify-timeout`) until each released repo holds binaries.
    6. **Restore** — in a `finally` (so it still runs even if verify times out or
-      raises), re-apply each project's exact snapshotted meta last (never a blanket
-      enable, since subprojects carry per-repo flags — e.g. tarballs' `publish:`
-      flags — that must survive the round trip).
+      raises), put back each project's snapshotted `<build>` element last (never a
+      blanket enable, since subprojects carry per-repo build flags that must survive
+      the round trip). Only `<build>` is restored, onto the meta as it is now: changes
+      made inside the freeze window (the hop's own release targets, added and removed
+      around `osc release`) are kept, not rolled back to the pre-freeze snapshot. If the
+      current meta cannot be read, the full snapshot is restored instead.
 5. Release-tier OBS projects with no local mirror are reported loudly as orphans;
    deletion is never automatic — run `sync delete` manually.
 

percona_obs/release_freeze.py

@@ -158,6 +158,32 @@ def _disabled_meta(raw: str) -> str:
     return ET.tostring(root, encoding="unicode")
 
 
+def _restored_meta(current: str, snapshot: str) -> str:
+    """*current* meta with its ``<build>`` replaced by the snapshot's.
+
+    The freeze only rewrites ``<build>``; everything else (e.g. release
+    targets added and removed inside the freeze window) stays as it is now
+    rather than being rolled back to the pre-freeze snapshot.
+    """
+    root = ET.fromstring(current)
+    for build_elem in root.findall("build"):
+        root.remove(build_elem)
+    for build_elem in ET.fromstring(snapshot).findall("build"):
+        root.append(build_elem)
+    ET.indent(root, space="  ")
+    return ET.tostring(root, encoding="unicode")
+
+
+def _meta_to_restore(fetch_current, snapshot: str) -> str:
+    """Restore target for *snapshot*; the snapshot itself if the current meta
+    cannot be read or parsed (unfreezing beats leaving builds disabled)."""
+    try:
+        return _restored_meta(_decode_obs_response(fetch_current()), snapshot)
+    except Exception as exc:
+        print(f"warning: restoring the full pre-freeze meta: {exc}", flush=True)
+        return snapshot
+
+
 def freeze_packages(
     apiurl: str, packages: "dict[str, set[str]]"
 ) -> "dict[tuple[str, str], str]":
@@ -190,9 +216,12 @@ def freeze_packages(
 
 
 def restore_packages(apiurl: str, snapshots: "dict[tuple[str, str], str]") -> None:
-    """Push package meta snapshots back verbatim.  Never raises."""
-    for (prj, pkg), meta in snapshots.items():
+    """Restore the snapshotted ``<build>`` flags on each package.  Never raises."""
+    for (prj, pkg), snapshot in snapshots.items():
         try:
+            meta = _meta_to_restore(
+                lambda: osc.core.show_package_meta(apiurl, prj, pkg), snapshot
+            )
             osc.core.edit_meta(
                 metatype="pkg",
                 path_args=(prj, pkg),
@@ -211,9 +240,9 @@ def restore_packages(apiurl: str, snapshots: "dict[tuple[str, str], str]") -> No
 def freeze_builds(apiurl: str, obs_projects: "list[str]") -> "dict[str, str]":
     """Disable builds on every project; return {project: prior_meta_xml}.
 
-    The snapshot is the exact meta XML fetched before modification, so
-    restore_builds round-trips per-repo flags (publish, debuginfo, partial
-    build enables) without reconstructing them.
+    The snapshot is the exact meta XML fetched before modification;
+    restore_builds puts back only its ``<build>`` element (per-repo partial
+    build enables included), the one element the freeze rewrites.
 
     If freezing any project fails partway through, every project frozen so
     far is restored before the exception propagates — otherwise the earlier
@@ -235,11 +264,15 @@ def freeze_builds(apiurl: str, obs_projects: "list[str]") -> "dict[str, str]":
 
 
 def restore_builds(apiurl: str, snapshots: "dict[str, str]") -> None:
-    """Push snapshotted meta back verbatim.  Never raises: a restore failure
-    is printed as a warning so the caller's finally-block does not mask the
-    primary exception, and the remaining projects are still restored."""
-    for prj, meta in snapshots.items():
+    """Restore the snapshotted ``<build>`` flags on each project.  Never
+    raises: a restore failure is printed as a warning so the caller's
+    finally-block does not mask the primary exception, and the remaining
+    projects are still restored."""
+    for prj, snapshot in snapshots.items():
         try:
+            meta = _meta_to_restore(
+                lambda: osc.core.show_project_meta(apiurl, prj), snapshot
+            )
             _edit_project_meta(apiurl, prj, meta, force=True)
             _print_update(f"{prj}  (builds restored)")
         except Exception as exc:

tests/test_release_freeze.py

@@ -119,7 +119,41 @@ def test_freeze_snapshots_and_restores_exact_meta(monkeypatch):
     assert 'repository="R8"' in written[0][1]
 
     rf.restore_builds("http://obs", snaps)
-    assert written[-1] == ("prj", orig, True)
+    prj, restored, force = written[-1]
+    assert prj == "prj" and force
+    assert "<build" not in restored and 'repository="R8"' in restored
+
+
+def test_restore_builds_keeps_changes_made_while_frozen(monkeypatch):
+    """Only <build> is restored: a release target removed inside the freeze
+    window must not come back from the pre-freeze snapshot."""
+    store = {
+        "prj": '<project name="prj"><title/><description/>'
+        '<build><disable repository="R9"/></build>'
+        '<repository name="R8">'
+        '<releasetarget project="deleted" repository="deleted" trigger="manual"/>'
+        "</repository></project>"
+    }
+
+    def fake_edit(apiurl, prj, meta, force):
+        store[prj] = meta
+
+    monkeypatch.setattr(
+        rf.osc.core, "show_project_meta", lambda apiurl, prj: store[prj].encode()
+    )
+    monkeypatch.setattr(rf, "_edit_project_meta", fake_edit)
+    snaps = rf.freeze_builds("http://obs", ["prj"])
+    assert "<disable />" in store["prj"]
+    # the release hop cleans the targets while the project is frozen
+    store["prj"] = store["prj"].replace(
+        '<releasetarget project="deleted" repository="deleted" trigger="manual" />',
+        "",
+    )
+    assert "releasetarget" not in store["prj"]
+    rf.restore_builds("http://obs", snaps)
+    assert "releasetarget" not in store["prj"]
+    assert '<disable repository="R9" />' in store["prj"]
+    assert "<disable />" not in store["prj"]
 
 
 def test_freeze_partial_failure_restores_already_frozen_projects(monkeypatch):
@@ -148,17 +182,31 @@ def fake_edit(apiurl, prj, meta, force):
     # p1 was frozen then restored; p2's failed edit is the second write.
     assert [w[0] for w in written] == ["p1", "p2", "p1"]
     assert "<disable" in written[0][1]  # p1 frozen
-    assert written[2] == ("p1", origs["p1"], True)  # p1 restored to exact prior meta
+    assert written[2][0] == "p1" and "<build" not in written[2][1]  # p1 unfrozen
 
 
 def test_restore_never_raises(monkeypatch):
     def boom(*a, **k):
         raise RuntimeError("obs down")
 
+    monkeypatch.setattr(rf.osc.core, "show_project_meta", boom)
     monkeypatch.setattr(rf, "_edit_project_meta", boom)
     rf.restore_builds("http://obs", {"prj": "<project/>"})  # must not raise
 
 
+def test_restore_falls_back_to_snapshot_when_meta_unreadable(monkeypatch):
+    def boom(*a, **k):
+        raise RuntimeError("obs down")
+
+    written = []
+    monkeypatch.setattr(rf.osc.core, "show_project_meta", boom)
+    monkeypatch.setattr(
+        rf, "_edit_project_meta", lambda a, prj, meta, force: written.append(meta)
+    )
+    rf.restore_builds("http://obs", {"prj": "<project/>"})
+    assert written == ["<project/>"]
+
+
 def test_verify_release_landed_ok(monkeypatch):
     sets = {"src": {"a", "b"}, "rel": {"a", "b", "extra"}}
     monkeypatch.setattr(
@@ -243,7 +291,7 @@ def fake_edit(metatype, path_args, data, force, apiurl):
     assert edits[0][0] == "pkg" and edits[0][1] == ("deps", "etcd")
     assert "<disable" in edits[0][2]
     rf.restore_packages("http://obs", snaps)
-    assert edits[1][2] == metas[("deps", "etcd")]
+    assert "<build" not in edits[1][2] and 'name="etcd"' in edits[1][2]
 
 
 def test_freeze_packages_restores_on_partial_failure(monkeypatch):
@@ -270,4 +318,4 @@ def fake_show(api, prj, pkg):
         rf.freeze_packages("http://obs", {"deps": {"a", "b"}})
     # a was frozen then restored
     assert edits[0][0] == ("deps", "a") and "<disable" in edits[0][1]
-    assert edits[-1] == (("deps", "a"), metas[("deps", "a")])
+    assert edits[-1][0] == ("deps", "a") and "<build" not in edits[-1][1]