errors: drop remaining errors.Wrap-like fmtcompat.Errorf users

The remaining users of fmtcompat.Errorf are all something resembling

  err := someFunc(...)
  return fmtcompat.Errorf("...: %w", err)

which can easily be rewritten to be more standard Go and doing explicit
error checks. This removes the final users of fmtcompat.Errorf.

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

cmd/umoci/gc.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/opencontainers/umoci/oci/cas/dir"
 	"github.com/opencontainers/umoci/oci/casext"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/urfave/cli"
 )
 
@@ -67,5 +66,8 @@ func gc(ctx *cli.Context) error {
 	defer engine.Close()
 
 	// Run the GC.
-	return fmtcompat.Errorf("gc: %w", engineExt.GC(context.Background()))
+	if err := engineExt.GC(context.Background()); err != nil {
+		return fmt.Errorf("gc: %w", err)
+	}
+	return nil
 }

oci/casext/blob.go

@@ -26,7 +26,6 @@ import (
 
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/umoci/oci/casext/mediatype"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/system"
 )
 
@@ -75,11 +74,11 @@ func (e Engine) FromDescriptor(ctx context.Context, descriptor ispec.Descriptor)
 
 	if fn := mediatype.GetParser(descriptor.MediaType); fn != nil {
 		defer func() {
-			if _, err := system.Copy(ioutil.Discard, reader); Err == nil {
-				Err = fmtcompat.Errorf("discard trailing %q blob: %w", descriptor.MediaType, err)
+			if _, err := system.Copy(ioutil.Discard, reader); Err == nil && err != nil {
+				Err = fmt.Errorf("discard trailing %q blob: %w", descriptor.MediaType, err)
 			}
-			if err := reader.Close(); Err == nil {
-				Err = fmtcompat.Errorf("close %q blob: %w", descriptor.MediaType, err)
+			if err := reader.Close(); Err == nil && err != nil {
+				Err = fmt.Errorf("close %q blob: %w", descriptor.MediaType, err)
 			}
 		}()
 

oci/casext/map.go

@@ -24,7 +24,6 @@ import (
 	"github.com/apex/log"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/umoci/oci/casext/mediatype"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 )
 
 // Used by walkState.mark() to determine which struct members are descriptors to
@@ -75,8 +74,10 @@ func mapDescriptors(V reflect.Value, mapFunc DescriptorMapFunc) error {
 		if V.IsNil() {
 			return nil
 		}
-		err := mapDescriptors(V.Elem(), mapFunc)
-		return fmtcompat.Errorf("%v: %w", V.Type(), err)
+		if err := mapDescriptors(V.Elem(), mapFunc); err != nil {
+			return fmt.Errorf("%v: %w", V.Type(), err)
+		}
+		return nil
 
 	case reflect.Slice, reflect.Array:
 		// Iterate over each element.

oci/layer/generate.go

@@ -26,7 +26,6 @@ import (
 	"sort"
 
 	"github.com/apex/log"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/unpriv"
 	"github.com/vbatts/go-mtree"
 )
@@ -55,11 +54,13 @@ func GenerateLayer(path string, deltas []mtree.InodeDelta, opt *RepackOptions) (
 	go func() (Err error) {
 		// Close with the returned error.
 		defer func() {
+			var closeErr error
 			if Err != nil {
 				log.Warnf("could not generate layer: %v", Err)
+				closeErr = fmt.Errorf("generate layer: %w", Err)
 			}
 			// #nosec G104
-			_ = writer.CloseWithError(fmtcompat.Errorf("generate layer: %w", Err))
+			_ = writer.CloseWithError(closeErr)
 		}()
 
 		// We can't just dump all of the file contents into a tar file. We need
@@ -138,11 +139,13 @@ func GenerateInsertLayer(root string, target string, opaque bool, opt *RepackOpt
 
 	go func() (Err error) {
 		defer func() {
+			var closeErr error
 			if Err != nil {
 				log.Warnf("could not generate insert layer: %v", Err)
+				closeErr = fmt.Errorf("generate insert layer: %w", Err)
 			}
 			// #nosec G104
-			_ = writer.CloseWithError(fmtcompat.Errorf("generate insert layer: %w", Err))
+			_ = writer.CloseWithError(closeErr)
 		}()
 
 		tg := newTarGenerator(writer, packOptions.MapOptions)

oci/layer/tar_extract.go

@@ -30,7 +30,6 @@ import (
 
 	"github.com/apex/log"
 	securejoin "github.com/cyphar/filepath-securejoin"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/fseval"
 	"github.com/opencontainers/umoci/pkg/system"
 	"github.com/opencontainers/umoci/third_party/shared"
@@ -321,7 +320,7 @@ func (te *TarExtractor) ociWhiteout(root string, dir string, file string) error
 	// we iterate over any children and try again. The only difference
 	// between opaque whiteouts and regular whiteouts is that we don't
 	// delete the directory itself with opaque whiteouts.
-	err := te.fsEval.Walk(path, func(subpath string, info os.FileInfo, err error) error {
+	if err := te.fsEval.Walk(path, func(subpath string, info os.FileInfo, err error) error {
 		// If we are passed an error, bail unless it's ENOENT.
 		if err != nil {
 			// If something was deleted outside of our knowledge it's not
@@ -352,24 +351,29 @@ func (te *TarExtractor) ociWhiteout(root string, dir string, file string) error
 			// Purge the path. We skip anything underneath (if it's a
 			// directory) since we just purged it -- and we don't want to
 			// hit ENOENT during iteration for no good reason.
-			err := fmtcompat.Errorf("whiteout subpath: %w", te.fsEval.RemoveAll(subpath))
-			if err == nil && info.IsDir() {
-				err = filepath.SkipDir
+			if err := te.fsEval.RemoveAll(subpath); err != nil {
+				return fmt.Errorf("whiteout subpath: %w", err)
+			}
+			if info.IsDir() {
+				return filepath.SkipDir
 			}
-			return err
 		}
 		return nil
-	})
-	return fmtcompat.Errorf("whiteout remove: %w", err)
+	}); err != nil {
+		return fmt.Errorf("whiteout remove: %w", err)
+	}
+	return nil
 }
 
 func (te *TarExtractor) overlayFSWhiteout(dir string, file string) error {
 	isOpaque := file == whOpaque
 
 	// if this is an opaque whiteout, whiteout the directory
 	if isOpaque {
-		err := te.fsEval.Lsetxattr(dir, "trusted.overlay.opaque", []byte("y"), 0)
-		return fmtcompat.Errorf("couldn't set overlayfs whiteout attr for %s: %w", dir, err)
+		if err := te.fsEval.Lsetxattr(dir, "trusted.overlay.opaque", []byte("y"), 0); err != nil {
+			return fmt.Errorf("couldn't set overlayfs whiteout attr for %s: %w", dir, err)
+		}
+		return nil
 	}
 
 	// otherwise, white out the file itself.
@@ -378,8 +382,10 @@ func (te *TarExtractor) overlayFSWhiteout(dir string, file string) error {
 		return fmt.Errorf("couldn't create overlayfs whiteout for %s: %w", p, err)
 	}
 
-	err := te.fsEval.Mknod(p, unix.S_IFCHR|0666, unix.Mkdev(0, 0))
-	return fmtcompat.Errorf("couldn't create overlayfs whiteout for %s: %w", p, err)
+	if err := te.fsEval.Mknod(p, unix.S_IFCHR|0666, unix.Mkdev(0, 0)); err != nil {
+		return fmt.Errorf("couldn't create overlayfs whiteout for %s: %w", p, err)
+	}
+	return nil
 }
 
 // UnpackEntry extracts the given tar.Header to the provided root, ensuring

oci/layer/tar_generate.go

@@ -27,7 +27,6 @@ import (
 	"strings"
 
 	"github.com/apex/log"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/fseval"
 	"github.com/opencontainers/umoci/pkg/system"
 	"github.com/opencontainers/umoci/pkg/testutils"
@@ -315,7 +314,10 @@ func (tg *tarGenerator) addWhiteout(name string, opaque bool) error {
 	}
 
 	// Add a dummy header for the whiteout file.
-	return fmtcompat.Errorf("write whiteout header: %w", tg.tw.WriteHeader(&tar.Header{Name: whiteout, Size: 0}))
+	if err := tg.tw.WriteHeader(&tar.Header{Name: whiteout, Size: 0}); err != nil {
+		return fmt.Errorf("write whiteout header: %w", err)
+	}
+	return nil
 }
 
 // AddWhiteout creates a whiteout for the provided path.

oci/layer/unpack.go

@@ -40,7 +40,6 @@ import (
 	"github.com/opencontainers/umoci/oci/cas"
 	"github.com/opencontainers/umoci/oci/casext"
 	iconv "github.com/opencontainers/umoci/oci/config/convert"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/fseval"
 	"github.com/opencontainers/umoci/pkg/idtools"
 	"github.com/opencontainers/umoci/pkg/system"
@@ -374,5 +373,8 @@ func UnpackRuntimeJSON(ctx context.Context, engine cas.Engine, configFile io.Wri
 	// Save the config.json.
 	enc := json.NewEncoder(configFile)
 	enc.SetIndent("", "\t")
-	return fmtcompat.Errorf("write config.json: %w", enc.Encode(spec))
+	if err := enc.Encode(spec); err != nil {
+		return fmt.Errorf("write config.json: %w", err)
+	}
+	return nil
 }

pkg/unpriv/unpriv.go

@@ -28,7 +28,6 @@ import (
 	"time"
 
 	securejoin "github.com/cyphar/filepath-securejoin"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/system"
 	"golang.org/x/sys/unix"
 )
@@ -143,7 +142,10 @@ func Open(path string) (*os.File, error) {
 		fh, err = os.Open(path)
 		return err
 	})
-	return fh, fmtcompat.Errorf("unpriv.open: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.open: %w", err)
+	}
+	return fh, nil
 }
 
 // Create is a wrapper around os.Create which has been wrapped with unpriv.Wrap
@@ -158,7 +160,10 @@ func Create(path string) (*os.File, error) {
 		fh, err = os.Create(path)
 		return err
 	})
-	return fh, fmtcompat.Errorf("unpriv.create: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.create: %w", err)
+	}
+	return fh, nil
 }
 
 // Readdir is a wrapper around (*os.File).Readdir which has been wrapper with
@@ -193,7 +198,10 @@ func Readdir(path string) ([]os.FileInfo, error) {
 		infos, err = fh.Readdir(-1)
 		return err
 	})
-	return infos, fmtcompat.Errorf("unpriv.readdir: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.readdir: %w", err)
+	}
+	return infos, nil
 }
 
 // Lstat is a wrapper around os.Lstat which has been wrapped with unpriv.Wrap
@@ -209,7 +217,10 @@ func Lstat(path string) (os.FileInfo, error) {
 		fi, err = os.Lstat(path)
 		return err
 	})
-	return fi, fmtcompat.Errorf("unpriv.lstat: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.lstat: %w", err)
+	}
+	return fi, nil
 }
 
 // Lstatx is like Lstat but uses unix.Lstat and returns unix.Stat_t instead
