Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch most calls to filepath.Walk to filepath.WalkDir #1176

Merged
merged 2 commits into from Apr 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .cirrus.yml
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
6 changes: 4 additions & 2 deletions drivers/aufs/aufs.go
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

/*
Expand Down Expand Up @@ -26,6 +27,7 @@ import (
"bufio"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions drivers/btrfs/btrfs.go
@@ -1,3 +1,4 @@
//go:build linux && cgo
// +build linux,cgo

package btrfs
Expand All @@ -16,6 +17,7 @@ import "C"

import (
"fmt"
"io/fs"
"io/ioutil"
"math"
"os"
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
1 change: 1 addition & 0 deletions drivers/copy/copy_test.go
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

package copy
Expand Down
29 changes: 13 additions & 16 deletions drivers/devmapper/deviceset.go
@@ -1,3 +1,4 @@
//go:build linux && cgo
// +build linux,cgo

package devmapper
Expand All @@ -6,6 +7,7 @@ import (
"bufio"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"os/exec"
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
42 changes: 0 additions & 42 deletions drivers/overlay/check_115.go

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
@@ -1,4 +1,4 @@
go 1.14
go 1.16

module github.com/containers/storage

Expand Down
7 changes: 4 additions & 3 deletions pkg/archive/archive.go
Expand Up @@ -7,6 +7,7 @@ import (
"compress/bzip2"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion pkg/archive/changes_other.go
@@ -1,9 +1,11 @@
//go:build !linux
// +build !linux

package archive

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions 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.
Expand All @@ -10,6 +11,7 @@ import (
"encoding/hex"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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)
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/archive/diff.go
Expand Up @@ -4,6 +4,7 @@ import (
"archive/tar"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions pkg/archive/diff_test.go
Expand Up @@ -3,6 +3,7 @@ package archive
import (
"archive/tar"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions pkg/archive/utils_test.go
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"fmt"
"io"
"io/fs"
"io/ioutil"
"os"
"path/filepath"
Expand Down Expand Up @@ -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
Expand Down