From 999efa498874612503d457084400dce5e3934637 Mon Sep 17 00:00:00 2001
From: Nicolas Peugnet <nicolas@club1.fr>
Date: Sun, 5 Jan 2025 23:42:29 +0100
Subject: [PATCH] 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.
---
 estimate.go | 13 +++++--------
 fs_utils.go | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)
 create mode 100644 fs_utils.go

diff --git a/estimate.go b/estimate.go
index a3c2625..e7e478f 100644
--- a/estimate.go
+++ b/estimate.go
@@ -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.
diff --git a/fs_utils.go b/fs_utils.go
new file mode 100644
index 0000000..0bc9fce
--- /dev/null
+++ b/fs_utils.go
@@ -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)
+}