Skip to content

merge drops a mode change from one side (chmod-only branch merges as 100644) #18

Closed

Description

@Hazzng

Summary

git merge (and anything else that goes through merge-ort) loses a file's mode when one side changes it. Git merges the mode separately from the content: if one side kept the base mode, the result takes the other side's mode. just-git compares blob hashes only, then takes whichever side's mode that branch of the code happens to use. Three cases go wrong:

  1. Theirs changes only the mode (chmod 755). The hashes match, so the path resolves trivially and keeps ours' 100644.
  2. Ours changes only the mode, theirs changes the content. By hash only theirs changed, so the result takes theirs' entry, mode included, and ours' 100755 is lost.
  3. Both change the content and theirs also sets the exec bit. The clean three-way merge always writes ours.mode.

The merge commit records 100644 without any message, and the diffstat shows the lost mode as create mode 100644 p / delete mode 100755 p.

Repro (just-git 1.9.1 + just-bash 3.6.0)

The script never commits after a branch switch, so the stale worktree modes from #10 can't leak in.

import { Bash, InMemoryFs } from "just-bash";
import { createGit } from "just-git";

const env = { GIT_AUTHOR_NAME: "a", GIT_AUTHOR_EMAIL: "a@b", GIT_COMMITTER_NAME: "a", GIT_COMMITTER_EMAIL: "a@b" };
const bash = new Bash({ fs: new InMemoryFs(), customCommands: [createGit()], env, cwd: "/home/user" });
const r = await bash.exec(`
c() { git add -A && git commit -qm "$1"; }
mkdir r && cd r && git init -q -b main
printf "1\\n2\\n3\\n4\\n5\\n" > f && echo k > m && echo q > p && c base
printf "O\\n2\\n3\\n4\\n5\\n" > f && chmod 755 p && echo z > z && c ours
git checkout -q -b t main~1
printf "1\\n2\\n3\\n4\\nT\\n" > f && printf "q\\nT\\n" > p && chmod 755 m f && c theirs
git checkout -q main && git merge -m mg t; git ls-files -s f m p
git checkout -q -b c2 main~1 && printf "X\\n2\\n3\\n4\\nX\\n" > f && c c2 && git merge t; git ls-files -s f
`);
console.log(r.stdout + r.stderr);

just-git 1.9.1:

Auto-merging f
Merge made by the 'ort' strategy.
 f | 2 +-
 p | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)
 create mode 100644 p
 delete mode 100755 p
100644 9d8d2f23dd74ac4621ad6af8f309ae779b7d4323 0	f
100644 b68fde2a051d9af2fe3ff4c96c0898e5a3212e4d 0	m
100644 aa9acc5b1bfab5b4d310271e5cc52df34cb5c1c4 0	p

git 2.34.1, same script:

Auto-merging f
Merge made by the 'ort' strategy.
 f | 2 +-
 m | 0
 p | 1 +
 3 files changed, 2 insertions(+), 1 deletion(-)
 mode change 100644 => 100755 f
 mode change 100644 => 100755 m
100755 9d8d2f23dd74ac4621ad6af8f309ae779b7d4323 0	f
100755 b68fde2a051d9af2fe3ff4c96c0898e5a3212e4d 0	m
100755 aa9acc5b1bfab5b4d310271e5cc52df34cb5c1c4 0	p

The conflicted merge (c2) gets the same stages from both: 100644/100644/100755. Git also writes the conflicted worktree file with the merged mode, which here is 100755 because ours kept the base mode. just-git writes it with ours' mode (worktreeBlobs.set(path, { ..., mode: ours!.mode })). You only see that once checkout can set modes (#15).

The same results come out with #14 and #15 merged in, so this is separate from #10.

Where (at b509001, the 1.9.1 commit)

  • src/lib/merge-ort.ts collectMergeInfo builds matchMask from hashes only.
  • triviallyResolve decides "unchanged", "only ours changed" and "only theirs changed" by hash, then copies that side's mode. That covers cases 1 and 2.
  • processEntry, three-way branch: the shortcuts oh === bh, th === bh and oh === th copy one side's mode. The clean diff3Merge result is written with ours!.mode, which is case 3. Conflicted results put ours!.mode into worktreeBlobs.

Git compares mode and oid together when building the match mask (collect_merge_info_callback). Then handle_content_merge picks the mode:

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

Suggested fix

Compare (hash, mode) pairs in matchMask and triviallyResolve. Wherever a three-way path with a base picks a regular-file mode, use ours.mode === base.mode ? theirs.mode : ours.mode. That covers the processEntry shortcuts, the clean merge result and the conflicted worktree blob. With only 100644 and 100755 possible, the rule never conflicts.

The rename paths (mergeRenameContent, handleRenameAddAdd) also pick oursEntry.mode in places and probably want the same rule.

Why it matters

Agents in our sandboxes merge branches of client repos. A branch that only makes scripts/deploy.sh executable merges cleanly, says nothing, and the merge commit drops the bit again, so the client's CI fails later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions