Repository navigation
ci: merge .xcstrings per key instead of per line #13692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| #!/usr/bin/env python3 | ||
| """Git merge driver for Xcode string catalogs (.xcstrings). | ||
|
|
||
| A string catalog is one large JSON object keyed by string id. Two pull requests | ||
| that each add a new key collide positionally even though the keys are disjoint, | ||
| because the additions land in the same region of the file. On | ||
| Resources/Localizable.xcstrings (~6,700 entries, ~541k lines) that produced 42 | ||
| conflict hunks in a single pull request, none of which were semantic. | ||
|
|
||
| This driver merges per key instead of per line. It is deliberately conservative: | ||
| when the same key is changed on both sides it exits non-zero and lets git write | ||
| normal conflict markers, so a real disagreement is never resolved silently. | ||
|
|
||
| Formatting is preserved because the catalog round-trips byte-identically through | ||
| `json.dumps(..., ensure_ascii=False, indent=2)` plus a trailing newline; the | ||
| driver asserts that on the inputs before writing. | ||
|
|
||
| Usage (git passes these): merge-xcstrings.py %O %A %B %P | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| SERIALIZED_SUFFIX = "\n" | ||
|
|
||
|
|
||
| def render(document: dict) -> str: | ||
| return json.dumps(document, ensure_ascii=False, indent=2) + SERIALIZED_SUFFIX | ||
|
|
||
|
|
||
| def load(path: Path) -> tuple[dict, bool]: | ||
| """Return the parsed catalog and whether it re-renders byte-identically.""" | ||
| text = path.read_text(encoding="utf-8") | ||
| document = json.loads(text) | ||
| return document, render(document) == text | ||
|
|
||
|
|
||
| def merge_mapping(base: dict, ours: dict, theirs: dict, label: str) -> tuple[dict, list[str]]: | ||
| """Three-way merge one mapping. Returns (merged, conflicting keys).""" | ||
| merged: dict = {} | ||
| conflicts: list[str] = [] | ||
| # Preserve ours' order, then append keys only theirs introduced. | ||
| for key in list(ours) + [k for k in theirs if k not in ours]: | ||
| in_base, in_ours, in_theirs = key in base, key in ours, key in theirs | ||
| ours_value = ours.get(key) | ||
| theirs_value = theirs.get(key) | ||
| base_value = base.get(key) | ||
| ours_changed = ours_value != base_value if in_base else in_ours | ||
| theirs_changed = theirs_value != base_value if in_base else in_theirs | ||
| if not in_ours and not in_theirs: | ||
| continue | ||
| if ours_changed and theirs_changed: | ||
| if ours_value == theirs_value: | ||
| if in_ours: | ||
| merged[key] = ours_value | ||
| continue | ||
| conflicts.append(f"{label}.{key}") | ||
| continue | ||
| if ours_changed: | ||
| if in_ours: | ||
| merged[key] = ours_value | ||
| continue | ||
| if theirs_changed: | ||
| if in_theirs: | ||
| merged[key] = theirs_value | ||
| continue | ||
| merged[key] = ours_value | ||
| return merged, conflicts | ||
|
|
||
|
|
||
| def merge_catalog(base: dict, ours: dict, theirs: dict) -> tuple[dict, list[str]]: | ||
| strings, conflicts = merge_mapping( | ||
| base.get("strings", {}), ours.get("strings", {}), theirs.get("strings", {}), "strings" | ||
| ) | ||
| top_base = {k: v for k, v in base.items() if k != "strings"} | ||
| top_ours = {k: v for k, v in ours.items() if k != "strings"} | ||
| top_theirs = {k: v for k, v in theirs.items() if k != "strings"} | ||
| merged, top_conflicts = merge_mapping(top_base, top_ours, top_theirs, "catalog") | ||
| merged["strings"] = strings | ||
| ordered = {k: merged[k] for k in ("sourceLanguage", "strings", "version") if k in merged} | ||
| ordered.update({k: v for k, v in merged.items() if k not in ordered}) | ||
| return ordered, conflicts + top_conflicts | ||
|
|
||
|
|
||
| def main(argv: list[str]) -> int: | ||
| if len(argv) < 4: | ||
| print("usage: merge-xcstrings.py %O %A %B [%P]", file=sys.stderr) | ||
| return 2 | ||
| base_path, ours_path, theirs_path = (Path(p) for p in argv[1:4]) | ||
| name = argv[4] if len(argv) > 4 else str(ours_path) | ||
| try: | ||
| base, base_exact = load(base_path) | ||
| ours, ours_exact = load(ours_path) | ||
| theirs, theirs_exact = load(theirs_path) | ||
| except (OSError, ValueError) as error: | ||
| print(f"merge-xcstrings: {name}: cannot parse ({error}); falling back", file=sys.stderr) | ||
| return 1 | ||
| if not (base_exact and ours_exact and theirs_exact): | ||
| print( | ||
| f"merge-xcstrings: {name}: input is not canonically serialized; " | ||
| "falling back to the default driver so formatting is not rewritten", | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| merged, conflicts = merge_catalog(base, ours, theirs) | ||
| if conflicts: | ||
| print( | ||
| f"merge-xcstrings: {name}: {len(conflicts)} key(s) changed on both sides; " | ||
| "leaving them to the default driver: " + ", ".join(conflicts[:5]), | ||
| file=sys.stderr, | ||
| ) | ||
| return 1 | ||
| ours_path.write_text(render(merged), encoding="utf-8") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main(sys.argv)) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| #!/usr/bin/env python3 | ||
| """Contracts for the .xcstrings git merge driver. | ||
|
|
||
| The driver must merge disjoint key additions (the common case) and must refuse | ||
| to resolve a key that both sides changed differently, so a real disagreement | ||
| still reaches the author as a normal git conflict. | ||
| """ | ||
|
|
||
| import json | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
| DRIVER = ROOT / "scripts" / "merge-xcstrings.py" | ||
|
|
||
|
|
||
| def unit(value): | ||
| return {"localizations": {"en": {"stringUnit": {"state": "translated", "value": value}}}} | ||
|
|
||
|
|
||
| def catalog(strings): | ||
| return {"sourceLanguage": "en", "strings": strings, "version": "1.0"} | ||
|
|
||
|
|
||
| def render(document): | ||
| return json.dumps(document, ensure_ascii=False, indent=2) + "\n" | ||
|
|
||
|
|
||
| def run(base, ours, theirs): | ||
| with tempfile.TemporaryDirectory() as directory: | ||
| paths = {} | ||
| for name, document in (("O", base), ("A", ours), ("B", theirs)): | ||
| path = Path(directory) / f"{name}.json" | ||
| path.write_text(document if isinstance(document, str) else render(document), encoding="utf-8") | ||
| paths[name] = path | ||
| result = subprocess.run( | ||
| [sys.executable, str(DRIVER), str(paths["O"]), str(paths["A"]), str(paths["B"]), "Localizable.xcstrings"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| merged = paths["A"].read_text(encoding="utf-8") | ||
| return result.returncode, merged, result.stderr | ||
|
|
||
|
|
||
| def test_disjoint_additions_merge(): | ||
| base = catalog({"a": unit("A")}) | ||
| ours = catalog({"a": unit("A"), "b": unit("B")}) | ||
| theirs = catalog({"a": unit("A"), "c": unit("C")}) | ||
| code, merged, _ = run(base, ours, theirs) | ||
| assert code == 0, "disjoint additions must merge" | ||
| strings = json.loads(merged)["strings"] | ||
| assert set(strings) == {"a", "b", "c"}, strings.keys() | ||
|
|
||
|
|
||
| def test_same_key_same_value_is_not_a_conflict(): | ||
| base = catalog({"a": unit("old")}) | ||
| ours = catalog({"a": unit("new")}) | ||
| theirs = catalog({"a": unit("new")}) | ||
| code, merged, _ = run(base, ours, theirs) | ||
| assert code == 0 | ||
| assert json.loads(merged)["strings"]["a"] == unit("new") | ||
|
|
||
|
|
||
| def test_same_key_diverging_falls_back_to_git(): | ||
| base = catalog({"a": unit("old")}) | ||
| ours = catalog({"a": unit("ours")}) | ||
| theirs = catalog({"a": unit("theirs")}) | ||
| code, merged, stderr = run(base, ours, theirs) | ||
| assert code == 1, "a real disagreement must not be resolved silently" | ||
| assert "strings.a" in stderr, stderr | ||
| assert json.loads(merged)["strings"]["a"] == unit("ours"), "ours must be left untouched for git" | ||
|
|
||
|
|
||
| def test_one_sided_delete_applies(): | ||
| base = catalog({"a": unit("A"), "b": unit("B")}) | ||
| ours = catalog({"a": unit("A"), "b": unit("B")}) | ||
| theirs = catalog({"a": unit("A")}) | ||
| code, merged, _ = run(base, ours, theirs) | ||
| assert code == 0 | ||
| assert set(json.loads(merged)["strings"]) == {"a"} | ||
|
|
||
|
|
||
| def test_delete_versus_modify_conflicts(): | ||
| base = catalog({"a": unit("A")}) | ||
| ours = catalog({"a": unit("changed")}) | ||
| theirs = catalog({}) | ||
| code, _, stderr = run(base, ours, theirs) | ||
| assert code == 1, stderr | ||
|
|
||
|
|
||
| def test_non_canonical_input_falls_back(): | ||
| base = catalog({"a": unit("A")}) | ||
| ours = json.dumps(catalog({"a": unit("A"), "b": unit("B")}), indent=4) # wrong indent | ||
| theirs = catalog({"a": unit("A"), "c": unit("C")}) | ||
| code, _, stderr = run(base, ours, theirs) | ||
| assert code == 1, "must not rewrite a catalog it cannot reproduce byte-for-byte" | ||
| assert "canonically serialized" in stderr, stderr | ||
|
|
||
|
|
||
| def test_unparseable_input_falls_back(): | ||
| code, _, stderr = run(catalog({}), "{not json", catalog({})) | ||
| assert code == 1 | ||
| assert "cannot parse" in stderr, stderr | ||
|
|
||
|
|
||
| def main(): | ||
| tests = [value for name, value in sorted(globals().items()) if name.startswith("test_")] | ||
| for test in tests: | ||
| test() | ||
| print(f"ok {test.__name__}") | ||
| print(f"\n{len(tests)} tests passed") | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 9759
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 34560
🌐 Web query:
official Git gitattributes custom merge driver nonzero exit status %A result file conflict markers💡 Result:
<source_evidence>
Citations:
Materialize the text-driver fallback in
%A..gitattributesroutes*.xcstringsto the custommerge.xcstringsdriver. A custom driver must write its result to%A. A nonzero exit reports that the result contains conflicts; it does not cause Git to invoke the built-in text driver.The parse-error, non-canonical-input, and key-conflict branches return nonzero without writing
ours_path. A fallback can therefore leave%Aunchanged instead of producing the text driver's merge result or conflict markers. Delegate each fallback togit merge-fileor an equivalent operation that writes toours_path, then return its status. Update the fallback tests so they assert the materialized result rather than unchangedours.Suggested fix
The divergent-key test should also assert conflict markers in
mergedinstead of parsing%Aas unchanged JSON.🤖 Prompt for AI Agents