]> Cypherpunks repositories - gostls13.git/commitdiff
os: add Root.RemoveAll
authorDamien Neil <dneil@google.com>
Fri, 28 Mar 2025 23:40:34 +0000 (16:40 -0700)
committerGopher Robot <gobot@golang.org>
Wed, 21 May 2025 16:30:51 +0000 (09:30 -0700)
For #67002

Change-Id: If59dab4fd934a115d8ff383826525330de750b54
Reviewed-on: https://go-review.googlesource.com/c/go/+/661595
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>

api/next/67002.txt
doc/next/6-stdlib/99-minor/os/67002.md
src/os/removeall_at.go
src/os/root.go
src/os/root_noopenat.go
src/os/root_openat.go
src/os/root_test.go
src/os/root_unix.go
src/os/root_windows.go

index 112f477e8ec6c747e3ba19fc52936e228a2ca86a..274f20053863a95c36da95d9368158b2be372f57 100644 (file)
@@ -4,5 +4,6 @@ pkg os, method (*Root) Chtimes(string, time.Time, time.Time) error #67002
 pkg os, method (*Root) Lchown(string, int, int) error #67002
 pkg os, method (*Root) Link(string, string) error #67002
 pkg os, method (*Root) Readlink(string) (string, error) #67002
+pkg os, method (*Root) RemoveAll(string) error #67002
 pkg os, method (*Root) Rename(string, string) error #67002
 pkg os, method (*Root) Symlink(string, string) error #67002
index 84661c6c407579c603955a36ce327d97385fc610..62f1b36054169dfa83df4019b86eb62328c421c7 100644 (file)
@@ -6,5 +6,6 @@ The [os.Root] type supports the following additional methods:
   * [os.Root.Lchown]
   * [os.Root.Link]
   * [os.Root.Readlink]
+  * [os.Root.RemoveAll]
   * [os.Root.Rename]
   * [os.Root.Symlink]
index 0d9ebd2e4fb19c8439cbe2c81a8832aa19d3bf1b..a613aeeb9147d49d87961bc7c0fa4e36a0dc4754 100644 (file)
@@ -44,7 +44,7 @@ func removeAll(path string) error {
        }
        defer parent.Close()
 
