Fix support for dyff as external diff program

Makefile

@@ -57,7 +57,7 @@ GINKGO_FLAGS ?=
 TEST_FLAGS ?=
 
 .PHONY: test-e2e
-test-e2e: $(GINKGO) $(KUBECTL) ## Run e2e tests.
+test-e2e: $(GINKGO) $(KUBECTL) $(DYFF) ## Run e2e tests.
 	ginkgo run --timeout=10m --poll-progress-after=10s --poll-progress-interval=5s --randomize-all --randomize-suites --keep-going --show-node-events $(GINKGO_FLAGS) ./test/e2e/... -- $(TEST_FLAGS)
 
 ##@ Verification

README.md

@@ -88,13 +88,13 @@ comparing revisions 2 and 3 of deployment.apps/nginx
 
 The `k revisions diff` command uses `diff -u -N` to compare revisions by default.
 It also respects the `KUBECTL_EXTERNAL_DIFF` environment variable like the `kubectl diff` command.
-To get a nicer diff view, you can use one of these:
+To get a nicer diff output, you can use one of these:
 
 ```bash
-# Add color to the diff output
-k revisions diff deploy nginx | colordiff
-# Specify an external diff programm
-KUBECTL_EXTERNAL_DIFF="colordiff -u" k revisions diff deploy nginx
+# Use a colored external diff program
+export KUBECTL_EXTERNAL_DIFF="colordiff -u"
+# Use dyff as a rich diff program
+export KUBECTL_EXTERNAL_DIFF="dyff between --omit-header"
 # Show diff in VS Code
-KUBECTL_EXTERNAL_DIFF="code --diff --wait" k revisions diff deploy nginx
+export KUBECTL_EXTERNAL_DIFF="code --diff --wait"
 ```

hack/tools.mk

@@ -18,6 +18,12 @@ $(TOOLS_BIN_DIR)/.version_%:
 	@version_file=$@; rm -f $${version_file%_*}*
 	@touch $@
 
+DYFF := $(TOOLS_BIN_DIR)/dyff
+# renovate: datasource=github-releases depName=homeport/dyff
+DYFF_VERSION ?= 1.7.1
+$(DYFF): $(call tool_version_file,$(DYFF),$(DYFF_VERSION))
+	curl -sSfL https://github.com/homeport/dyff/releases/download/v$(DYFF_VERSION)/dyff_$(DYFF_VERSION)_$(shell uname -s | tr '[:upper:]' '[:lower:]')_$(shell uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/').tar.gz | tar -xzmf - -C $(TOOLS_BIN_DIR) dyff
+
 GINKGO := $(TOOLS_BIN_DIR)/ginkgo
 GINKGO_VERSION ?= $(call version_gomod,github.com/onsi/ginkgo/v2)
 $(GINKGO): $(call tool_version_file,$(GINKGO),$(GINKGO_VERSION))

pkg/cmd/diff/diff.go

@@ -77,12 +77,12 @@ files as empty) options.`,
   # Compare the previous revision and the revision before that
   kubectl revisions diff deploy nginx --revision=-2
 
-  # Add color to the diff output
-  kubectl revisions diff deploy nginx | colordiff
-
-  # Specify an external diff programm
+  # Use a colored external diff program
   KUBECTL_EXTERNAL_DIFF="colordiff -u" kubectl revisions diff deploy nginx
 
+  # Use dyff as a rich diff program
+  KUBECTL_EXTERNAL_DIFF="dyff between --omit-header" kubectl revisions diff deploy nginx
+
   # Show diff in VS Code
   KUBECTL_EXTERNAL_DIFF="code --diff --wait" kubectl revisions diff deploy nginx
 `,
@@ -202,8 +202,9 @@ func (o *Options) Run(ctx context.Context, f util.Factory, args []string) (err e
 		return err
 	}
 
-	// prepare files for diff programm
-	files, err := diff.NewFiles(kindString+"_"+info.Name, ToFileName(a), ToFileName(b))
+	// prepare files for diff program
+	fileName := kindString + "." + info.Namespace + "." + info.Name
+	files, err := diff.NewFiles(ToDirName(a), ToDirName(b))
 	if err != nil {
 		return err
 	}
@@ -220,22 +221,15 @@ func (o *Options) Run(ctx context.Context, f util.Factory, args []string) (err e
 		return err
 	}
 
