From 9dc151a791d5076853b61141eedf342362adff16 Mon Sep 17 00:00:00 2001 From: onee-only Date: Mon, 7 Apr 2025 17:26:48 +0900 Subject: [PATCH 1/2] utils: merkletrie, Fix diff on sparse-checkout index. Fixes #1406 --- utils/merkletrie/change.go | 4 +++- utils/merkletrie/difftree.go | 20 ++++++++++++++++---- worktree_test.go | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/utils/merkletrie/change.go b/utils/merkletrie/change.go index c3fd49e3b..d32b5a7d4 100644 --- a/utils/merkletrie/change.go +++ b/utils/merkletrie/change.go @@ -131,7 +131,9 @@ func (l *Changes) addRecursive(root noder.Path, ctor noderToChangeFn) error { } if !root.IsDir() { - l.Add(ctor(root)) + if !root.Skip() { + l.Add(ctor(root)) + } return nil } diff --git a/utils/merkletrie/difftree.go b/utils/merkletrie/difftree.go index 4ef2d9907..c5bd02423 100644 --- a/utils/merkletrie/difftree.go +++ b/utils/merkletrie/difftree.go @@ -321,8 +321,14 @@ func DiffTreeContext(ctx context.Context, fromTree, toTree noder.Noder, if err = ret.AddRecursiveDelete(from); err != nil { return nil, err } - if err := ii.nextBoth(); err != nil { - return nil, err + if from.Name() == to.Name() { + if err := ii.nextBoth(); err != nil { + return nil, err + } + } else { + if err := ii.nextFrom(); err != nil { + return nil, err + } } break } @@ -330,8 +336,14 @@ func DiffTreeContext(ctx context.Context, fromTree, toTree noder.Noder, if err = ret.AddRecursiveDelete(to); err != nil { return nil, err } - if err := ii.nextBoth(); err != nil { - return nil, err + if from.Name() == to.Name() { + if err := ii.nextBoth(); err != nil { + return nil, err + } + } else { + if err := ii.nextTo(); err != nil { + return nil, err + } } break } diff --git a/worktree_test.go b/worktree_test.go index b976cb92b..c550205c1 100644 --- a/worktree_test.go +++ b/worktree_test.go @@ -1373,6 +1373,24 @@ func (s *WorktreeSuite) TestStatusAfterCheckout() { s.True(status.IsClean()) } +func (s *WorktreeSuite) TestStatusAfterSparseCheckout() { + fs := memfs.New() + w := &Worktree{ + r: s.Repository, + Filesystem: fs, + } + + err := w.Checkout(&CheckoutOptions{ + SparseCheckoutDirectories: []string{"php"}, + Force: true, + }) + s.Require().NoError(err) + + status, err := w.Status() + s.Require().NoError(err) + s.True(status.IsClean(), status) +} + func (s *WorktreeSuite) TestStatusModified() { fs := s.TemporalFilesystem() From e8c9fcd5571191ca3227c5e67bf9d2ebc2533135 Mon Sep 17 00:00:00 2001 From: onee-only Date: Mon, 14 Apr 2025 19:15:45 +0900 Subject: [PATCH 2/2] utils: merkletrie, Simplify DiffTreeContext. When node.Skip() == true, it means the node and all its descendants should be skipped. Previously, we were still calling AddRecursiveDelete() on such nodes, which is redundant, since addRecursive() skips the entire subtree. This change removes those unnecessary AddRecursiveDelete() calls, and also adds a missing check to skip the 'from' node in the 'onlyFromRemains' case. Additionally, prettified the if-else structure in the switch case for better readability. --- utils/merkletrie/difftree.go | 47 ++++++++++++------------------------ 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/utils/merkletrie/difftree.go b/utils/merkletrie/difftree.go index c5bd02423..7fc8d02d7 100644 --- a/utils/merkletrie/difftree.go +++ b/utils/merkletrie/difftree.go @@ -297,18 +297,16 @@ func DiffTreeContext(ctx context.Context, fromTree, toTree noder.Noder, case noMoreNoders: return ret, nil case onlyFromRemains: - if err = ret.AddRecursiveDelete(from); err != nil { - return nil, err + if !from.Skip() { + if err = ret.AddRecursiveDelete(from); err != nil { + return nil, err + } } if err = ii.nextFrom(); err != nil { return nil, err } case onlyToRemains: - if to.Skip() { - if err = ret.AddRecursiveDelete(to); err != nil { - return nil, err - } - } else { + if !to.Skip() { if err = ret.AddRecursiveInsert(to); err != nil { return nil, err } @@ -317,38 +315,25 @@ func DiffTreeContext(ctx context.Context, fromTree, toTree noder.Noder, return nil, err } case bothHaveNodes: - if from.Skip() { - if err = ret.AddRecursiveDelete(from); err != nil { - return nil, err - } + var err error + switch { + case from.Skip(): if from.Name() == to.Name() { - if err := ii.nextBoth(); err != nil { - return nil, err - } + err = ii.nextBoth() } else { - if err := ii.nextFrom(); err != nil { - return nil, err - } - } - break - } - if to.Skip() { - if err = ret.AddRecursiveDelete(to); err != nil { - return nil, err + err = ii.nextFrom() } + case to.Skip(): if from.Name() == to.Name() { - if err := ii.nextBoth(); err != nil { - return nil, err - } + err = ii.nextBoth() } else { - if err := ii.nextTo(); err != nil { - return nil, err - } + err = ii.nextTo() } - break + default: + err = diffNodes(&ret, ii) } - if err = diffNodes(&ret, ii); err != nil { + if err != nil { return nil, err } default: