sync: don't promote on vendor.tar.* drift in branch content checks

The --branch-from content check compares raw file md5s against OBS.  For
packages with a cargo_vendor service whose upstream ships no Cargo.lock
(pgvectorscale), cargo vendor re-resolves the crate graph on every run, so
vendor.tar.gz bytes track the crates.io state at generation time and differ
between machines and dates even when no real input changed (verified:
identical back-to-back local runs, but OBS rev5 vendored camino-1.2.4 while
a fresh run vendors camino-1.2.5).

Seen on percona/obs-packaging PR #5: a PR touching only ppg:staging:18
packages promoted percona-pgvectorscale in staging:16/17/18:extras because
the content check reported 'vendor.tar.gz differs' (the gate before it fell
through to the content check because the OBS sync comments record SHAs from
a fork checkout that do not exist in the CI clone).

Compare vendor.tar.* by presence instead of bytes when the package declares
a cargo_vendor service and both sides have the archive: it is a derived
artifact of the other uploaded files, so if those all match, a vendor byte
difference is pure resolution drift and must not flip an aggregate decision
to promote.  A vendor archive missing from either side is still a mismatch.

The comparison is extracted into _content_mismatches so it is unit-testable
and reports every differing file instead of stopping at the first.

Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>

Author: rjd15372

percona_obs/cmd_sync.py