-	if err := p.PrintObj(a, files.A); err != nil {
-		return err
-	}
-	if err := files.A.Close(); err != nil {
-		return err
-	}
-
-	if err := p.PrintObj(b, files.B); err != nil {
+	if err := files.From.Print(fileName, a, p); err != nil {
 		return err
 	}
-	if err := files.B.Close(); err != nil {
+	if err := files.To.Print(fileName, b, p); err != nil {
 		return err
 	}
 
-	// run diff programm against prepared files
-	if err := o.Diff.Run(files.A.Name(), files.B.Name()); err != nil {
+	// run diff program against prepared files
+	if err := o.Diff.Run(files.From.Dir, files.To.Dir); err != nil {
 		// don't propagate exit status 1 (signaling a diff) upwards and exit cleanly instead
 		// there will always be a diff between revisions, there is no point in checking that
 		var exitError exec.ExitError
@@ -248,7 +242,7 @@ func (o *Options) Run(ctx context.Context, f util.Factory, args []string) (err e
 	return nil
 }
 
-// ToFileName returns a name for a file which the given revision should be written to.
-func ToFileName(rev history.Revision) string {
+// ToDirName returns a name for a directory which the given revision should be written to.
+func ToDirName(rev history.Revision) string {
 	return fmt.Sprintf("%d-%s", rev.Number(), rev.Name())
 }

pkg/diff/files.go

@@ -4,46 +4,83 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/hashicorp/go-multierror"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/cli-runtime/pkg/printers"
+
 	"github.com/timebertt/kubectl-revisions/pkg/runutil"
 )
 
-// Files is a compound handle for multiple files in a directory that shall be compared using a diff programm.
+// Files is a compound handle for multiple directories and files that shall be compared using a diff program.
+// This is similar to how `kubectl diff` works. This should behave similarly (e.g., create files in two different
+// directories) as some external diff tools might have some heuristic detections in places, e.g., see dyff:
+// https://github.com/homeport/dyff/blame/c382d5132c86d2280335f4cb71754ab20776a85a/internal/cmd/root.go#L85-L98
 type Files struct {
-	Dir  string
-	A, B *os.File
+	From, To *Version
 }
 
-// NewFiles creates a temporary directory and two files in it, and returns a Files handle.
-func NewFiles(dirNamePrefix, fileNameA, fileNameB string) (f *Files, err error) {
+// NewFiles creates two Version handles (i.e., two temporary directories).
+func NewFiles(from, to string) (f *Files, err error) {
 	f = &Files{}
 
-	f.Dir, err = os.MkdirTemp("", dirNamePrefix+"-")
-	if err != nil {
-		return nil, err
-	}
-
 	defer func() {
-		// if we weren't able to create both files, clean up the directory before returning the error
+		// if we weren't able to create both versions, clean up the leftovers before returning the error
 		if err != nil {
 			runutil.CaptureError(&err, f.TearDown)
 		}
 	}()
 
-	flags := os.O_WRONLY | os.O_CREATE | os.O_TRUNC
-	// nolint:gosec // no additional permissions given if file name escapes f.Dir
-	if f.A, err = os.OpenFile(filepath.Join(f.Dir, fileNameA), flags, 0600); err != nil {
+	f.From, err = NewVersion(from)
+	if err != nil {
 		return nil, err
 	}
-
-	// nolint:gosec // no additional permissions given if file name escapes f.Dir
-	if f.B, err = os.OpenFile(filepath.Join(f.Dir, fileNameB), flags, 0600); err != nil {
+	f.To, err = NewVersion(to)
+	if err != nil {
 		return nil, err
 	}
 
 	return f, nil
 }
 
-// TearDown removes the temporary directory held by this handle.
+// TearDown removes any temporary directories held by this handle.
 func (f Files) TearDown() error {
-	return os.RemoveAll(f.Dir)
+	return multierror.Append(nil, f.From.TearDown(), f.To.TearDown()).ErrorOrNil()
+}
+
+// Version is a handle for a directory that can hold multiple files of a single version to compare.
+type Version struct {
+	Dir string
+}
+
+// NewVersion creates a temporary directory.
+func NewVersion(name string) (*Version, error) {
+	dir, err := os.MkdirTemp("", name+"-")
+	if err != nil {
+		return nil, err
+	}
+
+	return &Version{
+		Dir: dir,
+	}, nil
+}
+
+// Print prints the given object using the given printer to a file with the specified name in Version.Dir.
+func (v *Version) Print(name string, obj runtime.Object, printer printers.ResourcePrinter) (err error) {
+	// nolint:gosec // no additional permissions given if file name escapes f.Dir
+	file, err := os.OpenFile(filepath.Join(v.Dir, name), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600)
+	if err != nil {
+		return err
+	}
+
+	defer runutil.CaptureError(&err, file.Close)
+	return printer.PrintObj(obj, file)
+}
+
+// TearDown removes the temporary directory held by this handle.
+func (v *Version) TearDown() error {
+	if v == nil {
+		return nil
+	}
+
+	return os.RemoveAll(v.Dir)
 }

pkg/diff/files_test.go

@@ -1,41 +1,71 @@
 package diff_test
 
 import (
-	"io"
 	"os"
 	"path/filepath"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	. "github.com/onsi/gomega/gbytes"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/cli-runtime/pkg/printers"
 
 	. "github.com/timebertt/kubectl-revisions/pkg/diff"
 )
 
-var _ = Describe("Files", func() {
-	It("should create a temp dir with two files", func() {
-		f, err := NewFiles("test", "a", "b")
+var _ = Describe("diff files", func() {
+	var (
+		f *Files
+	)
+
+	BeforeEach(func() {
+		var err error
+		f, err = NewFiles("a", "b")
 		Expect(err).NotTo(HaveOccurred())
+	})
 
-		DeferCleanup(func() {
-			Expect(f.TearDown()).To(Succeed())
-			Expect(f.Dir).NotTo(BeADirectory())
-			Expect(f.A.Name()).NotTo(BeARegularFile())
-			Expect(f.B.Name()).NotTo(BeARegularFile())
-		})
+	AfterEach(func() {
+		Expect(f.TearDown()).To(Succeed())
+	})
+
+	It("should create and tear down two temp dirs", func() {
+		Expect(f.From.Dir).To(BeADirectory())
+		Expect(f.To.Dir).To(BeADirectory())
+
+		Expect(f.TearDown()).To(Succeed())
+		Expect(f.From.Dir).NotTo(BeADirectory())
+		Expect(f.To.Dir).NotTo(BeADirectory())
+	})
 
-		Expect(f.Dir).NotTo(BeEmpty())
-		Expect(f.Dir).To(BeADirectory())
+	It("should print the given object", func() {
+		name := "foo"
+		fileName := filepath.Join(f.From.Dir, name)
 
-		testDiffFile := func(file *os.File, name string) {
-			ExpectWithOffset(1, file).NotTo(BeNil())
-			ExpectWithOffset(1, file.Name()).To(BeARegularFile())
-			ExpectWithOffset(1, filepath.Dir(file.Name())).To(Equal(f.Dir))
-			ExpectWithOffset(1, filepath.Base(file.Name())).To(Equal(name))
+		obj := &corev1.Namespace{}
+		obj.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Namespace"))
 
-			Expect(io.WriteString(file, "test "+name)).NotTo(BeZero())
-		}
+		Expect(f.From.Print(name, obj, &printers.YAMLPrinter{})).To(Succeed())
+		Expect(fileName).To(BeARegularFile())
 
-		testDiffFile(f.A, "a")
-		testDiffFile(f.B, "b")
+		// nolint:gosec // this is test code
+		bytes, err := os.ReadFile(fileName)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(BufferWithBytes(bytes)).To(Say("kind: Namespace"))
+	})
+
+	Describe("TearDown", func() {
+		It("should handle partial creation error", func() {
+			Expect(f.From.Dir).To(BeADirectory())
+			Expect(f.To.Dir).To(BeADirectory())
+
+			// simulate partial creation error: we failed to create the second version
+			Expect(os.RemoveAll(f.To.Dir)).To(Succeed())
+			Expect(f.To.Dir).NotTo(BeADirectory())
+			f.To = nil
+
+			// should delete all directories
+			Expect(f.TearDown()).To(Succeed())
+			Expect(f.From.Dir).NotTo(BeADirectory())
+		})
 	})
 })

pkg/diff/program.go

@@ -6,9 +6,9 @@ import (
 	"k8s.io/utils/exec"
 )
 
-// Program is a diff programm that compares two files.
+// Program is a diff program that compares two files.
 type Program interface {
-	// Run executes the diff programm to compare the given files.
+	// Run executes the diff program to compare the given files.
 	Run(a, b string) error
 }