diff --git a/internal/action/delete.go b/internal/action/delete.go index 6ce0143d62..7f2abd9be1 100644 --- a/internal/action/delete.go +++ b/internal/action/delete.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/gopasspw/gopass/pkg/ctxutil" + "github.com/gopasspw/gopass/pkg/debug" "github.com/gopasspw/gopass/pkg/termio" "github.com/urfave/cli/v2" @@ -39,17 +40,21 @@ func (s *Action) Delete(c *cli.Context) error { } if recursive { + debug.Log("pruning %q", name) if err := s.Store.Prune(ctx, name); err != nil { return ExitError(ExitUnknown, err, "failed to prune '%s': %s", name, err) } + debug.Log("pruned %q", name) return nil } // deletes a single key from a YAML doc if key != "" { + debug.Log("removing key %q from %q", key, name) return s.deleteKeyFromYAML(ctx, name, key) } + debug.Log("removing entry %q", name) if err := s.Store.Delete(ctx, name); err != nil { return ExitError(ExitIO, err, "Can not delete '%s': %s", name, err) } diff --git a/internal/backend/storage/fs/fsck.go b/internal/backend/storage/fs/fsck.go index f433fe75b9..15094fd239 100644 --- a/internal/backend/storage/fs/fsck.go +++ b/internal/backend/storage/fs/fsck.go @@ -2,8 +2,11 @@ package fs import ( "context" + "io/ioutil" "os" "path/filepath" + "sort" + "strings" "syscall" "github.com/gopasspw/gopass/internal/out" @@ -23,7 +26,7 @@ func (s *Store) Fsck(ctx context.Context) error { dirs := make(map[string]struct{}, len(entries)) for _, entry := range entries { pcb() - debug.Log("Checking %s", entry) + debug.Log("checking entry %q", entry) filename := filepath.Join(s.path, entry) dirs[filepath.Dir(filename)] = struct{}{} @@ -34,11 +37,18 @@ func (s *Store) Fsck(ctx context.Context) error { } for dir := range dirs { + debug.Log("checking dir %q", dir) if err := s.fsckCheckDir(ctx, dir); err != nil { return err } } - return nil + + if err := s.fsckCheckEmptyDirs(); err != nil { + return err + } + + debug.Log("checking root dir %q", s.path) + return s.fsckCheckDir(ctx, s.path) } func (s *Store) fsckCheckFile(ctx context.Context, filename string) error { @@ -90,3 +100,54 @@ func (s *Store) fsckCheckDir(ctx context.Context, dirname string) error { } return nil } + +func (s *Store) fsckCheckEmptyDirs() error { + v := []string{} + if err := filepath.Walk(s.path, func(fp string, fi os.FileInfo, ferr error) error { + if ferr != nil { + return ferr + } + if !fi.IsDir() { + return nil + } + if strings.HasPrefix(fi.Name(), ".") { + return filepath.SkipDir + } + if fp == s.path { + return nil + } + + // add candidate + debug.Log("adding candidate %q", fp) + v = append(v, fp) + return nil + }); err != nil { + return err + } + + // start with longest path (deepest dir) + sort.Slice(v, func(i, j int) bool { + return len(v[i]) > len(v[j]) + }) + + for _, d := range v { + if err := fsckRemoveEmptyDir(d); err != nil { + return err + } + } + return nil +} + +func fsckRemoveEmptyDir(fp string) error { + ls, err := ioutil.ReadDir(fp) + if err != nil { + return err + } + if len(ls) > 0 { + debug.Log("dir %q is not empty (%d)", fp, len(ls)) + return nil + } + + debug.Log("removing %q ...", fp) + return os.Remove(fp) +} diff --git a/internal/backend/storage/fs/store.go b/internal/backend/storage/fs/store.go index 255e403d0b..a41917c165 100644 --- a/internal/backend/storage/fs/store.go +++ b/internal/backend/storage/fs/store.go @@ -88,6 +88,7 @@ func (s *Store) removeEmptyParentDirectories(path string) error { return nil } + debug.Log("removing empty parent dir: %q", parent) err := os.Remove(parent) switch { case err == nil: @@ -162,7 +163,12 @@ func (s *Store) IsDir(ctx context.Context, name string) bool { func (s *Store) Prune(ctx context.Context, prefix string) error { path := filepath.Join(s.path, filepath.Clean(prefix)) debug.Log("Purning %s from %s", prefix, path) - return os.RemoveAll(path) + + if err := os.RemoveAll(path); err != nil { + return err + } + + return s.removeEmptyParentDirectories(path) } // Name returns the name of this backend diff --git a/internal/store/leaf/move.go b/internal/store/leaf/move.go index 06abc6ad97..9658e3f70b 100644 --- a/internal/store/leaf/move.go +++ b/internal/store/leaf/move.go @@ -76,6 +76,8 @@ func (s *Store) delete(ctx context.Context, name string, recurse bool) error { } } if err := s.deleteSingle(ctx, path); err != nil { + // might fail if we deleted the root of a tree which isn't a secret + // itself if !recurse { return err } @@ -115,6 +117,7 @@ func (s *Store) deleteRecurse(ctx context.Context, name, path string) error { debug.Log("Pruning %s", name) if err := s.storage.Prune(ctx, name); err != nil { + debug.Log("storage.Prune(%v) failed", name) return err } @@ -124,6 +127,7 @@ func (s *Store) deleteRecurse(ctx context.Context, name, path string) error { } return errors.Wrapf(err, "failed to add '%s' to git", path) } + debug.Log("pruned") return nil } diff --git a/internal/store/leaf/move_test.go b/internal/store/leaf/move_test.go index c3d0ebb61f..e630786838 100644 --- a/internal/store/leaf/move_test.go +++ b/internal/store/leaf/move_test.go @@ -280,8 +280,7 @@ func TestPrune(t *testing.T) { assert.Error(t, err) // delete empty folder - assert.NoError(t, s.Prune(ctx, "foo/")) - assert.Error(t, s.Prune(ctx, "foo/")) + assert.Error(t, s.Prune(ctx, "foo/"), "delete non-existing entry") } }, },