-       if err := removeAllFrom(parent, base); err != nil {
+       if err := removeAllFrom(sysfdType(parent.Fd()), base); err != nil {
                if pathErr, ok := err.(*PathError); ok {
                        pathErr.Path = parentDir + string(PathSeparator) + pathErr.Path
                        err = pathErr
@@ -54,9 +54,7 @@ func removeAll(path string) error {
        return nil
 }
 
-func removeAllFrom(parent *File, base string) error {
-       parentFd := sysfdType(parent.Fd())
-
+func removeAllFrom(parentFd sysfdType, base string) error {
        // Simple case: if Unlink (aka remove) works, we're done.
        err := removefileat(parentFd, base)
        if err == nil || IsNotExist(err) {
@@ -109,7 +107,7 @@ func removeAllFrom(parent *File, base string) error {
 
                        respSize = len(names)
                        for _, name := range names {
-                               err := removeAllFrom(file, name)
+                               err := removeAllFrom(sysfdType(file.Fd()), name)
                                if err != nil {
                                        if pathErr, ok := err.(*PathError); ok {
                                                pathErr.Path = base + string(PathSeparator) + pathErr.Path
index fb2bb8350a716d563d42e3a1105bc4a757f67300..9b9deaecc43974aed7580e262d1db6bd6959346b 100644 (file)
@@ -177,6 +177,12 @@ func (r *Root) Remove(name string) error {
        return rootRemove(r, name)
 }
 
+// RemoveAll removes the named file or directory and any children that it contains.
+// See [RemoveAll] for more details.
+func (r *Root) RemoveAll(name string) error {
+       return rootRemoveAll(r, name)
+}
+
 // Stat returns a [FileInfo] describing the named file in the root.
 // See [Stat] for more details.
 func (r *Root) Stat(name string) (FileInfo, error) {
index 47d6ebfa823d865f08115d11c5c531a88b255cef..b34416284ff3e1e7d3cb039ea6eb77e12779e2d9 100644 (file)
@@ -9,6 +9,7 @@ package os
 import (
        "errors"
        "sync/atomic"
+       "syscall"
        "time"
 )
 
@@ -156,6 +157,25 @@ func rootRemove(r *Root, name string) error {
        return nil
 }
 
+func rootRemoveAll(r *Root, name string) error {
+       if endsWithDot(name) {
+               // Consistency with os.RemoveAll: Return EINVAL when trying to remove .
+               return &PathError{Op: "RemoveAll", Path: name, Err: syscall.EINVAL}
+       }
+       if err := checkPathEscapesLstat(r, name); err != nil {
+               if err == syscall.ENOTDIR {
+                       // Some intermediate path component is not a directory.
+                       // RemoveAll treats this as success (since the target doesn't exist).
+                       return nil
+               }
+               return &PathError{Op: "RemoveAll", Path: name, Err: err}
+       }
+       if err := RemoveAll(joinPath(r.root.name, name)); err != nil {
+               return &PathError{Op: "RemoveAll", Path: name, Err: underlyingError(err)}
+       }
+       return nil
+}
+
 func rootReadlink(r *Root, name string) (string, error) {
        if err := checkPathEscapesLstat(r, name); err != nil {
                return "", &PathError{Op: "readlinkat", Path: name, Err: err}
index db2ae2295f810b2a60a445099812d52956bb97fc..b57506a2ebd2f3df992bf5332e5d4ad6ddca7ee8 100644 (file)
@@ -138,6 +138,28 @@ func rootRemove(r *Root, name string) error {
        return nil
 }
 
+func rootRemoveAll(r *Root, name string) error {
+       // Consistency with os.RemoveAll: Strip trailing /s from the name,
+       // so RemoveAll("not_a_directory/") succeeds.
+       for len(name) > 0 && IsPathSeparator(name[len(name)-1]) {
+               name = name[:len(name)-1]
+       }
+       if endsWithDot(name) {
+               // Consistency with os.RemoveAll: Return EINVAL when trying to remove .
+               return &PathError{Op: "RemoveAll", Path: name, Err: syscall.EINVAL}
+       }
+       _, err := doInRoot(r, name, func(parent sysfdType, name string) (struct{}, error) {
+               return struct{}{}, removeAllFrom(parent, name)
+       })
+       if IsNotExist(err) {
+               return nil
+       }
+       if err != nil {
+               return &PathError{Op: "RemoveAll", Path: name, Err: underlyingError(err)}
+       }
+       return err
+}
+
 func rootRename(r *Root, oldname, newname string) error {
        _, err := doInRoot(r, oldname, func(oldparent sysfdType, oldname string) (struct{}, error) {
                _, err := doInRoot(r, newname, func(newparent sysfdType, newname string) (struct{}, error) {
index 530d5abda632cf9c6e5115fb9630ea63e6e51282..c75a0947302f667556ca3d2911ba5f2672931acd 100644 (file)
@@ -630,6 +630,46 @@ func TestRootRemoveDirectory(t *testing.T) {
        }
 }
 
+func TestRootRemoveAll(t *testing.T) {
+       for _, test := range rootTestCases {
+               test.run(t, func(t *testing.T, target string, root *os.Root) {
+                       wantError := test.wantError
+                       if test.ltarget != "" {
+                               // Remove doesn't follow symlinks in the final path component,
+                               // so it will successfully remove ltarget.
+                               wantError = false
+                               target = filepath.Join(root.Name(), test.ltarget)
+                       } else if target != "" {
+                               if err := os.Mkdir(target, 0o777); err != nil {
+                                       t.Fatal(err)
+                               }
+                               if err := os.WriteFile(filepath.Join(target, "file"), nil, 0o666); err != nil {
+                                       t.Fatal(err)
+                               }
+                       }
+                       targetExists := true
+                       if _, err := root.Lstat(test.open); errors.Is(err, os.ErrNotExist) {
+                               // If the target doesn't exist, RemoveAll succeeds rather
+                               // than returning ErrNotExist.
+                               targetExists = false
+                               wantError = false
+                       }
+
+                       err := root.RemoveAll(test.open)
+                       if errEndsTest(t, err, wantError, "root.RemoveAll(%q)", test.open) {
+                               return
+                       }
+                       if !targetExists {
+                               return
+                       }
+                       _, err = os.Lstat(target)
+                       if !errors.Is(err, os.ErrNotExist) {
+                               t.Fatalf(`stat file removed with Root.Remove(%q): %v, want ErrNotExist`, test.open, err)
+                       }
+               })
+       }
+}
+
 func TestRootOpenFileAsRoot(t *testing.T) {
        dir := t.TempDir()
        target := filepath.Join(dir, "target")
@@ -958,6 +998,9 @@ type rootConsistencyTest struct {
        // detailedErrorMismatch indicates that os.Root and the corresponding non-Root
        // function return different errors for this test.
        detailedErrorMismatch func(t *testing.T) bool
+
+       // check is called before the test starts, and may t.Skip if necessary.
+       check func(t *testing.T)
 }
 
 var rootConsistencyTestCases = []rootConsistencyTest{{
@@ -1115,6 +1158,16 @@ var rootConsistencyTestCases = []rootConsistencyTest{{
                // and os.Open returns "The file cannot be accessed by the system.".
                return runtime.GOOS == "windows"
        },
+       check: func(t *testing.T) {
+               if runtime.GOOS == "windows" && strings.HasPrefix(t.Name(), "TestRootConsistencyRemoveAll/") {
+                       // Root.RemoveAll notices that a/ is not a directory,
+                       // and returns success.
+                       // os.RemoveAll tries to open a/ and fails because
+                       // it is not a regular file.
+                       // The inconsistency here isn't worth fixing, so just skip this test.
+                       t.Skip("known inconsistency on windows")
+               }
+       },
 }, {
        name: "question mark",
        open: "?",
@@ -1156,6 +1209,10 @@ func (test rootConsistencyTest) run(t *testing.T, f func(t *testing.T, path stri
        }
 
        t.Run(test.name, func(t *testing.T) {
+               if test.check != nil {
+                       test.check(t)
+               }
+
                dir1 := makefs(t, test.fs)
                dir2 := makefs(t, test.fs)
                if test.fsFunc != nil {
@@ -1321,6 +1378,23 @@ func TestRootConsistencyRemove(t *testing.T) {
        }
 }
 
+func TestRootConsistencyRemoveAll(t *testing.T) {
+       for _, test := range rootConsistencyTestCases {
+               if test.open == "." || test.open == "./" {
+                       continue // can't remove the root itself
+               }
+               test.run(t, func(t *testing.T, path string, r *os.Root) (string, error) {
+                       var err error
+                       if r == nil {
+                               err = os.RemoveAll(path)
+                       } else {
+                               err = r.RemoveAll(path)
+                       }
+                       return "", err
+               })
+       }
+}
+
 func TestRootConsistencyStat(t *testing.T) {
        for _, test := range rootConsistencyTestCases {
                test.run(t, func(t *testing.T, path string, r *os.Root) (string, error) {
@@ -1759,3 +1833,21 @@ func TestOpenInRoot(t *testing.T) {
                }
        }
 }
+
+func TestRootRemoveDot(t *testing.T) {
+       dir := t.TempDir()
+       root, err := os.OpenRoot(dir)
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer root.Close()
+       if err := root.Remove("."); err == nil {
+               t.Errorf(`root.Remove(".") = %v, want error`, err)
+       }
+       if err := root.RemoveAll("."); err == nil {
+               t.Errorf(`root.RemoveAll(".") = %v, want error`, err)
+       }
+       if _, err := os.Stat(dir); err != nil {
+               t.Error(`root.Remove(All)?(".") removed the root`)
+       }
+}
index 74320451d5b536432dd2b7312d70c3421d31198f..45462c9e10c42373912b31640fa40c260158b77d 100644 (file)
@@ -14,6 +14,9 @@ import (
        "time"
 )
 
+// sysfdType is the native type of a file handle
+// (int on Unix, syscall.Handle on Windows),
+// permitting helper functions to be written portably.
 type sysfdType = int
 
 // openRootNolog is OpenRoot.
index b096d78b68f221071b49010587b4f505d2b9b54a..3d3db1916e96750996b397e5f429e6885531b760 100644 (file)
@@ -77,6 +77,9 @@ func rootCleanPath(s string, prefix, suffix []string) (string, error) {
        return s, nil
 }
 
+// sysfdType is the native type of a file handle
+// (int on Unix, syscall.Handle on Windows),
+// permitting helper functions to be written portably.
 type sysfdType = syscall.Handle
 
 // openRootNolog is OpenRoot.