Fix #18: merge keeps a mode change from either side

Merged
HA
5d ago
+488-79

Fixes #18

Summary

git merge now keeps a file mode change made on either side. The issue's script prints the same output as git 2.34.1, byte for byte. That covers the m | 0 row, the mode change 100644 => 100755 lines, 100755 for f, m and p in the clean merge, and the stages of the conflicted one. Cherry-pick, rebase, revert and stash apply go through the same merge-ort code, so they pick up the fix too. A cherry-pick of a commit that only runs chmod +x used to stop with "The previous cherry-pick is now empty". It now commits the mode change, as git does.

Two related changes came out of comparing against git:

  • When both sides add the same path with different modes, git reports CONFLICT (add/add), even if the content is identical. just-git used to resolve it silently with ours' mode. It now conflicts the same way.
  • Commit and merge summaries printed a mode change as a create mode / delete mode pair. They now print mode change 100644 => 100755 f, and a renamed path whose mode changed gets git's mode change 100644 => 100755 line after the rename line. This also fixes the third Open item in #15.

Root cause

collectMergeInfo built matchMask from blob hashes, and triviallyResolve decided "unchanged", "only ours changed" and "only theirs changed" by hash, then copied that side's mode. A side that changed only the mode looked unchanged. In processEntry the three-way shortcuts (oh === bh, th === bh, oh === th) copied one side's mode, and the clean diff3 result and the conflicted worktree blob used ours.mode. The plain rename branches in detectAndProcessRenames had the same pattern, as did rename/rename to one path and rename/rename(1to2).

Git compares mode and oid together in collect_merge_info_callback, and handle_content_merge merges the mode on its own:

if (a->mode == b->mode || a->mode == o->mode)
	result->mode = b->mode;
else
	result->mode = a->mode;

The fix

All in src/lib/merge-ort.ts, plus the summary lines in src/lib/commit-summary.ts.

  • sameVersion(a, b) compares hash and mode. matchMask and triviallyResolve use it.
  • mergeFileMode(base, ours, theirs) returns ours === base ? theirs : ours when all three are 100644 or 100755, and null otherwise. Callers fall back to the mode they used before, so symlink, gitlink and type-change handling is unchanged.
  • processEntry uses the merged mode in the three shortcuts, the clean diff3 result, the clean merge driver result, and the worktree blob of binary, driver and diff3 conflicts. Stages 1, 2 and 3 keep each side's own mode, which was already right.
  • The plain rename branches (either side renamed), rename/rename to the same path, and rename/rename(1to2) use the merged mode. For 1to2 git puts the merged mode on both new paths, at stages 2 and 3 and in the worktree.
  • The add/add branch reports a conflict when both sides are regular files with different modes. Its four copies of the conflict bookkeeping now go through one local addAddConflict helper. The conflict message has Auto-merging only when the contents differ, which matches git.

Messages for mode-only cases match git. A mode-only difference prints no Auto-merging, because the shortcuts return before it is pushed.

Tests

  • test/commands/merge.test.ts, "file modes", 9 tests. Each asserts exact stdout, empty stderr, the exit code and git ls-files -s. They cover theirs mode-only, ours mode-only plus theirs content, both content plus theirs mode, a content conflict with theirs' mode change, both sides making the same mode change, add/add with different modes, theirs' mode change to a file ours renamed, rename/rename(1to2), and the fast-forward summary. Every expected string was checked against git 2.34.1 with the same setup.
  • test/lib/merge.test.ts, 2 tests. A conflicted content merge where theirs or ours changed the mode writes 100755 for the file in the result tree, which is what checkout writes to the worktree. Stages 1, 2 and 3 carry each side's mode.
  • test/commands/cherry-pick.test.ts: cherry-pick of a mode-only commit commits it and prints the mode change line.
  • test/commands/commit.test.ts: commit summaries for a mode-only change and for a content plus mode change.
  • test/interop/merge-modes.test.ts, 2 tests. Real git builds the issue's repository on disk and is copied twice. Real git merges in one copy and just-git in the other. stdout, stderr, exit code and real git's ls-files -s must match, for the clean merge and the conflicted one. The conflicted file's content is compared too.

The tests land in a commit before the fix. On the b509001 source, 13 of the 15 new tests fail. The two that pass are the "same mode change on both sides" test and the lib test where ours changed the mode. The old code got both right by accident, and they are there to pin the new rule.

bun test: 3527 pass, 29 skip, 0 fail. The baseline at b509001 is 3512 pass, 29 skip, 0 fail. bun check is clean and bun gen-docs produces no change. I did not run the oracle suite.

Open

  • On 1.9.1 the FileSystem has no chmod, so the worktree file keeps its old permission bits after a merge. The result tree now carries the merged mode, so checkout will apply it once #15 lands. Until then, resolving a conflict with git add stages the worktree's mode, which can still be the old one.
  • Stash apply uses the same code. git stash on main cannot save a mode-only change (Open item 2 in #15), and an applied mode only reaches the worktree, so there is no stash test here.
  • handleRenameAddAdd and rename/delete still pick a side's mode. In the cases I tried, their index result matched git, so I left them alone.
  • A file that becomes a symlink on one side keeps the old per-branch behaviour. Git reports a "distinct types" conflict there.
  • Unrelated to modes, found while comparing with git:
    • rename/rename(1to2) with a clean content merge does not print Auto-merging f.
    • Ours renames f to g. Theirs edits f and adds g with the base content. Git takes theirs' edit at g. just-git keeps the unedited content and prints an extra Auto-merging f.