From 7285cd397463cf647c28ab04dd6862c911efcd7e Mon Sep 17 00:00:00 2001 From: mbohy Date: Sat, 28 Jan 2023 19:42:46 +0000 Subject: [PATCH] git: worktree: check for empty parent dirs during Reset (Fixes #670) (#671) When we delete dir1/dir2/file1, we currently check if dir2 becomes empty with the deletion of file1, and if so, we delete dir2. If dir1 becomes empty with the deletion of dir2, we don't notice that, and dir1 is left behind. This commit adds a loop to check each parent directory in the file path for emptiness, removing empty directories along the way until a non-empty directory is found (or an error occurs). --- worktree.go | 49 ++++++++++++++++++++++++++++++++++++---------- worktree_test.go | 51 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/worktree.go b/worktree.go index 1b6d0bd42..5cdb4b518 100644 --- a/worktree.go +++ b/worktree.go @@ -415,7 +415,7 @@ func (w *Worktree) checkoutChange(ch merkletrie.Change, t *object.Tree, idx *ind isSubmodule = e.Mode == filemode.Submodule case merkletrie.Delete: - return rmFileAndDirIfEmpty(w.Filesystem, ch.From.String()) + return rmFileAndDirsIfEmpty(w.Filesystem, ch.From.String()) } if isSubmodule { @@ -783,8 +783,10 @@ func (w *Worktree) doClean(status Status, opts *CleanOptions, dir string, files } if opts.Dir && dir != "" { - return doCleanDirectories(w.Filesystem, dir) + _, err := removeDirIfEmpty(w.Filesystem, dir) + return err } + return nil } @@ -925,25 +927,52 @@ func findMatchInFile(file *object.File, treeName string, opts *GrepOptions) ([]G return grepResults, nil } -func rmFileAndDirIfEmpty(fs billy.Filesystem, name string) error { +// will walk up the directory tree removing all encountered empty +// directories, not just the one containing this file +func rmFileAndDirsIfEmpty(fs billy.Filesystem, name string) error { if err := util.RemoveAll(fs, name); err != nil { return err } dir := filepath.Dir(name) - return doCleanDirectories(fs, dir) + for { + removed, err := removeDirIfEmpty(fs, dir) + if err != nil { + return err + } + + if !removed { + // directory was not empty and not removed, + // stop checking parents + break + } + + // move to parent directory + dir = filepath.Dir(dir) + } + + return nil } -// doCleanDirectories removes empty subdirs (without files) -func doCleanDirectories(fs billy.Filesystem, dir string) error { +// removeDirIfEmpty will remove the supplied directory `dir` if +// `dir` is empty +// returns true if the directory was removed +func removeDirIfEmpty(fs billy.Filesystem, dir string) (bool, error) { files, err := fs.ReadDir(dir) if err != nil { - return err + return false, err } - if len(files) == 0 { - return fs.Remove(dir) + + if len(files) > 0 { + return false, nil } - return nil + + err = fs.Remove(dir) + if err != nil { + return false, err + } + + return true, nil } type indexBuilder struct { diff --git a/worktree_test.go b/worktree_test.go index d545b01eb..b57a77dbf 100644 --- a/worktree_test.go +++ b/worktree_test.go @@ -3,6 +3,7 @@ package git import ( "bytes" "context" + "errors" "io" "io/ioutil" "os" @@ -2210,6 +2211,56 @@ func (s *WorktreeSuite) TestGrep(c *C) { } } +func (s *WorktreeSuite) TestResetLingeringDirectories(c *C) { + dir, clean := s.TemporalDir() + defer clean() + + commitOpts := &CommitOptions{Author: &object.Signature{ + Name: "foo", + Email: "foo@foo.foo", + When: time.Now(), + }} + + repo, err := PlainInit(dir, false) + c.Assert(err, IsNil) + + w, err := repo.Worktree() + c.Assert(err, IsNil) + + os.WriteFile(filepath.Join(dir, "README"), []byte("placeholder"), 0o644) + + _, err = w.Add(".") + c.Assert(err, IsNil) + + initialHash, err := w.Commit("Initial commit", commitOpts) + c.Assert(err, IsNil) + + os.MkdirAll(filepath.Join(dir, "a", "b"), 0o755) + os.WriteFile(filepath.Join(dir, "a", "b", "1"), []byte("1"), 0o644) + + _, err = w.Add(".") + c.Assert(err, IsNil) + + _, err = w.Commit("Add file in nested sub-directories", commitOpts) + c.Assert(err, IsNil) + + // reset to initial commit, which should remove a/b/1, a/b, and a + err = w.Reset(&ResetOptions{ + Commit: initialHash, + Mode: HardReset, + }) + c.Assert(err, IsNil) + + _, err = os.Stat(filepath.Join(dir, "a", "b", "1")) + c.Assert(errors.Is(err, os.ErrNotExist), Equals, true) + + _, err = os.Stat(filepath.Join(dir, "a", "b")) + c.Assert(errors.Is(err, os.ErrNotExist), Equals, true) + + _, err = os.Stat(filepath.Join(dir, "a")) + c.Assert(errors.Is(err, os.ErrNotExist), Equals, true) +} + func (s *WorktreeSuite) TestAddAndCommit(c *C) { expectedFiles := 2