Switch most calls to filepath.Walk to filepath.WalkDir

It is faster then Walk, when you don't need to stat every file and directory. Signed-off-by: Daniel J Walsh <[email protected]>
containers · Apr 6, 2022 · 04b2f0f · 04b2f0f
1 parent 11fb94f
commit 04b2f0f
Show file tree

Hide file tree

Showing 15 changed files with 65 additions and 96 deletions.
diff --git a/.cirrus.yml b/.cirrus.yml
@@ -117,7 +117,7 @@ lint_task:
     env:
         CIRRUS_WORKING_DIR: "/go/src/github.com/containers/storage"
     container:
-        image: golang:1.15
+        image: golang:1.16
     modules_cache:
         fingerprint_script: cat go.sum
         folder: $GOPATH/pkg/mod
@@ -154,7 +154,7 @@ meta_task:
 
 vendor_task:
     container:
-        image: golang:1.15
+        image: golang:1.16
     modules_cache:
         fingerprint_script: cat go.sum
         folder: $GOPATH/pkg/mod
@@ -172,6 +172,6 @@ success_task:
         - meta
         - vendor
     container:
-        image: golang:1.15
+        image: golang:1.16
     clone_script: 'mkdir -p "$CIRRUS_WORKING_DIR"'  # Source code not needed
     script: /bin/true
