Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/skills/update-skia/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,17 @@ milestone numbers and paste your breaking change analysis table. The default exp
3. **Resolve conflicts** — each conflict must be resolved individually.
Never use `git merge -s ours` or `git read-tree --reset` — this destroys `git blame` attribution.

**⚠️ MANDATORY: Before resolving ANY conflict, check file history for fork-specific patches.**
Run `git log --oneline skiasharp -- <conflicted-file>` — if the log shows intentional
fork patches, keep our version. See [gotcha #15](references/known-gotchas.md) for details.

| File Category | Strategy |
|--------------|----------|
| `BUILD.gn` | **Combine both** — keep upstream structure AND SkiaSharp's platform flags + `skiasharp_build` target |
| `DEPS` | **Combine** — keep our dependency pins, accept upstream structure |
| `RELEASE_NOTES.md`, `infra/bots/` | **Take upstream** |
| C API (`include/c/`, `src/c/`) | **Keep SkiaSharp** — adapt includes/API calls in post-merge commits |
| Other upstream source (`src/`, `include/`) | **Check history first** — see [gotcha #15](references/known-gotchas.md) |

4. **Commit the merge**:
```bash
Expand Down
27 changes: 24 additions & 3 deletions .github/skills/update-skia/references/known-gotchas.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,39 @@ Never use a tree-override merge (`git merge -s ours`, `git read-tree --reset`).
| `RELEASE_NOTES.md`, `infra/bots/` | **Take upstream** |
| C API headers (`include/c/`) | **Keep SkiaSharp** — these don't exist upstream |
| C API source (`src/c/`) | **Keep SkiaSharp + adapt** — fix includes and API calls in post-merge commits |
| Other upstream source (`src/`, `include/`) | **Check history first** — see gotcha #15 |

### 15. Never `--theirs` Without Checking File History

**Failure mode**: A merge conflict in an upstream file (outside `src/c/` / `include/c/`) is resolved
with `git checkout --theirs`, silently overwriting an intentional SkiaSharp fork patch.

**Mandatory process for EVERY conflicted file:**

```bash
# BEFORE resolving, check if the fork has intentional patches
git log --oneline skiasharp -- <conflicted-file>
```

- If the log shows fork-specific commits (look for "Restore", "patch", "fix", or any non-merge
commit), **keep our version** and only absorb upstream's harmless additive changes (new includes).
- If the log shows only merge commits from prior upstream merges, taking `--theirs` is likely safe.
- **Never use `git checkout --theirs` as a shortcut** for files you haven't investigated.

**Key signal words** in commit messages that indicate intentional fork patches:
`Restore`, `patch`, `fix for`, `platform`, `workaround`, `SkiaSharp`, `iOS`, `Tizen`

## Testing

### 15. Version Compatibility Errors
### 16. Version Compatibility Errors

`InvalidOperationException: The version of the native libSkiaSharp library (X) is incompatible` means VERSIONS.txt wasn't fully updated. Fix the root cause — do NOT work around it.

### 16. Pixel Value Precision
### 17. Pixel Value Precision

Upstream periodically improves color conversion precision, shifting expected pixel values by ±1. When pixel-exact test assertions break, check if upstream changed the conversion and update expected values.

### 17. Test Runner
### 18. Test Runner

Tests use `Skip.If()` for unsupported platforms. Run `dotnet test tests/SkiaSharp.Tests.Console.sln` for the full suite. Backend-specific tests self-skip when hardware isn't available.

Expand Down
Loading