diff: rename differ to comparer

Remove combined interface and split implementations.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>

client.go

@@ -24,7 +24,6 @@ import (
 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/dialer"
-	"github.com/containerd/containerd/diff"
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/namespaces"
@@ -458,7 +457,7 @@ func (c *Client) ImageService() images.Store {
 }
 
 // DiffService returns the underlying Differ
-func (c *Client) DiffService() diff.DiffApplier {
+func (c *Client) DiffService() DiffService {
 	return NewDiffServiceFromClient(diffapi.NewDiffClient(c.conn))
 }
 

cmd/containerd/builtins.go

@@ -2,7 +2,7 @@ package main
 
 // register containerd builtins here
 import (
-	_ "github.com/containerd/containerd/diff/walking"
+	_ "github.com/containerd/containerd/diff/walking/plugin"
 	_ "github.com/containerd/containerd/gc/scheduler"
 	_ "github.com/containerd/containerd/services/containers"
 	_ "github.com/containerd/containerd/services/content"

cmd/ctr/commands/snapshots/snapshots.go

@@ -129,7 +129,7 @@ var diffCommand = cli.Command{
 		}
 
 		if idB == "" {
-			desc, err = rootfs.Diff(ctx, idA, snapshotter, client.DiffService(), opts...)
+			desc, err = rootfs.CreateDiff(ctx, idA, snapshotter, client.DiffService(), opts...)
 			if err != nil {
 				return err
 			}
@@ -145,7 +145,7 @@ var diffCommand = cli.Command{
 			if err != nil {
 				return err
 			}
-			desc, err = ds.DiffMounts(ctx, a, b, opts...)
+			desc, err = ds.Compare(ctx, a, b, opts...)
 			if err != nil {
 				return err
 			}

diff.go

@@ -1,17 +1,24 @@
 package containerd
 
 import (
+	"context"
+
 	diffapi "github.com/containerd/containerd/api/services/diff/v1"
 	"github.com/containerd/containerd/api/types"
 	"github.com/containerd/containerd/diff"
 	"github.com/containerd/containerd/mount"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"golang.org/x/net/context"
 )
 
+// DiffService handles the computation and application of diffs
+type DiffService interface {
+	diff.Comparer
+	diff.Applier
+}
+
 // NewDiffServiceFromClient returns a new diff service which communicates
 // over a GRPC connection.
-func NewDiffServiceFromClient(client diffapi.DiffClient) diff.DiffApplier {
+func NewDiffServiceFromClient(client diffapi.DiffClient) DiffService {
 	return &diffRemote{
 		client: client,
 	}
@@ -33,7 +40,7 @@ func (r *diffRemote) Apply(ctx context.Context, diff ocispec.Descriptor, mounts
 	return toDescriptor(resp.Applied), nil
 }
 
-func (r *diffRemote) Diff(ctx context.Context, a, b []mount.Mount, opts ...diff.Opt) (ocispec.Descriptor, error) {
+func (r *diffRemote) Compare(ctx context.Context, a, b []mount.Mount, opts ...diff.Opt) (ocispec.Descriptor, error) {
 	var config diff.Config
 	for _, opt := range opts {
 		if err := opt(&config); err != nil {

diff/apply/apply.go

@@ -0,0 +1,112 @@
+package apply
+
+import (
+	"context"
+	"io"
+	"io/ioutil"
+	"time"
+
+	"github.com/containerd/containerd/archive"
+	"github.com/containerd/containerd/archive/compression"
+	"github.com/containerd/containerd/content"
+	"github.com/containerd/containerd/diff"
+	"github.com/containerd/containerd/errdefs"
+	"github.com/containerd/containerd/images"
+	"github.com/containerd/containerd/log"
+	"github.com/containerd/containerd/mount"
+	digest "github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// NewFileSystemApplier returns an applier which simply mounts
+// and applies diff onto the mounted filesystem.
+func NewFileSystemApplier(cs content.Store) diff.Applier {
+	return &fsApplier{
+		store: cs,
+	}
+}
+
+type fsApplier struct {
+	store content.Store
+}
+
+var emptyDesc = ocispec.Descriptor{}
+
+// Apply applies the content associated with the provided digests onto the
+// provided mounts. Archive content will be extracted and decompressed if
+// necessary.
+func (s *fsApplier) Apply(ctx context.Context, desc ocispec.Descriptor, mounts []mount.Mount) (d ocispec.Descriptor, err error) {
+	t1 := time.Now()
+	defer func() {
+		if err == nil {
+			log.G(ctx).WithFields(logrus.Fields{
+				"d":     time.Now().Sub(t1),
+				"dgst":  desc.Digest,
+				"size":  desc.Size,
+				"media": desc.MediaType,
+			}).Debugf("diff applied")
+		}
+	}()
+
+	isCompressed, err := images.IsCompressedDiff(ctx, desc.MediaType)
+	if err != nil {
+		return emptyDesc, errors.Wrapf(errdefs.ErrNotImplemented, "unsupported diff media type: %v", desc.MediaType)
+	}
+
+	var ocidesc ocispec.Descriptor
+	if err := mount.WithTempMount(ctx, mounts, func(root string) error {
+		ra, err := s.store.ReaderAt(ctx, desc.Digest)
+		if err != nil {
+			return errors.Wrap(err, "failed to get reader from content store")
+		}
+		defer ra.Close()
+
+		r := content.NewReader(ra)
+		if isCompressed {
+			ds, err := compression.DecompressStream(r)
+			if err != nil {
+				return err
+			}
+			defer ds.Close()
+			r = ds
+		}
+
+		digester := digest.Canonical.Digester()
+		rc := &readCounter{
+			r: io.TeeReader(r, digester.Hash()),
+		}
+
+		if _, err := archive.Apply(ctx, root, rc); err != nil {
+			return err
+		}
+
+		// Read any trailing data
+		if _, err := io.Copy(ioutil.Discard, rc); err != nil {
+			return err
+		}
+
+		ocidesc = ocispec.Descriptor{
+			MediaType: ocispec.MediaTypeImageLayer,
+			Size:      rc.c,
+			Digest:    digester.Digest(),
+		}
+		return nil
+
+	}); err != nil {
+		return emptyDesc, err
+	}
+	return ocidesc, nil
+}
+
+type readCounter struct {
+	r io.Reader
+	c int64
+}
+
+func (rc *readCounter) Read(p []byte) (n int, err error) {
+	n, err = rc.r.Read(p)
+	rc.c += int64(n)
+	return
+}

diff/diff.go

@@ -1,9 +1,10 @@
 package diff
 
 import (
+	"context"
+
 	"github.com/containerd/containerd/mount"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"golang.org/x/net/context"
 )
 
 // Config is used to hold parameters needed for a diff operation
@@ -24,14 +25,14 @@ type Config struct {
 // Opt is used to configure a diff operation
 type Opt func(*Config) error
 
-// Differ allows creation of filesystem diffs between mounts
+// Comparer allows creation of filesystem diffs between mounts
-type Differ interface {
+type Comparer interface {
-	// Diff computes the difference between two mounts and returns a
+	// Compare computes the difference between two mounts and returns a
 	// descriptor for the computed diff. The options can provide
 	// a ref which can be used to track the content creation of the diff.
 	// The media type which is used to determine the format of the created
 	// content can also be provided as an option.
-	Diff(ctx context.Context, lower, upper []mount.Mount, opts ...Opt) (ocispec.Descriptor, error)
+	Compare(ctx context.Context, lower, upper []mount.Mount, opts ...Opt) (ocispec.Descriptor, error)
 }
 
 // Applier allows applying diffs between mounts
@@ -44,15 +45,6 @@ type Applier interface {
 	Apply(ctx context.Context, desc ocispec.Descriptor, mount []mount.Mount) (ocispec.Descriptor, error)
 }
 
-// DiffApplier is the interface that groups the basic Apply and Diff methods.
-//
-// golint says `type name will be used as diff.DiffApplier by other packages, and that stutters`,
-// but that can be ignored now.
-type DiffApplier interface { // nolint: golint
-	Applier
-	Differ
-}
-
 // WithMediaType sets the media type to use for creating the diff, without
 // specifying the differ will choose a default.
 func WithMediaType(m string) Opt {

diff/walking/differ.go

@@ -1,11 +1,11 @@
 package walking
 
 import (
+	"context"
 	"crypto/rand"
 	"encoding/base64"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"time"
 
 	"github.com/containerd/containerd/archive"
@@ -13,121 +13,34 @@ import (
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/diff"
 	"github.com/containerd/containerd/errdefs"
-	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/log"
-	"github.com/containerd/containerd/metadata"
 	"github.com/containerd/containerd/mount"
-	"github.com/containerd/containerd/platforms"
-	"github.com/containerd/containerd/plugin"
 	digest "github.com/opencontainers/go-digest"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
-	"golang.org/x/net/context"
 )
 
-func init() {
-	plugin.Register(&plugin.Registration{
-		Type: plugin.DiffPlugin,
-		ID:   "walking",
-		Requires: []plugin.Type{
-			plugin.MetadataPlugin,
-		},
-		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
-			md, err := ic.Get(plugin.MetadataPlugin)
-			if err != nil {
-				return nil, err
-			}
-
-			ic.Meta.Platforms = append(ic.Meta.Platforms, platforms.DefaultSpec())
-			return NewWalkingDiff(md.(*metadata.DB).ContentStore())
-		},
-	})
-}
-
 type walkingDiff struct {
 	store content.Store
 }
 
 var emptyDesc = ocispec.Descriptor{}
 
-// NewWalkingDiff is a generic implementation of diff.DiffApplier.
+// NewWalkingDiff is a generic implementation of diff.Comparer.  The diff is
-// NewWalkingDiff is expected to work with any filesystem.
+// calculated by mounting both the upper and lower mount sets and walking the
-func NewWalkingDiff(store content.Store) (diff.DiffApplier, error) {
+// mounted directories concurrently. Changes are calculated by comparing files
+// against each other or by comparing file existence between directories.
+// NewWalkingDiff uses no special characteristics of the mount sets and is
+// expected to work with any filesystem.
+func NewWalkingDiff(store content.Store) diff.Comparer {
 	return &walkingDiff{
 		store: store,
-	}, nil
+	}
 }
 
-// Apply applies the content associated with the provided digests onto the
+// Compare creates a diff between the given mounts and uploads the result
-// provided mounts. Archive content will be extracted and decompressed if
-// necessary.
-func (s *walkingDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts []mount.Mount) (d ocispec.Descriptor, err error) {
-	t1 := time.Now()
-	defer func() {
-		if err == nil {
-			log.G(ctx).WithFields(logrus.Fields{
-				"d":     time.Now().Sub(t1),
-				"dgst":  desc.Digest,
-				"size":  desc.Size,
-				"media": desc.MediaType,
-			}).Debugf("diff applied")
-		}
-	}()
-
-	isCompressed, err := images.IsCompressedDiff(ctx, desc.MediaType)
-	if err != nil {
-		return emptyDesc, errors.Wrapf(errdefs.ErrNotImplemented, "unsupported diff media type: %v", desc.MediaType)
-	}
-
-	var ocidesc ocispec.Descriptor
-	if err := mount.WithTempMount(ctx, mounts, func(root string) error {
-		ra, err := s.store.ReaderAt(ctx, desc.Digest)
-		if err != nil {
-			return errors.Wrap(err, "failed to get reader from content store")
-		}
-		defer ra.Close()
-
-		r := content.NewReader(ra)
-		if isCompressed {
-			ds, err := compression.DecompressStream(r)
-			if err != nil {
-				return err
-			}
-			defer ds.Close()
-			r = ds
-		}
-
-		digester := digest.Canonical.Digester()
-		rc := &readCounter{
-			r: io.TeeReader(r, digester.Hash()),
-		}
-
-		if _, err := archive.Apply(ctx, root, rc); err != nil {
-			return err
-		}
-
-		// Read any trailing data
-		if _, err := io.Copy(ioutil.Discard, rc); err != nil {
-			return err
-		}
-
-		ocidesc = ocispec.Descriptor{
-			MediaType: ocispec.MediaTypeImageLayer,
-			Size:      rc.c,
-			Digest:    digester.Digest(),
-		}
-		return nil
-
-	}); err != nil {
-		return emptyDesc, err
-	}
-	return ocidesc, nil
-}
-
-// Diff creates a diff between the given mounts and uploads the result
 // to the content store.
-func (s *walkingDiff) Diff(ctx context.Context, lower, upper []mount.Mount, opts ...diff.Opt) (d ocispec.Descriptor, err error) {
+func (s *walkingDiff) Compare(ctx context.Context, lower, upper []mount.Mount, opts ...diff.Opt) (d ocispec.Descriptor, err error) {
 	var config diff.Config
 	for _, opt := range opts {
 		if err := opt(&config); err != nil {
@@ -228,17 +141,6 @@ func (s *walkingDiff) Diff(ctx context.Context, lower, upper []mount.Mount, opts
 	return ocidesc, nil
 }
 
-type readCounter struct {
-	r io.Reader
-	c int64
-}
-
-func (rc *readCounter) Read(p []byte) (n int, err error) {
-	n, err = rc.r.Read(p)
-	rc.c += int64(n)
-	return
-}
-
 func uniqueRef() string {
 	t := time.Now()
 	var b [3]byte

diff/walking/plugin/plugin.go

@@ -0,0 +1,39 @@
+package plugin
+
+import (
+	"github.com/containerd/containerd/diff"
+	"github.com/containerd/containerd/diff/apply"
+	"github.com/containerd/containerd/diff/walking"
+	"github.com/containerd/containerd/metadata"
+	"github.com/containerd/containerd/platforms"
+	"github.com/containerd/containerd/plugin"
+)
+
+func init() {
+	plugin.Register(&plugin.Registration{
+		Type: plugin.DiffPlugin,
+		ID:   "walking",
+		Requires: []plugin.Type{
+			plugin.MetadataPlugin,
+		},
+		InitFn: func(ic *plugin.InitContext) (interface{}, error) {
+			md, err := ic.Get(plugin.MetadataPlugin)
+			if err != nil {
+				return nil, err
+			}
+
+			ic.Meta.Platforms = append(ic.Meta.Platforms, platforms.DefaultSpec())
+			cs := md.(*metadata.DB).ContentStore()
+
+			return diffPlugin{
+				Comparer: walking.NewWalkingDiff(cs),
+				Applier:  apply.NewFileSystemApplier(cs),
+			}, nil
+		},
+	})
+}
+
+type diffPlugin struct {
+	diff.Comparer
+	diff.Applier
+}

diff/windows/windows.go

@@ -3,6 +3,7 @@
 package windows
 
 import (
+	"context"
 	"io"
 	"io/ioutil"
 	"time"
@@ -23,7 +24,6 @@ import (
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"golang.org/x/net/context"
 )
 
 func init() {
@@ -45,15 +45,25 @@ func init() {
 	})
 }
 
+// CompareApplier handles both comparision and
+// application of layer diffs.
+type CompareApplier interface {
+	diff.Applier
+	diff.Comparer
+}
+
+// windowsDiff does filesystem comparison and application
+// for Windows specific layer diffs.
 type windowsDiff struct {
 	store content.Store
 }
 
 var emptyDesc = ocispec.Descriptor{}
 
-// NewWindowsDiff is the Windows container layer implementation of diff.Differ.
+// NewWindowsDiff is the Windows container layer implementation
-func NewWindowsDiff(store content.Store) (diff.Differ, error) {
+// for comparing and applying filesystem layers
-	return &windowsDiff{
+func NewWindowsDiff(store content.Store) (CompareApplier, error) {
+	return windowsDiff{
 		store: store,
 	}, nil
 }
@@ -61,7 +71,7 @@ func NewWindowsDiff(store content.Store) (diff.Differ, error) {
 // Apply applies the content associated with the provided digests onto the
 // provided mounts. Archive content will be extracted and decompressed if
 // necessary.
-func (s *windowsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts []mount.Mount) (d ocispec.Descriptor, err error) {
+func (s windowsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts []mount.Mount) (d ocispec.Descriptor, err error) {
 	t1 := time.Now()
 	defer func() {
 		if err == nil {
@@ -128,9 +138,9 @@ func (s *windowsDiff) Apply(ctx context.Context, desc ocispec.Descriptor, mounts
 	}, nil
 }
 
-// DiffMounts creates a diff between the given mounts and uploads the result
+// Compare creates a diff between the given mounts and uploads the result
 // to the content store.
-func (s *windowsDiff) DiffMounts(ctx context.Context, lower, upper []mount.Mount, opts ...diff.Opt) (d ocispec.Descriptor, err error) {
+func (s windowsDiff) Compare(ctx context.Context, lower, upper []mount.Mount, opts ...diff.Opt) (d ocispec.Descriptor, err error) {
 	return emptyDesc, errdefs.ErrNotImplemented
 }
 

rootfs/diff.go

@@ -10,11 +10,11 @@ import (
 	"golang.org/x/net/context"
 )
 
-// Diff creates a layer diff for the given snapshot identifier from the parent
+// CreateDiff creates a layer diff for the given snapshot identifier from the
-// of the snapshot. A content ref is provided to track the progress of the
+// parent of the snapshot. A content ref is provided to track the progress of
-// content creation and the provided snapshotter and mount differ are used
+// the content creation and the provided snapshotter and mount differ are used
 // for calculating the diff. The descriptor for the layer diff is returned.
-func Diff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter, d diff.Differ, opts ...diff.Opt) (ocispec.Descriptor, error) {
+func CreateDiff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter, d diff.Comparer, opts ...diff.Opt) (ocispec.Descriptor, error) {
 	info, err := sn.Stat(ctx, snapshotID)
 	if err != nil {
 		return ocispec.Descriptor{}, err
@@ -42,5 +42,5 @@ func Diff(ctx context.Context, snapshotID string, sn snapshots.Snapshotter, d di
 		defer sn.Remove(ctx, upperKey)
 	}
 
-	return d.Diff(ctx, lower, upper, opts...)
+	return d.Compare(ctx, lower, upper, opts...)
 }

services/diff/service.go

@@ -23,6 +23,11 @@ type config struct {
 	Order []string `toml:"default"`
 }
 
+type differ interface {
+	diff.Comparer
+	diff.Applier
+}
+
 func init() {
 	plugin.Register(&plugin.Registration{
 		Type: plugin.GRPCPlugin,
@@ -38,20 +43,20 @@ func init() {
 			}
 
 			orderedNames := ic.Config.(*config).Order
-			ordered := make([]diff.DiffApplier, len(orderedNames))
+			ordered := make([]differ, len(orderedNames))
 			for i, n := range orderedNames {
 				differp, ok := differs[n]
 				if !ok {
 					return nil, errors.Errorf("needed differ not loaded: %s", n)
 				}
-				differ, err := differp.Instance()
+				d, err := differp.Instance()
 				if err != nil {
 					return nil, errors.Wrapf(err, "could not load required differ due plugin init error: %s", n)
 				}
 
-				ordered[i], ok = differ.(diff.DiffApplier)
+				ordered[i], ok = d.(differ)
 				if !ok {
-					return nil, errors.Errorf("differ does not implement diff.DiffApplier interface: %s", n)
+					return nil, errors.Errorf("differ does not implement Comparer and Applier interface: %s", n)
 				}
 			}
 
@@ -63,7 +68,7 @@ func init() {
 }
 
 type service struct {
-	differs []diff.DiffApplier
+	differs []differ
 }
 
 func (s *service) Register(gs *grpc.Server) error {
@@ -115,8 +120,8 @@ func (s *service) Diff(ctx context.Context, dr *diffapi.DiffRequest) (*diffapi.D
 		opts = append(opts, diff.WithLabels(dr.Labels))
 	}
 
-	for _, differ := range s.differs {
+	for _, d := range s.differs {
-		ocidesc, err = differ.Diff(ctx, aMounts, bMounts, opts...)
+		ocidesc, err = d.Compare(ctx, aMounts, bMounts, opts...)
 		if !errdefs.IsNotImplemented(err) {
 			break
 		}

task.go

@@ -536,7 +536,7 @@ func (t *task) checkpointRWSnapshot(ctx context.Context, index *v1.Index, snapsh
 	opts := []diff.Opt{
 		diff.WithReference(fmt.Sprintf("checkpoint-rw-%s", id)),
 	}
-	rw, err := rootfs.Diff(ctx, id, t.client.SnapshotService(snapshotterName), t.client.DiffService(), opts...)
+	rw, err := rootfs.CreateDiff(ctx, id, t.client.SnapshotService(snapshotterName), t.client.DiffService(), opts...)
 	if err != nil {
 		return err
 	}