Skip to content

Commit e45784c

Browse files
mattleibowCopilot
andauthored
Fix release-line Skia sync state preservation (#4660)
[skia-sync] Preserve release-line sync state (#4660) Context: https://github.com/mono/SkiaSharp/actions/runs/30937099860 Context: https://github.com/mono/SkiaSharp/actions/runs/30942491593 Validation: https://github.com/mono/SkiaSharp/actions/runs/30956446553 Release-line syncs replaced the triggering revision's current update-skia skill with stale servicing-branch helpers. This downgraded versions such as 4.150.2 to 4.150.0 and left initialized submodules at SHAs from the previous parent checkout. Stage the complete update-skia skill outside the mutable product checkout and prepare branches deterministically from the resolved parent and Skia SHAs. Recursively align every submodule before creating the mutable mono/skia branch. Hydrate Skia dependencies before version reconciliation. Preserve servicing and semantic versions unless the corresponding DEPS identity changes, while permitting source-backed provenance backfills. Generated mono/skia PRs now require merge commits so upstream ancestry remains available to future sync detection. The final m150 validation preserved 4.150.2, built from source, and passed the full test suite. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 48b4e0a commit e45784c

11 files changed

Lines changed: 533 additions & 55 deletions

.agents/skills/update-skia/SKILL.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,9 @@ An update is complete only when:
7878
have a matching final decision, exact-SHA evidence, and reconciled Component Governance metadata.
7979
A dependency recorded as preserved must still equal the fork-base revision.
8080
- Let `update_versions.py` identify dependency changes from exact base/final DEPS. For every tracked
81-
changed dependency, derive the semantic version from checked-out source and complete the
82-
`skia_dependency` verification fields before the helper can pass. Never update an unrelated
83-
manifest version when its DEPS identity did not change.
81+
changed dependency, and every legacy tracked registration missing verification evidence, derive
82+
the semantic version from checked-out source and complete the `skia_dependency` fields before the
83+
helper can pass. Never update a manifest version when its DEPS identity did not change.
8484
- Components not sourced from Skia DEPS, including ANGLE and its dependencies, are updated in
8585
separate dependency PRs rather than bundled into a Skia upstream sync.
8686
- Never use `externals-download` after a submodule/native/C API change.
@@ -128,6 +128,9 @@ needed for that phase.
128128

129129
## Deterministic helpers
130130

131+
- Automation stages this entire skill outside the mutable product checkout and exports its path as
132+
`SKIA_SYNC_SKILL_DIR`. After changing the product branch, continue reading phase references and
133+
running helpers from that staged directory rather than the branch-local `.agents` tree.
131134
- `scripts/update_versions.py` updates and validates version surfaces, Skia hashes, and deterministic
132135
DEPS-to-Component-Governance identity/review signals.
133136
- `scripts/regenerate_bindings.py` runs every binding configuration, restores HarfBuzz,

.agents/skills/update-skia/references/phases/04-05-branch-and-merge.md

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,31 +5,27 @@ Read only `DEPS: Fork-Customized Dependencies`, `Genuine Merge Required`,
55
`Conflict Resolution by File Category`, and `Verify-Upstream-or-Reapply` in
66
[../known-gotchas.md](../known-gotchas.md).
77

8-
## Phase 04 — branch from the resolved bases
8+
## Phase 04 — create matched feature branches
99

10-
Create or verify the parent feature branch:
10+
Use the deterministic branch helper for a new update. It consumes the exact parent and Skia SHAs
11+
resolved in Phase 01; do not replace them with current branch tips or reproduce its Git operations
12+
manually.
1113

1214
```bash
13-
git fetch origin "{BASE_BRANCH}"
14-
git checkout -b "{HEAD_BRANCH}" "origin/{BASE_BRANCH}"
15+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/prepare_branches.py" \
16+
--repo-root "${GITHUB_WORKSPACE:-$PWD}"
1517
```
1618

17-
Create or verify the mono/skia feature branch at the exact submodule pointer recorded by the
18-
parent base:
19+
The helper fails instead of resetting existing branches or dirty files. On success:
1920

20-
```bash
21-
BASE_SUB_SHA=$(git ls-tree "origin/{BASE_BRANCH}" -- externals/skia | awk '{print $3}')
22-
export SKIA_SYNC_SKIA_BASE_SHA="$BASE_SUB_SHA"
23-
git -C externals/skia fetch origin "{SKIA_BASE_BRANCH}"
24-
git -C externals/skia checkout "$BASE_SUB_SHA"
25-
git -C externals/skia branch -r --contains "$BASE_SUB_SHA" |
26-
grep "origin/{SKIA_BASE_BRANCH}"
27-
git -C externals/skia checkout -b "{HEAD_BRANCH}"
28-
```
21+
- the parent feature branch starts at `{PARENT_BASE_SHA}`;
22+
- every recursive submodule matches that parent commit;
23+
- the mono/skia pointer is verified against `{SKIA_BASE_SHA}` and
24+
`origin/{SKIA_BASE_BRANCH}`; and
25+
- the mono/skia feature branch is created from that exact pointer.
2926

30-
If a branch already exists, inspect its base and commits before continuing. Reuse only the
31-
intended update branch; never reset or overwrite unrelated work. Automation may start with the
32-
submodule already aligned, but must still verify the pointer and bases.
27+
If either feature branch already exists, stop and inspect its base and commits. Reuse it only when
28+
it is the intended continuation; never reset or overwrite unrelated work.
3329

3430
## Phase 05 — genuine upstream merge
3531

@@ -117,7 +113,7 @@ fork patch. Return to the parent repository root and generate the diff-of-diffs
117113
merge:
118114

119115
```bash
120-
python3 .agents/skills/update-skia/scripts/audit_fork_patches.py \
116+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/audit_fork_patches.py" \
121117
--old-upstream "$SKIA_SYNC_BASE_UPSTREAM_SHA" \
122118
--new-upstream "$SKIA_SYNC_TARGET_UPSTREAM_SHA" \
123119
--fork-base "$SKIA_SYNC_SKIA_BASE_SHA" \

.agents/skills/update-skia/references/phases/06-07-update-and-build.md

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,19 @@
22

33
## Phase 06 — update version surfaces
44

5-
From the parent repository, run the maintained helper:
5+
Hydrate the dependencies recorded by the merged Skia `DEPS` before inspecting their
6+
semantic-version evidence:
67

78
```bash
8-
python3 .agents/skills/update-skia/scripts/update_versions.py
9+
python3 externals/skia/tools/git-sync-deps
10+
```
11+
12+
This synchronizes dependency source only; it does not build native binaries. Then run the
13+
maintained version helper from the parent repository:
14+
15+
```bash
16+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/update_versions.py" \
17+
--repo-root "${GITHUB_WORKSPACE:-$PWD}"
918
```
1019

1120
The helper updates and validates `scripts/VERSIONS.txt`, the Skia registrations in
@@ -34,8 +43,9 @@ A revision-only roll may retain the same semantic version, but still requires th
3443
identity and source evidence. The helper rejects a manifest version bump when that dependency's
3544
DEPS identity did not change. It is idempotent; rerun it after every final DEPS/native adaptation
3645
and after the final mono/skia fix commit so the parent records the exact tested state.
37-
Every tracked registration, including an unchanged baseline entry, must retain non-empty
38-
`version_source` evidence.
46+
Every tracked registration, including an unchanged legacy entry, must have non-empty
47+
`version_source` evidence. Backfill missing `skia_dependency` evidence from the hydrated source so
48+
supported branches become compliant over time without changing semantic versions unnecessarily.
3949
The script proves coverage and consistency; the independent review must re-read each cited source
4050
to validate the agent's semantic-version claim.
4151

@@ -96,7 +106,8 @@ explanatory commit.
96106

97107
After every mono/skia adaptation, rerun `audit_fork_patches.py` with the Phase 05 arguments. Fill
98108
new or changed rows and require `--validate` to pass again. Reuse the exact Phase 05
99-
`python3 .agents/skills/update-skia/scripts/audit_fork_patches.py` command rather than searching
109+
`python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/audit_fork_patches.py"`
110+
command rather than searching
100111
for another copy of the helper.
101112

102113
Before Phase 08, rerun `update_versions.py` against final `DEPS`, then reconcile every row in

.agents/skills/update-skia/references/phases/08-10-bindings-and-tests.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
The native build must run first so dependency headers exist:
66

77
```bash
8-
python3 .agents/skills/update-skia/scripts/regenerate_bindings.py
8+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/regenerate_bindings.py" \
9+
--repo-root "${GITHUB_WORKSPACE:-$PWD}"
910
dotnet build binding/SkiaSharp/SkiaSharp.csproj
1011
```
1112

@@ -119,15 +120,16 @@ mono/skia tree before moving to Phase 11:
119120
2. From the parent root, run the metadata finalizer:
120121

121122
```bash
122-
python3 .agents/skills/update-skia/scripts/update_versions.py
123+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/update_versions.py" \
124+
--repo-root "${GITHUB_WORKSPACE:-$PWD}"
123125
```
124126

125127
If it fails, reconcile every `skia-dependency-changes.json` row with checked-out source,
126128
`cgmanifest.json`, and `skia-dependency-decisions.md`, then rerun until it passes.
127129
3. Refresh the fork audit against the **current final mono/skia HEAD**:
128130

129131
```bash
130-
python3 .agents/skills/update-skia/scripts/audit_fork_patches.py \
132+
python3 "${SKIA_SYNC_SKILL_DIR:-.agents/skills/update-skia}/scripts/audit_fork_patches.py" \
131133
--old-upstream "$SKIA_SYNC_BASE_UPSTREAM_SHA" \
132134
--new-upstream "$SKIA_SYNC_TARGET_UPSTREAM_SHA" \
133135
--fork-base "$SKIA_SYNC_SKIA_BASE_SHA" \
Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
#!/usr/bin/env python3
2+
3+
"""Create matched SkiaSharp and mono/skia update branches from exact bases."""
4+
5+
import argparse
6+
import os
7+
import subprocess
8+
from pathlib import Path
9+
10+
11+
class PreparationError(RuntimeError):
12+
pass
13+
14+
15+
def git(repo: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]:
16+
result = subprocess.run(
17+
["git", "-C", str(repo), *args],
18+
capture_output=True,
19+
text=True,
20+
)
21+
if check and result.returncode != 0:
22+
detail = result.stderr.strip() or result.stdout.strip()
23+
raise PreparationError(f"git {' '.join(args)} failed in {repo}: {detail}")
24+
return result
25+
26+
27+
def git_output(repo: Path, *args: str) -> str:
28+
return git(repo, *args).stdout.rstrip("\r\n")
29+
30+
31+
def branch_exists(repo: Path, branch: str) -> bool:
32+
result = git(
33+
repo,
34+
"show-ref",
35+
"--verify",
36+
"--quiet",
37+
f"refs/heads/{branch}",
38+
check=False,
39+
)
40+
if result.returncode not in (0, 1):
41+
raise PreparationError(f"Could not inspect branch {branch} in {repo}.")
42+
return result.returncode == 0
43+
44+
45+
def require_clean(repo: Path, *, ignore_submodules: bool = False) -> None:
46+
args = ["status", "--porcelain=v1", "--untracked-files=all"]
47+
if ignore_submodules:
48+
args.append("--ignore-submodules=all")
49+
status = git_output(repo, *args)
50+
if status:
51+
raise PreparationError(f"Worktree is not clean: {repo}\n{status}")
52+
53+
54+
def require_ancestor(repo: Path, commit: str, remote_branch: str) -> None:
55+
result = git(
56+
repo,
57+
"merge-base",
58+
"--is-ancestor",
59+
commit,
60+
remote_branch,
61+
check=False,
62+
)
63+
if result.returncode != 0:
64+
raise PreparationError(f"{commit} is not contained by {remote_branch}.")
65+
66+
67+
def prepare_branches(
68+
repo_root: Path,
69+
base_branch: str,
70+
parent_base_sha: str,
71+
skia_base_branch: str,
72+
skia_base_sha: str,
73+
head_branch: str,
74+
) -> None:
75+
repo_root = repo_root.resolve()
76+
skia_root = repo_root / "externals" / "skia"
77+
78+
git_output(repo_root, "rev-parse", "--show-toplevel")
79+
git_output(skia_root, "rev-parse", "--show-toplevel")
80+
git(repo_root, "check-ref-format", "--branch", head_branch)
81+
git(skia_root, "check-ref-format", "--branch", head_branch)
82+
83+
require_clean(repo_root, ignore_submodules=True)
84+
require_clean(skia_root)
85+
if branch_exists(repo_root, head_branch):
86+
raise PreparationError(f"Parent branch already exists: {head_branch}")
87+
if branch_exists(skia_root, head_branch):
88+
raise PreparationError(f"Skia branch already exists: {head_branch}")
89+
90+
git(
91+
repo_root,
92+
"fetch",
93+
"--no-tags",
94+
"origin",
95+
f"+refs/heads/{base_branch}:refs/remotes/origin/{base_branch}",
96+
)
97+
git(
98+
skia_root,
99+
"fetch",
100+
"--no-tags",
101+
"origin",
102+
f"+refs/heads/{skia_base_branch}:refs/remotes/origin/{skia_base_branch}",
103+
)
104+
105+
parent_base_sha = git_output(
106+
repo_root, "rev-parse", "--verify", f"{parent_base_sha}^{{commit}}"
107+
)
108+
skia_base_sha = git_output(
109+
skia_root, "rev-parse", "--verify", f"{skia_base_sha}^{{commit}}"
110+
)
111+
require_ancestor(repo_root, parent_base_sha, f"origin/{base_branch}")
112+
require_ancestor(skia_root, skia_base_sha, f"origin/{skia_base_branch}")
113+
114+
tree_entry = git_output(
115+
repo_root, "ls-tree", parent_base_sha, "--", "externals/skia"
116+
).split()
117+
if len(tree_entry) < 3 or tree_entry[1] != "commit":
118+
raise PreparationError(
119+
f"{parent_base_sha} does not contain the externals/skia submodule."
120+
)
121+
if tree_entry[2] != skia_base_sha:
122+
raise PreparationError(
123+
"Resolved Skia base does not match the parent gitlink: "
124+
f"{skia_base_sha} != {tree_entry[2]}"
125+
)
126+
127+
git(repo_root, "checkout", "-b", head_branch, parent_base_sha)
128+
git(repo_root, "submodule", "sync", "--recursive")
129+
git(repo_root, "submodule", "update", "--init", "--recursive")
130+
131+
submodule_status = git_output(
132+
repo_root, "submodule", "status", "--recursive"
133+
)
134+
mismatches = [
135+
line for line in submodule_status.splitlines() if line and line[0] in "-+U"
136+
]
137+
if mismatches:
138+
raise PreparationError(
139+
"Submodules do not match the selected parent:\n" + "\n".join(mismatches)
140+
)
141+
142+
actual_skia_sha = git_output(skia_root, "rev-parse", "HEAD")
143+
if actual_skia_sha != skia_base_sha:
144+
raise PreparationError(
145+
f"Skia is at {actual_skia_sha}, expected {skia_base_sha}."
146+
)
147+
148+
git(skia_root, "checkout", "-b", head_branch, skia_base_sha)
149+
print(f"Created {head_branch} from parent {parent_base_sha}.")
150+
print(f"Created externals/skia/{head_branch} from {skia_base_sha}.")
151+
152+
153+
def main() -> int:
154+
parser = argparse.ArgumentParser(
155+
description="Create matched SkiaSharp and mono/skia update branches."
156+
)
157+
parser.add_argument(
158+
"--repo-root",
159+
type=Path,
160+
default=Path(os.environ.get("GITHUB_WORKSPACE", ".")),
161+
)
162+
parser.add_argument("--base-branch", default=os.environ.get("SKIA_SYNC_BASE_BRANCH"))
163+
parser.add_argument(
164+
"--parent-base-sha", default=os.environ.get("SKIA_SYNC_PARENT_BASE_SHA")
165+
)
166+
parser.add_argument(
167+
"--skia-base-branch", default=os.environ.get("SKIA_SYNC_SKIA_BASE_BRANCH")
168+
)
169+
parser.add_argument(
170+
"--skia-base-sha", default=os.environ.get("SKIA_SYNC_SKIA_BASE_SHA")
171+
)
172+
parser.add_argument("--head-branch", default=os.environ.get("SKIA_SYNC_HEAD_BRANCH"))
173+
args = parser.parse_args()
174+
175+
required = {
176+
"--base-branch": args.base_branch,
177+
"--parent-base-sha": args.parent_base_sha,
178+
"--skia-base-branch": args.skia_base_branch,
179+
"--skia-base-sha": args.skia_base_sha,
180+
"--head-branch": args.head_branch,
181+
}
182+
missing = [name for name, value in required.items() if not value]
183+
if missing:
184+
parser.error("missing required values: " + ", ".join(missing))
185+
186+
try:
187+
prepare_branches(
188+
args.repo_root,
189+
args.base_branch,
190+
args.parent_base_sha,
191+
args.skia_base_branch,
192+
args.skia_base_sha,
193+
args.head_branch,
194+
)
195+
except PreparationError as error:
196+
parser.exit(1, f"error: {error}\n")
197+
return 0
198+
199+
200+
if __name__ == "__main__":
201+
raise SystemExit(main())

0 commit comments

Comments
 (0)