diff --git a/docs/performance.md b/docs/performance.md new file mode 100644 index 0000000..0fe1c11 --- /dev/null +++ b/docs/performance.md @@ -0,0 +1,71 @@ +# Working-tree performance (`status` and `diff`) + +Week 8, Module 2. Numbers below are best-of-5 wall-clock seconds for one call of `cmd_status` +and `cmd_diff` on a generated repository. + +## How it was measured + +- Repository: 3000 small files (40 lines each, in 50 folders) plus one 20000-line file, all + committed. +- Benchmark: `bench_working_tree.py` (`--profile` shows where the time goes). +- Machine: MacBook Air (Apple silicon), Python 3.13.5. +- Command: `PYTHONPATH=. python3 bench_working_tree.py` + +## Before + +`main`'s code. Old and new were run alternately, one right after the other, so both saw the same +machine conditions. + +| Situation | status | diff | +| --- | --- | --- | +| nothing changed | 0.133 s | 0.122 s | +| 1 small file changed | 0.125 s | 0.121 s | +| 50 small files changed | 0.123 s | 0.125 s | +| + big file, 2 edits | 0.123 s | 0.131 s | +| + big file, 500 scattered edits | 0.125 s | 0.223 s | + +## Where the time went + +With 3000 files, `status` and `diff` take about the same time however many files changed, so +the cost is per file visited, not per change. The profile of the "nothing changed" case showed: + +- `os.getcwd` called 6003 times (twice per file), about 0.05 s of the 0.19 s profiled run. + Cause: the file walk called `os.path.relpath` once for every file. `WorkingTree` uses the + relative root `"."`, so each call asked the operating system for the current directory + twice. +- Reading the files (`open`, 3055 calls, about 0.03 s) and the executable-bit check + (`os.access`, 3001 calls, about 0.01 s). Both are needed to decide whether a file changed. +- Line diffing (Myers) only matters for the big file with 500 scattered edits, where `diff` + rises from about 0.13 s to 0.23 s. That cost is real work, not repeated work. + +## Change made + +One bounded fix: the three file walks (`status`, `diff`, and the added-file check) now share +`walk_working_files()`, which works out the relative folder once per folder (about 50 times) +instead of once per file (3000 times). + +Nothing else changed. Every file is still read and hashed, so a same-size edit with the +original timestamp is still found (regression test: +`test_same_size_edit_with_preserved_timestamp_is_still_found`). Nothing is cached, and +`status` and `diff` still write nothing to the repository. + +## After + +Same command, on the code with `walk_working_files()`. + +| Situation | status | diff | +| --- | --- | --- | +| nothing changed | 0.070 s | 0.064 s | +| 1 small file changed | 0.066 s | 0.064 s | +| 50 small files changed | 0.066 s | 0.066 s | +| + big file, 2 edits | 0.070 s | 0.071 s | +| + big file, 500 scattered edits | 0.066 s | 0.166 s | + +With many files, `status` and `diff` take about 45% less time (about 0.12 s down to 0.066 s). +Every row improves by roughly the same 0.055 s, which matches the time the profile showed going +to the 6003 `getcwd` calls. The 500-edit `diff` improves by less (0.223 s to 0.166 s) because +most of its time is the Myers diff, which was not changed. + +## Re-run + +Run these numbers again with the shared benchmark (#47) once it is merged. diff --git a/minigit/index.py b/minigit/index.py index fd7c3da..51223c1 100644 --- a/minigit/index.py +++ b/minigit/index.py @@ -1,7 +1,7 @@ """Module 2 - Index and working tree. Owns the bridge between live files on disk and the immutable object store: -the index file, staging, diffing, and checkout. +the index file, staging, diffing, checkout, and the text `status` and `diff` print. Depends on: module 1. Serves: module 3. @@ -9,6 +9,7 @@ Build the `WorkingTree` class here, per the interface contract. """ +import json import os import re import stat @@ -167,6 +168,17 @@ def _bytes_to_lines(data: bytes) -> list[str] | None: return None +def walk_working_files(root: str): + """Yield (full_path, repo-relative path with '/') for every file under `root`, skipping + .minigit. The relative directory is worked out once per directory, not once per file.""" + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames if d != ".minigit"] + rel_dir = os.path.relpath(dirpath, root) + prefix = "" if rel_dir == "." else rel_dir.replace(os.sep, "/") + "/" + for filename in filenames: + yield os.path.join(dirpath, filename), prefix + filename + + class WorkingTree: def __init__(self, repo_path=".", store=None): self.root = repo_path @@ -193,11 +205,17 @@ def read_index(self) -> list[IndexEntry]: entries = [] with open(self.index_path, encoding="utf-8") as f: - for line in f: + for number, line in enumerate(f, start=1): line = line.rstrip("\n") if not line: continue - mode, hash_, path = line.split(" ", 2) + try: + mode, hash_, path = line.split(" ", 2) + except ValueError: + raise MiniGitError( + f"corrupt index: line {number} is not ' '; " + "delete .minigit/index and run 'minigit add' again" + ) from None entries.append(IndexEntry(mode, hash_, path)) return sorted(entries, key=lambda e: e.path) @@ -222,7 +240,9 @@ def stage_file(self, path) -> None: 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}") + raise MiniGitError( + f"No such file: {path}; check the path, or run 'minigit status' to list files" + ) self.write_index(remaining) return @@ -321,17 +341,13 @@ def _compare_with_tree(self, tree_hash) -> list[_FileChange]: self._make_change(entry.path, "modified", entry.mode, new_mode, old_data, new_data) ) - for dirpath, dirnames, filenames in os.walk(self.root): - dirnames[:] = [d for d in dirnames if d != ".minigit"] - for filename in filenames: - full_path = os.path.join(dirpath, filename) - rel_path = os.path.relpath(full_path, self.root).replace(os.sep, "/") - if rel_path in known_paths: - continue - with open(full_path, "rb") as f: - new_data = f.read() - new_mode = self._file_mode(full_path) - changes.append(self._make_change(rel_path, "added", None, new_mode, b"", new_data)) + for full_path, rel_path in walk_working_files(self.root): + if rel_path in known_paths: + continue + with open(full_path, "rb") as f: + new_data = f.read() + new_mode = self._file_mode(full_path) + changes.append(self._make_change(rel_path, "added", None, new_mode, b"", new_data)) return sorted(changes, key=lambda c: c.path) @@ -364,13 +380,9 @@ def _working_status(self) -> DiffResult: if current_hash != entry.hash or current_mode != entry.mode: result.modified.append(entry.path) - for dirpath, dirnames, filenames in os.walk(self.root): - dirnames[:] = [d for d in dirnames if d != ".minigit"] - for filename in filenames: - full_path = os.path.join(dirpath, filename) - rel_path = os.path.relpath(full_path, self.root) - if rel_path not in known_paths: - result.added.append(rel_path) + for _, rel_path in walk_working_files(self.root): + if rel_path not in known_paths: + result.added.append(rel_path) return result @@ -406,13 +418,18 @@ def checkout(self, tree_hash, conflicted_paths=()) -> None: self._validate_checkout_path(entry.path) full_path = os.path.join(self.root, entry.path) if not os.path.isfile(full_path): - raise MiniGitError(f"local changes would be lost: {entry.path} is missing") + raise MiniGitError( + f"local changes would be lost: {entry.path} is missing; " + "restore the file or commit its deletion first" + ) with open(full_path, "rb") as f: data = f.read() disk_hash = self.store.hash_object(data, "blob") disk_mode = self._file_mode(full_path) if disk_hash != entry.hash or disk_mode != entry.mode: - raise MiniGitError(f"local changes would be lost: {entry.path}") + raise MiniGitError( + f"local changes would be lost: {entry.path}; commit or undo your edits first" + ) # Preflight every parent and destination before deleting any tracked file. # Directories may become files only when all their contents are tracked. @@ -431,20 +448,31 @@ def checkout(self, tree_hash, conflicted_paths=()) -> None: full_parent = os.path.join(self.root, parent) if os.path.lexists(full_parent) and not os.path.isdir(full_parent): if parent not in removable: - raise MiniGitError(f"untracked path would be overwritten: {parent}") + raise MiniGitError( + f"untracked path would be overwritten: {parent}; " + "move or delete it first" + ) full_path = os.path.join(self.root, entry.path) if os.path.isdir(full_path): if entry.path not in tracked_dirs: - raise MiniGitError(f"untracked path would be overwritten: {entry.path}") + raise MiniGitError( + f"untracked path would be overwritten: {entry.path}; " + "move or delete it first" + ) for directory, dirnames, filenames in os.walk(full_path): for name in dirnames + filenames: child = os.path.join(directory, name) relative = os.path.relpath(child, self.root).replace(os.sep, "/") if os.path.islink(child) or relative not in removable | tracked_dirs: - raise MiniGitError(f"untracked path would be overwritten: {relative}") + raise MiniGitError( + f"untracked path would be overwritten: {relative}; " + "move or delete it first" + ) elif os.path.lexists(full_path) and entry.path not in current_by_path: - raise MiniGitError(f"untracked path would be overwritten: {entry.path}") + raise MiniGitError( + f"untracked path would be overwritten: {entry.path}; move or delete it first" + ) # Remove tracked files the target doesn't have, then prune for entry in current_entries: @@ -477,7 +505,7 @@ def _validate_checkout_path(self, path: str) -> None: or any(part in {"", ".", ".."} for part in path.split("/")) or any(char in path for char in "\\\0\r\n") ): - raise MiniGitError(f"unsafe path: {path}") + raise MiniGitError(f"unsafe path: {display_path(path)}") if any(part == ".minigit" for part in path.split("/")): raise MiniGitError(f"path under .minigit: {path}") @@ -521,7 +549,66 @@ def cmd_add(args) -> int: return 0 +# --------------------------------------------------------------------------- +# Output formatting for `status` and `diff` (plain text; no Myers code in here) +# --------------------------------------------------------------------------- + +CONTEXT_LINES = 3 # unchanged lines shown around each change in a diff hunk + + +def display_path(path: str, prefix: str = "") -> str: + """Return `prefix + path` the way status and diff print it. + + Ordinary paths (spaces and Unicode included) are printed as they are. A path with + a control character or a backslash is printed as a quoted, escaped string, so one + path always stays on one line and the output reads the same everywhere. + """ + text = prefix + path + if any(not char.isprintable() or char == "\\" for char in text): + return json.dumps(text) + return text + + +def format_status(header, unborn, unresolved, staged, not_staged, untracked) -> list[str]: + """Return the lines `minigit status` prints. Empty sections are left out.""" + lines = [header] + if unborn: + lines.append("no commits yet") + sections = ( + ("unresolved (merge conflict):", unresolved), + ("staged:", staged), + ("not staged:", not_staged), + ("untracked:", untracked), + ) + shown = False + for title, paths in sections: + if paths: + lines.append(title) + lines.extend(f" {display_path(path)}" for path in sorted(paths)) + shown = True + if not shown: + lines.append("clean") + return lines + + +def _head_header(head) -> str: + """First status line. `head` is what CommitManager.read_head() returned.""" + if head is None: + return "On branch main" # no HEAD file yet: same default as CommitManager + if len(head) == 40 and all(char in "0123456789abcdef" for char in head): + return f"HEAD detached at {head[:7]}" + return f"On branch {head}" + + +def _unresolved_merge_paths(manager) -> list[str]: + """Paths of a pending merge that `add` has not resolved yet (M3 owns the merge state).""" + pending = manager._read_pending_merge() + return sorted(pending[3]) if pending else [] + + def cmd_status(args) -> int: + """minigit status: branch, then staged (HEAD tree vs index), not staged (index vs + working files) and untracked files. Read-only.""" wt = WorkingTree() index_entries = wt.read_index() index_by_path = {e.path: e for e in index_entries} @@ -557,59 +644,94 @@ 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) + # A path with an unresolved merge conflict is listed once, under its own heading. + unresolved = set(_unresolved_merge_paths(manager)) + not_staged_paths -= unresolved # untracked: on disk, absent from the index 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"] - for filename in filenames: - full_path = os.path.join(dirpath, filename) - rel_path = os.path.relpath(full_path, wt.root).replace(os.sep, "/") - if rel_path not in known_paths: - untracked_paths.add(rel_path) - - staged = sorted(staged_paths) - not_staged = sorted(not_staged_paths) - untracked = sorted(untracked_paths) + for _, rel_path in walk_working_files(wt.root): + if rel_path not in known_paths: + untracked_paths.add(rel_path) + + lines = format_status( + _head_header(manager.read_head()), + head_tree_hash is None, + sorted(unresolved), + staged_paths, + not_staged_paths, + untracked_paths, + ) + for line in lines: + print(line) + return 0 - printed_anything = False - if staged: - print("staged:") - for path in staged: - print(f" {path}") - printed_anything = True +def _hunk_range(start: int, count: int) -> str: + """`start` is a 0-based line index. A non-empty range is written 1-based + ("3,2" = lines 3 and 4); an empty range is written as the line before it ("2,0").""" + return f"{start if count == 0 else start + 1},{count}" - if not_staged: - print("not staged:") - for path in not_staged: - print(f" {path}") - printed_anything = True - if untracked: - print("untracked:") - for path in untracked: - print(f" {path}") - printed_anything = True +def _diff_lines(prefix: str, line: str) -> list[str]: + out = [prefix + line.removesuffix("\n")] + if not line.endswith("\n"): + out.append("\\ No newline at end of file") + return out - if not printed_anything: - print("clean") - - return 0 +def _group_edits(edits: list, context: int) -> list[list]: + """Group edits that are close enough for their context lines to touch or overlap.""" + groups: list[list] = [] + for edit in edits: + if groups: + last = groups[-1][-1] + if edit.start - (last.start + last.old_count) <= 2 * context: + groups[-1].append(edit) + continue + groups.append([edit]) + return groups + + +def _render_hunk(old_lines: list, group: list, shift: int, context: int): + """Return (hunk lines, change in line count). `shift` = lines added minus removed so far.""" + first, last = group[0], group[-1] + old_from = max(0, first.start - context) + old_to = min(len(old_lines), last.start + last.old_count + context) + body: list[str] = [] + removed = added = 0 + position = old_from + for edit in group: + for line in old_lines[position : edit.start]: + body.extend(_diff_lines(" ", line)) + for line in old_lines[edit.start : edit.start + edit.old_count]: + body.extend(_diff_lines("-", line)) + for line in edit.new_lines: + body.extend(_diff_lines("+", line)) + removed += edit.old_count + added += len(edit.new_lines) + position = edit.start + edit.old_count + for line in old_lines[position:old_to]: + body.extend(_diff_lines(" ", line)) + old_count = old_to - old_from + new_count = old_count - removed + added + header = ( + f"@@ -{_hunk_range(old_from, old_count)} +{_hunk_range(old_from + shift, new_count)} @@" + ) + return [header, *body], added - removed -def _hunk_range(start: int, count: int) -> str: - return f"{start if count == 0 else start + 1},{count}" +def _format_change(change: _FileChange, context: int = CONTEXT_LINES) -> list[str]: + """Render one file's changes as printable lines. -def _format_change(change: _FileChange) -> list[str]: - """Render one file's changes as printable lines.""" + Hunk header: `@@ -, +, @@`, with the + old numbers from the HEAD version and the new ones from the working file. + Each hunk shows `context` unchanged lines around the changes; lines start with + " " (unchanged), "-" (removed) or "+" (added). + """ path = change.path - out = [f"diff --minigit a/{path} b/{path}"] + out = [f"diff --minigit {display_path(path, 'a/')} {display_path(path, 'b/')}"] if change.status == "added": out.append(f"new file mode {change.new_mode}") @@ -620,37 +742,29 @@ def _format_change(change: _FileChange) -> list[str]: out.append(f"new mode {change.new_mode}") if change.binary: - if change.content_changed: - out.append(f"binary file changed: {path}") + if change.status == "added": + out.append(f"binary file added: {display_path(path)}") + elif change.status == "deleted": + out.append(f"binary file deleted: {display_path(path)}") + elif change.content_changed: + out.append(f"binary file changed: {display_path(path)}") return out if not change.edits: return out # mode-only change, or an empty file added / deleted - out.append("--- /dev/null" if change.status == "added" else f"--- a/{path}") - out.append("+++ /dev/null" if change.status == "deleted" else f"+++ b/{path}") - - def emit(prefix: str, line: str) -> None: - out.append(prefix + line.removesuffix("\n")) - if not line.endswith("\n"): - out.append("\\ No newline at end of file") + out.append("--- /dev/null" if change.status == "added" else f"--- {display_path(path, 'a/')}") + out.append("+++ /dev/null" if change.status == "deleted" else f"+++ {display_path(path, 'b/')}") - offset = 0 - for edit in change.edits: - new_start = edit.start + offset - out.append( - f"@@ -{_hunk_range(edit.start, edit.old_count)} " - f"+{_hunk_range(new_start, len(edit.new_lines))} @@" - ) - for line in change.old_lines[edit.start : edit.start + edit.old_count]: - emit("-", line) - for line in edit.new_lines: - emit("+", line) - offset += len(edit.new_lines) - edit.old_count + shift = 0 + for group in _group_edits(change.edits, context): + hunk, delta = _render_hunk(change.old_lines, group, shift, context) + out.extend(hunk) + shift += delta return out def cmd_diff(args) -> int: - """minigit diff: HEAD tree vs the working files (unborn HEAD = empty tree).""" + """minigit diff: HEAD tree vs the working files (unborn HEAD = empty tree). Read-only.""" wt = WorkingTree() from minigit.commits import CommitManager @@ -667,8 +781,12 @@ def register_index_commands(subparsers) -> None: add_parser.add_argument("path") add_parser.set_defaults(handler=cmd_add) - status_parser = subparsers.add_parser("status", help="show staged files") + status_parser = subparsers.add_parser( + "status", help="show the branch and staged, not staged and untracked files" + ) status_parser.set_defaults(handler=cmd_status) - diff_parser = subparsers.add_parser("diff", help="show line changes since HEAD") + diff_parser = subparsers.add_parser( + "diff", help="show line changes: the HEAD tree compared with the working files" + ) diff_parser.set_defaults(handler=cmd_diff) diff --git a/tests/test_index.py b/tests/test_index.py index 61b433e..446391c 100644 --- a/tests/test_index.py +++ b/tests/test_index.py @@ -5,6 +5,7 @@ import hashlib import os import random +import re import shutil import sys import types @@ -20,11 +21,15 @@ cmd_add, cmd_diff, cmd_status, + display_path, myers_diff, split_lines, + walk_working_files, ) from minigit.objects import ObjectStore, TreeEntry +BRANCH = "On branch main\n" # first line of `minigit status` on the default branch + def test_build_tree_empty_index(tmp_path): store = ObjectStore(str(tmp_path)) @@ -209,7 +214,7 @@ def test_status_clean_after_commit(tmp_path, monkeypatch, capsys): cmd_status(_Args()) output = capsys.readouterr().out - assert output.strip() == "clean" + assert output == BRANCH + "clean\n" def test_status_staged_and_unstaged_on_one_file(tmp_path, monkeypatch, capsys): @@ -266,7 +271,7 @@ def test_status_and_diff_do_not_store_unstaged_content(tmp_path, monkeypatch, ca file.write_text("unstaged") monkeypatch.chdir(tmp_path) cmd_status(_Args()) - assert capsys.readouterr().out == "not staged:\n file\n" + assert capsys.readouterr().out == BRANCH + "not staged:\n file\n" assert wt.diff_working_tree_vs(tree).modified == ["file"] assert set(wt.store.objects_dir.rglob("*")) == before @@ -279,7 +284,7 @@ def test_status_reports_file_removed_from_index_as_untracked(tmp_path, monkeypat wt.write_index([]) monkeypatch.chdir(tmp_path) cmd_status(_Args()) - assert capsys.readouterr().out == "staged:\n file\nuntracked:\n file\n" + assert capsys.readouterr().out == BRANCH + "staged:\n file\nuntracked:\n file\n" def test_checkout_restores_changed_file(tmp_path): @@ -681,6 +686,9 @@ def __init__(self, repo_path=".", store=None, tree=None): def get_head_tree(self): return state["head_tree"] + def read_head(self): + return None + def _read_pending_merge(self): return None @@ -718,10 +726,8 @@ def test_cli_shows_headers_and_added_removed_lines(cli_repo, capsys): assert "diff --minigit a/a.txt b/a.txt" in out assert "--- a/a.txt" in out assert "+++ b/a.txt" in out - assert "@@ -2,1 +2,1 @@" in out - assert "-two" in out - assert "+TWO" in out - assert " one" not in out + assert "@@ -1,3 +1,3 @@" in out + assert out[out.index("@@ -1,3 +1,3 @@") + 1 :] == [" one", "-two", "+TWO", " three"] def test_cli_removed_lines_come_from_the_old_text(cli_repo, capsys): @@ -735,7 +741,7 @@ def test_cli_removed_lines_come_from_the_old_text(cli_repo, capsys): assert out.index("-old1") < out.index("-old2") < out.index("+new") -def test_cli_second_hunk_line_numbers_account_for_earlier_edits(cli_repo, capsys): +def test_cli_nearby_edits_share_one_hunk(cli_repo, capsys): repo, commit = cli_repo write(repo, "a.txt", "a\nb\nc\nd\ne\nf\n") commit("a.txt") @@ -743,8 +749,23 @@ def test_cli_second_hunk_line_numbers_account_for_earlier_edits(cli_repo, capsys out = run_diff(capsys).splitlines() - assert "@@ -1,0 +2,2 @@" in out - assert "@@ -6,1 +8,1 @@" in out + assert [line for line in out if line.startswith("@@")] == ["@@ -1,6 +1,8 @@"] + + +def test_cli_distant_edits_are_separate_hunks_with_shifted_numbers(cli_repo, capsys): + repo, commit = cli_repo + old = "".join(f"{i}\n" for i in range(1, 21)) + new = "1\nTWO\n" + "".join(f"{i}\n" for i in range(3, 19)) + "TWENTY\n" + write(repo, "a.txt", old) + commit("a.txt") + write(repo, "a.txt", new) + + out = run_diff(capsys).splitlines() + + assert [line for line in out if line.startswith("@@")] == [ + "@@ -1,5 +1,5 @@", + "@@ -16,5 +16,4 @@", + ] def test_cli_new_and_deleted_files(cli_repo, capsys): @@ -843,7 +864,15 @@ def test_status_sections_still_staged_not_staged_untracked(cli_repo, capsys): cmd_status(None) out = capsys.readouterr().out.splitlines() - assert out == ["staged:", " b.txt", "not staged:", " a.txt", "untracked:", " c.txt"] + assert out == [ + "On branch main", + "staged:", + " b.txt", + "not staged:", + " a.txt", + "untracked:", + " c.txt", + ] # --------------------------------------------------------------------------- @@ -1021,7 +1050,7 @@ def test_fast_forward_into_fresh_repo_restores_everything(source, local, monkeyp assert [e.path for e in local.wt.read_index()] == sorted(e.path for e in source.wt.read_index()) assert local.cm._current_branch() == "main" assert local.cm.read_ref("main") == source.cm.read_ref("main") - assert status_text(local, monkeypatch, capsys) == "clean\n" + assert status_text(local, monkeypatch, capsys) == BRANCH + "clean\n" assert diff_text(local, monkeypatch, capsys) == "" @@ -1047,7 +1076,7 @@ def test_second_fast_forward_applies_edit_delete_new_dir_and_mode( assert local.read("docs/guide/intro.md") == b"intro\n" assert not os.access(local.path("run.sh"), os.X_OK) assert local.cm._current_branch() == "main" - assert status_text(local, monkeypatch, capsys) == "clean\n" + assert status_text(local, monkeypatch, capsys) == BRANCH + "clean\n" assert diff_text(local, monkeypatch, capsys) == "" @@ -1074,7 +1103,9 @@ def test_dirty_tracked_file_blocks_pull_and_is_kept(synced): local.write("README.md", "my local edit\n") before = local.snapshot() - with pytest.raises(MiniGitError): + with pytest.raises( + MiniGitError, match="local changes would be lost: README.md; commit or undo your edits" + ): local.cm.merge("theirs") assert local.snapshot() == before @@ -1187,7 +1218,10 @@ def test_conflicted_path_gets_markers_but_keeps_pre_merge_index_entry(merging, m index = {e.path: e for e in local.wt.read_index()} assert index["a.txt"] == old_a assert index["c.txt"].hash == hashlib.sha1(b"blob 16\0added by theirs\n").hexdigest() - assert status_text(local, monkeypatch, capsys) == "staged:\n c.txt\nnot staged:\n a.txt\n" + assert ( + status_text(local, monkeypatch, capsys) + == BRANCH + "staged:\n c.txt\nnot staged:\n a.txt\n" + ) def test_conflicted_checkout_of_unknown_path_is_refused_before_any_write(merging): @@ -1212,13 +1246,13 @@ def test_resolve_with_add_then_commit_is_clean_across_fresh_objects(merging, mon assert cmd_add(argparse.Namespace(path="a.txt")) == 0 fresh = Repo(local.root) - assert status_text(fresh, monkeypatch, capsys) == "staged:\n a.txt\n c.txt\n" + assert status_text(fresh, monkeypatch, capsys) == BRANCH + "staged:\n a.txt\n c.txt\n" fresh = Repo(local.root) fresh.commit("merge", extra_parents=[theirs_tip]) fresh = Repo(local.root) - assert status_text(fresh, monkeypatch, capsys) == "clean\n" + assert status_text(fresh, monkeypatch, capsys) == BRANCH + "clean\n" assert fresh.read("a.txt") == b"resolved by hand\n" assert len(fresh.cm.read_commit(fresh.cm.read_ref("main")).parents) == 2 @@ -1233,13 +1267,299 @@ def test_resolved_deletion_with_add_then_commit(merging, monkeypatch, capsys): fresh = Repo(local.root) assert [e.path for e in fresh.wt.read_index()] == ["b.txt", "c.txt"] - assert status_text(fresh, monkeypatch, capsys) == "staged:\n a.txt\n c.txt\n" + assert status_text(fresh, monkeypatch, capsys) == BRANCH + "staged:\n a.txt\n c.txt\n" fresh.commit("merge: delete a.txt") fresh = Repo(local.root) - assert status_text(fresh, monkeypatch, capsys) == "clean\n" + assert status_text(fresh, monkeypatch, capsys) == BRANCH + "clean\n" assert not fresh.exists("a.txt") def test_add_of_a_missing_untracked_path_still_fails(local): with pytest.raises(MiniGitError, match="No such file"): local.wt.stage_file("never-existed.txt") + + +# --------------------------------------------------------------------------- +# Week 7: status / diff output, path display, error messages +# --------------------------------------------------------------------------- + + +def run_status(capsys): + assert cmd_status(None) == 0 + return capsys.readouterr().out + + +def test_status_empty_unborn_repo(cli_repo, capsys): + assert run_status(capsys) == BRANCH + "no commits yet\nclean\n" + + +def test_status_unborn_repo_lists_staged_files(cli_repo, capsys): + repo, _ = cli_repo + write(repo, "a.txt", "hi\n") + repo.stage_file("a.txt") + + assert run_status(capsys) == BRANCH + "no commits yet\nstaged:\n a.txt\n" + + +def test_status_staged_then_edited_file_is_in_both_sections(cli_repo, capsys): + repo, commit = cli_repo + write(repo, "a.txt", "v1\n") + commit("a.txt") + write(repo, "a.txt", "v2\n") + repo.stage_file("a.txt") + write(repo, "a.txt", "v3\n") + + assert run_status(capsys) == BRANCH + "staged:\n a.txt\nnot staged:\n a.txt\n" + + +def test_status_paths_are_sorted_and_handle_nesting_spaces_and_unicode(cli_repo, capsys): + repo, _ = cli_repo + for name in ["z.txt", "dir/sub/b file.txt", "café/é.txt", "a.txt"]: + write(repo, name, "x\n") + + expected = ["a.txt", "café/é.txt", "dir/sub/b file.txt", "z.txt"] + assert run_status(capsys) == ( + BRANCH + "no commits yet\nuntracked:\n" + "".join(f" {p}\n" for p in expected) + ) + + +def test_display_path_escapes_only_unprintable_paths(): + assert display_path("my dir/café é.txt") == "my dir/café é.txt" + assert display_path("a\nb.txt") == '"a\\nb.txt"' + assert display_path("a\\b.txt") == '"a\\\\b.txt"' + assert display_path("x.txt", "a/") == "a/x.txt" + assert display_path("x\ty.txt", "a/") == '"a/x\\ty.txt"' + + +def test_status_first_line_names_the_branch_or_detached_head(tmp_path, monkeypatch, capsys): + (tmp_path / "f.txt").write_text("x") + wt = WorkingTree(tmp_path) + wt.stage_file("f.txt") + manager = CommitManager(tmp_path) + commit = manager.create_commit(wt.build_tree_from_index(), [], "Test", "initial") + monkeypatch.chdir(tmp_path) + + manager.write_head("feature") + assert run_status(capsys).splitlines()[0] == "On branch feature" + + (tmp_path / ".minigit" / "HEAD").write_text(commit + "\n") + assert run_status(capsys).splitlines()[0] == f"HEAD detached at {commit[:7]}" + + +def test_status_lists_unresolved_merge_paths_from_the_manager(cli_repo, capsys, monkeypatch): + repo, _ = cli_repo + write(repo, "a.txt", "x\n") + monkeypatch.setattr( + sys.modules["minigit.commits"].CommitManager, + "_read_pending_merge", + lambda self: ("ours", "theirs", "merge", ["a.txt"]), + ) + + # one heading, no duplicates: a.txt is not also "untracked" or "not staged" + assert run_status(capsys) == BRANCH + "no commits yet\nunresolved (merge conflict):\n a.txt\n" + + +def apply_printed_hunks(old_lines, diff_lines): + """Apply the printed hunks to old_lines (all lines end in "\\n") - checks the numbers.""" + result, pos, hunks = [], 0, [] + for line in diff_lines: + if line.startswith("@@"): + hunks.append((re.fullmatch(r"@@ -(\d+),(\d+) \+(\d+),(\d+) @@", line).groups(), [])) + elif hunks: + hunks[-1][1].append(line) + for (old_start, old_count, new_start, new_count), body in hunks: + old_start, old_count, new_start, new_count = map( + int, (old_start, old_count, new_start, new_count) + ) + first_old = old_start if old_count == 0 else old_start - 1 + result += old_lines[pos:first_old] + pos = first_old + assert len(result) == (new_start if new_count == 0 else new_start - 1) + seen_old = seen_new = 0 + for line in body: + text = line[1:] + "\n" + if line[0] in " -": + assert old_lines[pos] == text + pos += 1 + seen_old += 1 + if line[0] in " +": + result.append(text) + seen_new += 1 + assert (seen_old, seen_new) == (old_count, new_count) + return result + old_lines[pos:] + + +def test_printed_hunks_rebuild_the_new_file_for_any_context_size(repo): + from minigit.index import _format_change + + random.seed(3) + for _ in range(300): + old = [random.choice("abc") + "\n" for _ in range(random.randint(0, 14))] + new = [random.choice("abc") + "\n" for _ in range(random.randint(0, 14))] + change = repo._make_change( + "f.txt", "modified", "100644", "100644", "".join(old).encode(), "".join(new).encode() + ) + for context in (0, 1, 3): + printed = _format_change(change, context) + assert apply_printed_hunks(old, printed) == new + + +def test_cli_empty_and_repeated_line_files(cli_repo, capsys): + repo, commit = cli_repo + write(repo, "rep.txt", "x\nx\nx\n") + commit("rep.txt") + write(repo, "rep.txt", "x\nx\n") + write(repo, "empty.txt", "") + + out = run_diff(capsys).splitlines() + + assert "diff --minigit a/empty.txt b/empty.txt" in out + assert "new file mode 100644" in out + assert [line for line in out if line.startswith("@@")] == ["@@ -1,3 +1,2 @@"] + assert out.count("-x") == 1 + + +def test_cli_binary_files_added_and_deleted_get_a_summary(cli_repo, capsys): + repo, commit = cli_repo + write(repo, "gone.bin", b"\x00\x01") + commit("gone.bin") + os.remove("gone.bin") + write(repo, "new.bin", b"\x00\x02") + + out = run_diff(capsys).splitlines() + + assert "binary file deleted: gone.bin" in out + assert "binary file added: new.bin" in out + assert not any(line.startswith("@@") for line in out) + + +def test_cli_diff_shows_space_unicode_and_odd_paths_clearly(cli_repo, capsys): + repo, commit = cli_repo + write(repo, "my dir/café notes.txt", "one\n") + commit("my dir/café notes.txt") + write(repo, "my dir/café notes.txt", "two\n") + write(repo, "odd\nname.txt", "new\n") # cannot be staged, but it can sit in the folder + + out = run_diff(capsys).splitlines() + status = run_status(capsys) + + assert "diff --minigit a/my dir/café notes.txt b/my dir/café notes.txt" in out + assert "--- a/my dir/café notes.txt" in out + assert '+++ "b/odd\\nname.txt"' in out + assert not any(line == "name.txt" for line in out) + assert ' "odd\\nname.txt"\n' in status + + +def test_cli_diff_output_is_plain_and_repeatable(cli_repo, capsys): + repo, commit = cli_repo + write(repo, "a.txt", "one\n") + commit("a.txt") + write(repo, "a.txt", "two\n") + + first = run_diff(capsys) + + assert first == run_diff(capsys) + assert "\x1b" not in first + + +def test_diff_help_says_head_tree_versus_working_files(monkeypatch): + from minigit.index import register_index_commands + + monkeypatch.setenv("COLUMNS", "200") + parser = argparse.ArgumentParser() + register_index_commands(parser.add_subparsers()) + + assert "the HEAD tree compared with the working files" in parser.format_help() + + +def test_status_and_diff_leave_an_unborn_repo_unchanged(local, monkeypatch, capsys): + local.write("a.txt", "x\n") + before, objects = local.snapshot(), local.object_files() + + status_text(local, monkeypatch, capsys) + diff_text(local, monkeypatch, capsys) + + assert local.snapshot() == before + assert local.object_files() == objects + + +def test_status_and_diff_leave_a_conflicted_repo_unchanged(merging, monkeypatch, capsys): + local, merged_tree = merging + local.wt.checkout(merged_tree, conflicted_paths={"a.txt"}) + before, objects = local.snapshot(), local.object_files() + + status_text(local, monkeypatch, capsys) + out = diff_text(local, monkeypatch, capsys) + + assert "<<<<<<< ours" in out + assert local.snapshot() == before + assert local.object_files() == objects + + +def test_expected_failures_name_the_path_and_the_next_step(repo): + with pytest.raises(MiniGitError, match=r"No such file: nope\.txt; check the path"): + repo.stage_file("nope.txt") + with pytest.raises(MiniGitError, match=r"unsafe path: \.\./x\.txt"): + repo.stage_file("../x.txt") + write(repo, "odd\nname.txt", "x\n") + with pytest.raises(MiniGitError, match=r'unsafe path: "odd\\nname.txt"'): # one line + repo.stage_file("odd\nname.txt") + os.makedirs(os.path.dirname(repo.index_path)) + with open(repo.index_path, "w") as f: + f.write("garbage\n") + with pytest.raises(MiniGitError, match=r"corrupt index: line 1 .*minigit add"): + repo.read_index() + + +# --------------------------------------------------------------------------- +# Week 8: speed work must not change results +# --------------------------------------------------------------------------- + + +def test_same_size_edit_with_preserved_timestamp_is_still_found(cli_repo, capsys): + repo, commit = cli_repo + path = write(repo, "a.txt", "aaaa\nkeep\n") + commit("a.txt") + before = os.stat(path) + + write(repo, "a.txt", "bbbb\nkeep\n") # same size + os.utime(path, ns=(before.st_atime_ns, before.st_mtime_ns)) # same timestamps + after = os.stat(path) + assert (after.st_size, after.st_mtime_ns) == (before.st_size, before.st_mtime_ns) + + assert run_status(capsys) == BRANCH + "not staged:\n a.txt\n" + diff = run_diff(capsys) + assert "-aaaa\n+bbbb\n keep\n" in diff + + +def test_walk_working_files_gives_repo_relative_paths_and_skips_minigit(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + names = ["top.txt", "dir/sub/b file.txt", "café/é.txt", "dir/x.txt"] + for name in names: + os.makedirs(os.path.dirname(name) or ".", exist_ok=True) + with open(name, "w") as f: + f.write("x\n") + os.makedirs(".minigit/objects") + with open(".minigit/index", "w") as f: + f.write("") + + for root in (".", str(tmp_path)): + found = list(walk_working_files(root)) + assert sorted(rel for _, rel in found) == sorted(names) + for full, rel in found: + assert os.path.samefile(full, os.path.join(tmp_path, rel)) + + +def test_myers_on_a_larger_text_with_repeated_lines_is_valid_and_small(): + random.seed(7) + old = [f"line {random.randint(0, 30)}\n" for _ in range(1500)] + new = list(old) + for _ in range(40): # 40 scattered single-line changes + new[random.randrange(len(new))] = f"changed {random.randint(0, 5)}\n" + + edits = myers_diff(old, new) + + assert apply_edits(old, edits) == new + assert edit_cost(edits) <= 80 # 40 replacements cost at most 40 deletes + 40 inserts + for first, second in zip(edits, edits[1:], strict=False): + assert first.start + first.old_count < second.start diff --git a/tests/test_week3_integration.py b/tests/test_week3_integration.py index bbc9b69..f4d0ec0 100644 --- a/tests/test_week3_integration.py +++ b/tests/test_week3_integration.py @@ -16,7 +16,7 @@ def test_cli_commit_status_and_history(tmp_path, monkeypatch, capsys): assert main(["add", "src/hello.txt"]) == 0 capsys.readouterr() assert main(["status"]) == 0 - assert capsys.readouterr().out == "staged:\n src/hello.txt\n" + assert capsys.readouterr().out == "On branch main\nno commits yet\nstaged:\n src/hello.txt\n" assert main(["commit", "-m", "first"]) == 0 manager = CommitManager(tmp_path) first = manager.read_ref("main") @@ -26,12 +26,14 @@ def test_cli_commit_status_and_history(tmp_path, monkeypatch, capsys): assert manager.store.read_object(entries[0].hash) == ("blob", b"first") capsys.readouterr() assert main(["status"]) == 0 - assert capsys.readouterr().out == "clean\n" + assert capsys.readouterr().out == "On branch main\nclean\n" file.write_text("second") assert main(["add", "src/hello.txt"]) == 0 file.write_text("third") assert main(["status"]) == 0 - assert capsys.readouterr().out == "staged:\n src/hello.txt\nnot staged:\n src/hello.txt\n" + assert capsys.readouterr().out == ( + "On branch main\nstaged:\n src/hello.txt\nnot staged:\n src/hello.txt\n" + ) assert main(["commit", "-m", "second"]) == 0 second = manager.read_ref("main") assert manager.read_commit(second).parents == [first] diff --git a/tests/test_week4_integration.py b/tests/test_week4_integration.py index faf7810..d484914 100644 --- a/tests/test_week4_integration.py +++ b/tests/test_week4_integration.py @@ -31,7 +31,7 @@ def test_week4_cli_checkpoint_and_push(tmp_path, monkeypatch, capsys): assert manager.read_ref("main") == manager.read_ref("feature") == b capsys.readouterr() assert main(["status"]) == 0 - assert capsys.readouterr().out.strip() == "clean" + assert capsys.readouterr().out.strip() == "On branch main\nclean" remote = tmp_path / "remote" remote.mkdir() diff --git a/tests/test_week5_integration.py b/tests/test_week5_integration.py index e951433..9760bbc 100644 --- a/tests/test_week5_integration.py +++ b/tests/test_week5_integration.py @@ -84,7 +84,7 @@ def test_conflict_resolution_across_cli_processes(tmp_path, delete_ours, resolut 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 cli(tmp_path, "status").strip() == "On branch main\nclean" assert not list((tmp_path / ".minigit").glob("MERGE_*")) @@ -131,7 +131,7 @@ def test_packed_three_way_merge_push_and_stale_rejection(tmp_path): 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, "status").strip() == "On branch main\nclean" assert cli(local, "diff") == "" tip = manager.read_ref("main") assert manager.read_commit(tip).parents == parents