Skip to content

Commit bc7a640

Browse files
committed
Show meaningful paths in kpt pkg diff output
Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
1 parent d08fb0b commit bc7a640

2 files changed

Lines changed: 110 additions & 4 deletions

File tree

pkg/lib/pkg/diff/diff.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package diff
1717

1818
import (
19+
"bytes"
1920
"context"
2021
"fmt"
2122
"io"
@@ -245,6 +246,7 @@ func (c *Command) DefaultValues() {
245246
DiffToolOpts: c.DiffToolOpts,
246247
Debug: c.Debug,
247248
Output: c.Output,
249+
LocalPath: c.Path,
248250
}
249251
}
250252
}
@@ -271,15 +273,19 @@ type defaultPkgDiffer struct {
271273
// Output is an io.Writer where command will write the output of the
272274
// command.
273275
Output io.Writer
276+
277+
// LocalPath is the original local package path on the user's filesystem.
278+
// Used to replace temp staging paths in diff output with the real path.
279+
LocalPath string
274280
}
275281

276282
func (d *defaultPkgDiffer) Diff(pkgs ...string) error {
277283
// add merge comments before comparing so that there are no unwanted diffs
278284
if err := addmergecomment.Process(pkgs...); err != nil {
279285
return err
280286
}
281-
for _, pkg := range pkgs {
282-
if err := d.prepareForDiff(pkg); err != nil {
287+
for _, pkgPath := range pkgs {
288+
if err := d.prepareForDiff(pkgPath); err != nil {
283289
return err
284290
}
285291
}
@@ -291,8 +297,11 @@ func (d *defaultPkgDiffer) Diff(pkgs ...string) error {
291297
args = pkgs
292298
}
293299
cmd := exec.Command(d.DiffTool, args...)
294-
cmd.Stdout = d.Output
295-
cmd.Stderr = d.Output
300+
301+
// Capture output so we can replace temp paths with meaningful ones
302+
var buf bytes.Buffer
303+
cmd.Stdout = &buf
304+
cmd.Stderr = &buf
296305

297306
if d.Debug {
298307
fmt.Fprintf(d.Output, "%s\n", strings.Join(cmd.Args, " "))
@@ -310,6 +319,21 @@ func (d *defaultPkgDiffer) Diff(pkgs ...string) error {
310319
fmt.Printf(exitCodeDiffWarning, d.DiffTool, d.DiffType)
311320
}
312321
}
322+
323+
// Replace temp staging paths with meaningful ones in the output.
324+
// The local staging path is replaced with the real filesystem path.
325+
// Remote staging paths are replaced with their semantic directory name.
326+
output := buf.String()
327+
for _, stagingPath := range pkgs {
328+
name := filepath.Base(stagingPath)
329+
if d.LocalPath != "" && strings.HasPrefix(name, LocalPackageSource+"-") {
330+
output = strings.ReplaceAll(output, stagingPath, d.LocalPath)
331+
} else {
332+
output = strings.ReplaceAll(output, stagingPath, name)
333+
}
334+
}
335+
fmt.Fprint(d.Output, output)
336+
313337
return err
314338
}
315339

pkg/lib/pkg/diff/diff_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,3 +503,85 @@ func TestStagingDirectoryNames(t *testing.T) {
503503
})
504504
}
505505
}
506+
507+
// TestCommand_DiffOutputPaths verifies that diff output shows meaningful paths
508+
// instead of raw temp directory paths (https://github.com/kptdev/kpt/issues/591).
509+
func TestCommand_DiffOutputPaths(t *testing.T) {
510+
g := &testutil.TestSetupManager{
511+
T: t,
512+
ReposChanges: map[string][]testutil.Content{
513+
testutil.Upstream: {
514+
{
515+
Pkg: pkgbuilder.NewRootPkg().
516+
WithResource(pkgbuilder.DeploymentResource),
517+
Branch: "main",
518+
Tag: "v1",
519+
},
520+
{
521+
Pkg: pkgbuilder.NewRootPkg().
522+
WithResource(pkgbuilder.DeploymentResource,
523+
pkgbuilder.SetFieldPath("5", "spec", "replicas")),
524+
},
525+
},
526+
},
527+
GetRef: "v1",
528+
LocalChanges: []testutil.Content{
529+
{
530+
Pkg: pkgbuilder.NewRootPkg().
531+
WithKptfile(
532+
pkgbuilder.NewKptfile().
533+
WithUpstreamRef(testutil.Upstream, "/", "v1", "resource-merge").
534+
WithUpstreamLockRef(testutil.Upstream, "/", "v1", 0),
535+
).
536+
WithResource(pkgbuilder.DeploymentResource,
537+
pkgbuilder.SetFieldPath("10", "spec", "replicas")),
538+
},
539+
},
540+
}
541+
defer g.Clean()
542+
if !g.Init() {
543+
return
544+
}
545+
546+
localPath := g.LocalWorkspace.FullPackagePath()
547+
548+
tests := []struct {
549+
name string
550+
diffType diff.Type
551+
diffTool string
552+
wantLocal bool
553+
wantLabels []string
554+
}{
555+
{"local", diff.TypeLocal, "diff", true, []string{"remote-v1/"}},
556+
{"remote", diff.TypeRemote, "diff", false, []string{"remote-v1/", "target-main/"}},
557+
{"combined", diff.TypeCombined, "diff", true, []string{"target-main/"}},
558+
{"3way", diff.Type3Way, "echo", true, []string{"remote-v1", "target-main"}},
559+
}
560+
561+
for _, tt := range tests {
562+
t.Run(tt.name, func(t *testing.T) {
563+
var out bytes.Buffer
564+
diffToolOpts := "-r -i -w"
565+
if tt.diffTool == "echo" {
566+
diffToolOpts = ""
567+
}
568+
err := (&diff.Command{
569+
Path: localPath,
570+
Ref: "main",
571+
DiffType: tt.diffType,
572+
DiffTool: tt.diffTool,
573+
DiffToolOpts: diffToolOpts,
574+
Output: &out,
575+
}).Run(fake.CtxWithDefaultPrinter())
576+
assert.NoError(t, err)
577+
578+
output := out.String()
579+
if tt.wantLocal {
580+
assert.Contains(t, output, localPath)
581+
}
582+
for _, label := range tt.wantLabels {
583+
assert.Contains(t, output, label)
584+
}
585+
})
586+
}
587+
}

0 commit comments

Comments
 (0)