diff --git a/.gitattributes b/.gitattributes index 1d4232b23296..2024db716b8a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,3 +6,11 @@ Resources/markdown-viewer/diff-viewer/**/*.mjs -whitespace # source, so exclude them from GitHub Linguist's language stats. Resources/markdown-viewer/** linguist-vendored Resources/markdown-viewer/diff-viewer-app/** linguist-generated + +# Xcode string catalogs are one large JSON object keyed by string id. Two +# branches that each add a different key collide positionally, producing dozens +# of conflict hunks with no semantic disagreement. scripts/merge-xcstrings.py +# merges per key and defers to the default driver whenever both sides changed +# the same key. Register it with scripts/install-git-hooks.sh (run by setup.sh); +# without that config git silently uses the default driver. +*.xcstrings merge=xcstrings diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index d0c2dfae9af2..4b38f45b748a 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -120,6 +120,10 @@ jobs: if: ${{ matrix.group == 'preflight' }} run: python3 tests/test_localizable_xcstrings_structure.py + - name: Validate .xcstrings merge driver + if: ${{ matrix.group == 'preflight' }} + run: python3 tests/test_merge_xcstrings.py + - name: Validate Python test harness syntax if: ${{ matrix.group == 'preflight' }} run: git ls-files 'tests/*.py' 'tests_v2/*.py' 'scripts/*.py' | xargs python3 -m py_compile diff --git a/scripts/install-git-hooks.sh b/scripts/install-git-hooks.sh index dc5e66f65554..2b42cdaf7db5 100755 --- a/scripts/install-git-hooks.sh +++ b/scripts/install-git-hooks.sh @@ -10,3 +10,9 @@ cd "$REPO_ROOT" git config core.hooksPath scripts/git-hooks chmod +x scripts/git-hooks/* echo "==> Git hooks installed (core.hooksPath = scripts/git-hooks)." + +# Merge drivers named by .gitattributes have to be defined per clone; git will +# not run a driver it cannot resolve, it just falls back to the default one. +git config merge.xcstrings.name "Xcode string catalog (key-wise three-way merge)" +git config merge.xcstrings.driver "python3 scripts/merge-xcstrings.py %O %A %B %P" +echo "==> .xcstrings merge driver installed (merge.xcstrings.driver)." diff --git a/scripts/merge-xcstrings.py b/scripts/merge-xcstrings.py new file mode 100755 index 000000000000..e76a56b484f1 --- /dev/null +++ b/scripts/merge-xcstrings.py @@ -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)) diff --git a/tests/test_merge_xcstrings.py b/tests/test_merge_xcstrings.py new file mode 100644 index 000000000000..15c7bd773321 --- /dev/null +++ b/tests/test_merge_xcstrings.py @@ -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()