pico

fix(rsync): delete largest path first rss
created pr with 98.1 on 2025-12-19T01:28:25Z · by c8ef7d19
added 98.2 on 2025-12-19T01:39:08Z · by c8ef7d19
1: f8ad8c5 = 1: f8ad8c5 fix(rsync): delete largest path first
-: ------- > 2: 0d69224 chore: add tests
cmds
checkout latest patchset: 
ssh pr.pico.sh print 98 | git am -3
checkout any patchset in a patch request: 
ssh pr.pico.sh print 98.[rev] | git am -3
add changes to patch request: 
git format-patch main --stdout | ssh pr.pico.sh pr add 98

Patchset 98.2 on 2025-12-19T01:39:08Z · commit 0d69224

fix(rsync): delete largest path first
Eric Bower 2025-12-19T01:23:21Z
 → chore: add tests
Eric Bower 2025-12-19T01:36:11Z
+1 -1 pkg/send/protocols/rsync/rsync.go #
......@@ -148,7 +148,7 @@ func (h *handler) Remove(willReceive []*rsyncutils.ReceiverFile) error {
148148 return rf.Name == entry.Name()
149149 })
150150
151- if !exists && entry.Name() != "._pico_keep_dir" {
151+ if !exists && path.Base(entry.Name()) != "._pico_keep_dir" {
152152 toDelete = append(toDelete, entry.Name())
153153 }
154154 }
+237 -0 pkg/send/protocols/rsync/rsync_test.go #
......@@ -0,0 +1,237 @@
1+package rsync
2+
3+import (
4+ "bytes"
5+ "context"
6+ "io"
7+ "io/fs"
8+ "log/slog"
9+ "os"
10+ "testing"
11+ "time"
12+
13+ rsyncutils "github.com/picosh/go-rsync-receiver/utils"
14+ "github.com/picosh/pico/pkg/pssh"
15+ "github.com/picosh/pico/pkg/send/utils"
16+ "golang.org/x/crypto/ssh"
17+)
18+
19+// mockFileInfo implements fs.FileInfo for testing.
20+type mockFileInfo struct {
21+ name string
22+ size int64
23+ mode fs.FileMode
24+ modTime time.Time
25+ isDir bool
26+}
27+
28+func (m *mockFileInfo) Name() string { return m.name }
29+func (m *mockFileInfo) Size() int64 { return m.size }
30+func (m *mockFileInfo) Mode() fs.FileMode { return m.mode }
31+func (m *mockFileInfo) ModTime() time.Time { return m.modTime }
32+func (m *mockFileInfo) IsDir() bool { return m.isDir }
33+func (m *mockFileInfo) Sys() any { return nil }
34+
35+// mockWriteHandler implements utils.CopyFromClientHandler for testing.
36+// It records the order of Delete calls to verify deletion order.
37+type mockWriteHandler struct {
38+ entries []os.FileInfo
39+ deleteCalls []string
40+ deleteErrors map[string]error
41+}
42+
43+func (m *mockWriteHandler) Delete(_ *pssh.SSHServerConnSession, entry *utils.FileEntry) error {
44+ m.deleteCalls = append(m.deleteCalls, entry.Filepath)
45+ if m.deleteErrors != nil {
46+ if err, ok := m.deleteErrors[entry.Filepath]; ok {
47+ return err
48+ }
49+ }
50+ return nil
51+}
52+
53+func (m *mockWriteHandler) Write(_ *pssh.SSHServerConnSession, _ *utils.FileEntry) (string, error) {
54+ return "", nil
55+}
56+
57+func (m *mockWriteHandler) Read(_ *pssh.SSHServerConnSession, _ *utils.FileEntry) (os.FileInfo, utils.ReadAndReaderAtCloser, error) {
58+ return nil, nil, nil
59+}
60+
61+func (m *mockWriteHandler) List(_ *pssh.SSHServerConnSession, _ string, _ bool, _ bool) ([]os.FileInfo, error) {
62+ return m.entries, nil
63+}
64+
65+func (m *mockWriteHandler) GetLogger(_ *pssh.SSHServerConnSession) *slog.Logger {
66+ return slog.Default()
67+}
68+
69+func (m *mockWriteHandler) Validate(_ *pssh.SSHServerConnSession) error {
70+ return nil
71+}
72+
73+// mockChannel implements ssh.Channel for testing.
74+type mockChannel struct {
75+ stderr *bytes.Buffer
76+}
77+
78+func (m *mockChannel) Read(_ []byte) (int, error) { return 0, io.EOF }
79+func (m *mockChannel) Write(data []byte) (int, error) { return len(data), nil }
80+func (m *mockChannel) Close() error { return nil }
81+func (m *mockChannel) CloseWrite() error { return nil }
82+func (m *mockChannel) SendRequest(_ string, _ bool, _ []byte) (bool, error) {
83+ return true, nil
84+}
85+func (m *mockChannel) Stderr() io.ReadWriter { return m.stderr }
86+
87+var _ ssh.Channel = (*mockChannel)(nil)
88+
89+// newMockSession creates a mock SSHServerConnSession for testing.
90+func newMockSession() (*pssh.SSHServerConnSession, *bytes.Buffer) {
91+ stderr := &bytes.Buffer{}
92+ channel := &mockChannel{stderr: stderr}
93+
94+ ctx, cancel := context.WithCancel(context.Background())
95+ logger := slog.Default()
96+ server := pssh.NewSSHServer(ctx, logger, &pssh.SSHServerConfig{})
97+ serverConn := pssh.NewSSHServerConn(ctx, logger, &ssh.ServerConn{
98+ Permissions: &ssh.Permissions{
99+ Extensions: map[string]string{},
100+ },
101+ }, server)
102+
103+ session := &pssh.SSHServerConnSession{
104+ Channel: channel,
105+ SSHServerConn: serverConn,
106+ Ctx: ctx,
107+ CancelFunc: cancel,
108+ }
109+
110+ return session, stderr
111+}
112+
113+func TestRemove_DeletesChildrenBeforeParents(t *testing.T) {
114+ session, _ := newMockSession()
115+ mockHandler := &mockWriteHandler{
116+ entries: []os.FileInfo{
117+ &mockFileInfo{name: "a", isDir: true},
118+ &mockFileInfo{name: "a/file.txt", size: 100},
119+ &mockFileInfo{name: "b/c", isDir: true},
120+ &mockFileInfo{name: "b/c/deep.txt", size: 50},
121+ &mockFileInfo{name: "b", isDir: true},
122+ },
123+ }
124+
125+ h := &handler{
126+ session: session,
127+ writeHandler: mockHandler,
128+ root: "test",
129+ }
130+
131+ err := h.Remove([]*rsyncutils.ReceiverFile{})
132+ if err != nil {
133+ t.Fatalf("Remove() returned error: %v", err)
134+ }
135+
136+ if len(mockHandler.deleteCalls) != 5 {
137+ t.Fatalf("expected 5 delete calls, got %d: %v", len(mockHandler.deleteCalls), mockHandler.deleteCalls)
138+ }
139+
140+ indexOfA := -1
141+ indexOfAFile := -1
142+ indexOfB := -1
143+ indexOfBC := -1
144+ indexOfBCDeep := -1
145+
146+ for i, path := range mockHandler.deleteCalls {
147+ switch path {
148+ case "/test/a":
149+ indexOfA = i
150+ case "/test/a/file.txt":
151+ indexOfAFile = i
152+ case "/test/b":
153+ indexOfB = i
154+ case "/test/b/c":
155+ indexOfBC = i
156+ case "/test/b/c/deep.txt":
157+ indexOfBCDeep = i
158+ }
159+ }
160+
161+ if indexOfAFile > indexOfA {
162+ t.Errorf("a/file.txt (index %d) should be deleted before a (index %d)", indexOfAFile, indexOfA)
163+ }
164+ if indexOfBCDeep > indexOfBC {
165+ t.Errorf("b/c/deep.txt (index %d) should be deleted before b/c (index %d)", indexOfBCDeep, indexOfBC)
166+ }
167+ if indexOfBC > indexOfB {
168+ t.Errorf("b/c (index %d) should be deleted before b (index %d)", indexOfBC, indexOfB)
169+ }
170+}
171+
172+func TestRemove_IgnoresPicoKeepDir(t *testing.T) {
173+ session, _ := newMockSession()
174+ mockHandler := &mockWriteHandler{
175+ entries: []os.FileInfo{
176+ &mockFileInfo{name: "dir", isDir: true},
177+ &mockFileInfo{name: "dir/._pico_keep_dir", size: 0},
178+ &mockFileInfo{name: "dir/file.txt", size: 100},
179+ },
180+ }
181+
182+ h := &handler{
183+ session: session,
184+ writeHandler: mockHandler,
185+ root: "test",
186+ }
187+
188+ err := h.Remove([]*rsyncutils.ReceiverFile{})
189+ if err != nil {
190+ t.Fatalf("Remove() returned error: %v", err)
191+ }
192+
193+ for _, path := range mockHandler.deleteCalls {
194+ if path == "/test/dir/._pico_keep_dir" {
195+ t.Error("._pico_keep_dir should not be in delete list")
196+ }
197+ }
198+
199+ if len(mockHandler.deleteCalls) != 2 {
200+ t.Errorf("expected 2 delete calls (dir, dir/file.txt), got %d: %v", len(mockHandler.deleteCalls), mockHandler.deleteCalls)
201+ }
202+}
203+
204+func TestRemove_OnlyDeletesFilesNotInWillReceive(t *testing.T) {
205+ session, _ := newMockSession()
206+ mockHandler := &mockWriteHandler{
207+ entries: []os.FileInfo{
208+ &mockFileInfo{name: "a.txt", size: 100},
209+ &mockFileInfo{name: "b.txt", size: 100},
210+ &mockFileInfo{name: "c.txt", size: 100},
211+ },
212+ }
213+
214+ h := &handler{
215+ session: session,
216+ writeHandler: mockHandler,
217+ root: "test",
218+ }
219+
220+ willReceive := []*rsyncutils.ReceiverFile{
221+ {Name: "a.txt"},
222+ {Name: "c.txt"},
223+ }
224+
225+ err := h.Remove(willReceive)
226+ if err != nil {
227+ t.Fatalf("Remove() returned error: %v", err)
228+ }
229+
230+ if len(mockHandler.deleteCalls) != 1 {
231+ t.Fatalf("expected 1 delete call, got %d: %v", len(mockHandler.deleteCalls), mockHandler.deleteCalls)
232+ }
233+
234+ if mockHandler.deleteCalls[0] != "/test/b.txt" {
235+ t.Errorf("expected to delete /test/b.txt, got %s", mockHandler.deleteCalls[0])
236+ }
237+}
Back to top