@@ -218,7 +229,10 @@ func Lstatx(path string) (unix.Stat_t, error) {
 	err := Wrap(path, func(path string) error {
 		return unix.Lstat(path, &s)
 	})
-	return s, fmtcompat.Errorf("unpriv.lstatx: %w", err)
+	if err != nil {
+		return s, fmt.Errorf("unpriv.lstatx: %w", err)
+	}
+	return s, nil
 }
 
 // Readlink is a wrapper around os.Readlink which has been wrapped with
@@ -234,7 +248,10 @@ func Readlink(path string) (string, error) {
 		target, err = os.Readlink(path)
 		return err
 	})
-	return target, fmtcompat.Errorf("unpriv.readlink: %w", err)
+	if err != nil {
+		return "", fmt.Errorf("unpriv.readlink: %w", err)
+	}
+	return target, nil
 }
 
 // Symlink is a wrapper around os.Symlink which has been wrapped with
@@ -500,7 +517,10 @@ func Llistxattr(path string) ([]string, error) {
 		xattrs, err = system.Llistxattr(path)
 		return err
 	})
-	return xattrs, fmtcompat.Errorf("unpriv.llistxattr: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.llistxattr: %w", err)
+	}
+	return xattrs, nil
 }
 
 // Lremovexattr is a wrapper around system.Lremovexattr which has been wrapped
