fix: prevent duplicate content in merge-previous-releases auto-merge - #283
Conversation
| fi | ||
| # Reset all files to destination (new RC) version | ||
| echo "Resetting all files to destination branch version..." | ||
| git checkout HEAD -- . 2>/dev/null || true |
There was a problem hiding this comment.
@Gudahtt
Two edge cases: a) if old RC has content not yet in main, it gets discarded; b) if destination deleted file that source has, it gets un-deleted. Not sure if this can happen in real life?
There was a problem hiding this comment.
We expect all changes in the old RC to be on main in all cases, so I'm not worried about that case. If that happens, we're already in a mess that we'd have to clean up manually.
For the second case, could you elaborate? Why would the file get un-deleted? The goal here is to match the destination, so we don't want to restore files the destination has deleted.
There was a problem hiding this comment.
I can't recall how git checkout handles deleted files. If you're right about this, we may need special handling here of deleted files to ensure they remain deleted.
There was a problem hiding this comment.
Ran a quick test. After git merge --no-commit, if old RC has a file that destination doesn't have:
- git checkout HEAD -- . - file stays staged (overlay mode, doesn't remove files)
- git checkout --no-overlay HEAD -- . - file gets removed, matches destination exactly
From git-checkout docs:
- default overlay mode:
git checkoutnever removes files from the index or working tree --no-overlay: files present locally but not in are removed
So git checkout HEAD -- . only restores paths that exist in HEAD, it doesn't delete extras brought in by the merge.
So looks like we need to git checkout --no-overlay HEAD -- . (or git merge -s ours)?
There was a problem hiding this comment.
Ah... yes, maybe what we want here is just the -s ours option on git merge. That should eliminate the need for the checkout step as well.
All this trouble was caused by using -s ort -X ours instead of -s ours. ort auto-resolves, preferring ours just for conflicts. -s ours takes ours for everything, doing no conflict resolution. That's what we want here. TIL.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 0d3f577. Configure here.
|
|
||
| - Include `.yarn/patches` in `publish-preview` build artifacts ([#284](https://github.com/MetaMask/github-tools/pull/284)) | ||
| - Restrict `merge-previous-releases` to older branches that still have an open/draft release PR targeting `stable`, instead of every older `release/*` branch ([#282](https://github.com/MetaMask/github-tools/pull/282)) | ||
| - Fix `merge-previous-releases` creating duplicate content when git auto-merges identical changes from both branches ([#283](https://github.com/MetaMask/github-tools/pull/283)) |
There was a problem hiding this comment.
Oops, this was put under the wrong release header

When merging older RCs into a new RC, git's auto-merge can create duplicate content (e.g., duplicate locale keys) when both branches have identical changes. This happens because git doesn't recognize the changes are the same and tries to combine them.
Fix: use
--no-commit --no-ff, then reset all files to destination before committing. This preserves merge history while keeping destination content.Fixes: https://consensyssoftware.atlassian.net/browse/MCRM-143
Note
Medium Risk
Changes how release RC branches are merged in automation; wrong behavior could affect release branch content, though the new strategy is simpler and explicitly preserves the destination branch.
Overview
Fixes
merge-previous-releasesrecording older RC merges in a way that left duplicate content when Git auto-combined the same change from both branches (e.g. duplicated locale keys).merge_with_favor_destinationno longer runs a normal merge with-X oursand manual conflict cleanup. It now usesgit merge -s ours, which still adds a merge commit for history but keeps the new release branch tree unchanged. Script comments describe this behavior; CHANGELOG documents the fix under 1.18.1.Reviewed by Cursor Bugbot for commit 0d3f577. Bugbot is set up for automated code reviews on this repo. Configure here.