Skip to content

Commit

Permalink
Make sure to remove all temporary directories
Browse files Browse the repository at this point in the history
Previously, some files could not be removed because of the directories
permissions, which made os.RemoveAll() fail silently and left a lot of
garbage files in /tmp.

This commit adds a new forceRemoveAll() function that removes all files
and directories more reliably, in a new fs_utils.go file for filesystem
utilities. isFile() is also moved to this new file.

Also log errors when forceRemoveAll fails to detect these issues sooner
in the future.
  • Loading branch information
n-peugnet committed Jan 5, 2025
1 parent b1485e6 commit 999efa4
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 8 deletions.
13 changes: 5 additions & 8 deletions estimate.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,20 +68,17 @@ func removeVendor(gopath string) (found bool, _ error) {
return found, err
}

func isFile(path string) bool {
if info, err := os.Stat(path); err == nil {
return !info.IsDir()
}
return false
}

func estimate(importpath string) error {
// construct a separate GOPATH in a temporary directory
gopath, err := os.MkdirTemp("", "dh-make-golang")
if err != nil {
return fmt.Errorf("create temp dir: %w", err)
}
defer os.RemoveAll(gopath)
defer func() {
if err := forceRemoveAll(gopath); err != nil {
log.Printf("could not remove all %s: %v", gopath, err)
}
}()

// clone the repo inside the src directory of the GOPATH
// and init a Go module if it is not yet one.
Expand Down
35 changes: 35 additions & 0 deletions fs_utils.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package main

import (
"io/fs"
"os"
"path/filepath"
)

// isFile checks if a path exists and is a file (not a directory).
func isFile(path string) bool {
if info, err := os.Stat(path); err == nil {
return !info.IsDir()
}
return false
}

// forceRemoveAll is a more robust alternative to [os.RemoveAll] that tries
// harder to remove all the files and directories.
func forceRemoveAll(path string) error {
// first pass to make sure all the directories are writable
err := filepath.Walk(path, func(path string, info fs.FileInfo, err error) error {
if info.IsDir() {
os.Chmod(path, 0777)
} else {
// remove files by the way
os.Remove(path)
}
return err
})
if err != nil {
return err
}
// remove the remaining directories
return os.RemoveAll(path)
}

0 comments on commit 999efa4

Please sign in to comment.