diff --git a/minigit/remote.py b/minigit/remote.py index 883ad6f..912ee63 100644 --- a/minigit/remote.py +++ b/minigit/remote.py @@ -269,7 +269,7 @@ def _put_object(self, sock, buf: bytearray, obj_hash: str) -> None: raise NetworkProtocolError(f"remote rejected {obj_hash}: {reply}") def pull(self, remote_address: str, branch: str, token: str) -> None: - """Fetch `branch` from the remote and update the matching local ref.""" + """Fetch `branch` and its complete reachable object graph.""" host, port = self._parse_address(remote_address) if len(token) == 0: @@ -289,11 +289,65 @@ def pull(self, remote_address: str, branch: str, token: str) -> None: send_line(sock, f"REF {branch}") reply = receive_line(sock, buf) - remote_hash = reply.rsplit(" ", 1)[1] - print(f"remote {branch} is at {remote_hash}") - print("# Week 6 - same exchange in reverse") + fields = reply.split(" ") + if len(fields) != 3 or fields[:2] != ["REF", branch]: + raise NetworkProtocolError(f"invalid REF response: {reply!r}") + remote_hash = fields[2] + if remote_hash == "-": + raise NetworkProtocolError(f"remote branch {branch!r} has no commits") + if not _valid_hash(remote_hash): + raise NetworkProtocolError(f"invalid remote hash: {remote_hash!r}") + + commits_to_visit = [remote_hash] + visited_commits: set[str] = set() + visited_trees: set[str] = set() + while commits_to_visit: + commit_hash = commits_to_visit.pop() + if commit_hash in visited_commits: + continue + visited_commits.add(commit_hash) + self._ensure_pulled_object(sock, buf, commit_hash, "commit") + commit = self.commits.read_commit(commit_hash) + commits_to_visit.extend(commit.parents) + self._pull_tree(sock, buf, commit.tree, visited_trees) + + send_line(sock, "DONE") + print(f"{branch} now at {remote_hash}") + except (OSError, ObjectNotFoundError, ObjectCorruptError) as exc: + raise NetworkProtocolError(f"pull from {host}:{port} failed: {exc}") from exc finally: sock.close() + self.commits.write_ref(branch, remote_hash) + + def _ensure_pulled_object( + self, sock, buf: bytearray, obj_hash: str, expected_type: str + ) -> None: + """Ensure an object exists locally and has its expected graph type.""" + + try: + obj_type, _ = self.store.read_object(obj_hash) + except ObjectNotFoundError: + send_line(sock, f"WANT {obj_hash}") + self._receive_object(sock, buf, obj_hash) + obj_type, _ = self.store.read_object(obj_hash) + + if obj_type != expected_type: + raise ObjectCorruptError(obj_hash) + + def _pull_tree(self, sock, buf: bytearray, tree_hash: str, visited: set[str]) -> None: + """Fetch a tree and all nested trees and blobs, once each.""" + + if tree_hash in visited: + return + visited.add(tree_hash) + self._ensure_pulled_object(sock, buf, tree_hash, "tree") + for entry in self.store.read_tree(tree_hash): + if entry.type == "tree": + self._pull_tree(sock, buf, entry.hash, visited) + elif entry.type == "blob": + self._ensure_pulled_object(sock, buf, entry.hash, "blob") + else: + raise ObjectCorruptError(entry.hash) def fetch_objects(self, remote_address: str, hashes: list[str], token: str) -> None: """Fetch each of `hashes` from the remote and write it into the local store. diff --git a/tests/test_remote.py b/tests/test_remote.py index a4d4b01..d2b9925 100644 --- a/tests/test_remote.py +++ b/tests/test_remote.py @@ -9,7 +9,7 @@ from minigit.cli import main from minigit.commits import CommitManager -from minigit.errors import NetworkProtocolError +from minigit.errors import NetworkProtocolError, ObjectNotFoundError from minigit.objects import ObjectStore from minigit.objects import TreeEntry as RealTreeEntry from minigit.remote import RemoteClient, RemoteServer, receive_line, recv_exact, send_line @@ -207,6 +207,60 @@ def test_push_nested_files_multiple_commits(push_pair): assert not (Path(pair.server.repo_path) / ".minigit" / "index").exists() +def test_pull_fetches_missing_descendants_from_packed_remote(tmp_path, push_pair): + """Pull fetches a merge graph even when the local commit already exists.""" + + pair = push_pair + base = _commit( + pair.local_store, + pair.local_commits, + {"base.txt": b"base", "shared/empty": b""}, + [], + ) + side = _commit( + pair.local_store, + pair.local_commits, + {"side.bin": b"\x00\xff", "shared/name": "caf\u00e9".encode()}, + [base], + ) + merge = _commit( + pair.local_store, + pair.local_commits, + {"nested/run.sh": b"#!/bin/sh\n", "shared/name": "caf\u00e9".encode()}, + [base, side], + ) + pair.client.push(pair.address, "main", "tok") + + reachable = pair.client.collect_reachable("main") + pair.remote_store.pack_objects(reachable) + for obj_hash in reachable: + pair.remote_store._object_path(obj_hash).unlink() + + destination_root = tmp_path / "destination" + destination_root.mkdir() + destination_store = ObjectStore(destination_root) + destination_commits = CommitManager(str(destination_root), store=destination_store) + destination_client = RemoteClient( + repo_path=str(destination_root), + store=destination_store, + commits=destination_commits, + ) + + commit_type, commit_bytes = pair.local_store.read_object(merge) + assert commit_type == "commit" + destination_store.write_object(commit_bytes, "commit") + destination_commits.write_ref("main", merge) + with pytest.raises(ObjectNotFoundError): + destination_store.read_object(base) + + destination_client.pull(pair.address, "main", "tok") + + assert destination_commits.read_ref("main") == merge + assert destination_client.collect_reachable("main") == reachable + for obj_hash in reachable: + assert destination_store.read_object(obj_hash) == pair.local_store.read_object(obj_hash) + + def test_push_to_empty_branch_then_one_more_commit(push_pair): pair = push_pair assert pair.remote_commits.read_ref("main") is None