Skip to content

Commit 88ece11

Browse files
committed
Use the exact removaAll implementation from Go 1.24
1 parent a114241 commit 88ece11

1 file changed

Lines changed: 103 additions & 32 deletions

File tree

internal/pkg/agent/install/uninstall.go

Lines changed: 103 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ import (
88
"context"
99
"errors"
1010
"fmt"
11+
"io"
1112
"io/fs"
1213
"net/http"
1314
"os"
1415
"path/filepath"
1516
"runtime"
1617
"strings"
18+
"syscall"
1719
"time"
1820

1921
"github.com/kardianos/service"
@@ -333,58 +335,127 @@ func RemoveBut(path string, bestEffort bool, exceptions ...string) error {
333335
// state and fail on files that have been ADS-renamed. The simple path-based
334336
// approach using os.Remove works correctly with the ADS rename trick that
335337
// RemovePath uses to delete running executables on Windows.
338+
// See: https://cs.opensource.google/go/go/+/refs/tags/go1.24.13:src/os/removeall_noat.go;drc=a2baae6851a157d662dff7cc508659f66249698a;l=15
336339
func removeAll(path string) error {
337-
// Try simple remove first (handles files and empty directories).
340+
if path == "" {
341+
// fail silently to retain compatibility with previous behavior
342+
// of RemoveAll. See issue 28830.
343+
return nil
344+
}
345+
346+
// Simple case: if Remove works, we're done.
338347
err := os.Remove(path)
339-
if err == nil || errors.Is(err, fs.ErrNotExist) {
348+
if err == nil || os.IsNotExist(err) {
340349
return nil
341350
}
342351

343-
// Check if it's a directory.
344-
info, serr := os.Lstat(path)
352+
// Otherwise, is this a directory we need to recurse into?
353+
dir, serr := os.Lstat(path)
345354
if serr != nil {
346-
if errors.Is(serr, fs.ErrNotExist) {
355+
if serr, ok := serr.(*os.PathError); ok && (os.IsNotExist(serr.Err) || serr.Err == syscall.ENOTDIR) {
347356
return nil
348357
}
349358
return serr
350359
}
351-
if !info.IsDir() {
352-
// Not a directoryreturn the original Remove error.
360+
if !dir.IsDir() {
361+
// Not a directory; return the error from Remove.
353362
return err
354363
}
355364

356-
// Remove directory contents recursively.
357-
err = removeAllChildren(path)
358-
if err != nil {
359-
return err
360-
}
365+
// Remove contents & return first error.
366+
err = nil
367+
for {
368+
fd, err := os.Open(path)
369+
if err != nil {
370+
if os.IsNotExist(err) {
371+
// Already deleted by someone else.
372+
return nil
373+
}
374+
return err
375+
}
361376

362-
// Remove the now-empty directory.
363-
err = os.Remove(path)
364-
if errors.Is(err, fs.ErrNotExist) {
365-
return nil
366-
}
367-
return err
368-
}
377+
const reqSize = 1024
378+
var names []string
379+
var readErr error
380+
381+
for {
382+
numErr := 0
383+
names, readErr = fd.Readdirnames(reqSize)
384+
385+
for _, name := range names {
386+
err1 := os.RemoveAll(path + string(os.PathSeparator) + name)
387+
if err == nil {
388+
err = err1
389+
}
390+
if err1 != nil {
391+
numErr++
392+
}
393+
}
369394

370-
func removeAllChildren(path string) error {
371-
entries, err := os.ReadDir(path)
372-
if err != nil {
373-
if errors.Is(err, fs.ErrNotExist) {
374-
return nil
395+
// If we can delete any entry, break to start new iteration.
396+
// Otherwise, we discard current names, get next entries and try deleting them.
397+
if numErr != reqSize {
398+
break
399+
}
400+
}
401+
402+
// Removing files from the directory may have caused
403+
// the OS to reshuffle it. Simply calling Readdirnames
404+
// again may skip some entries. The only reliable way
405+
// to avoid this is to close and re-open the
406+
// directory. See issue 20841.
407+
fd.Close()
408+
409+
if readErr == io.EOF {
410+
break
411+
}
412+
// If Readdirnames returned an error, use it.
413+
if err == nil {
414+
err = readErr
415+
}
416+
if len(names) == 0 {
417+
break
418+
}
419+
420+
// We don't want to re-open unnecessarily, so if we
421+
// got fewer than request names from Readdirnames, try
422+
// simply removing the directory now. If that
423+
// succeeds, we are done.
424+
if len(names) < reqSize {
425+
err1 := os.Remove(path)
426+
if err1 == nil || os.IsNotExist(err1) {
427+
return nil
428+
}
429+
430+
if err != nil {
431+
// We got some error removing the
432+
// directory contents, and since we
433+
// read fewer names than we requested
434+
// there probably aren't more files to
435+
// remove. Don't loop around to read
436+
// the directory again. We'll probably
437+
// just get the same error.
438+
return err
439+
}
375440
}
376-
return err
377441
}
378442

379-
var firstErr error
380-
for _, entry := range entries {
381-
child := filepath.Join(path, entry.Name())
382-
err := removeAll(child)
383-
if err != nil && firstErr == nil {
384-
firstErr = err
443+
// Remove directory.
444+
err1 := os.Remove(path)
445+
if err1 == nil || os.IsNotExist(err1) {
446+
return nil
447+
}
448+
if runtime.GOOS == "windows" && os.IsPermission(err1) {
449+
if fileInfo, err := os.Stat(path); err == nil {
450+
if err = os.Chmod(path, os.FileMode(0200|int(fileInfo.Mode()))); err == nil {
451+
err1 = os.Remove(path)
452+
}
385453
}
386454
}
387-
return firstErr
455+
if err == nil {
456+
err = err1
457+
}
458+
return err
388459
}
389460

390461
func containsString(str string, a []string, caseSensitive bool) bool {

0 commit comments

Comments
 (0)