@@ -90,6 +90,7 @@
 )
 from .services import (
     _copy_local_packaging,
+    _has_cargo_vendor_service,
     _has_runnable_services,
     _run_local_services,
     upstream_scm_refs,
@@ -119,6 +120,9 @@
 # layout ("…:containers") intentionally does not match and is never filtered.
 _CONTAINER_SUBPROJ_RE = re.compile(r":containers:([^:]+)$")
 
+# Matches the vendored-crates archive produced by the cargo_vendor service.
+_VENDOR_TAR_RE = re.compile(r"^vendor\.tar\.[a-z0-9]+$")
+
 
 _OBS_SUBSTITUTABLE = {"_service", "_aggregate", "_link"}
 
@@ -435,6 +439,34 @@ def _is_link_package(obs_dir: Path) -> bool:
     return (obs_dir / "_link").is_file()
 
 
+def _content_mismatches(
+    local_md5s: dict[str, str],
+    obs_md5s: dict[str, str],
+    ignore_vendor: bool,
+) -> list[str]:
+    """Return the file names whose content differs between local and OBS.
+
+    With *ignore_vendor*, a vendor.tar.* present on both sides is compared by
+    presence only, never by bytes.  cargo_vendor output is a function of the
+    other uploaded files plus the crates.io state at generation time: upstream
+    projects without a committed Cargo.lock (e.g. pgvectorscale) re-resolve
+    the crate graph on every run, so its bytes drift between machines and
+    dates even when no real input changed.  A vendor-only difference must
+    therefore not flip a branch decision to promote.  A vendor archive
+    missing from either side is still a mismatch.
+    """
+    mismatches = [name for name, md5 in local_md5s.items() if obs_md5s.get(name) != md5]
+    mismatches += [name for name in obs_md5s if name not in local_md5s]
+    if ignore_vendor:
+        both = set(local_md5s) & set(obs_md5s)
+        mismatches = [
+            name
+            for name in mismatches
+            if not (_VENDOR_TAR_RE.match(name) and name in both)
+        ]
+    return sorted(mismatches)
+
+
 def _content_matches_branch(
     apiurl: str,
     branch_project: str,
@@ -515,21 +547,17 @@ def _content_matches_branch(
             if f.is_file():
                 local_md5s[f.name] = hashlib.md5(f.read_bytes()).hexdigest()
 
-        for fname, local_md5 in sorted(local_md5s.items()):
-            if obs_md5s.get(fname) != local_md5:
-                logger.debug(
-                    f"content check: {fname} differs  {branch_project}/{package_name}"
-                )
-                return False
-
-        for fname in obs_md5s:
-            if fname not in local_md5s:
-                logger.debug(
-                    f"content check: {fname} in OBS but not local  {branch_project}/{package_name}"
-                )
-                return False
-
-        return True
+        mismatches = _content_mismatches(
+            local_md5s,
+            obs_md5s,
+            ignore_vendor=run_services and _has_cargo_vendor_service(service_file),
+        )
+        for fname in mismatches:
+            detail = "differs" if fname in local_md5s else "in OBS but not local"
+            logger.debug(
+                f"content check: {fname} {detail}  {branch_project}/{package_name}"
+            )
+        return not mismatches
     finally:
         shutil.rmtree(combined, ignore_errors=True)
         if workdir is not None:

percona_obs/services.py

@@ -76,6 +76,15 @@ def _has_runnable_services(service_file: Path) -> bool:
     )
 
 
+def _has_cargo_vendor_service(service_file: Path) -> bool:
+    """Return True if *service_file* declares a cargo_vendor service."""
+    try:
+        root = ET.parse(service_file).getroot()
+    except (ET.ParseError, OSError):
+        return False
+    return any(svc.get("name") == "cargo_vendor" for svc in root.findall("service"))
+
+
 def _get_upstream_obs_scm_info(
     service_file: Path,
 ) -> tuple[str, str, str] | None:

tests/test_vendor_content_check.py

@@ -0,0 +1,78 @@
+"""Unit tests for vendor-aware content comparison (percona_obs.cmd_sync).
+
+Reproduces the percona/obs-packaging PR #5 bug: pgvectorscale ships no
+Cargo.lock, so cargo_vendor re-resolves the crate graph on every run and
+vendor.tar.gz bytes drift with the crates.io state at generation time.  A
+branch-from content check comparing raw md5s then reported "vendor.tar.gz
+differs" for packages whose real inputs were untouched, promoting
+percona-pgvectorscale in every extras project on unrelated PRs.
+"""
+
+from percona_obs.cmd_sync import _content_mismatches
+from percona_obs.services import _has_cargo_vendor_service
+
+SPEC = "percona-pgvectorscale.spec"
+SRC = "percona-pgvectorscale_16-0.9.0.tar.gz"
+VENDOR = "vendor.tar.gz"
+
+
+def test_vendor_only_drift_ignored_for_cargo_packages():
+    local = {SPEC: "a", SRC: "b", VENDOR: "local-bytes"}
+    obs = {SPEC: "a", SRC: "b", VENDOR: "obs-bytes"}
+    assert _content_mismatches(local, obs, ignore_vendor=True) == []
+
+
+def test_vendor_drift_reported_without_cargo_vendor_service():
+    local = {SPEC: "a", VENDOR: "local-bytes"}
+    obs = {SPEC: "a", VENDOR: "obs-bytes"}
+    assert _content_mismatches(local, obs, ignore_vendor=False) == [VENDOR]
+
+
+def test_real_change_still_reported_alongside_vendor_drift():
+    local = {SPEC: "changed", SRC: "b", VENDOR: "local-bytes"}
+    obs = {SPEC: "a", SRC: "b", VENDOR: "obs-bytes"}
+    assert _content_mismatches(local, obs, ignore_vendor=True) == [SPEC]
+
+
+def test_vendor_missing_on_obs_is_a_mismatch():
+    local = {SPEC: "a", VENDOR: "local-bytes"}
+    obs = {SPEC: "a"}
+    assert _content_mismatches(local, obs, ignore_vendor=True) == [VENDOR]
+
+
+def test_vendor_missing_locally_is_a_mismatch():
+    local = {SPEC: "a"}
+    obs = {SPEC: "a", VENDOR: "obs-bytes"}
+    assert _content_mismatches(local, obs, ignore_vendor=True) == [VENDOR]
+
+
+def test_other_compressions_ignored_too():
+    local = {SPEC: "a", "vendor.tar.xz": "x", "vendor.tar.zst": "y"}
+    obs = {SPEC: "a", "vendor.tar.xz": "p", "vendor.tar.zst": "q"}
+    assert _content_mismatches(local, obs, ignore_vendor=True) == []
+
+
+def test_identical_content_has_no_mismatches():
+    local = {SPEC: "a", SRC: "b"}
+    obs = {SPEC: "a", SRC: "b"}
+    assert _content_mismatches(local, obs, ignore_vendor=False) == []
+
+
+def test_has_cargo_vendor_service(tmp_path):
+    with_vendor = tmp_path / "with" / "_service"
+    with_vendor.parent.mkdir()
+    with_vendor.write_text(
+        "<services>"
+        '<service mode="buildtime" name="cargo_vendor">'
+        '<param name="compression">gz</param>'
+        "</service>"
+        "</services>"
+    )
+    without_vendor = tmp_path / "without" / "_service"
+    without_vendor.parent.mkdir()
+    without_vendor.write_text(
+        '<services><service mode="buildtime" name="tar" /></services>'
+    )
+    assert _has_cargo_vendor_service(with_vendor) is True
+    assert _has_cargo_vendor_service(without_vendor) is False
+    assert _has_cargo_vendor_service(tmp_path / "missing" / "_service") is False