diff --git a/drivers/aufs/aufs.go b/drivers/aufs/aufs.go
@@ -1,3 +1,4 @@
+//go:build linux
 // +build linux
 
 /*
@@ -26,6 +27,7 @@ import (
 	"bufio"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -649,11 +651,11 @@ func (a *Driver) mounted(mountpoint string) (bool, error) {
 // Cleanup aufs and unmount all mountpoints
 func (a *Driver) Cleanup() error {
 	var dirs []string
-	if err := filepath.Walk(a.mntPath(), func(path string, info os.FileInfo, err error) error {
+	if err := filepath.WalkDir(a.mntPath(), func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
-		if !info.IsDir() {
+		if !d.IsDir() {
 			return nil
 		}
 		dirs = append(dirs, path)

diff --git a/drivers/btrfs/btrfs.go b/drivers/btrfs/btrfs.go
@@ -1,3 +1,4 @@
+//go:build linux && cgo
 // +build linux,cgo
 
 package btrfs
@@ -16,6 +17,7 @@ import "C"
 
 import (
 	"fmt"
+	"io/fs"
 	"io/ioutil"
 	"math"
 	"os"
@@ -256,7 +258,7 @@ func subvolDelete(dirpath, name string, quotaEnabled bool) error {
 	var args C.struct_btrfs_ioctl_vol_args
 
 	// walk the btrfs subvolumes
-	walkSubvolumes := func(p string, f os.FileInfo, err error) error {
+	walkSubvolumes := func(p string, d fs.DirEntry, err error) error {
 		if err != nil {
 			if os.IsNotExist(err) && p != fullPath {
 				// missing most likely because the path was a subvolume that got removed in the previous iteration
@@ -267,20 +269,20 @@ func subvolDelete(dirpath, name string, quotaEnabled bool) error {
 		}
 		// we want to check children only so skip itself
 		// it will be removed after the filepath walk anyways
-		if f.IsDir() && p != fullPath {
+		if d.IsDir() && p != fullPath {
 			sv, err := isSubvolume(p)
 			if err != nil {
 				return fmt.Errorf("Failed to test if %s is a btrfs subvolume: %v", p, err)
 			}
 			if sv {
-				if err := subvolDelete(path.Dir(p), f.Name(), quotaEnabled); err != nil {
+				if err := subvolDelete(path.Dir(p), d.Name(), quotaEnabled); err != nil {
 					return fmt.Errorf("Failed to destroy btrfs child subvolume (%s) of parent (%s): %v", p, dirpath, err)
 				}
 			}
 		}
 		return nil
 	}
-	if err := filepath.Walk(path.Join(dirpath, name), walkSubvolumes); err != nil {
+	if err := filepath.WalkDir(path.Join(dirpath, name), walkSubvolumes); err != nil {
 		return fmt.Errorf("Recursively walking subvolumes for %s failed: %v", dirpath, err)
 	}
 

diff --git a/drivers/copy/copy_test.go b/drivers/copy/copy_test.go
@@ -1,3 +1,4 @@
+//go:build linux
 // +build linux
 
 package copy

diff --git a/drivers/devmapper/deviceset.go b/drivers/devmapper/deviceset.go
@@ -1,3 +1,4 @@
+//go:build linux && cgo
 // +build linux,cgo
 
 package devmapper
@@ -6,6 +7,7 @@ import (
 	"bufio"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -419,40 +421,35 @@ func (devices *DeviceSet) constructDeviceIDMap() {
 	}
 }
 
-func (devices *DeviceSet) deviceFileWalkFunction(path string, finfo os.FileInfo) error {
+func (devices *DeviceSet) deviceFileWalkFunction(path string, name string) error {
 
 	// Skip some of the meta files which are not device files.
-	if strings.HasSuffix(finfo.Name(), ".migrated") {
+	if strings.HasSuffix(name, ".migrated") {
 		logrus.Debugf("devmapper: Skipping file %s", path)
 		return nil
 	}
 
-	if strings.HasPrefix(finfo.Name(), ".") {
+	if strings.HasPrefix(name, ".") {
 		logrus.Debugf("devmapper: Skipping file %s", path)
 		return nil
 	}
 
-	if finfo.Name() == deviceSetMetaFile {
+	if name == deviceSetMetaFile {
 		logrus.Debugf("devmapper: Skipping file %s", path)
 		return nil
 	}
 
-	if finfo.Name() == transactionMetaFile {
+	if name == transactionMetaFile {
 		logrus.Debugf("devmapper: Skipping file %s", path)
 		return nil
 	}
 
 	logrus.Debugf("devmapper: Loading data for file %s", path)
 
-	hash := finfo.Name()
-	if hash == base {
-		hash = ""
-	}
-
 	// Include deleted devices also as cleanup delete device logic
 	// will go through it and see if there are any deleted devices.
-	if _, err := devices.lookupDevice(hash); err != nil {
-		return fmt.Errorf("devmapper: Error looking up device %s:%v", hash, err)
+	if _, err := devices.lookupDevice(name); err != nil {
+		return fmt.Errorf("devmapper: Error looking up device %s:%v", name, err)
 	}
 
 	return nil
@@ -462,21 +459,21 @@ func (devices *DeviceSet) loadDeviceFilesOnStart() error {
 	logrus.Debug("devmapper: loadDeviceFilesOnStart()")
 	defer logrus.Debug("devmapper: loadDeviceFilesOnStart() END")
 
-	var scan = func(path string, info os.FileInfo, err error) error {
+	var scan = func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			logrus.Debugf("devmapper: Can't walk the file %s", path)
 			return nil
 		}
 
 		// Skip any directories
-		if info.IsDir() {
+		if d.IsDir() {
 			return nil
 		}
 
-		return devices.deviceFileWalkFunction(path, info)
+		return devices.deviceFileWalkFunction(path, d.Name())
 	}
 
-	return filepath.Walk(devices.metadataDir(), scan)
+	return filepath.WalkDir(devices.metadataDir(), scan)
 }
 
 // Should be called with devices.Lock() held.

diff --git a/drivers/overlay/check_115.go b/drivers/overlay/check_115.go
diff --git a/go.mod b/go.mod
@@ -1,4 +1,4 @@
-go 1.14
+go 1.16
 
 module github.com/containers/storage
 

diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
@@ -7,6 +7,7 @@ import (
 	"compress/bzip2"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -863,14 +864,14 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 			rebaseName := options.RebaseNames[include]
 
 			walkRoot := getWalkRoot(srcPath, include)
-			filepath.Walk(walkRoot, func(filePath string, f os.FileInfo, err error) error {
+			filepath.WalkDir(walkRoot, func(filePath string, d fs.DirEntry, err error) error {
 				if err != nil {
 					logrus.Errorf("Tar: Can't stat file %s to tar: %s", srcPath, err)
 					return nil
 				}
 
 				relFilePath, err := filepath.Rel(srcPath, filePath)
-				if err != nil || (!options.IncludeSourceDir && relFilePath == "." && f.IsDir()) {
+				if err != nil || (!options.IncludeSourceDir && relFilePath == "." && d.IsDir()) {
 					// Error getting relative path OR we are looking
 					// at the source directory path. Skip in both situations.
 					return nil
@@ -903,7 +904,7 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 					// dir. If so then we can't skip this dir.
 
 					// Its not a dir then so we can just return/skip.
-					if !f.IsDir() {
+					if !d.IsDir() {
 						return nil
 					}
 

diff --git a/pkg/archive/changes_other.go b/pkg/archive/changes_other.go
@@ -1,9 +1,11 @@
+//go:build !linux
 // +build !linux
 
 package archive
 
 import (
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -41,7 +43,7 @@ func collectFileInfoForChanges(oldDir, newDir string, oldIDMap, newIDMap *idtool
 func collectFileInfo(sourceDir string, idMappings *idtools.IDMappings) (*FileInfo, error) {
 	root := newRootFileInfo(idMappings)
 
-	err := filepath.Walk(sourceDir, func(path string, f os.FileInfo, err error) error {
+	err := filepath.WalkDir(sourceDir, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}

diff --git a/pkg/archive/copy_unix_test.go b/pkg/archive/copy_unix_test.go
@@ -1,3 +1,4 @@
+//go:build !windows
 // +build !windows
 
 // TODO Windows: Some of these tests may be salvageable and portable to Windows.
@@ -10,6 +11,7 @@ import (
 	"encoding/hex"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -102,24 +104,22 @@ func dirContentsEqual(t *testing.T, newDir, oldDir string) (err error) {
 }
 
 func logDirContents(t *testing.T, dirPath string) {
-	logWalkedPaths := filepath.WalkFunc(func(path string, info os.FileInfo, err error) error {
+	t.Logf("logging directory contents: %q", dirPath)
+
+	err := filepath.WalkDir(dirPath, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			t.Errorf("stat error for path %q: %s", path, err)
 			return nil
 		}
 
-		if info.IsDir() {
+		if d.IsDir() {
 			path = joinTrailingSep(path)
 		}
 
 		t.Logf("\t%s", path)
 
 		return nil
 	})
-
-	t.Logf("logging directory contents: %q", dirPath)
-
-	err := filepath.Walk(dirPath, logWalkedPaths)
 	require.NoError(t, err)
 }
 

diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go
@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -134,7 +135,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64,
 				if err != nil {
 					return 0, err
 				}
-				err = filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+				err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
 					if err != nil {
 						if os.IsNotExist(err) {
 							err = nil // parent was deleted

diff --git a/pkg/archive/diff_test.go b/pkg/archive/diff_test.go
@@ -3,6 +3,7 @@ package archive
 import (
 	"archive/tar"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -362,7 +363,7 @@ func makeTestLayer(paths []string) (rc io.ReadCloser, err error) {
 
 func readDirContents(root string) ([]string, error) {
 	var files []string
-	err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error {
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
 		if err != nil {
 			return err
 		}
@@ -373,7 +374,7 @@ func readDirContents(root string) ([]string, error) {
 		if err != nil {
 			return err
 		}
-		if info.IsDir() {
+		if d.IsDir() {
 			rel = rel + "/"
 		}
 		files = append(files, rel)

diff --git a/pkg/archive/utils_test.go b/pkg/archive/utils_test.go
@@ -5,6 +5,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -140,8 +141,8 @@ func testBreakout(untarFn string, tmpdir string, headers []*tar.Header) error {
 	// Since victim/hello was generated with time.Now(), it is safe to assume
 	// that any file whose content matches exactly victim/hello, managed somehow
 	// to access victim/hello.
-	return filepath.Walk(dest, func(path string, info os.FileInfo, err error) error {
-		if info.IsDir() {
+	return filepath.WalkDir(dest, func(path string, d fs.DirEntry, err error) error {
+		if d.IsDir() {
 			if err != nil {
 				// skip directory if error
 				return filepath.SkipDir
-Original file line number
+Diff line change
@@ -1,4 +1,4 @@
-    go 1.14
+    go 1.16
     module github.com/containers/storage
@@ Expand Down @@