@@ -535,7 +555,10 @@ func Lgetxattr(path, name string) ([]byte, error) {
 		value, err = system.Lgetxattr(path, name)
 		return err
 	})
-	return value, fmtcompat.Errorf("unpriv.lgetxattr: %w", err)
+	if err != nil {
+		return nil, fmt.Errorf("unpriv.lgetxattr: %w", err)
+	}
+	return value, nil
 }
 
 // Lclearxattrs is similar to system.Lclearxattrs but in order to implement it
@@ -616,9 +639,9 @@ func Walk(root string, walkFn filepath.WalkFunc) error {
 		} else {
 			err = walk(root, info, walkFn)
 		}
-		if !errors.Is(err, filepath.SkipDir) {
-			return fmtcompat.Errorf("unpriv.walk: %w", err)
+		if err == nil || errors.Is(err, filepath.SkipDir) {
+			return nil
 		}
-		return nil
+		return fmt.Errorf("unpriv.walk: %w", err)
 	})
 }

utils.go

@@ -34,7 +34,6 @@ import (
 	"github.com/opencontainers/umoci/oci/casext"
 	igen "github.com/opencontainers/umoci/oci/config/generate"
 	"github.com/opencontainers/umoci/oci/layer"
-	"github.com/opencontainers/umoci/pkg/fmtcompat"
 	"github.com/opencontainers/umoci/pkg/idtools"
 	"github.com/urfave/cli"
 	"github.com/vbatts/go-mtree"
@@ -110,8 +109,10 @@ func WriteBundleMeta(bundle string, meta Meta) error {
 	}
 	defer fh.Close()
 
-	_, err = meta.WriteTo(fh)
-	return fmtcompat.Errorf("write metadata: %w", err)
+	if _, err := meta.WriteTo(fh); err != nil {
+		return fmt.Errorf("write metadata: %w", err)
+	}
+	return nil
 }
 
 // ReadBundleMeta reads and parses the umoci.json file from a given bundle path.
@@ -130,7 +131,10 @@ func ReadBundleMeta(bundle string) (Meta, error) {
 			err = fmt.Errorf("unsupported umoci.json version: %s", meta.Version)
 		}
 	}
-	return meta, fmtcompat.Errorf("decode metadata: %w", err)
+	if err != nil {
+		return meta, fmt.Errorf("decode metadata: %w", err)
+	}
+	return meta, nil
 }
 
 // ManifestStat has information about a given OCI manifest.