Skip to content

Commit 56171aa

Browse files
committed
Make sure to remove all temporary directories
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.
1 parent 6624657 commit 56171aa

File tree

2 files changed

+39
-8
lines changed

2 files changed

+39
-8
lines changed

estimate.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,20 +68,17 @@ func removeVendor(gopath string) (found bool, _ error) {
6868
return found, err
6969
}
7070

71-
func isFile(path string) bool {
72-
if info, err := os.Stat(path); err == nil {
73-
return !info.IsDir()
74-
}
75-
return false
76-
}
77-
7871
func estimate(importpath string) error {
7972
// construct a separate GOPATH in a temporary directory
8073
gopath, err := os.MkdirTemp("", "dh-make-golang")
8174
if err != nil {
8275
return fmt.Errorf("create temp dir: %w", err)
8376
}
84-
defer os.RemoveAll(gopath)
77+
defer func() {
78+
if err := forceRemoveAll(gopath); err != nil {
79+
log.Printf("could not remove all %s: %v", gopath, err)
80+
}
81+
}()
8582

8683
// clone the repo inside the src directory of the GOPATH
8784
// and init a Go module if it is not yet one.

fs_utils.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package main
2+
3+
import (
4+
"io/fs"
5+
"os"
6+
"path/filepath"
7+
)
8+
9+
// isFile checks if a path exists and is a file (not a directory).
10+
func isFile(path string) bool {
11+
if info, err := os.Stat(path); err == nil {
12+
return !info.IsDir()
13+
}
14+
return false
15+
}
16+
17+
// forceRemoveAll is a more robust alternative to [os.RemoveAll] that tries
18+
// harder to remove all the files and directories.
19+
func forceRemoveAll(path string) error {
20+
// first pass to make sure all the directories are writable
21+
err := filepath.Walk(path, func(path string, info fs.FileInfo, err error) error {
22+
if info.IsDir() {
23+
return os.Chmod(path, 0777)
24+
} else {
25+
// remove files by the way
26+
return os.Remove(path)
27+
}
28+
})
29+
if err != nil {
30+
return err
31+
}
32+
// remove the remaining directories
33+
return os.RemoveAll(path)
34+
}

0 commit comments

Comments
 (0)