From bd9356871a79c52bba7d680fe6b4ad8a9bd5509f Mon Sep 17 00:00:00 2001 From: Santhosh Vaiyapuri Date: Mon, 17 Aug 2026 20:11:19 +0200 Subject: [PATCH] docs: track upstream sync progress with marker refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging upstream refs into master to record sync progress makes every upstream commit an ancestor of master. semantic-release analyses ..master by walking all parents, so commits we deliberately skipped enter the version calculation and changelog — one upstream 'feat:' turns a patch into a minor and credits us with code we never shipped. Ghost merges have the same problem: the tree is unchanged but the upstream parent is still recorded. Track progress with a sync-marker/ ref instead. It is not an ancestor of master, so releases never see it, and because tracking no longer depends on history shape the PR merge strategy stops mattering — squashing a sync PR used to silently destroy the record. Also folds in what the last sync surfaced: don't run both example builds concurrently, pipefail so a failed xcodebuild isn't hidden by tail, keep only the formatting for files the sync touched, and don't trust a shadowed grep. --- .claude/skills/upstream-sync.md | 108 ++++++++++++++++++++++---------- CLAUDE.md | 2 +- 2 files changed, 75 insertions(+), 35 deletions(-) diff --git a/.claude/skills/upstream-sync.md b/.claude/skills/upstream-sync.md index b0f2db696..099c94b10 100644 --- a/.claude/skills/upstream-sync.md +++ b/.claude/skills/upstream-sync.md @@ -1,24 +1,43 @@ # Upstream Sync Skill -Sync fork with one or more upstream remotes via cherry-pick + merge-base advance. +Sync fork with one or more upstream remotes via cherry-pick + marker refs. ## When to use User says: "sync with upstream", "cherry-pick from X", "merge upstream", or similar. +## How we track "already reviewed" + +A ref per upstream, `sync-marker/`, points at the last upstream commit we +triaged. Divergence is `sync-marker/../master` — nothing is inferred +from history shape. + +**Never merge an upstream ref into `master`.** Doing so makes every upstream commit an +ancestor of `master`, and semantic-release analyses `..master` by walking all +parents — so skipped upstream commits land in the version calculation and the changelog. +One stray `feat:` upstream turns a patch into a minor and credits us with code we never +shipped. Marker refs are not ancestors of `master`, so releases never see them. + +That also means the PR merge strategy is irrelevant: squash, rebase, and merge are all +safe. (Before markers, tracking lived in merge commits and squashing silently broke it.) + ## Workflow ### Phase 1: Explore divergence ```bash git fetch -git log --oneline --right-only .../master --no-merges +git log --oneline sync-marker/../master --no-merges ``` +If the marker does not exist yet, bootstrap it from the last real merge of that remote +(`git log --merges --oneline master | grep `) or ask the user, then create it: +`git branch sync-marker/ `. + For each upstream-only commit, get files changed: ```bash -git log --right-only .../master --no-merges --format="%h %s" | while read hash msg; do +git log sync-marker/../master --no-merges --format="%h %s" | while read hash msg; do echo "=== $hash $msg ==="; git diff-tree --no-commit-id --name-only -r $hash; echo done ``` @@ -38,7 +57,7 @@ done | Refactoring | CHERRY-PICK (evaluate risk) | | Docs/CI/tools | Ask user | -Check for equivalents: `git log --oneline --left-only .../master | grep -i ""` +Check for equivalents: `git log --oneline /master.. | grep -i ""` ### Phase 3: Ask user @@ -55,31 +74,7 @@ Order: TS fixes → Android fixes → iOS fixes → small features → large fea If conflict: resolve, `git add`, `git cherry-pick --continue --no-edit`. If empty after resolution: `git cherry-pick --skip`. -### Phase 5: Merge to advance merge-base - -Without this, future merges replay ALL upstream commits including skipped ones. - -```bash -git merge /master --no-commit - -# Conflicted files — keep ours -git diff --name-only --diff-filter=U | xargs git checkout --ours -# Files deleted in our branch — remove -git rm -# Auto-merged files — reset to our version -git diff --cached --name-only --diff-filter=M | xargs git checkout HEAD -- -# Unwanted new files from upstream — remove -git diff --cached --name-only --diff-filter=A # review, then: -git rm -f - -git add -A -git diff --cached --stat HEAD # should be empty or near-empty -git commit -m "merge: sync merge-base with /master" -``` - -Verify: `git log --oneline --right-only .../master | wc -l` should be `0`. - -### Phase 6: Verify +### Phase 5: Verify Run ALL of these. Do not skip any. @@ -91,23 +86,64 @@ cd examples/GumTestApp/ios && pod install && \ -sdk iphonesimulator -configuration Debug build ``` -### Phase 7: Format native files +Run these one at a time, never two concurrently against `examples/GumTestApp` — a second +run's `rm -rf Pods` will pull files out from under the first and both fail confusingly. + +Use `set -o pipefail`, or check the build's own exit code. Piping `xcodebuild` into `tail` +reports *tail's* status, so a failed build looks like exit 0. Confirm by grepping for +`** BUILD SUCCEEDED **`. + +If `pod install` cannot satisfy the pinned `StreamWebRTC` version, the local CocoaPods +spec repo is stale — `pod install --repo-update`. Not a sync problem; CI checks out fresh. + +### Phase 6: Format native files ```bash git ls-files | grep -e "\(\.java\|\.h\|\.m\)$" | grep -v examples | xargs npx clang-format -i ``` +This reformats pre-existing drift across many untouched files, because the local +clang-format version differs from whatever produced the committed formatting. Keep only +the files this sync actually touched and revert the rest — formatting is not a CI gate, +and a sync PR full of unrelated reformatting is unreviewable. + Rebuild Android + iOS to confirm, then commit. -### Phase 8: Update package-lock.json +### Phase 7: Update package-lock.json -If `package.json` dependencies changed, lock file will be stale. +If `package.json` dependencies changed, lock file will be stale. Note a `scripts`-only +change does **not** require this. ```bash npm install git add package-lock.json && git commit -m "chore: update package-lock.json" ``` +### Phase 8: After the sync PR merges, move the markers + +Do this **only once the PR has landed on `master`** — not at cherry-pick time. Moving a +marker for a PR that is later abandoned skips those upstream commits permanently. + +Any merge strategy is fine (squash, rebase, or merge). + +```bash +# for each remote synced in this round +git fetch +git branch -f sync-marker/ +git push --force-with-lease origin sync-marker/ +``` + +Point the marker at the upstream tip you actually triaged, not a freshly fetched one — +upstream may have moved on while the PR was in review, and those newer commits have not +been reviewed. Then verify the divergence is empty: + +```bash +git log --oneline sync-marker/../master --no-merges # only untriaged commits +``` + +Marker pushes trigger no CI: every workflow is scoped to `master` pushes, PRs targeting +`master`, or manual dispatch. + ## Preservation rules These MUST NOT change during sync: @@ -134,7 +170,7 @@ Post-sync: `grep -r "org.webrtc:google-webrtc\|webrtc-ios" --include="*.gradle" 5. **Watch for duplicates after conflict resolution.** Duplicate variable declarations, closing braces, or imports when keeping both sides of a conflict. -6. **Advance merge-base for EVERY upstream remote.** If syncing with multiple upstreams, merge each one separately. Otherwise the un-advanced remote replays all its history on the next merge. +6. **Move the marker for EVERY upstream remote.** If syncing with multiple upstreams, each needs its own `sync-marker/`. A remote whose marker was not moved replays all its history next round. 7. **Upstream podspec/build files leak into cherry-picks.** Other forks have their own podspec (e.g., `livekit-react-native-webrtc.podspec`). Always `git rm` them when they appear. @@ -142,4 +178,8 @@ Post-sync: `grep -r "org.webrtc:google-webrtc\|webrtc-ios" --include="*.gradle" 9. **Verify cherry-picked changes still exist on upstream HEAD.** A commit could have been added and later reverted/modified by a subsequent commit on the same upstream. After cherry-picking, verify the actual code still matches upstream's current state: `git show /master: | grep ""`. Don't just trust that a commit was made — it may have been undone. -10. **Squash-merging the sync PR destroys the merge-base advancement.** The merge commits from Phase 5 (`git merge /master`) are the only thing that tells git "we've seen up to this point." Squash-merge flattens them into a single commit, losing that information. When merging the sync PR, use **"Create a merge commit"** (regular merge), not squash. If already squashed, create a follow-up PR with ghost merges: `git merge -s ours /master` for each upstream, then merge that PR with a regular merge commit. +10. **Never merge an upstream ref into `master` to record progress.** This includes ghost merges (`git merge -s ours /master`), which keep the tree but still add the upstream parent. semantic-release analyses `..master` by walking all parents, so every skipped upstream commit enters the version calculation and the changelog — one upstream `feat:` turns a patch into a minor and credits us with code we never shipped. Move `sync-marker/` instead (Phase 8); it is not an ancestor of `master`, so releases never see it. Historical merges like `eface9a` / `d66ff60` predate this rule and are harmless only because later release tags absorbed them. + +11. **Don't infer "already synced" from the tree.** A squashed sync PR copies upstream *content* into `master` without making upstream *commits* ancestors, so content checks and marker checks can disagree. `sync-marker/` is the only record of what was triaged; the tree tells you nothing about it. + +12. **`grep` may be shadowed by a shell function or `ugrep`.** A wrapper can report zero matches for a string that is plainly present, which reads as "the cherry-pick did not apply." Confirm with `/usr/bin/grep` or `git show : | sed -n 'N,Mp'` before concluding anything is missing. diff --git a/CLAUDE.md b/CLAUDE.md index db51f39e7..db25d1bad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,7 +65,7 @@ Sanity check after any dependency edit: `grep -r "org.webrtc:google-webrtc\|webr ## Keeping the fork in sync with upstream -Pulling fixes/features from upstream `react-native-webrtc` (and sibling forks) is a recurring, error-prone task with strict preservation rules and merge-base mechanics. **Read `.claude/skills/upstream-sync.md` before doing any sync/cherry-pick/merge work** — it documents the triage table, the merge-base advancement step (and why squash-merging the sync PR breaks it), and the files that must never change during a sync. +Pulling fixes/features from upstream `react-native-webrtc` (and sibling forks) is a recurring, error-prone task with strict preservation rules. **Read `.claude/skills/upstream-sync.md` before doing any sync/cherry-pick/merge work** — it documents the triage table, the `sync-marker/` refs that record how far each upstream has been reviewed (and why upstream refs must never be merged into `master`), and the files that must never change during a sync. ## Releases