directory: simplify newImageDestination

The current code is unnecessarily complicated:
 - it does unnecessary checks if a file/directory exists before opening it;
 - it uses three helper functions not used anywhere else;
 - directory contents is read twice.

Untangle all this, making the code simpler.

Signed-off-by: Kir Kolyshkin <[email protected]>

directory/directory_dest.go

@@ -1,6 +1,7 @@
 package directory
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -15,7 +16,6 @@ import (
 	"github.com/containers/image/v5/internal/putblobdigest"
 	"github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/types"
-	"github.com/containers/storage/pkg/fileutils"
 	"github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 )
@@ -54,49 +54,41 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (private.Im
 	// If directory exists check if it is empty
 	// if not empty, check whether the contents match that of a container image directory and overwrite the contents
 	// if the contents don't match throw an error
-	dirExists, err := pathExists(ref.resolvedPath)
-	if err != nil {
-		return nil, fmt.Errorf("checking for path %q: %w", ref.resolvedPath, err)
-	}
-	if dirExists {
-		isEmpty, err := isDirEmpty(ref.resolvedPath)
-		if err != nil {
+	dir, err := os.Open(ref.resolvedPath)
+	switch {
+	case err == nil: // Directory exists.
+		contents, err := dir.Readdirnames(-1)
+		_ = dir.Close()
+		if err != nil { // Unexpected error.
 			return nil, err
 		}
-
-		if !isEmpty {
-			versionExists, err := pathExists(ref.versionPath())
-			if err != nil {
-				return nil, fmt.Errorf("checking if path exists %q: %w", ref.versionPath(), err)
-			}
-			if versionExists {
-				contents, err := os.ReadFile(ref.versionPath())
-				if err != nil {
-					return nil, err
-				}
-				// check if contents of version file is what we expect it to be
-				if string(contents) != version {
-					return nil, ErrNotContainerImageDir
+		if len(contents) > 0 { // Not empty.
+			// Check if contents of version file is what we expect it to be.
+			ver, err := os.ReadFile(ref.versionPath())
+			if err == nil && bytes.Equal(ver, []byte(version)) {
+				// Indeed an image directory. Reuse by removing all the old contents
+				// (including the version file, which will be recreated below).
+				logrus.Debugf("overwriting existing container image directory %q", ref.resolvedPath)
+				for _, name := range contents {
+					if os.RemoveAll(filepath.Join(ref.resolvedPath, name)) != nil {
+						return nil, err
+					}
 				}
-			} else {
-				return nil, ErrNotContainerImageDir
-			}
-			// delete directory contents so that only one image is in the directory at a time
-			if err = removeDirContents(ref.resolvedPath); err != nil {
-				return nil, fmt.Errorf("erasing contents in %q: %w", ref.resolvedPath, err)
 			}
-			logrus.Debugf("overwriting existing container image directory %q", ref.resolvedPath)
 		}
-	} else {
-		// create directory if it doesn't exist
-		if err := os.MkdirAll(ref.resolvedPath, 0755); err != nil {
-			return nil, fmt.Errorf("unable to create directory %q: %w", ref.resolvedPath, err)
+	case errors.Is(err, os.ErrNotExist): // Directory does not exist; create it.
+		if err := os.MkdirAll(ref.resolvedPath, 0o755); err != nil {
+			return nil, err
 		}
+	default:
+		// Unexpected error.
+		return nil, err
 	}
-	// create version file
-	err = os.WriteFile(ref.versionPath(), []byte(version), 0644)
+
+	// Create version file.
+	err = os.WriteFile(ref.versionPath(), []byte(version), 0o644)
 	if err != nil {
-		return nil, fmt.Errorf("creating version file %q: %w", ref.versionPath(), err)
+		return nil, err
 	}
 
 	d := &dirImageDestination{
@@ -261,39 +253,3 @@ func (d *dirImageDestination) PutSignaturesWithFormat(ctx context.Context, signa
 func (d *dirImageDestination) Commit(context.Context, types.UnparsedImage) error {
 	return nil
 }
-
-// returns true if path exists
-func pathExists(path string) (bool, error) {
-	err := fileutils.Exists(path)
-	if err == nil {
-		return true, nil
-	}
-	if os.IsNotExist(err) {
-		return false, nil
-	}
-	return false, err
-}
-
-// returns true if directory is empty
-func isDirEmpty(path string) (bool, error) {
-	files, err := os.ReadDir(path)
-	if err != nil {
-		return false, err
-	}
-	return len(files) == 0, nil
-}
-
-// deletes the contents of a directory
-func removeDirContents(path string) error {
-	files, err := os.ReadDir(path)
-	if err != nil {
-		return err
-	}
-
-	for _, file := range files {
-		if err := os.RemoveAll(filepath.Join(path, file.Name())); err != nil {
-			return err
-		}
-	}
-	return nil
-}