fix(push): stop the pack walk at remote haves and shallow boundaries

Taskfile.pkl

@@ -313,7 +313,7 @@ local testUpdate: Task = new {
 local testJsBuild: Task = new {
   name = "test-js-build"
   description = "Node-side smoke tests for the published JS lib + npm wrapper"
-  cmd = "node --test tools/js-build.test.mjs tools/npm-lib.test.mjs tools/npm-cli.test.mjs tools/demo-relay.test.mjs tools/demo-editor-link.test.mjs tools/playground-commands.test.mjs tools/playground-view.test.mjs tools/flaker-affected-rules.test.mjs tools/select-affected-tests.test.mjs"
+  cmd = "node --test tools/js-build.test.mjs tools/npm-lib.test.mjs tools/shallow-push.test.mjs tools/npm-cli.test.mjs tools/demo-relay.test.mjs tools/demo-editor-link.test.mjs tools/playground-commands.test.mjs tools/playground-view.test.mjs tools/flaker-affected-rules.test.mjs tools/select-affected-tests.test.mjs"
   deps {
     syncNpmLibRaw
     syncNpmBitMjs

Test.pkl

@@ -66,6 +66,6 @@ tests {
   }
   new {
     name = "js-build"
-    cmd = "node --test tools/js-build.test.mjs tools/npm-lib.test.mjs tools/npm-cli.test.mjs tools/demo-relay.test.mjs tools/demo-editor-link.test.mjs tools/playground-commands.test.mjs tools/playground-view.test.mjs"
+    cmd = "node --test tools/js-build.test.mjs tools/npm-lib.test.mjs tools/shallow-push.test.mjs tools/npm-cli.test.mjs tools/demo-relay.test.mjs tools/demo-editor-link.test.mjs tools/playground-commands.test.mjs tools/playground-view.test.mjs"
   }
 }

modules/bit_lib/src/js_api_exports.mbt

@@ -2579,7 +2579,7 @@ pub async fn js_clone_remote(
       }
       Some((refname, commit_id)) => {
         if wants.length() > 0 {
-          let pack = @bitnative.fetch_pack_with_http(
+          let result = @bitnative.fetch_pack_with_http_result(
             remote_url,
             wants,
             prefer_v2,
@@ -2599,6 +2599,7 @@ pub async fn js_clone_remote(
               js_transport_post(transport_id, url, body, headers)
             },
           )
+          let pack = result.pack
           let objects = @pack.parse_packfile(pack)
           let git_dir = join_path(root, ".git")
           @bit.FileSystem::mkdir_p(fs, join_path(git_dir, "objects/pack"))
@@ -2607,6 +2608,16 @@ pub async fn js_clone_remote(
           @bit.materialize_clone_to_fs(
             store, commit_id, refname, remote_url, fs, root, fs,
           )
+          // Record the history boundary so later walks (push in particular)
+          // know the parents of these commits were never fetched. The server
+          // reports it for a `depth` request and for an already-shallow repo.
+          @repo.apply_shallow_updates(
+            fs,
+            fs,
+            js_resolve_git_dir(fs, root),
+            result.shallow,
+            result.unshallow,
+          )
         } else {
           init_repo(
             fs,
@@ -2680,7 +2691,7 @@ pub async fn js_fetch_remote(
     match js_select_fetch_target(refs, symrefs, refspec) {
       None => { status: "empty", commit_id: "", refname: "", remote_ref: "" }
       Some((refname, commit_id)) => {
-        let pack = @bitnative.fetch_pack_with_http(
+        let result = @bitnative.fetch_pack_with_http_result(
           remote_url,
           [commit_id],
           prefer_v2,
@@ -2699,10 +2710,19 @@ pub async fn js_fetch_remote(
           ) -> (@bit.HttpResponse, Bytes) raise @bit.GitError {
             js_transport_post(transport_id, url, body, headers)
           },
+          shallow=@repo.read_shallow_boundaries(fs, git_dir),
         )
+        let pack = result.pack
         let objects = @pack.parse_packfile(pack)
         @bit.FileSystem::mkdir_p(fs, join_path(git_dir, "objects/pack"))
         @pack.write_packfile_with_index(fs, git_dir, pack, objects)
+        @repo.apply_shallow_updates(
+          fs,
+          fs,
+          git_dir,
+          result.shallow,
+          result.unshallow,
+        )
         let remote_ref = js_remote_tracking_refname(refname)
         let remote_ref_path = join_path(git_dir, remote_ref)
         @bit.FileSystem::mkdir_p(fs, rebase_parent_dir(remote_ref_path))
@@ -2769,9 +2789,11 @@ pub async fn js_push_remote(
       js_normalize_push_refname(refname)
     }
     let db = ObjectDb::load(fs, git_dir)
-    let objects = collect_reachable_objects(db, fs, head_id)
-    let pack = @pack.create_packfile(objects)
     let remote = @protocol.Remote::new(remote_url)
+    // Discover before collecting: the refs the receiver advertises bound the
+    // walk. Packing HEAD's full history first both sends objects the remote
+    // already has and fails outright when this clone (or the remote itself)
+    // is shallow, because the advertised tip's parents are not local.
     let (refs, _caps) = @protocol.discover_refs_with_http(remote, async fn(
       url : String,
       headers : Map[String, String],
@@ -2786,6 +2808,14 @@ pub async fn js_push_remote(
         )
       }
     }
+    let objects = collect_reachable_objects_excluding_commits(
+      db,
+      fs,
+      [head_id],
+      advertised_have_ids(refs),
+      shallow=@repo.read_shallow_boundaries(fs, git_dir),
+    )
+    let pack = @pack.create_packfile(objects)
     let req = @protocol.PushRequest::new(old_id, head_id, target_ref, pack)
     @protocol.push_with_http(
       remote,

modules/bit_lib/src/native/sync_http.mbt

@@ -160,6 +160,7 @@ pub async fn push_http(
         rfs,
         [head_id],
         sync_remote_have_ids(refs),
+        shallow=@repo.read_shallow_boundaries(rfs, actual_git_dir),
       )
       let pack = @pack.create_packfile(objects)
       sync_raise_lfs_upload_errors(
@@ -204,6 +205,7 @@ pub async fn push_http_from_commit(
     rfs,
     [commit_id],
     sync_remote_have_ids(refs),
+    shallow=@repo.read_shallow_boundaries(rfs, actual_git_dir),
   )
   let pack = @pack.create_packfile(objects)
   sync_raise_lfs_upload_errors(
@@ -274,18 +276,7 @@ fn sync_find_ref(
 fn sync_remote_have_ids(
   refs : Array[(@bit.ObjectId, String)],
 ) -> Array[@bit.ObjectId] {
-  let zero = @bit.ObjectId::zero()
-  let seen : Map[String, Bool] = Map([])
-  let haves : Array[@bit.ObjectId] = []
-  for item in refs {
-    let (id, _) = item
-    let hex = id.to_hex()
-    if id != zero && !seen.contains(hex) {
-      seen[hex] = true
-      haves.push(id)
-    }
-  }
-  haves
+  @bitlib.advertised_have_ids(refs)
 }
 
 ///|

modules/bit_lib/src/pack_collect.mbt

@@ -54,6 +54,7 @@ fn collect_object_ids_only(
   fs : &@bit.RepoFileSystem,
   commit_id : @bit.ObjectId,
   seen : Map[String, Bool],
+  shallow? : Map[String, Bool] = Map([]),
 ) -> Unit raise @bit.GitError {
   let hex = commit_id.to_hex()
   if seen.contains(hex) {
@@ -67,8 +68,12 @@ fn collect_object_ids_only(
       if o.obj_type == @bit.ObjectType::Commit {
         let info = @bit.parse_commit(o.data)
         collect_tree_ids_only(db, fs, info.tree, seen)
+        if shallow.contains(hex) {
+          // Shallow boundary: the parents are absent by construction.
+          return
+        }
         for p in info.parents {
-          collect_object_ids_only(db, fs, p, seen)
+          collect_object_ids_only(db, fs, p, seen, shallow~)
         }
       }
   }
@@ -202,10 +207,12 @@ pub fn collect_reachable_objects(
   db : ObjectDb,
   fs : &@bit.RepoFileSystem,
   commit_id : @bit.ObjectId,
+  shallow? : Array[@bit.ObjectId] = [],
 ) -> Array[@bit.PackObject] raise @bit.GitError {
   let seen : Map[String, Bool] = Map([])
   let out : Array[@bit.PackObject] = []
-  collect_commit_objects(db, fs, commit_id, seen, out)
+  let boundaries = shallow_boundary_set(shallow)
+  collect_commit_objects(db, fs, commit_id, seen, out, shallow=boundaries)
   out
 }
 
@@ -215,11 +222,13 @@ pub fn collect_reachable_objects_from_commits(
   db : ObjectDb,
   fs : &@bit.RepoFileSystem,
   commits : Array[@bit.ObjectId],
+  shallow? : Array[@bit.ObjectId] = [],
 ) -> Array[@bit.PackObject] raise @bit.GitError {
   let seen : Map[String, Bool] = Map([])
   let out : Array[@bit.PackObject] = []
+  let boundaries = shallow_boundary_set(shallow)
   for commit_id in commits {
-    collect_commit_objects(db, fs, commit_id, seen, out)
+    collect_commit_objects(db, fs, commit_id, seen, out, shallow=boundaries)
   }
   out
 }
@@ -234,17 +243,52 @@ pub fn collect_reachable_objects_excluding_commits(
   fs : &@bit.RepoFileSystem,
   commits : Array[@bit.ObjectId],
   excluded_commits : Array[@bit.ObjectId],
+  shallow? : Array[@bit.ObjectId] = [],
 ) -> Array[@bit.PackObject] raise @bit.GitError {
   // Mark the complete object closure already owned by the receiver. Merely
   // stopping the commit walk at a have commit is insufficient: the wanted
   // commit's new tree can still reference unchanged blobs from that commit.
   let seen : Map[String, Bool] = Map([])
+  let boundaries = shallow_boundary_set(shallow)
   for id in excluded_commits {
-    collect_object_ids_only(db, fs, id, seen)
+    collect_object_ids_only(db, fs, id, seen, shallow=boundaries)
   }
   let out : Array[@bit.PackObject] = []
   for commit_id in commits {
-    collect_commit_objects(db, fs, commit_id, seen, out)
+    collect_commit_objects(db, fs, commit_id, seen, out, shallow=boundaries)
+  }
+  out
+}
+
+///|
+/// The distinct non-zero commit ids a peer advertises, usable as the exclusion
+/// set of a push walk: everything reachable from them is already on the other
+/// side, so it must not be packed again.
+pub fn advertised_have_ids(
+  refs : Array[(@bit.ObjectId, String)],
+) -> Array[@bit.ObjectId] {
+  let zero = @bit.ObjectId::zero()
+  let seen : Map[String, Bool] = Map([])
+  let haves : Array[@bit.ObjectId] = []
+  for item in refs {
+    let (id, _) = item
+    let hex = id.to_hex()
+    if id != zero && !seen.contains(hex) {
+      seen[hex] = true
+      haves.push(id)
+    }
+  }
+  haves
+}
+
+///|
+/// Index shallow boundary commits by hex id for O(1) lookups during a walk.
+fn shallow_boundary_set(
+  shallow : Array[@bit.ObjectId],
+) -> Map[String, Bool] {
+  let out : Map[String, Bool] = Map([])
+  for id in shallow {
+    out[id.to_hex()] = true
   }
   out
 }
@@ -256,6 +300,7 @@ fn collect_commit_objects(
   commit_id : @bit.ObjectId,
   seen : Map[String, Bool],
   out : Array[@bit.PackObject],
+  shallow? : Map[String, Bool] = Map([]),
 ) -> Unit raise @bit.GitError {
   let hex = commit_id.to_hex()
   if seen.contains(hex) {
@@ -272,8 +317,13 @@ fn collect_commit_objects(
       out.push(o)
       let info = @bit.parse_commit(o.data)
       collect_tree_objects(db, fs, info.tree, seen, out)
+      if shallow.contains(hex) {
+        // Shallow boundary: this commit's parents were never fetched, so the
+        // history stops here just as it does for git's grafted walk.
+        return
+      }
       for p in info.parents {
-        collect_commit_objects(db, fs, p, seen, out)
+        collect_commit_objects(db, fs, p, seen, out, shallow~)
       }
     }
   }

modules/bit_lib/src/pack_collect_test.mbt

@@ -397,3 +397,105 @@ test "collect_reachable_objects_excluding_commits: stops at have commits" {
   assert_eq(nested_shared_blob_count, 0)
   assert_eq(new_blob_count, 1)
 }
+
+///|
+/// Drop a loose object so the walk sees the truncated history a shallow clone
+/// actually has on disk.
+fn drop_loose_object(
+  fs : @bit.TestFs,
+  git_dir : String,
+  id : @bit.ObjectId,
+) -> Unit raise @bit.GitError {
+  let hex = id.to_hex()
+  let prefix = String::unsafe_substring(hex, start=0, end=2)
+  let suffix = String::unsafe_substring(hex, start=2, end=hex.length())
+  fs.remove_file(git_dir + "/objects/" + prefix + "/" + suffix)
+}
+
+///|
+test "collect_reachable_objects_excluding_commits: shallow tip whose parent is absent" {
+  let fs = @bit.TestFs::new()
+  init_repo(fs, "/repo") catch {
+    _ => ()
+  }
+  fs.write_file("/repo/base.txt", b"base")
+  add_paths(fs, fs, "/repo", ["."])
+  let base = commit(fs, fs, "/repo", "base", "test <test@test.com>", 1000L)
+  fs.write_file("/repo/boundary.txt", b"boundary")
+  add_paths(fs, fs, "/repo", ["."])
+  let boundary = commit(
+    fs, fs, "/repo", "boundary", "test <test@test.com>", 2000L,
+  )
+  fs.write_file("/repo/new.txt", b"new")
+  add_paths(fs, fs, "/repo", ["."])
+  let tip = commit(fs, fs, "/repo", "tip", "test <test@test.com>", 3000L)
+  // A depth-1 clone (or a clone of an already shallow remote) holds `boundary`
+  // but never received its parent.
+  drop_loose_object(fs, "/repo/.git", base)
+  let db = ObjectDb::load(fs, "/repo/.git")
+
+  // The receiver advertises `boundary`, so the walk must stop there instead of
+  // trying to load the absent parent.
+  let objects = collect_reachable_objects_excluding_commits(db, fs, [tip], [
+    boundary,
+  ])
+  let ids : Map[String, Bool] = Map([])
+  for obj in objects {
+    ids[obj.id.to_hex()] = true
+  }
+  assert_true(ids.contains(tip.to_hex()))
+  assert_true(!ids.contains(boundary.to_hex()))
+}
+
+///|
+test "collect_reachable_objects: stops at a shallow boundary commit" {
+  let fs = @bit.TestFs::new()
+  init_repo(fs, "/repo") catch {
+    _ => ()
+  }
+  fs.write_file("/repo/base.txt", b"base")
+  add_paths(fs, fs, "/repo", ["."])
+  let base = commit(fs, fs, "/repo", "base", "test <test@test.com>", 1000L)
+  fs.write_file("/repo/boundary.txt", b"boundary")
+  add_paths(fs, fs, "/repo", ["."])
+  let boundary = commit(
+    fs, fs, "/repo", "boundary", "test <test@test.com>", 2000L,
+  )
+  drop_loose_object(fs, "/repo/.git", base)
+  let db = ObjectDb::load(fs, "/repo/.git")
+
+  // Without the boundary the walk reaches the absent parent and fails.
+  let mut failed = false
+  (collect_reachable_objects(db, fs, boundary) |> ignore) catch {
+    _ => failed = true
+  }
+  assert_true(failed)
+
+  // Declaring the boundary grafts the history closed, as git's shallow file does.
+  let objects = collect_reachable_objects(db, fs, boundary, shallow=[boundary])
+  let ids : Map[String, Bool] = Map([])
+  for obj in objects {
+    ids[obj.id.to_hex()] = true
+  }
+  assert_true(ids.contains(boundary.to_hex()))
+  assert_true(!ids.contains(base.to_hex()))
+}
+
+///|
+test "advertised_have_ids: drops zero and duplicate ids" {
+  let first = @bit.ObjectId::from_hex(
+    "1111111111111111111111111111111111111111",
+  )
+  let second = @bit.ObjectId::from_hex(
+    "2222222222222222222222222222222222222222",
+  )
+  let haves = advertised_have_ids([
+    (first, "HEAD"),
+    (first, "refs/heads/main"),
+    (@bit.ObjectId::zero(), "refs/heads/empty"),
+    (second, "refs/heads/topic"),
+  ])
+  assert_eq(haves.length(), 2)
+  assert_eq(haves[0].to_hex(), first.to_hex())
+  assert_eq(haves[1].to_hex(), second.to_hex())
+}