Merge pull request #939 from dhoizner/fix-pull-after-shallow

pjbgf · web-flow · commit ae552ce0bf32 · 2023-12-01T09:42:24.000Z
git: stop iterating at oldest shallow when pulling. Fixes #305
diff --git a/remote.go b/remote.go
@@ -1070,7 +1070,7 @@ func checkFastForwardUpdate(s storer.EncodedObjectStorer, remoteRefs storer.Refe
 		return fmt.Errorf("non-fast-forward update: %s", cmd.Name.String())
 	}
 
-	ff, err := isFastForward(s, cmd.Old, cmd.New)
+	ff, err := isFastForward(s, cmd.Old, cmd.New, nil)
 	if err != nil {
 		return err
 	}
@@ -1082,14 +1082,28 @@ func checkFastForwardUpdate(s storer.EncodedObjectStorer, remoteRefs storer.Refe
 	return nil
 }
 
-func isFastForward(s storer.EncodedObjectStorer, old, new plumbing.Hash) (bool, error) {
+func isFastForward(s storer.EncodedObjectStorer, old, new plumbing.Hash, earliestShallow *plumbing.Hash) (bool, error) {
 	c, err := object.GetCommit(s, new)
 	if err != nil {
 		return false, err
 	}
 
+	parentsToIgnore := []plumbing.Hash{}
+	if earliestShallow != nil {
+		earliestCommit, err := object.GetCommit(s, *earliestShallow)
+		if err != nil {
+			return false, err
+		}
+
+		parentsToIgnore = earliestCommit.ParentHashes
+	}
+
 	found := false
-	iter := object.NewCommitPreorderIter(c, nil, nil)
+	// stop iterating at the earlist shallow commit, ignoring its parents
+	// note: when pull depth is smaller than the number of new changes on the remote, this fails due to missing parents.
+	//       as far as i can tell, without the commits in-between the shallow pull and the earliest shallow, there's no
+	//       real way of telling whether it will be a fast-forward merge.
+	iter := object.NewCommitPreorderIter(c, nil, parentsToIgnore)
 	err = iter.ForEach(func(c *object.Commit) error {
 		if c.Hash != old {
 			return nil
@@ -1205,7 +1219,7 @@ func (r *Remote) updateLocalReferenceStorage(
 			// If the ref exists locally as a non-tag and force is not
 			// specified, only update if the new ref is an ancestor of the old
 			if old != nil && !old.Name().IsTag() && !force && !spec.IsForceUpdate() {
-				ff, err := isFastForward(r.s, old.Hash(), new.Hash())
+				ff, err := isFastForward(r.s, old.Hash(), new.Hash(), nil)
 				if err != nil {
 					return updated, err
 				}
@@ -1390,7 +1404,6 @@ func pushHashes(
 	useRefDeltas bool,
 	allDelete bool,
 ) (*packp.ReportStatus, error) {
-
 	rd, wr := io.Pipe()
 
 	config, err := s.Config()
diff --git a/worktree.go b/worktree.go
@@ -95,7 +95,15 @@ func (w *Worktree) PullContext(ctx context.Context, o *PullOptions) error {
 
 	head, err := w.r.Head()
 	if err == nil {
-		headAheadOfRef, err := isFastForward(w.r.Storer, ref.Hash(), head.Hash())
+		// if we don't have a shallows list, just ignore it
+		shallowList, _ := w.r.Storer.Shallow()
+
+		var earliestShallow *plumbing.Hash
+		if len(shallowList) > 0 {
+			earliestShallow = &shallowList[0]
+		}
+
+		headAheadOfRef, err := isFastForward(w.r.Storer, ref.Hash(), head.Hash(), earliestShallow)
 		if err != nil {
 			return err
 		}
@@ -104,7 +112,7 @@ func (w *Worktree) PullContext(ctx context.Context, o *PullOptions) error {
 			return NoErrAlreadyUpToDate
 		}
 
-		ff, err := isFastForward(w.r.Storer, head.Hash(), ref.Hash())
+		ff, err := isFastForward(w.r.Storer, head.Hash(), ref.Hash(), earliestShallow)
 		if err != nil {
 			return err
 		}
@@ -188,6 +196,7 @@ func (w *Worktree) Checkout(opts *CheckoutOptions) error {
 
 	return w.Reset(ro)
 }
+
 func (w *Worktree) createBranch(opts *CheckoutOptions) error {
 	if err := opts.Branch.Validate(); err != nil {
 		return err
diff --git a/worktree_test.go b/worktree_test.go
@@ -313,6 +313,42 @@ func (s *WorktreeSuite) TestPullDepth(c *C) {
 	c.Assert(err, Equals, nil)
 }
 
+func (s *WorktreeSuite) TestPullAfterShallowClone(c *C) {
+	tempDir, clean := s.TemporalDir()
+	defer clean()
+	remoteURL := filepath.Join(tempDir, "remote")
+	repoDir := filepath.Join(tempDir, "repo")
+
+	remote, err := PlainInit(remoteURL, false)
+	c.Assert(err, IsNil)
+	c.Assert(remote, NotNil)
+
+	_ = CommitNewFile(c, remote, "File1")
+	_ = CommitNewFile(c, remote, "File2")
+
+	repo, err := PlainClone(repoDir, false, &CloneOptions{
+		URL:           remoteURL,
+		Depth:         1,
+		Tags:          NoTags,
+		SingleBranch:  true,
+		ReferenceName: "master",
+	})
+	c.Assert(err, IsNil)
+
+	_ = CommitNewFile(c, remote, "File3")
+	_ = CommitNewFile(c, remote, "File4")
+
+	w, err := repo.Worktree()
+	c.Assert(err, IsNil)
+
+	err = w.Pull(&PullOptions{
+		RemoteName:    DefaultRemoteName,
+		SingleBranch:  true,
+		ReferenceName: plumbing.NewBranchReferenceName("master"),
+	})
+	c.Assert(err, IsNil)
+}
+
 func (s *WorktreeSuite) TestCheckout(c *C) {
 	fs := memfs.New()
 	w := &Worktree{
