diff --git a/Taskfile.pkl b/Taskfile.pkl index afc51c77..b3b4a75b 100644 --- a/Taskfile.pkl +++ b/Taskfile.pkl @@ -332,8 +332,12 @@ local e2e: Task = new { local testSubdir: Task = new { name = "test-subdir" - description = "subdir-clone/push integration tests (t/ directory)" - cmd = "bash t/run-tests.sh t900" + description = "subdir-clone/push and transport integration tests (t/ directory)" + // The filter is a substring match, so `t900` alone silently skips t902x. + cmd = #""" + bash t/run-tests.sh t900 + bash t/run-tests.sh t902 + """# } local testDistributed: Task = new { diff --git a/modules/bit/cmd/bit/fsck.mbt b/modules/bit/cmd/bit/fsck.mbt index 03bf0ff8..9bb3a1d0 100644 --- a/modules/bit/cmd/bit/fsck.mbt +++ b/modules/bit/cmd/bit/fsck.mbt @@ -84,6 +84,7 @@ async fn handle_fsck(args : Array[String]) -> Unit raise Error { fs, ref_tips, collect_extra=need_extra, + shallow=@repo.read_shallow_boundaries(fs, git_dir), ) errors += result.errors // Report missing objects diff --git a/modules/bit/cmd/bit/rev_list.mbt b/modules/bit/cmd/bit/rev_list.mbt index 87233eb3..5113fa65 100644 --- a/modules/bit/cmd/bit/rev_list.mbt +++ b/modules/bit/cmd/bit/rev_list.mbt @@ -600,6 +600,13 @@ async fn handle_rev_list(args : Array[String]) -> Unit raise Error { } else { Map([]) } + // Commits on the shallow boundary are grafted: their parents were never + // fetched. Following those links emits an id whose object is not in the + // store, so the walk must stop here exactly as `log` does. + let shallow_boundary : Map[String, Bool] = Map([]) + for id in @repo.read_shallow_boundaries(fs, git_dir) { + shallow_boundary[id.to_hex()] = true + } let visited : Map[String, Bool] = Map([]) let result : Array[@bitcore.ObjectId] = [] // BFS/DFS traversal — collect ALL reachable commits, then sort/slice @@ -618,7 +625,9 @@ async fn handle_rev_list(args : Array[String]) -> Unit raise Error { // Get commit and add parents match rev_list_load_commit_info(db, fs, id) { Some(info) => - if first_parent && info.parents.length() > 0 { + if shallow_boundary.contains(hex) { + () + } else if first_parent && info.parents.length() > 0 { if !visited.contains(info.parents[0].to_hex()) { queue.push(info.parents[0]) } diff --git a/modules/bit_io_native/src/upload_pack_process.mbt b/modules/bit_io_native/src/upload_pack_process.mbt index 9721de53..cdf17531 100644 --- a/modules/bit_io_native/src/upload_pack_process.mbt +++ b/modules/bit_io_native/src/upload_pack_process.mbt @@ -1261,7 +1261,10 @@ pub async fn clone_process_to_fs( if wants.length() == 0 { return refs } - let pack = fetch_pack_process(remote, wants, prefer_v2, depth~, filter~) + let fetched = fetch_pack_process_result( + remote, wants, prefer_v2, depth~, filter~, + ) + let pack = fetched.pack let objects = @pack.parse_packfile(pack) let git_dir = @bit.join_path(root, ".git") let pack_id = read_pack_trailer_id(pack) @@ -1290,24 +1293,13 @@ pub async fn clone_process_to_fs( ) } } - if depth > 0 { - let shallow_ids : Array[String] = [] - let seen : Map[String, Bool] = Map([]) - for id in wants { - let hex = id.to_hex() - if seen.contains(hex) { - continue - } - seen[hex] = true - shallow_ids.push(hex) - } - if shallow_ids.length() > 0 { - fs.write_string( - @bit.join_path(git_dir, "shallow"), - shallow_ids.join("\n") + "\n", - ) - } - } + // The boundary is whatever the server reported as shallow, not the refs we + // wanted: those coincide only at depth 1. Recording the tips instead makes + // every deeper clone look parentless at its tip, which then truncates the + // history a repack keeps. + @repo.apply_shallow_updates( + fs, rfs, git_dir, fetched.shallow, fetched.unshallow, + ) refs } diff --git a/modules/bit_lib/src/fsck.mbt b/modules/bit_lib/src/fsck.mbt index 6611fcc5..ad139a29 100644 --- a/modules/bit_lib/src/fsck.mbt +++ b/modules/bit_lib/src/fsck.mbt @@ -18,7 +18,15 @@ pub fn fsck_connectivity_check( fs : &@bit.RepoFileSystem, tips : Array[@bit.ObjectId], collect_extra? : Bool = false, + shallow? : Array[@bit.ObjectId] = [], ) -> FsckResult { + // Commits on the shallow boundary are grafted: their parents were never + // fetched, so following those links would report a well-formed shallow + // clone as corrupt. + let boundary : Map[String, Bool] = Map([]) + for id in shallow { + boundary[id.to_hex()] = true + } let reachable : Map[String, Bool] = Map([]) let missing : Map[String, Bool] = Map([]) let root_commits : Array[String] = [] @@ -68,10 +76,12 @@ pub fn fsck_connectivity_check( if !reachable.contains(tree_hex) { queue.push(info.tree) } - for parent in info.parents { - let phex = parent.to_hex() - if !reachable.contains(phex) { - queue.push(parent) + if !boundary.contains(hex) { + for parent in info.parents { + let phex = parent.to_hex() + if !reachable.contains(phex) { + queue.push(parent) + } } } } diff --git a/modules/bit_lib/src/fsck_test.mbt b/modules/bit_lib/src/fsck_test.mbt index 373e1b10..da5031e4 100644 --- a/modules/bit_lib/src/fsck_test.mbt +++ b/modules/bit_lib/src/fsck_test.mbt @@ -127,3 +127,48 @@ test "fsck_verify_loose_hash valid" { ), ) } + +///| +test "fsck_connectivity_check treats a shallow boundary as grafted" { + let fs = @bit.TestFs::new() + let git_dir = "/repo/.git" + fs.mkdir_p(git_dir) + fs.mkdir_p(git_dir + "/objects") + fs.mkdir_p(git_dir + "/refs") + let (tree_id, tree_bytes) = @bit.create_tree([]) + @bit_lib.write_object_bytes(fs, git_dir, tree_id, tree_bytes) + // A boundary commit whose parent was never fetched, exactly what a + // `clone --depth` leaves behind. + let absent_parent_hex = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" + let absent_parent = @bit.ObjectId::from_hex(absent_parent_hex) + let commit = @bit.Commit::new( + tree_id, + [absent_parent], + "A ", + 1000000000L, + "+0000", + "A ", + 1000000000L, + "+0000", + "boundary\n", + ) + let (commit_id, commit_bytes) = @bit.create_commit(commit) + @bit_lib.write_object_bytes(fs, git_dir, commit_id, commit_bytes) + let db = @bit_lib.ObjectDb::load(fs, git_dir) + + // Without the boundary the absent parent is reported as corruption. + let plain = @bit_lib.fsck_connectivity_check(db, fs, [commit_id]) + assert_true(plain.errors > 0) + assert_true(plain.missing.contains(absent_parent_hex)) + + // Declaring the boundary makes the same repository read as clean. + let grafted = @bit_lib.fsck_connectivity_check( + db, + fs, + [commit_id], + shallow=[commit_id], + ) + assert_eq(grafted.errors, 0) + assert_true(!grafted.missing.contains(absent_parent_hex)) + assert_true(grafted.reachable.contains(tree_id.to_hex())) +} diff --git a/modules/bit_lib/src/gc.mbt b/modules/bit_lib/src/gc.mbt index f6066129..f09cf5bf 100644 --- a/modules/bit_lib/src/gc.mbt +++ b/modules/bit_lib/src/gc.mbt @@ -83,7 +83,10 @@ fn repack_git_dir( if roots.length() == 0 { return None } - let objects = collect_reachable_objects_from_commits(db, rfs, roots) + // A shallow clone's boundary commits have no parents on disk. Walking past + // them either aborts the run or, worse, truncates what a repack keeps. + let shallow = @repo.read_shallow_boundaries(rfs, git_dir) + let objects = collect_reachable_objects_from_commits(db, rfs, roots, shallow~) collect_reachable_tag_objects(db, rfs, ref_ids, objects) // Reflog-referenced objects are roots too (git keeps them); repacking without // them would drop reflog-only objects from the new pack. @@ -101,10 +104,11 @@ fn gc_git_dir( let db = ObjectDb::load(rfs, git_dir) let roots = resolve_commit_roots(db, rfs, ref_ids) let prune_enabled = roots.length() > 0 + let shallow = @repo.read_shallow_boundaries(rfs, git_dir) let reachable = if roots.length() == 0 { [] } else { - collect_reachable_objects_from_commits(db, rfs, roots) + collect_reachable_objects_from_commits(db, rfs, roots, shallow~) } // Also include tag objects that refs point to directly (annotated tags). collect_reachable_tag_objects(db, rfs, ref_ids, reachable) @@ -154,7 +158,8 @@ fn prune_git_dir( if roots.length() == 0 { return { pruned: [] } } - let reachable = collect_reachable_objects_from_commits(db, rfs, roots) + let shallow = @repo.read_shallow_boundaries(rfs, git_dir) + let reachable = collect_reachable_objects_from_commits(db, rfs, roots, shallow~) collect_reachable_tag_objects(db, rfs, ref_ids, reachable) // Match gc_git_dir: reflog-referenced objects are protected roots, so a bare // `bit gc --prune` must not delete reflog-only loose objects. @@ -374,6 +379,7 @@ fn collect_reflog_referenced_objects( git_dir : String, objects : Array[@bit.PackObject], ) -> Unit raise @bit.GitError { + let shallow = @repo.read_shallow_boundaries(fs, git_dir) let seen : Map[String, Bool] = Map([]) for obj in objects { seen[@bit.hash_object_content(obj.obj_type, obj.data).to_hex()] = true @@ -391,7 +397,9 @@ fn collect_reflog_referenced_objects( objects.push(obj) // If it's a commit, also collect its tree and blobs if obj.obj_type == @bit.ObjectType::Commit { - let sub = collect_reachable_objects_from_commits(db, fs, [id]) + let sub = collect_reachable_objects_from_commits( + db, fs, [id], shallow~, + ) for s in sub { let sh = @bit.hash_object_content(s.obj_type, s.data).to_hex() if !seen.contains(sh) { diff --git a/modules/bit_protocol/src/upload_pack_http_common.mbt b/modules/bit_protocol/src/upload_pack_http_common.mbt index ced83496..ac4da0c7 100644 --- a/modules/bit_protocol/src/upload_pack_http_common.mbt +++ b/modules/bit_protocol/src/upload_pack_http_common.mbt @@ -253,6 +253,8 @@ priv struct PreparedClone { pack : Bytes pack_id : @bit.ObjectId objects : Array[@bit.PackObject] + shallow : Array[@bit.ObjectId] + unshallow : Array[@bit.ObjectId] } ///| @@ -276,9 +278,13 @@ async fn prepare_clone_with_http( if default_ref is None || wants.length() == 0 { return (refs, None) } - let pack = fetch_pack_with_http( + // Keep the shallow/unshallow lines: without them a --depth clone leaves no + // boundary on disk, so the repository reports itself complete and a later + // `fetch --unshallow` has nothing to deepen from. + let result = fetch_pack_with_http_result( remote, wants, prefer_v2, depth, filter, http_get, http_post, ) + let pack = result.pack ( refs, Some({ @@ -286,6 +292,8 @@ async fn prepare_clone_with_http( pack, pack_id: read_pack_trailer_id_http(pack), objects: @pack.parse_packfile(pack), + shallow: result.shallow, + unshallow: result.unshallow, }), ) } @@ -316,6 +324,9 @@ pub async fn clone_to_fs_with_http( let git_dir = @bit.join_path(root, ".git") fs.mkdir_p(@bit.join_path(git_dir, "objects/pack")) @pack.write_packfile_with_index(fs, git_dir, prepared.pack, prepared.objects) + @repo.apply_shallow_updates( + fs, rfs, git_dir, prepared.shallow, prepared.unshallow, + ) if filter.is_partial() { write_promisor_file(fs, git_dir, remote) write_pack_promisor_marker_http( @@ -384,6 +395,7 @@ pub async fn[ prepared.pack, prepared.objects, ) + @repo.write_shallow_boundaries_async(fs, git_dir, prepared.shallow) if filter.is_partial() { write_promisor_file_async(fs, git_dir, remote) write_pack_promisor_marker_http_async( diff --git a/modules/bit_repo/src/shallow.mbt b/modules/bit_repo/src/shallow.mbt index c5f0c513..a85d0038 100644 --- a/modules/bit_repo/src/shallow.mbt +++ b/modules/bit_repo/src/shallow.mbt @@ -53,3 +53,35 @@ pub fn apply_shallow_updates( let lines = [ for _, id in boundaries => id.to_hex() ] fs.write_string(path, lines.join("\n") + "\n") } + +///| +/// Record the shallow boundary of a freshly cloned repository. +/// +/// A clone starts with no `shallow` file, so there is nothing to merge and no +/// prior boundary an `unshallow` line could lift: writing the reported set is +/// the whole job. `apply_shallow_updates` covers the incremental fetch case, +/// which needs to read what is already there. +pub async fn[FS : @types.AsyncFileSystem] write_shallow_boundaries_async( + fs : FS, + git_dir : String, + shallow : Array[@object.ObjectId], +) -> Unit raise @object.GitError { + if shallow.length() == 0 { + return + } + let seen : Map[String, Bool] = Map([]) + let lines : Array[String] = [] + for id in shallow { + let hex = id.to_hex() + if seen.contains(hex) { + continue + } + seen[hex] = true + lines.push(hex) + } + @types.AsyncFileSystem::write_string( + fs, + join_path(git_dir, "shallow"), + lines.join("\n") + "\n", + ) +} diff --git a/t/t9021-shallow-clone-integrity.sh b/t/t9021-shallow-clone-integrity.sh new file mode 100755 index 00000000..644f3456 --- /dev/null +++ b/t/t9021-shallow-clone-integrity.sh @@ -0,0 +1,193 @@ +#!/bin/sh +# +# Shallow clones must record the boundary the server reported, and every +# history walk must stop there instead of running into objects that were +# never fetched. +# + +test_description='bit shallow clone records its boundary and keeps history intact' + +TEST_DIRECTORY=$(cd "$(dirname "$0")" && pwd) +. "$TEST_DIRECTORY/test-lib.sh" + +if ! test_have_prereq GIT; then + test_skip "shallow clone integrity" "git not found" + test_done +fi + +# `bit` is run with --no-git-fallback throughout: without it clone and gc are +# delegated to the real git binary and none of this exercises bit at all. +bit_cmd() { + "$BIT" --no-git-fallback "$@" +} + +test_expect_success 'setup a six-commit origin' ' + git init -q -b main src && + (cd src && + git config user.email test@example.com && + git config user.name Test && + git config commit.gpgsign false && + for n in 1 2 3 4 5 6; do + echo $n >f$n.txt && + git add . && + git commit -q -m "c$n" || exit 1 + done) && + git clone -q --bare src origin.git && + git -C origin.git symbolic-ref HEAD refs/heads/main && + git -C origin.git config http.receivepack true +' + +# The boundary is the oldest commit fetched, not the ref tip. Recording the tip +# makes the clone look parentless one commit in, so git walks less history than +# the repository actually holds. Comparing the objects on disk against what a +# boundary-honouring walk shows catches that; comparing two walks would not, +# since both would be truncated by the same wrong boundary. +# +# The loop runs in a subshell: `eval` runs a test body in the current shell, so +# a bare `exit` would abandon the whole file. +test_expect_success 'clone --depth keeps every fetched commit walkable' ' + ( for depth in 1 2 3 4; do + rm -rf "d$depth" && + bit_cmd clone -q --depth "$depth" "file://$(pwd)/origin.git" "d$depth" && + present=$(git -C "d$depth" cat-file --batch-all-objects --batch-check | + grep -c commit) && + visible=$(git -C "d$depth" log --oneline | wc -l | tr -d " ") && + test "$present" = "$depth" && + test "$visible" = "$depth" || { + echo "depth $depth: $present commits on disk, $visible walkable" >&2 + exit 1 + } + done ) +' + +test_expect_success 'clone --depth marks the repository shallow' ' + rm -rf d && + bit_cmd clone -q --depth 2 "file://$(pwd)/origin.git" d && + test "$(bit_cmd -C d rev-parse --is-shallow-repository)" = true +' + +# rev-list used to emit the boundary commit'\''s parent, an id with no object +# behind it, so it reported one commit more than the repository holds. +test_expect_success 'rev-list stops at the boundary and matches log' ' + rm -rf d && + bit_cmd clone -q --depth 2 "file://$(pwd)/origin.git" d && + test "$(bit_cmd -C d rev-list --count HEAD)" = 2 && + test "$(bit_cmd -C d rev-list HEAD | wc -l)" = 2 && + test "$(bit_cmd -C d log --oneline | wc -l)" = 2 && + bit_cmd -C d rev-list HEAD | while read -r oid; do + git -C d cat-file -e "$oid" || exit 1 + done +' + +test_expect_success 'gc succeeds in a shallow clone and keeps every commit' ' + rm -rf d && + bit_cmd clone -q --depth 3 "file://$(pwd)/origin.git" d && + bit_cmd -C d gc && + test "$(bit_cmd -C d log --oneline | wc -l)" = 3 +' + +test_expect_success 'prune succeeds in a shallow clone' ' + rm -rf d && + bit_cmd clone -q --depth 3 "file://$(pwd)/origin.git" d && + bit_cmd -C d prune && + test "$(bit_cmd -C d log --oneline | wc -l)" = 3 +' + +# repack -ad used to pack only the tip and then delete the pack holding the +# rest, losing commits with a zero exit status. +test_expect_success 'repack -ad keeps every commit in a shallow clone' ' + ( for depth in 1 2 3 4; do + rm -rf "r$depth" && + bit_cmd clone -q --depth "$depth" "file://$(pwd)/origin.git" "r$depth" && + before=$(git -C "r$depth" cat-file --batch-all-objects --batch-check | + grep -c commit) && + bit_cmd -C "r$depth" repack -ad && + after=$(git -C "r$depth" cat-file --batch-all-objects --batch-check | + grep -c commit) && + test "$before" = "$after" || { + echo "depth $depth: repack lost commits, $before -> $after" >&2 + exit 1 + } + done ) +' + +test_expect_success 'fsck is clean in a shallow clone' ' + rm -rf d && + bit_cmd clone -q --depth 2 "file://$(pwd)/origin.git" d && + bit_cmd -C d fsck +' + +test_expect_success 'committing on top of a shallow tip keeps gc and fsck happy' ' + rm -rf d && + bit_cmd clone -q --depth 2 "file://$(pwd)/origin.git" d && + echo new >d/new.txt && + bit_cmd -C d add . && + bit_cmd -C d commit -q -m "on top of shallow" && + test "$(bit_cmd -C d rev-list --count HEAD)" = 3 && + bit_cmd -C d gc && + bit_cmd -C d fsck +' + +test_expect_success 'fetch --unshallow completes the history' ' + rm -rf d && + bit_cmd clone -q --depth 2 "file://$(pwd)/origin.git" d && + bit_cmd -C d fetch --unshallow && + test "$(bit_cmd -C d rev-list --count HEAD)" = 6 && + test_path_is_missing d/.git/shallow && + bit_cmd -C d fsck +' + +test_expect_success 'fetch --depth deepens to the requested depth' ' + rm -rf d && + bit_cmd clone -q --depth 1 "file://$(pwd)/origin.git" d && + bit_cmd -C d fetch --depth 4 && + test "$(bit_cmd -C d rev-list --count HEAD)" = 4 +' + +# The native HTTP clone used to drop the shallow lines entirely, leaving a +# repository that was shallow in content but reported itself complete, so +# --unshallow afterwards had nothing to deepen from. +if command -v node >/dev/null 2>&1; then + PORT=$((10000 + $$ % 50000)) + SERVER_PID="" + + cleanup_http() { + if test -n "$SERVER_PID"; then + kill "$SERVER_PID" 2>/dev/null || true + SERVER_PID="" + fi + } + trap 'cleanup_http; cleanup' EXIT + + test_expect_success 'start a smart HTTP server' ' + USE_REAL_GIT=1 node "$BIT_BUILD_DIR/tools/http-test-server.js" \ + "$(pwd)/src" "$PORT" >server.log 2>&1 & + SERVER_PID=$! && + ready= && + for _ in 1 2 3 4 5 6 7 8 9 10; do + if git ls-remote "http://localhost:$PORT" >/dev/null 2>&1; then + ready=yes + break + fi + sleep 1 + done && + test -n "$ready" + ' + + # The tear-down must not sit at the end of the && chain: `cmd || true` + # there would swallow every assertion before it and the test could never + # fail. It belongs in the EXIT trap above. + test_expect_success 'http clone --depth records the boundary and can unshallow' ' + rm -rf h && + bit_cmd clone -q --depth 2 "http://localhost:$PORT" h && + test_path_is_file h/.git/shallow && + test "$(bit_cmd -C h rev-parse --is-shallow-repository)" = true && + test "$(bit_cmd -C h rev-list --count HEAD)" = 2 && + bit_cmd -C h fetch --unshallow && + test "$(bit_cmd -C h rev-list --count HEAD)" = 6 + ' +else + test_skip "http shallow clone" "node not found" +fi + +test_done