diff --git a/gitmirror/crates/gitmirror-xrpc/src/routes/git.rs b/gitmirror/crates/gitmirror-xrpc/src/routes/git.rs index 8c8a7447c..78581e590 100644 --- a/gitmirror/crates/gitmirror-xrpc/src/routes/git.rs +++ b/gitmirror/crates/gitmirror-xrpc/src/routes/git.rs @@ -185,6 +185,18 @@ pub(crate) fn find_commit(repo: &gix::Repository, sha: &str) -> Result Result, XrpcError> { + let oid = gix::ObjectId::from_hex(sha.as_bytes()) + .map_err(|e| XrpcError::InvalidRequest(format!("bad commit sha {sha:?}: {e}")))?; + if oid == gix::ObjectId::empty_tree(repo.object_hash()) { + return Ok(None); + } + find_commit(repo, sha).map(Some) +} + // ── blame ────────────────────────────────────────────────────────────── #[derive(Debug, thiserror::Error)] @@ -452,12 +464,17 @@ pub(crate) async fn get_blame( fn get_diff_inner( repo: &gix::Repository, - base: gix::ObjectId, + base: Option, head: gix::ObjectId, ) -> Result, XrpcError> { let compare = || -> anyhow::Result> { - let merge_base = repo.merge_base(base, head)?.detach(); - let old = repo.find_tree(repo.find_commit(merge_base)?.tree_id()?)?; + let old = match base { + Some(base) => { + let merge_base = repo.merge_base(base, head)?.detach(); + repo.find_tree(repo.find_commit(merge_base)?.tree_id()?)? + } + None => repo.empty_tree(), + }; let new = repo.find_tree(repo.find_commit(head)?.tree_id()?)?; diff::diff(repo, &old, &new, false)? .map(|d| d.map(Into::into)) @@ -474,7 +491,7 @@ pub(crate) async fn get_diff( let scratch = state .layout .open_scratch(&[&args.head_repo, &args.base_repo])?; - let base = find_commit(&scratch, args.base_commit.as_ref())?; + let base = find_diff_base(&scratch, args.base_commit.as_ref())?; let head = find_commit(&scratch, args.head_commit.as_ref())?; tokio::task::spawn_blocking(move || get_diff_inner(&scratch, base, head)) @@ -1148,11 +1165,49 @@ mod diff_tests { let repo_base = root.join("repos"); let scratch = Layout::new(repo_base) .open_scratch(&[&Did::raw(HEAD_DID.into()), &Did::raw(BASE_DID.into())])?; - let base = find_commit(&scratch, base_commit)?; + let base = find_diff_base(&scratch, base_commit)?; let head = find_commit(&scratch, head_commit)?; get_diff_inner(&scratch, base, head) } + fn empty_tree() -> String { + gix::ObjectId::empty_tree(gix::hash::Kind::Sha1).to_string() + } + + fn paths(diffs: &[sh_tangled::git::FileDiff]) -> Vec { + let mut paths: Vec = diffs.iter().map(|d| d.rhs_src.path.to_string()).collect(); + paths.sort(); + paths + } + + #[test] + fn a_root_commit_diffs_against_the_empty_tree() { + let (root, _base, _head) = fork_fixture(); + let root_commit = rev_parse(&root.path().join("upstream"), "HEAD~1"); + + let diffs = diff_fixture(root.path(), &empty_tree(), &root_commit).unwrap(); + + assert_eq!(paths(&diffs), ["a.txt", "b.txt"]); + let null = gix::ObjectId::null(gix::hash::Kind::Sha1).to_string(); + for diff in &diffs { + assert_eq!(diff.lhs_src.oid.to_string(), null); + } + + let a = diffs.iter().find(|d| d.rhs_src.path == "a.txt").unwrap(); + assert_eq!(a.hunks.len(), 1); + assert!(a.hunks[0].novel_lhs.is_empty()); + assert_eq!(a.hunks[0].novel_rhs, [0, 1, 2]); + } + + #[test] + fn the_empty_tree_base_skips_the_merge_base_and_reports_the_whole_head() { + let (root, base, _head) = fork_fixture(); + + let diffs = diff_fixture(root.path(), &empty_tree(), &base).unwrap(); + + assert_eq!(paths(&diffs), ["a.txt", "b.txt", "upstream.txt"]); + } + #[test] fn a_cross_repo_diff_reports_only_what_the_head_side_changed() { let (root, base, head) = fork_fixture(); diff --git a/web/src/lib/api/gitmirror.ts b/web/src/lib/api/gitmirror.ts index 186471b1f..cbecb7531 100644 --- a/web/src/lib/api/gitmirror.ts +++ b/web/src/lib/api/gitmirror.ts @@ -97,6 +97,12 @@ export type MirrorFileKind = "new" | "deleted" | "renamed" | "changed"; export const diffSideExists = (src: ShTangledGitDefs.DiffSrc): boolean => src.path !== "" && !/^0+$/.test(src.oid); +const EMPTY_TREE_SHA1 = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; +const EMPTY_TREE_SHA256 = "6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321"; + +export const emptyTreeOid = (like: string): string => + like.length === 64 ? EMPTY_TREE_SHA256 : EMPTY_TREE_SHA1; + export const diffFileName = (file: MirrorFileDiff): string => file.rhsSrc.path || file.lhsSrc.path; export const diffFileKind = (file: MirrorFileDiff): MirrorFileKind => { diff --git a/web/src/lib/api/repoDiff.test.ts b/web/src/lib/api/repoDiff.test.ts index 82ad4c43c..839a2bd49 100644 --- a/web/src/lib/api/repoDiff.test.ts +++ b/web/src/lib/api/repoDiff.test.ts @@ -9,7 +9,7 @@ import { loadDiff, resolveChange } from "$lib/api/repoDiff"; -import type { MirrorFileDiff } from "$lib/api/gitmirror"; +import { emptyTreeOid, type MirrorFileDiff } from "$lib/api/gitmirror"; const src = (path: string, extra: Partial = {}) => ({ path, @@ -228,6 +228,37 @@ describe("fetchSides", () => { expect(blobCalls(calls)).toHaveLength(3); }); + it("diffs a root commit against the empty tree and reads only the new side", async () => { + const head = "a".repeat(40); + const { deps, calls } = harness( + [ + { + lhsSrc: absent("new.ts"), + rhsSrc: src("new.ts"), + hunks: [{ novelLhs: [], novelRhs: [0], lines: [{ rhs: 0 }] }], + hasSyntacticChanges: false + } + ], + { [`${head}:new.ts`]: "hello\n" } + ); + const spec = { + kind: "diff", + repo: REPO, + version: { base: emptyTreeOid(head), head } + } as const; + + const diff = await loadDiff(deps, spec); + const added = await fetchSides(deps, diff.contents, diff.files[0]); + + expect(diff.contents).toEqual({ oldRef: emptyTreeOid(head), newRef: head }); + expect(diff.files.map((file) => file.kind)).toEqual(["new"]); + expect(added).toEqual({ + oldFile: { name: "new.ts", contents: "" }, + newFile: { name: "new.ts", contents: "hello\n" } + }); + expect(blobCalls(calls)).toHaveLength(1); + }); + it("takes an interdiff's left side inline and fetches only the right", async () => { const { deps, calls } = harness( [ @@ -348,6 +379,15 @@ describe("resolveChange", () => { }); }); +describe("emptyTreeOid", () => { + it("picks the sentinel matching the repo's hash algorithm", () => { + expect(emptyTreeOid("a".repeat(40))).toBe("4b825dc642cb6eb9a060e54bf8d69288fbee4904"); + expect(emptyTreeOid("a".repeat(64))).toBe( + "6ef19b41225c5369f1c104d45d8d85efa9b057b53b14b4b9b939dd74decc5321" + ); + }); +}); + describe("cappedFetch", () => { const bigJson = (bytes: number) => new Response("x".repeat(bytes), { diff --git a/web/src/routes/[handle]/[repo]/commit/[ref]/+page.svelte b/web/src/routes/[handle]/[repo]/commit/[ref]/+page.svelte index 276c84708..2843beab1 100644 --- a/web/src/routes/[handle]/[repo]/commit/[ref]/+page.svelte +++ b/web/src/routes/[handle]/[repo]/commit/[ref]/+page.svelte @@ -88,7 +88,6 @@ : undefined} {@const mirror = { diff: commitPage.diff, - error: commitPage.diffError, pendingSpec: commitPage.diffSpec, deps: { ctx: createBobbinClient({ serviceUrl: data.publicConfig.bobbinUrl }), diff --git a/web/src/routes/[handle]/[repo]/commit/[ref]/+page.ts b/web/src/routes/[handle]/[repo]/commit/[ref]/+page.ts index ebe947146..1996b482c 100644 --- a/web/src/routes/[handle]/[repo]/commit/[ref]/+page.ts +++ b/web/src/routes/[handle]/[repo]/commit/[ref]/+page.ts @@ -2,7 +2,12 @@ import { error } from "@sveltejs/kit"; import { browser } from "$app/environment"; import { createBobbinClient } from "$lib/api/client"; import { knotMirrorTarget } from "$lib/api/gitclient"; -import { commitAtRef, mirrorChangeId, toMirrorCommitDetail } from "$lib/api/gitmirror"; +import { + commitAtRef, + emptyTreeOid, + mirrorChangeId, + toMirrorCommitDetail +} from "$lib/api/gitmirror"; import { formatPatchUrl } from "$lib/api/knotmirror"; import { getPatchStream } from "$lib/api/patchStream"; import { settle, stream, toHttpError } from "$lib/api/load"; @@ -48,9 +53,11 @@ export const load: PageLoad = async (event) => { }) ); } - const spec: RepoDiffSpec | undefined = parentRef - ? { kind: "diff", repo: repoDid, version: { base: parentRef, head: wire.oid } } - : undefined; + const spec: RepoDiffSpec = { + kind: "diff", + repo: repoDid, + version: { base: parentRef ?? emptyTreeOid(wire.oid), head: wire.oid } + }; const git = knotMirrorTarget(parent.publicConfig, { repoDid }, event.fetch); // the ssr worker has a memory ceiling a multi-megabyte diff payload blows // through, so above the configured byte cap the body is abandoned mid-read @@ -66,7 +73,7 @@ export const load: PageLoad = async (event) => { let diff; let diffSpec: RepoDiffSpec | undefined; try { - diff = spec ? await loadDiff(diffDeps, spec) : undefined; + diff = await loadDiff(diffDeps, spec); } catch (cause) { if (!isDiffTooLarge(cause)) { toHttpError(cause, "Could not load commit diff from the mirror"); @@ -79,8 +86,7 @@ export const load: PageLoad = async (event) => { parent: parentRef ?? "", changeId: mirrorChangeId(wire) ?? "", diff, - diffSpec, - diffError: parentRef ? undefined : "Root commit diffs are not supported by the mirror." + diffSpec }; });