From 47a0aae4c1c0d85bce938d17504cd016945005fb Mon Sep 17 00:00:00 2001 From: "Myeongjin (Daniel)" Date: Tue, 13 Oct 2026 00:49:37 -0400 Subject: [PATCH 1/3] Add three-way merge and conflict resolution. --- minigit/commits.py | 339 ++++++++++++++++++++++++++++++++++++++++-- minigit/index.py | 11 +- tests/test_commits.py | 290 +++++++++++++++++++++++++++++++++++- 3 files changed, 621 insertions(+), 19 deletions(-) diff --git a/minigit/commits.py b/minigit/commits.py index d060ef6..dc76ef6 100644 --- a/minigit/commits.py +++ b/minigit/commits.py @@ -11,32 +11,46 @@ import os import time +from dataclasses import dataclass from minigit.errors import ( + MergeConflictError, MiniGitError, ObjectCorruptError, RefExistsError, RefNotFoundError, ) from minigit.index import WorkingTree -from minigit.objects import ObjectStore +from minigit.objects import ObjectStore, TreeEntry def _cmd_commit(args) -> int: """Handle the 'minigit commit -m ' command""" manager = CommitManager() + pending = manager._read_pending_merge() + if pending is not None: + _, _, _, unresolved = pending + if unresolved: + raise MiniGitError("unresolved merge paths: " + ", ".join(unresolved)) tree_hash = manager.tree.build_tree_from_index() branch = manager._current_branch() - last_commit = manager.read_ref(branch) - parents = [last_commit] if last_commit else [] + if pending is None: + last_commit = manager.read_ref(branch) + parents = [last_commit] if last_commit else [] + message = args.message + else: + ours, theirs, message, _ = pending + parents = [ours, theirs] author = manager.read_author() commit_hash = manager.create_commit( tree_hash, parents, author, - args.message, + message, ) - print(f"[{branch} {commit_hash[:7]}] {args.message}") + if pending is not None: + manager._clear_pending_merge() + print(f"[{branch} {commit_hash[:7]}] {message}") return 0 @@ -222,6 +236,8 @@ def switch_branch(self, name) -> None: """ Point HEAD at name and restore the working tree from that branch's commit """ + if self._read_pending_merge() is not None: + raise MiniGitError("merge in progress") if not os.path.exists(self._ref_path(name)): raise RefNotFoundError(name) self._require_no_staged_changes() @@ -334,12 +350,13 @@ def list_branches(self) -> list[str]: return sorted(os.listdir(self._refs_dir())) def merge(self, branch_name) -> str | None: - # Week 4 fast-forward / Week 5 three-way - """Fast-forward the current branch to `branch_name` when history allows it + """Fast-forward or three-way merge `branch_name` into the current branch. - Raises RefNotFoundError for an - unknown branch, and MiniGitError when the two histories have diverged. + Fast-forward and no-op cases return None. A clean three-way merge returns + the new commit hash. A text conflict raises MergeConflictError. """ + if self._read_pending_merge() is not None: + raise MiniGitError("merge in progress") if not os.path.exists(self._ref_path(branch_name)): raise RefNotFoundError(branch_name) @@ -357,7 +374,227 @@ def merge(self, branch_name) -> str | None: self.write_ref(current, target) return None - raise MiniGitError("three-way merge is not implemented yet") + return self._three_way_merge(current_hash, target, branch_name) + + def find_merge_base(self, ours: str, theirs: str) -> str: + """Return the single lowest common ancestor of two commits.""" + common = set(self.walk_history(ours)) & set(self.walk_history(theirs)) + if not common: + raise MiniGitError("no common ancestor") + lowest = [ + commit + for commit in common + if not any(other != commit and self.is_ancestor(commit, other) for other in common) + ] + if len(lowest) != 1: + raise MiniGitError("multiple merge bases") + return lowest[0] + + def mark_path_resolved(self, path: str) -> None: + """Drop a path from MERGE_UNRESOLVED after an explicit add.""" + pending = self._read_pending_merge() + if pending is None: + return + ours, theirs, message, unresolved = pending + remaining = [item for item in unresolved if item != path] + self._write_pending_merge(ours, theirs, message, remaining) + + def _three_way_merge(self, current_hash: str, target: str, branch_name: str) -> str: + """Merge two diverged commits and either commit or raise MergeConflictError.""" + base_hash = self.find_merge_base(current_hash, target) + base_map = self._flatten(self.read_commit(base_hash).tree) + ours_map = self._flatten(self.read_commit(current_hash).tree) + theirs_map = self._flatten(self.read_commit(target).tree) + + result_files: dict[str, tuple[str, str]] = {} + conflict_paths: list[str] = [] + for path in sorted(set(base_map) | set(ours_map) | set(theirs_map)): + base = base_map.get(path) + ours = ours_map.get(path) + theirs = theirs_map.get(path) + taken = self._take_side(base, ours, theirs) + if taken == "conflict": + conflict_paths.append(path) + marker = self._conflict_text( + self._decode_entry(ours), self._decode_entry(theirs), branch_name + ) + mode = ours[0] if ours else theirs[0] + result_files[path] = (mode, self.store.write_object(marker.encode(), "blob")) + elif taken == "merge-text": + merged = self._merge_blob_pair(base, ours, theirs) + if merged is None: + conflict_paths.append(path) + marker = self._conflict_text( + self._decode_entry(ours), self._decode_entry(theirs), branch_name + ) + result_files[path] = (ours[0], self.store.write_object(marker.encode(), "blob")) + else: + result_files[path] = (ours[0], self.store.write_object(merged.encode(), "blob")) + elif taken is not None: + result_files[path] = taken + + if self._has_file_directory_conflict(result_files): + raise MiniGitError("file/directory conflict") + self._require_no_staged_changes() + result_tree = self._build_tree_from_paths(result_files) + message = f"Merge branch '{branch_name}'" + + if not conflict_paths: + self.tree.checkout(result_tree) + return self.create_commit( + result_tree, [current_hash, target], self.read_author(), message + ) + + pre_index = {entry.path: entry for entry in self.tree.read_index()} + self.tree.checkout(result_tree) + restored = [] + for entry in self.tree.read_index(): + if entry.path in conflict_paths: + if entry.path in pre_index: + restored.append(pre_index[entry.path]) + continue + restored.append(entry) + for path in conflict_paths: + if path in pre_index and all(entry.path != path for entry in restored): + restored.append(pre_index[path]) + self.tree.write_index(restored) + self._write_pending_merge(current_hash, target, message, conflict_paths) + raise MergeConflictError(sorted(conflict_paths)) + + def _flatten(self, tree_hash: str) -> dict[str, tuple[str, str]]: + """Return path -> (mode, hash) using M2.""" + return {e.path: (e.mode, e.hash) for e in self.tree.read_tree_entries(tree_hash)} + + def _take_side(self, base, ours, theirs): + """Return the winning (mode, hash), None to delete, or a conflict/text-merge sentinel.""" + if ours == theirs: + return ours + if ours == base: + return theirs + if theirs == base: + return ours + if ours is None or theirs is None: + return "conflict" + return "merge-text" + + def _blob_bytes(self, blob_hash: str) -> bytes: + """Read a blob through M1.""" + obj_type, data = self.store.read_object(blob_hash) + if obj_type != "blob": + raise ObjectCorruptError(blob_hash) + return data + + def _decode_entry(self, entry: tuple[str, str] | None) -> str: + """Return blob text for a flattened entry, or empty string if the path is missing.""" + if entry is None: + return "" + data = self._blob_bytes(entry[1]) + if b"\0" in data: + raise MiniGitError("binary file cannot be merged") + try: + return data.decode() + except UnicodeDecodeError as error: + raise MiniGitError("binary file cannot be merged") from error + + def _merge_blob_pair(self, base, ours, theirs) -> str | None: + """Merge two changed blobs against base. None means a text conflict.""" + if ours[0] != theirs[0]: + raise MiniGitError("mode conflict") + ours_data = self._blob_bytes(ours[1]) + theirs_data = self._blob_bytes(theirs[1]) + base_data = self._blob_bytes(base[1]) if base else b"" + if b"\0" in ours_data or b"\0" in theirs_data or b"\0" in base_data: + raise MiniGitError("binary file cannot be merged") + try: + return _merge_text(base_data.decode(), ours_data.decode(), theirs_data.decode()) + except UnicodeDecodeError as error: + raise MiniGitError("binary file cannot be merged") from error + + def _has_file_directory_conflict(self, result: dict[str, tuple[str, str]]) -> bool: + """Return True when a path is both a file and a parent of another path.""" + paths = sorted(result) + for i, path in enumerate(paths): + prefix = path + "/" + for other in paths[i + 1 :]: + if other.startswith(prefix): + return True + return False + + def _build_tree_from_paths(self, files: dict[str, tuple[str, str]]) -> str: + """Write the result tree through M1.""" + root: dict = {} + for path, (mode, blob_hash) in files.items(): + node = root + parts = path.split("/") + for part in parts[:-1]: + node = node.setdefault(part, {}) + node[parts[-1]] = (mode, blob_hash) + + def write_dir(node: dict) -> str: + """Write one tree object from a nested path map.""" + entries = [] + for name in sorted(node): + value = node[name] + if isinstance(value, dict): + entries.append(TreeEntry("40000", "tree", write_dir(value), name)) + else: + mode, blob_hash = value + entries.append(TreeEntry(mode, "blob", blob_hash, name)) + return self.store.write_tree(entries) + + return write_dir(root) + + def _conflict_text(self, ours_text: str, theirs_text: str, branch_name: str) -> str: + """Build a conflict-marker blob.""" + return f"<<<<<<< HEAD\n{ours_text}=======\n{theirs_text}>>>>>>> {branch_name}\n" + + def _merge_state_paths(self) -> dict[str, str]: + """Return the pending-merge file paths under `.minigit/`.""" + meta = os.path.join(self.root, ".minigit") + return { + "ours": os.path.join(meta, "MERGE_OURS"), + "theirs": os.path.join(meta, "MERGE_HEAD"), + "message": os.path.join(meta, "MERGE_MSG"), + "unresolved": os.path.join(meta, "MERGE_UNRESOLVED"), + } + + def _write_pending_merge(self, ours: str, theirs: str, message: str, paths: list[str]) -> None: + """Save both parents, the merge message, and unresolved paths.""" + paths_map = self._merge_state_paths() + os.makedirs(os.path.join(self.root, ".minigit"), exist_ok=True) + with open(paths_map["ours"], "w") as handle: + handle.write(ours + "\n") + with open(paths_map["theirs"], "w") as handle: + handle.write(theirs + "\n") + with open(paths_map["message"], "w") as handle: + handle.write(message + "\n") + with open(paths_map["unresolved"], "w") as handle: + handle.write("".join(path + "\n" for path in sorted(paths))) + + def _read_pending_merge(self) -> tuple[str, str, str, list[str]] | None: + """Return (ours, theirs, message, unresolved paths), or None if no merge is pending.""" + paths_map = self._merge_state_paths() + if not os.path.exists(paths_map["theirs"]): + return None + + def read(path: str) -> str: + """Read a pending-merge file as text.""" + with open(path) as handle: + return handle.read() + + unresolved = [line for line in read(paths_map["unresolved"]).splitlines() if line] + return ( + read(paths_map["ours"]).strip(), + read(paths_map["theirs"]).strip(), + read(paths_map["message"]).strip(), + unresolved, + ) + + def _clear_pending_merge(self) -> None: + """Delete pending-merge files after a completed merge commit.""" + for path in self._merge_state_paths().values(): + if os.path.exists(path): + os.remove(path) def log(self) -> list[str]: """Return one summary line per commit reachable from HEAD, newest first""" @@ -375,3 +612,85 @@ def log(self) -> list[str]: lines.append(f"{i[:7]} {summary}") return lines + + +@dataclass +class Hunk: + """One edit against the base file""" + + base_start: int + base_count: int + lines: list[str] + + +def _diff_lines(base: list[str], other: list[str]) -> list[Hunk]: + """Fake line diff""" + hunks = [] + i = 0 + j = 0 + while i < len(base) or j < len(other): + if i < len(base) and j < len(other) and base[i] == other[j]: + i += 1 + j += 1 + continue + start_i, start_j = i, j + if i < len(base) and (j >= len(other) or base[i] not in other[j:]): + i += 1 + elif j < len(other) and (i >= len(base) or other[j] not in base[i:]): + j += 1 + else: + i += 1 + j += 1 + hunks.append(Hunk(start_i, i - start_i, other[start_j:j])) + return hunks + + +def _ranges_overlap(a: Hunk, b: Hunk) -> bool: + """True if hunks overlap or insert at the same base index""" + if a.base_count == 0 and b.base_count == 0: + return a.base_start == b.base_start + return a.base_start < b.base_start + b.base_count and b.base_start < a.base_start + a.base_count + + +def _combine_hunks(ours: list[Hunk], theirs: list[Hunk]) -> list[Hunk] | None: + """Combine disjoint hunks""" + merged = [] + for hunk in ours + theirs: + skip = False + for existing in merged: + if (existing.base_start, existing.base_count, existing.lines) == ( + hunk.base_start, + hunk.base_count, + hunk.lines, + ): + skip = True + break + if _ranges_overlap(existing, hunk): + return None + if not skip: + merged.append(hunk) + return sorted(merged, key=lambda hunk: hunk.base_start) + + +def _apply_hunks(base_lines: list[str], hunks: list[Hunk]) -> list[str]: + """Apply combined hunks to base.""" + result = [] + index = 0 + for hunk in hunks: + result.extend(base_lines[index : hunk.base_start]) + result.extend(hunk.lines) + index = hunk.base_start + hunk.base_count + result.extend(base_lines[index:]) + return result + + +def _merge_text(base_text: str, ours_text: str, theirs_text: str) -> str | None: + """Diff each side against base. Combine or conflict""" + base_lines = base_text.split("\n") + combined = _combine_hunks( + _diff_lines(base_lines, ours_text.split("\n")), + _diff_lines(base_lines, theirs_text.split("\n")), + ) + if combined is None: + return None + return "\n".join(_apply_hunks(base_lines, combined)) diff --git a/minigit/index.py b/minigit/index.py index f842c8f..7a37cd4 100644 --- a/minigit/index.py +++ b/minigit/index.py @@ -299,8 +299,17 @@ def _prune_empty_dirs(self, dir_path: str) -> None: def cmd_add(args) -> int: + """Stage a path. During a merge, that path is marked resolved.""" wt = WorkingTree() - wt.stage_file(args.path) + full_path = os.path.join(wt.root, args.path) + if os.path.isfile(full_path): + wt.stage_file(args.path) + else: + entries = [entry for entry in wt.read_index() if entry.path != args.path] + wt.write_index(entries) + from minigit.commits import CommitManager + + CommitManager(wt.root, store=wt.store, tree=wt).mark_path_resolved(args.path) return 0 diff --git a/tests/test_commits.py b/tests/test_commits.py index e6fef7a..e85163e 100644 --- a/tests/test_commits.py +++ b/tests/test_commits.py @@ -5,8 +5,10 @@ import pytest +from minigit.cli import main from minigit.commits import CommitManager from minigit.errors import ( + MergeConflictError, MiniGitError, ObjectCorruptError, ObjectNotFoundError, @@ -14,7 +16,7 @@ RefNotFoundError, ) from minigit.index import WorkingTree -from minigit.objects import ObjectStore +from minigit.objects import ObjectStore, TreeEntry AUTHOR = "Daniel " @@ -37,11 +39,23 @@ def build_tree_from_index(self) -> str: return self.index_tree def checkout(self, tree_hash: str) -> None: - "record checkout" + """Record checkout.""" if self.fail: raise MiniGitError("checkout failed") self.checked_out.append(tree_hash) + def read_tree_entries(self, tree_hash: str): + """Delegate flatten to a real WorkingTree over the same store.""" + return WorkingTree(str(self.store.root), store=self.store).read_tree_entries(tree_hash) + + def read_index(self): + """Return recorded index entries, if any.""" + return list(getattr(self, "index_entries", [])) + + def write_index(self, entries) -> None: + """Record index writes from merge conflict restoration.""" + self.index_entries = list(entries) + def make_manager(temp_path, tree=None): """Return CommitManager with real store and fake workign tree""" @@ -552,8 +566,8 @@ def test_merge_into_unborn_branch_creates_the_ref(tmp_path): assert m.tree.checked_out == [tree] -def test_merge_diverged_history_raises_and_changes_nothing(tmp_path): - """Diverged histories raise MiniGitError and leave both refs and HEAD alone.""" +def test_merge_diverged_identical_trees_creates_merge_commit(tmp_path): + """Diverged history with the same tree is a clean two-parent merge.""" m = make_manager(tmp_path) tree = make_tree(m) a = m.create_commit(tree, [], AUTHOR, "a") @@ -562,12 +576,12 @@ def test_merge_diverged_history_raises_and_changes_nothing(tmp_path): b = m.create_commit(tree, [a], AUTHOR, "b") m.write_head("main") c = m.create_commit(tree, [a], AUTHOR, "c") - with pytest.raises(MiniGitError, match="three-way merge is not implemented yet"): - m.merge("feature") - assert m.read_ref("main") == c + merge_hash = m.merge("feature") + assert merge_hash is not None + assert m.read_commit(merge_hash).parents == [c, b] + assert m.read_ref("main") == merge_hash assert m.read_ref("feature") == b assert m.read_head() == "main" - assert m.tree.checked_out == [] def test_merge_with_staged_changes_leaves_refs_unchanged(tmp_path): @@ -630,3 +644,263 @@ def test_real_fast_forward_restores_target_files(tmp_path): assert m.read_head() == "main" assert count_object_files(tmp_path) == objects_before assert a != b + + +# week 5 three-way merge + + +def write_file_tree(m, files: dict[str, str]) -> str: + """Write a flat tree of path -> text content and return its hash.""" + entries = [ + TreeEntry("100644", "blob", m.store.write_object(content.encode(), "blob"), path) + for path, content in files.items() + ] + return m.store.write_tree(entries) + + +def commit_tree_files(m, files: dict[str, str], message: str) -> str: + """Commit `files` on the current branch and keep the fake index in sync.""" + tree = write_file_tree(m, files) + parent = m.read_ref(m._current_branch()) + parents = [parent] if parent else [] + commit_hash = m.create_commit(tree, parents, AUTHOR, message) + m.tree.index_tree = tree + return commit_hash + + +def setup_diverged(m, base_files, ours_files, theirs_files): + """Create diverged `main` (ours) and `feature` (theirs) from a shared base.""" + base = commit_tree_files(m, base_files, "base") + m.create_branch("feature", base) + ours = commit_tree_files(m, ours_files, "ours") + m.write_head("feature") + theirs = commit_tree_files(m, theirs_files, "theirs") + m.write_head("main") + m.tree.index_tree = m.read_commit(ours).tree + return ours, theirs + + +def blob_text(m, tree_hash: str, path: str) -> str: + """Return the decoded blob for `path` in `tree_hash`.""" + blob = next(entry.hash for entry in m.tree.read_tree_entries(tree_hash) if entry.path == path) + _, data = m.store.read_object(blob) + return data.decode() + + +def test_merge_separate_files_is_clean(tmp_path): + """Each side added a different file; merge keeps both and writes two parents.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"shared.txt": "base\n"}, + {"shared.txt": "base\n", "ours.txt": "ours\n"}, + {"shared.txt": "base\n", "theirs.txt": "theirs\n"}, + ) + merge_hash = m.merge("feature") + commit = m.read_commit(merge_hash) + assert commit.parents == [ours, theirs] + assert m.read_ref("main") == merge_hash + assert m.read_ref("feature") == theirs + assert m.read_head() == "main" + paths = {entry.path for entry in m.tree.read_tree_entries(commit.tree)} + assert paths == {"shared.txt", "ours.txt", "theirs.txt"} + + +def test_merge_disjoint_lines_is_clean(tmp_path): + """Edits on different lines combine into one file.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "a\nb\nc\n"}, + {"file.txt": "A\nb\nc\n"}, + {"file.txt": "a\nb\nC\n"}, + ) + merge_hash = m.merge("feature") + assert m.read_commit(merge_hash).parents == [ours, theirs] + assert blob_text(m, m.read_commit(merge_hash).tree, "file.txt") == "A\nb\nC\n" + + +def test_merge_same_line_conflict_writes_markers_and_keeps_refs(tmp_path): + """Same line edited two ways: markers in the result tree, refs not moved.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "x\n"}, + {"file.txt": "ours\n"}, + {"file.txt": "theirs\n"}, + ) + with pytest.raises(MergeConflictError) as info: + m.merge("feature") + assert info.value.paths == ["file.txt"] + assert m.read_ref("main") == ours + assert m.read_ref("feature") == theirs + assert m.read_head() == "main" + text = blob_text(m, m.tree.checked_out[-1], "file.txt") + assert "<<<<<<< HEAD" in text + assert "ours" in text + assert "=======" in text + assert "theirs" in text + assert ">>>>>>> feature" in text + + +def test_merge_identical_edits_taken_once(tmp_path): + """Both sides made the same change; merge takes it once.""" + m = make_manager(tmp_path) + setup_diverged( + m, + {"file.txt": "a\n"}, + {"file.txt": "b\n"}, + {"file.txt": "b\n"}, + ) + merge_hash = m.merge("feature") + assert blob_text(m, m.read_commit(merge_hash).tree, "file.txt") == "b\n" + + +def test_merge_add_add_same_content_is_clean(tmp_path): + """Both sides added the same new file.""" + m = make_manager(tmp_path) + setup_diverged( + m, + {"keep.txt": "k\n"}, + {"keep.txt": "k\n", "new.txt": "same\n"}, + {"keep.txt": "k\n", "new.txt": "same\n"}, + ) + merge_hash = m.merge("feature") + paths = {entry.path for entry in m.tree.read_tree_entries(m.read_commit(merge_hash).tree)} + assert paths == {"keep.txt", "new.txt"} + assert blob_text(m, m.read_commit(merge_hash).tree, "new.txt") == "same\n" + + +def test_merge_delete_delete_is_clean(tmp_path): + """Both sides deleted the same file.""" + m = make_manager(tmp_path) + setup_diverged(m, {"gone.txt": "x\n"}, {}, {}) + merge_hash = m.merge("feature") + assert m.tree.read_tree_entries(m.read_commit(merge_hash).tree) == [] + + +def test_merge_modify_delete_is_conflict(tmp_path): + """One side edited, the other deleted.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "a\n"}, + {"file.txt": "b\n"}, + {}, + ) + with pytest.raises(MergeConflictError) as info: + m.merge("feature") + assert info.value.paths == ["file.txt"] + assert m.read_ref("main") == ours + assert m.read_ref("feature") == theirs + + +def test_merge_same_position_insertions_conflict(tmp_path): + """Insertions at the same base index conflict.""" + m = make_manager(tmp_path) + with pytest.raises(MergeConflictError) as info: + setup_diverged( + m, + {"file.txt": "a\nc\n"}, + {"file.txt": "a\nours\nc\n"}, + {"file.txt": "a\ntheirs\nc\n"}, + ) + m.merge("feature") + assert info.value.paths == ["file.txt"] + + +def test_find_merge_base_through_second_parent(tmp_path): + """Ancestry follows every parent when locating the merge base.""" + m = make_manager(tmp_path) + tree = make_tree(m) + base = m.create_commit(tree, [], AUTHOR, "base") + left = m.create_commit(tree, [base], AUTHOR, "left") + right = m.create_commit(tree, [base], AUTHOR, "right") + merged = m.create_commit(tree, [left, right], AUTHOR, "merge") + later = m.create_commit(tree, [merged], AUTHOR, "later") + assert m.find_merge_base(later, right) == right + + +def test_merge_unrelated_history_raises(tmp_path): + """Two root commits have no common ancestor.""" + m = make_manager(tmp_path) + tree = make_tree(m) + a = m.create_commit(tree, [], AUTHOR, "a") + m.create_branch("feature", a) + m.create_commit(tree, [], AUTHOR, "b") + with pytest.raises(MiniGitError, match="no common ancestor"): + m.merge("feature") + + +def test_merge_multiple_bases_raises(tmp_path): + """Two incomparable common ancestors raise MiniGitError.""" + m = make_manager(tmp_path) + tree = make_tree(m) + root = m.create_commit(tree, [], AUTHOR, "root") + a = m.create_commit(tree, [root], AUTHOR, "a") + b = m.create_commit(tree, [root], AUTHOR, "b") + ours = m.create_commit(tree, [a, b], AUTHOR, "ours") + m.create_branch("feature", ours) + m.write_head("feature") + m.create_commit(tree, [b, a], AUTHOR, "theirs") + m.write_head("main") + with pytest.raises(MiniGitError, match="multiple merge bases"): + m.merge("feature") + + +def test_merge_dirty_index_leaves_refs_and_no_pending_state(tmp_path): + """Staged changes block a three-way merge before writes.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "a\n"}, + {"file.txt": "ours\n"}, + {"file.txt": "theirs-side\n"}, + ) + m.tree.index_tree = m.store.hash_object(b"staged", "tree") + with pytest.raises(MiniGitError): + m.merge("feature") + assert m.read_ref("main") == ours + assert m.read_ref("feature") == theirs + assert m.read_head() == "main" + assert not (tmp_path / ".minigit" / "MERGE_HEAD").exists() + + +def test_merge_failed_checkout_leaves_refs_and_no_pending_state(tmp_path): + """A checkout that raises during three-way merge writes no pending state.""" + m = make_manager(tmp_path) + ours, theirs = setup_diverged( + m, + {"file.txt": "a\n"}, + {"file.txt": "ours\n"}, + {"file.txt": "theirs-side\n"}, + ) + m.tree.fail = True + with pytest.raises(MiniGitError, match="checkout failed"): + m.merge("feature") + assert m.read_ref("main") == ours + assert m.read_ref("feature") == theirs + assert not (tmp_path / ".minigit" / "MERGE_HEAD").exists() + + +def test_cli_conflict_then_resolve_add_commit(tmp_path, monkeypatch): + """A new process cannot commit until conflicts are added; then both parents are kept.""" + monkeypatch.chdir(tmp_path) + m = make_real_manager(tmp_path) + base = commit_file(m, "file.txt", "x\n", "base") + m.create_branch("feature", base) + ours = commit_file(m, "file.txt", "ours\n", "ours") + m.write_head("feature") + theirs = commit_file(m, "file.txt", "theirs\n", "theirs") + m.write_head("main") + m.tree.write_index(m.tree.read_tree_entries(m.get_head_tree())) + with pytest.raises(MergeConflictError): + CommitManager(str(tmp_path), store=m.store, tree=m.tree).merge("feature") + assert main(["commit", "-m", "too-soon"]) == 1 + (tmp_path / "file.txt").write_text("resolved\n") + assert main(["add", "file.txt"]) == 0 + assert main(["commit", "-m", "unused"]) == 0 + fresh = CommitManager(str(tmp_path)) + commit = fresh.read_commit(fresh.read_ref("main")) + assert commit.parents == [ours, theirs] + assert not (tmp_path / ".minigit" / "MERGE_HEAD").exists() From 59035e2bbb720b213d4b560d8efcf07e983ccc3d Mon Sep 17 00:00:00 2001 From: "Myeongjin (Daniel)" Date: Tue, 13 Oct 2026 00:52:19 -0400 Subject: [PATCH 2/3] Fix merge CLI test to switch branches before checkout. --- tests/test_commits.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/test_commits.py b/tests/test_commits.py index e85163e..af82260 100644 --- a/tests/test_commits.py +++ b/tests/test_commits.py @@ -890,10 +890,9 @@ def test_cli_conflict_then_resolve_add_commit(tmp_path, monkeypatch): base = commit_file(m, "file.txt", "x\n", "base") m.create_branch("feature", base) ours = commit_file(m, "file.txt", "ours\n", "ours") - m.write_head("feature") + m.switch_branch("feature") theirs = commit_file(m, "file.txt", "theirs\n", "theirs") - m.write_head("main") - m.tree.write_index(m.tree.read_tree_entries(m.get_head_tree())) + m.switch_branch("main") with pytest.raises(MergeConflictError): CommitManager(str(tmp_path), store=m.store, tree=m.tree).merge("feature") assert main(["commit", "-m", "too-soon"]) == 1 From b3e5dd5211b65411753450d65a90c441d8d202cc Mon Sep 17 00:00:00 2001 From: aman shah Date: Tue, 13 Oct 2026 09:42:37 -0400 Subject: [PATCH 3/3] Use Myers edits and preserve explicit conflict resolutions across CLI calls --- README.md | 13 ++- minigit/commits.py | 45 +++----- minigit/index.py | 43 +++++--- tests/test_index.py | 3 + tests/test_week5_integration.py | 175 ++++++++++++++++++++++++++++++++ 5 files changed, 236 insertions(+), 43 deletions(-) create mode 100644 tests/test_week5_integration.py diff --git a/README.md b/README.md index 9424b67..d6034f4 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,18 @@ minigit status # clean Checkout restores files, executable modes, and the index. It rejects staged or unstaged changes and untracked paths that would be overwritten. Fast-forward merges keep HEAD on the current branch and create no additional commit. -Diverged branches report that three-way merging is not implemented yet. +Diverged branches use Myers line edits for a three-way merge. Disjoint text edits +produce a commit with both parents. Conflicts write markers and leave branch refs +unchanged. Edit each conflicted file, run `minigit add `, then +`minigit commit -m "Resolve merge"`. To resolve by deleting a file, remove it and +add its path. Pending conflicts survive new CLI processes and block another +merge, checkout, or commit until explicitly resolved. + +`minigit diff` shows line changes between HEAD and working files, including +untracked files, binary changes, executable modes, and missing final newlines. +`minigit status` retains its staged / not staged / untracked sections. Both +commands are read-only. Object storage also supports local +[packfiles](docs/packfiles.md) without changing object hashes. To test push, initialize another scratch repository and run `minigit serve --port 9418 --token demo-token` there. From the first repository, diff --git a/minigit/commits.py b/minigit/commits.py index dc76ef6..83f8330 100644 --- a/minigit/commits.py +++ b/minigit/commits.py @@ -20,7 +20,7 @@ RefExistsError, RefNotFoundError, ) -from minigit.index import WorkingTree +from minigit.index import WorkingTree, myers_diff, split_lines from minigit.objects import ObjectStore, TreeEntry @@ -37,10 +37,10 @@ def _cmd_commit(args) -> int: if pending is None: last_commit = manager.read_ref(branch) parents = [last_commit] if last_commit else [] - message = args.message else: - ours, theirs, message, _ = pending + ours, theirs, _, _ = pending parents = [ours, theirs] + message = args.message author = manager.read_author() commit_hash = manager.create_commit( tree_hash, @@ -121,7 +121,7 @@ def register_subcommands(subparsers) -> None: log_parser = subparsers.add_parser("log", help="show commit history") log_parser.set_defaults(handler=_cmd_log) - merge_parser = subparsers.add_parser("merge", help="fast-forward the current branch") + merge_parser = subparsers.add_parser("merge", help="merge a branch into the current branch") merge_parser.add_argument("branch", help="branch name to merge in") merge_parser.set_defaults(handler=_cmd_merge) @@ -546,6 +546,10 @@ def write_dir(node: dict) -> str: def _conflict_text(self, ours_text: str, theirs_text: str, branch_name: str) -> str: """Build a conflict-marker blob.""" + if ours_text and not ours_text.endswith("\n"): + ours_text += "\n" + if theirs_text and not theirs_text.endswith("\n"): + theirs_text += "\n" return f"<<<<<<< HEAD\n{ours_text}=======\n{theirs_text}>>>>>>> {branch_name}\n" def _merge_state_paths(self) -> dict[str, str]: @@ -582,7 +586,7 @@ def read(path: str) -> str: with open(path) as handle: return handle.read() - unresolved = [line for line in read(paths_map["unresolved"]).splitlines() if line] + unresolved = [line for line in read(paths_map["unresolved"]).split("\n") if line] return ( read(paths_map["ours"]).strip(), read(paths_map["theirs"]).strip(), @@ -624,25 +628,8 @@ class Hunk: def _diff_lines(base: list[str], other: list[str]) -> list[Hunk]: - """Fake line diff""" - hunks = [] - i = 0 - j = 0 - while i < len(base) or j < len(other): - if i < len(base) and j < len(other) and base[i] == other[j]: - i += 1 - j += 1 - continue - start_i, start_j = i, j - if i < len(base) and (j >= len(other) or base[i] not in other[j:]): - i += 1 - elif j < len(other) and (i >= len(base) or other[j] not in base[i:]): - j += 1 - else: - i += 1 - j += 1 - hunks.append(Hunk(start_i, i - start_i, other[start_j:j])) - return hunks + """Adapt M2's Myers edits without changing their base coordinates.""" + return [Hunk(edit.start, edit.old_count, edit.new_lines) for edit in myers_diff(base, other)] def _ranges_overlap(a: Hunk, b: Hunk) -> bool: @@ -669,7 +656,7 @@ def _combine_hunks(ours: list[Hunk], theirs: list[Hunk]) -> list[Hunk] | None: return None if not skip: merged.append(hunk) - return sorted(merged, key=lambda hunk: hunk.base_start) + return sorted(merged, key=lambda hunk: (hunk.base_start, hunk.base_count)) def _apply_hunks(base_lines: list[str], hunks: list[Hunk]) -> list[str]: @@ -686,11 +673,11 @@ def _apply_hunks(base_lines: list[str], hunks: list[Hunk]) -> list[str]: def _merge_text(base_text: str, ours_text: str, theirs_text: str) -> str | None: """Diff each side against base. Combine or conflict""" - base_lines = base_text.split("\n") + base_lines = split_lines(base_text) combined = _combine_hunks( - _diff_lines(base_lines, ours_text.split("\n")), - _diff_lines(base_lines, theirs_text.split("\n")), + _diff_lines(base_lines, split_lines(ours_text)), + _diff_lines(base_lines, split_lines(theirs_text)), ) if combined is None: return None - return "\n".join(_apply_hunks(base_lines, combined)) + return "".join(_apply_hunks(base_lines, combined)) diff --git a/minigit/index.py b/minigit/index.py index f0b042b..e9eec86 100644 --- a/minigit/index.py +++ b/minigit/index.py @@ -216,11 +216,15 @@ def write_index(self, entries) -> None: def stage_file(self, path) -> None: full_path = os.path.join(self.root, path) + rel_path = os.path.relpath(full_path, self.root).replace(os.sep, "/") + self._validate_checkout_path(rel_path) if not os.path.isfile(full_path): - raise MiniGitError(f"No such file: {path}") - - rel_path = os.path.relpath(full_path, self.root) - rel_path = rel_path.replace(os.sep, "/") + entries = self.read_index() + remaining = [entry for entry in entries if entry.path != rel_path] + if os.path.lexists(full_path) or len(remaining) == len(entries): + raise MiniGitError(f"No such file: {path}") + self.write_index(remaining) + return with open(full_path, "rb") as f: data = f.read() @@ -480,15 +484,23 @@ def _prune_empty_dirs(self, dir_path: str) -> None: def cmd_add(args) -> int: """Stage a path. During a merge, that path is marked resolved.""" wt = WorkingTree() - full_path = os.path.join(wt.root, args.path) - if os.path.isfile(full_path): - wt.stage_file(args.path) - else: - entries = [entry for entry in wt.read_index() if entry.path != args.path] - wt.write_index(entries) from minigit.commits import CommitManager - CommitManager(wt.root, store=wt.store, tree=wt).mark_path_resolved(args.path) + manager = CommitManager(wt.root, store=wt.store, tree=wt) + full_path = os.path.join(wt.root, args.path) + rel_path = os.path.relpath(full_path, wt.root).replace(os.sep, "/") + wt._validate_checkout_path(rel_path) + pending = manager._read_pending_merge() + # A delete/modify conflict can have no pre-merge index entry. Explicitly + # adding its absent path still resolves the conflict as a deletion. + missing_conflict = ( + pending is not None and rel_path in pending[3] and not os.path.lexists(full_path) + ) + if missing_conflict: + wt.write_index([entry for entry in wt.read_index() if entry.path != rel_path]) + else: + wt.stage_file(args.path) + manager.mark_path_resolved(rel_path) return 0 @@ -499,7 +511,8 @@ def cmd_status(args) -> int: from minigit.commits import CommitManager - head_tree_hash = CommitManager(wt.root, store=wt.store, tree=wt).get_head_tree() + manager = CommitManager(wt.root, store=wt.store, tree=wt) + head_tree_hash = manager.get_head_tree() head_entries = wt.read_tree_entries(head_tree_hash) if head_tree_hash else [] head_by_path = {e.path: e for e in head_entries} @@ -527,8 +540,12 @@ def cmd_status(args) -> int: if current_hash != entry.hash or current_mode != entry.mode: not_staged_paths.add(entry.path) + pending = manager._read_pending_merge() + unresolved = set(pending[3]) if pending else set() + not_staged_paths.update(unresolved) + # untracked: on disk, absent from the index - known_paths = set(index_by_path) + known_paths = set(index_by_path) | unresolved untracked_paths = set() for dirpath, dirnames, filenames in os.walk(wt.root): dirnames[:] = [d for d in dirnames if d != ".minigit"] diff --git a/tests/test_index.py b/tests/test_index.py index 3361f0c..049217c 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -676,6 +676,9 @@ def __init__(self, repo_path=".", store=None, tree=None): def get_head_tree(self): return state["head_tree"] + def _read_pending_merge(self): + return None + fake = types.ModuleType("minigit.commits") fake.CommitManager = FakeCommitManager monkeypatch.setitem(sys.modules, "minigit.commits", fake) diff --git a/tests/test_week5_integration.py b/tests/test_week5_integration.py new file mode 100644 index 0000000..e951433 --- /dev/null +++ b/tests/test_week5_integration.py @@ -0,0 +1,175 @@ +"""Week 5 regressions across real storage, diffing, merging, and the CLI.""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from minigit.commits import _merge_text + +PROJECT = Path(__file__).resolve().parents[1] + + +def cli(root, *args, expected=0): + env = dict(os.environ, PYTHONPATH=str(PROJECT)) + result = subprocess.run( + [sys.executable, "-m", "minigit.cli", *args], + cwd=root, + env=env, + capture_output=True, + text=True, + timeout=10, + ) + assert result.returncode == expected, result.stdout + result.stderr + return result.stdout + result.stderr + + +@pytest.mark.parametrize("reverse", [False, True]) +@pytest.mark.parametrize( + "base,ours,theirs,expected", + [ + ("a\nb\nc\n", "A\nb\nc\n", "insert\na\nb\nc\n", "insert\nA\nb\nc\n"), + ("a\nb\nc\n", "a\nc\n", "a\nB\nc\n", None), + ("a\nb\nc", "A\nb\nc", "a\nb\nC", "A\nb\nC"), + ("", "ours", "theirs", None), + ("a\nb\nc\n", "a\nx\nb\nc\n", "a\nb\nC\n", "a\nx\nb\nC\n"), + ], +) +def test_text_merge_uses_base_line_coordinates(base, ours, theirs, expected, reverse): + if reverse: + ours, theirs = theirs, ours + assert _merge_text(base, ours, theirs) == expected + + +@pytest.mark.parametrize("delete_ours", [False, True]) +@pytest.mark.parametrize("resolution", ["./file.txt", "file.txt", "delete"]) +def test_conflict_resolution_across_cli_processes(tmp_path, delete_ours, resolution): + from minigit.commits import CommitManager + + cli(tmp_path, "init") + path = tmp_path / "file.txt" + path.write_text("base") + cli(tmp_path, "add", "file.txt") + cli(tmp_path, "commit", "-m", "base") + cli(tmp_path, "branch", "feature") + if delete_ours: + path.unlink() + else: + path.write_text("ours") + cli(tmp_path, "add", "./file.txt") + cli(tmp_path, "commit", "-m", "ours") + cli(tmp_path, "checkout", "feature") + path.write_text("theirs") + cli(tmp_path, "add", "file.txt") + cli(tmp_path, "commit", "-m", "theirs") + cli(tmp_path, "checkout", "main") + manager = CommitManager(tmp_path) + parents = [manager.read_ref("main"), manager.read_ref("feature")] + cli(tmp_path, "merge", "feature", expected=1) + markers = path.read_text().splitlines() + assert "=======" in markers + assert ">>>>>>> feature" in markers + cli(tmp_path, "commit", "-m", "early", expected=1) + cli(tmp_path, "checkout", "feature", expected=1) + cli(tmp_path, "merge", "feature", expected=1) + if resolution == "delete": + path.unlink() + cli(tmp_path, "add", "./file.txt") + else: + path.write_text("resolved\n") + cli(tmp_path, "add", resolution) + cli(tmp_path, "commit", "-m", "Resolved by hand") + commit = manager.read_commit(manager.read_ref("main")) + assert commit.parents == parents + assert commit.message == "Resolved by hand" + assert cli(tmp_path, "status").strip() == "clean" + assert not list((tmp_path / ".minigit").glob("MERGE_*")) + + +def test_add_unknown_path_fails_without_changing_index(tmp_path): + cli(tmp_path, "init") + index = tmp_path / ".minigit/index" + before = index.read_bytes() + cli(tmp_path, "add", "missing.txt", expected=1) + assert index.read_bytes() == before + + +def test_packed_three_way_merge_push_and_stale_rejection(tmp_path): + import threading + + from minigit.commits import CommitManager + from minigit.remote import RemoteClient, RemoteServer + + local = tmp_path / "local" + remote = tmp_path / "remote" + local.mkdir() + remote.mkdir() + for root in (local, remote): + cli(root, "init") + path = local / "file.txt" + path.write_bytes(b"a\nb\nc\n") + cli(local, "add", "file.txt") + cli(local, "commit", "-m", "base") + cli(local, "branch", "feature") + path.write_bytes(b"A\nb\nc\n") + cli(local, "add", "file.txt") + cli(local, "commit", "-m", "ours") + cli(local, "branch", "stale") + cli(local, "checkout", "feature") + path.write_bytes(b"a\nb\nC\n") + cli(local, "add", "file.txt") + cli(local, "commit", "-m", "theirs") + cli(local, "checkout", "main") + manager = CommitManager(local) + parents = [manager.read_ref("main"), manager.read_ref("feature")] + client = RemoteClient(local) + reachable = client.collect_reachable("main") | client.collect_reachable("feature") + manager.store.pack_objects(reachable) + for obj_hash in reachable: + manager.store._object_path(obj_hash).unlink() + cli(local, "merge", "feature") + assert path.read_bytes() == b"A\nb\nC\n" + assert cli(local, "status").strip() == "clean" + assert cli(local, "diff") == "" + tip = manager.read_ref("main") + assert manager.read_commit(tip).parents == parents + reachable = client.collect_reachable("main") + manager.store.pack_objects(reachable) + for obj_hash in reachable: + manager.store._object_path(obj_hash).unlink(missing_ok=True) + + server = RemoteServer(remote, token="test-token", port=0) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + address = f"127.0.0.1:{server.port}" + receiver = CommitManager(remote) + original_head = (remote / ".minigit/HEAD").read_bytes() + original_index = (remote / ".minigit/index").read_bytes() + try: + cli(local, "push", address, "main", "--token", "test-token") + assert receiver.read_ref("main") == tip + receiver.store.pack_objects(reachable) + for obj_hash in reachable: + receiver.store._object_path(obj_hash).unlink() + assert receiver.store.read_object(obj_hash) == manager.store.read_object(obj_hash) + before = sorted(receiver.store.objects_dir.rglob("*")) + assert "up to date" in cli(local, "push", address, "main", "--token", "test-token") + assert sorted(receiver.store.objects_dir.rglob("*")) == before + cli(local, "checkout", "stale") + path.write_bytes(b"stale writer\n") + cli(local, "add", "file.txt") + cli(local, "commit", "-m", "stale writer") + manager.write_ref("main", manager.read_ref("stale")) + assert "diverged" in cli( + local, "push", address, "main", "--token", "test-token", expected=1 + ) + assert receiver.read_ref("main") == tip + assert (remote / ".minigit/HEAD").read_bytes() == original_head + assert (remote / ".minigit/index").read_bytes() == original_index + assert sorted(remote.iterdir()) == [remote / ".minigit"] + finally: + server.close() + thread.join(timeout=2) + assert not thread.is_alive()