From bd6217bb74644d245ce6271138f4c660415fa0fb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 5 Oct 2022 11:09:35 +0200 Subject: [PATCH 1/2] pkg/directory: minor refactor of Size() - separate exported function from implementation, to allow for GoDoc to be maintained in a single location. - don't use named return variables (no "bare" return, and potentially shadowing variables) - reverse the `os.IsNotExist(err) && d != dir` condition, putting the "lighter" `d != dir` first. Signed-off-by: Sebastiaan van Stijn --- pkg/directory/directory.go | 6 ++++++ pkg/directory/directory_unix.go | 13 +++++++------ pkg/directory/directory_windows.go | 13 +++++++------ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/pkg/directory/directory.go b/pkg/directory/directory.go index f0c45303cd..998b93fd8c 100644 --- a/pkg/directory/directory.go +++ b/pkg/directory/directory.go @@ -1,6 +1,7 @@ package directory // import "github.com/docker/docker/pkg/directory" import ( + "context" "os" "path/filepath" ) @@ -22,3 +23,8 @@ func MoveToSubdir(oldpath, subdir string) error { } return nil } + +// Size walks a directory tree and returns its total size in bytes. +func Size(ctx context.Context, dir string) (int64, error) { + return calcSize(ctx, dir) +} diff --git a/pkg/directory/directory_unix.go b/pkg/directory/directory_unix.go index eeedff18a4..e0b15d0b05 100644 --- a/pkg/directory/directory_unix.go +++ b/pkg/directory/directory_unix.go @@ -10,14 +10,15 @@ import ( "syscall" ) -// Size walks a directory tree and returns its total size in bytes. -func Size(ctx context.Context, dir string) (size int64, err error) { +// calcSize walks a directory tree and returns its total size in bytes. +func calcSize(ctx context.Context, dir string) (int64, error) { + var size int64 data := make(map[uint64]struct{}) - err = filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { + err := filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { if err != nil { - // if dir does not exist, Size() returns the error. // if dir/x disappeared while walking, Size() ignores dir/x. - if os.IsNotExist(err) && d != dir { + // if dir does not exist, Size() returns the error. + if d != dir && os.IsNotExist(err) { return nil } return err @@ -51,5 +52,5 @@ func Size(ctx context.Context, dir string) (size int64, err error) { return nil }) - return + return size, err } diff --git a/pkg/directory/directory_windows.go b/pkg/directory/directory_windows.go index f07f241880..fc72ff62e6 100644 --- a/pkg/directory/directory_windows.go +++ b/pkg/directory/directory_windows.go @@ -6,13 +6,14 @@ import ( "path/filepath" ) -// Size walks a directory tree and returns its total size in bytes. -func Size(ctx context.Context, dir string) (size int64, err error) { - err = filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { +// calcSize walks a directory tree and returns its total calcSize in bytes. +func calcSize(ctx context.Context, dir string) (int64, error) { + var size int64 + err := filepath.Walk(dir, func(d string, fileInfo os.FileInfo, err error) error { if err != nil { - // if dir does not exist, Size() returns the error. // if dir/x disappeared while walking, Size() ignores dir/x. - if os.IsNotExist(err) && d != dir { + // if dir does not exist, Size() returns the error. + if d != dir && os.IsNotExist(err) { return nil } return err @@ -38,5 +39,5 @@ func Size(ctx context.Context, dir string) (size int64, err error) { return nil }) - return + return size, err } From 26659d5eb83330269ef634713435a995caa1e2e6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 5 Oct 2022 11:11:26 +0200 Subject: [PATCH 2/2] pkg/directory: remove unused MoveToSubdir() utility This utility was added in 442b45628ee12ebd8e8bd08497896d5fa8eec4bd as part of user-namespaces, and first used in 44e1023a93a0107d63d5400695cbbc6da498a425 to set up the daemon root, and move the existing content; https://github.com/docker/docker/blob/44e1023a93a0107d63d5400695cbbc6da498a425/daemon/daemon_experimental.go#L68-L71 A later iteration no longer _moved_ the existing root directory, and removed the use of `directory.MoveToSubdir()` e8532023f20498e6eb1ce5c079dc8a09aeae3061 It looks like there's no external consumers of this utility, so we should be save to remove it. Signed-off-by: Sebastiaan van Stijn --- pkg/directory/directory.go | 24 +---------------- pkg/directory/directory_test.go | 48 --------------------------------- 2 files changed, 1 insertion(+), 71 deletions(-) diff --git a/pkg/directory/directory.go b/pkg/directory/directory.go index 998b93fd8c..7b8d74a356 100644 --- a/pkg/directory/directory.go +++ b/pkg/directory/directory.go @@ -1,28 +1,6 @@ package directory // import "github.com/docker/docker/pkg/directory" -import ( - "context" - "os" - "path/filepath" -) - -// MoveToSubdir moves all contents of a directory to a subdirectory underneath the original path -func MoveToSubdir(oldpath, subdir string) error { - infos, err := os.ReadDir(oldpath) - if err != nil { - return err - } - for _, info := range infos { - if info.Name() != subdir { - oldName := filepath.Join(oldpath, info.Name()) - newName := filepath.Join(oldpath, subdir, info.Name()) - if err := os.Rename(oldName, newName); err != nil { - return err - } - } - } - return nil -} +import "context" // Size walks a directory tree and returns its total size in bytes. func Size(ctx context.Context, dir string) (int64, error) { diff --git a/pkg/directory/directory_test.go b/pkg/directory/directory_test.go index ec9c97e699..3bfa1e0fd7 100644 --- a/pkg/directory/directory_test.go +++ b/pkg/directory/directory_test.go @@ -3,9 +3,6 @@ package directory // import "github.com/docker/docker/pkg/directory" import ( "context" "os" - "path/filepath" - "reflect" - "sort" "testing" ) @@ -144,51 +141,6 @@ func TestSizeFileAndNestedDirectoryNonempty(t *testing.T) { } } -// Test migration of directory to a subdir underneath itself -func TestMoveToSubdir(t *testing.T) { - var outerDir, subDir string - var err error - - if outerDir, err = os.MkdirTemp(os.TempDir(), "TestMoveToSubdir"); err != nil { - t.Fatalf("failed to create directory: %v", err) - } - - if subDir, err = os.MkdirTemp(outerDir, "testSub"); err != nil { - t.Fatalf("failed to create subdirectory: %v", err) - } - - // write 4 temp files in the outer dir to get moved - filesList := []string{"a", "b", "c", "d"} - for _, fName := range filesList { - if file, err := os.Create(filepath.Join(outerDir, fName)); err != nil { - t.Fatalf("couldn't create temp file %q: %v", fName, err) - } else { - file.WriteString(fName) - file.Close() - } - } - - if err = MoveToSubdir(outerDir, filepath.Base(subDir)); err != nil { - t.Fatalf("Error during migration of content to subdirectory: %v", err) - } - // validate that the files were moved to the subdirectory - infos, err := os.ReadDir(subDir) - if err != nil { - t.Fatal(err) - } - if len(infos) != 4 { - t.Fatalf("Should be four files in the subdir after the migration: actual length: %d", len(infos)) - } - var results []string - for _, info := range infos { - results = append(results, info.Name()) - } - sort.Strings(results) - if !reflect.DeepEqual(filesList, results) { - t.Fatalf("Results after migration do not equal list of files: expected: %v, got: %v", filesList, results) - } -} - // Test a non-existing directory func TestSizeNonExistingDirectory(t *testing.T) { if _, err := Size(context.Background(), "/thisdirectoryshouldnotexist/TestSizeNonExistingDirectory"); err == nil {