From 9354ec3c1f21591097e5490063394cff2758e491 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 11:31:20 -0500 Subject: [PATCH 1/7] Moved manifest_diff to bufcasdiff in a new pkg --- .../bufcasdiff}/manifest_diff.go | 97 ++++++++++--------- .../bufcasdiff}/manifest_diff_test.go | 4 +- .../testdata/manifest_diff/README.md | 0 .../testdata/manifest_diff/from/changes.txt | 0 .../testdata/manifest_diff/from/to_remove.txt | 0 .../manifest_diff/from/to_rename_bar/1.txt | 0 .../manifest_diff/from/to_rename_bar/2.txt | 0 .../manifest_diff/from/to_rename_bar/3.txt | 0 .../manifest_diff/from/to_rename_foo/1.txt | 0 .../manifest_diff/from/to_rename_foo/2.txt | 0 .../manifest_diff/from/to_rename_foo/3.txt | 0 .../testdata/manifest_diff/to/added.txt | 0 .../testdata/manifest_diff/to/changes.txt | 0 .../manifest_diff/to/renamed_bar/1.txt | 0 .../manifest_diff/to/renamed_bar/2.txt | 0 .../manifest_diff/to/renamed_bar/3.txt | 0 .../manifest_diff/to/renamed_foo/1.txt | 0 .../manifest_diff/to/renamed_foo/2.txt | 0 .../manifest_diff/to/renamed_foo/3.txt | 0 19 files changed, 54 insertions(+), 47 deletions(-) rename {cmd/casdiff => internal/bufcasdiff}/manifest_diff.go (78%) rename {cmd/casdiff => internal/bufcasdiff}/manifest_diff_test.go (98%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/README.md (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/changes.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_remove.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_bar/1.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_bar/2.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_bar/3.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_foo/1.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_foo/2.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/from/to_rename_foo/3.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/added.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/changes.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_bar/1.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_bar/2.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_bar/3.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_foo/1.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_foo/2.txt (100%) rename {cmd/casdiff => internal/bufcasdiff}/testdata/manifest_diff/to/renamed_foo/3.txt (100%) diff --git a/cmd/casdiff/manifest_diff.go b/internal/bufcasdiff/manifest_diff.go similarity index 78% rename from cmd/casdiff/manifest_diff.go rename to internal/bufcasdiff/manifest_diff.go index cecd9e63..57d84648 100644 --- a/cmd/casdiff/manifest_diff.go +++ b/internal/bufcasdiff/manifest_diff.go @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -package main +package bufcasdiff import ( + "bytes" "context" "encoding/hex" "fmt" - "os" "buf.build/go/standard/xslices" "github.com/bufbuild/buf/private/pkg/cas" @@ -26,7 +26,7 @@ import ( "github.com/bufbuild/buf/private/pkg/storage" ) -type manifestDiff struct { +type ManifestDiff struct { pathsAdded map[string]cas.FileNode pathsRenamed map[string]fileDiff pathsRemoved map[string]cas.FileNode @@ -39,8 +39,8 @@ type fileDiff struct { diff string } -func newManifestDiff() *manifestDiff { - return &manifestDiff{ +func NewManifestDiff() *ManifestDiff { + return &ManifestDiff{ pathsAdded: make(map[string]cas.FileNode), pathsRenamed: make(map[string]fileDiff), pathsRemoved: make(map[string]cas.FileNode), @@ -48,14 +48,14 @@ func newManifestDiff() *manifestDiff { } } -func buildManifestDiff( +func BuildManifestDiff( ctx context.Context, from cas.Manifest, to cas.Manifest, //nolint:varnamelen // from/to used symmetrically bucket storage.ReadBucket, -) (*manifestDiff, error) { +) (*ManifestDiff, error) { var ( - diff = newManifestDiff() + diff = NewManifestDiff() digestToAddedPaths = make(map[string][]string) digestToRemovedPaths = make(map[string][]string) ) @@ -130,8 +130,11 @@ func buildManifestDiff( return diff, nil } -func (d *manifestDiff) printText() { - fmt.Fprintf(os.Stdout, +// FIXME: define a single `.String()` output with a list of formats. +func (d *ManifestDiff) AsText() string { + var b bytes.Buffer + fmt.Fprintf( + &b, "%d files changed: %d removed, %d renamed, %d added, %d changed content\n", len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), len(d.pathsRemoved), @@ -140,43 +143,46 @@ func (d *manifestDiff) printText() { len(d.pathsChangedContent), ) if len(d.pathsRemoved) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("Files removed:\n\n") + b.WriteString("\n") + b.WriteString("Files removed:\n\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRemoved) for _, path := range sortedPaths { - os.Stdout.WriteString("- " + d.pathsRemoved[path].String() + "\n") + b.WriteString("- " + d.pathsRemoved[path].String() + "\n") } } if len(d.pathsRenamed) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("Files renamed:\n\n") + b.WriteString("\n") + b.WriteString("Files renamed:\n\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRenamed) for _, path := range sortedPaths { - os.Stdout.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") - os.Stdout.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") + b.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") + b.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") } } if len(d.pathsAdded) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("Files added:\n\n") + b.WriteString("\n") + b.WriteString("Files added:\n\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsAdded) for _, path := range sortedPaths { - os.Stdout.WriteString("+ " + d.pathsAdded[path].String() + "\n") + b.WriteString("+ " + d.pathsAdded[path].String() + "\n") } } if len(d.pathsChangedContent) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("Files changed content:\n\n") + b.WriteString("\n") + b.WriteString("Files changed content:\n\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsChangedContent) for _, path := range sortedPaths { fnDiff := d.pathsChangedContent[path] - os.Stdout.WriteString(fnDiff.diff + "\n") + b.WriteString(fnDiff.diff + "\n") } } + return b.String() } -func (d *manifestDiff) printMarkdown() { - fmt.Fprintf(os.Stdout, +func (d *ManifestDiff) AsMarkdown() string { + var b bytes.Buffer + fmt.Fprintf( + &b, "> _%d files changed: %d removed, %d renamed, %d added, %d changed content_\n", len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), len(d.pathsRemoved), @@ -185,51 +191,52 @@ func (d *manifestDiff) printMarkdown() { len(d.pathsChangedContent), ) if len(d.pathsRemoved) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("# Files removed:\n\n") - os.Stdout.WriteString("```diff\n") + b.WriteString("\n") + b.WriteString("# Files removed:\n\n") + b.WriteString("```diff\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRemoved) for _, path := range sortedPaths { - os.Stdout.WriteString("- " + d.pathsRemoved[path].String() + "\n") + b.WriteString("- " + d.pathsRemoved[path].String() + "\n") } - os.Stdout.WriteString("```\n") + b.WriteString("```\n") } if len(d.pathsRenamed) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("# Files renamed:\n\n") - os.Stdout.WriteString("```diff\n") + b.WriteString("\n") + b.WriteString("# Files renamed:\n\n") + b.WriteString("```diff\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRenamed) for _, path := range sortedPaths { - os.Stdout.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") - os.Stdout.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") + b.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") + b.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") } - os.Stdout.WriteString("```\n") + b.WriteString("```\n") } if len(d.pathsAdded) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("# Files added:\n\n") - os.Stdout.WriteString("```diff\n") + b.WriteString("\n") + b.WriteString("# Files added:\n\n") + b.WriteString("```diff\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsAdded) for _, path := range sortedPaths { - os.Stdout.WriteString("+ " + d.pathsAdded[path].String() + "\n") + b.WriteString("+ " + d.pathsAdded[path].String() + "\n") } - os.Stdout.WriteString("```\n") + b.WriteString("```\n") } if len(d.pathsChangedContent) > 0 { - os.Stdout.WriteString("\n") - os.Stdout.WriteString("# Files changed content:\n\n") + b.WriteString("\n") + b.WriteString("# Files changed content:\n\n") sortedPaths := xslices.MapKeysToSortedSlice(d.pathsChangedContent) for _, path := range sortedPaths { fdiff := d.pathsChangedContent[path] // the path we use here can be from/to, is the same, what changed was the content. - os.Stdout.WriteString("## `" + fdiff.from.Path() + "`:\n") - os.Stdout.WriteString( + b.WriteString("## `" + fdiff.from.Path() + "`:\n") + b.WriteString( "```diff\n" + fdiff.diff + "\n" + "```\n", ) } } + return b.String() } func calculateFileNodeDiff( diff --git a/cmd/casdiff/manifest_diff_test.go b/internal/bufcasdiff/manifest_diff_test.go similarity index 98% rename from cmd/casdiff/manifest_diff_test.go rename to internal/bufcasdiff/manifest_diff_test.go index 2d7bf960..2f39ea21 100644 --- a/cmd/casdiff/manifest_diff_test.go +++ b/internal/bufcasdiff/manifest_diff_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package main +package bufcasdiff import ( "context" @@ -31,7 +31,7 @@ func TestManifestDiff(t *testing.T) { t.Parallel() ctx := t.Context() casBucket, mFrom, mTo := prepareDiffCASBucket(ctx, t) - mdiff, err := buildManifestDiff(ctx, mFrom, mTo, casBucket) + mdiff, err := BuildManifestDiff(ctx, mFrom, mTo, casBucket) require.NoError(t, err) require.NotNil(t, mdiff) t.Run("removed", func(t *testing.T) { diff --git a/cmd/casdiff/testdata/manifest_diff/README.md b/internal/bufcasdiff/testdata/manifest_diff/README.md similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/README.md rename to internal/bufcasdiff/testdata/manifest_diff/README.md diff --git a/cmd/casdiff/testdata/manifest_diff/from/changes.txt b/internal/bufcasdiff/testdata/manifest_diff/from/changes.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/changes.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/changes.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_remove.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_remove.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_remove.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_remove.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/1.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/1.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/2.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/2.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/3.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_bar/3.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/1.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/1.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/2.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/2.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt diff --git a/cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/3.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/from/to_rename_foo/3.txt rename to internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/added.txt b/internal/bufcasdiff/testdata/manifest_diff/to/added.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/added.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/added.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/changes.txt b/internal/bufcasdiff/testdata/manifest_diff/to/changes.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/changes.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/changes.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_bar/1.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_bar/1.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_bar/2.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_bar/2.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_bar/3.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_bar/3.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_foo/1.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_foo/1.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_foo/2.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_foo/2.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt diff --git a/cmd/casdiff/testdata/manifest_diff/to/renamed_foo/3.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt similarity index 100% rename from cmd/casdiff/testdata/manifest_diff/to/renamed_foo/3.txt rename to internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt From 3b7c581ad2d7a0e8b54637a23d0e1f09ced384b0 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 11:45:54 -0500 Subject: [PATCH 2/7] Move run logic --- cmd/casdiff/casdiff.go | 120 ++------------------ internal/bufcasdiff/bufcasdiff.go | 130 ++++++++++++++++++++++ internal/bufcasdiff/manifest_diff.go | 6 +- internal/bufcasdiff/manifest_diff_test.go | 2 +- 4 files changed, 142 insertions(+), 116 deletions(-) create mode 100644 internal/bufcasdiff/bufcasdiff.go diff --git a/cmd/casdiff/casdiff.go b/cmd/casdiff/casdiff.go index 74930a40..6997944e 100644 --- a/cmd/casdiff/casdiff.go +++ b/cmd/casdiff/casdiff.go @@ -16,19 +16,14 @@ package main import ( "context" - "errors" "fmt" - "io/fs" "strconv" "buf.build/go/app/appcmd" "buf.build/go/app/appext" "buf.build/go/standard/xslices" - "github.com/bufbuild/buf/private/pkg/cas" "github.com/bufbuild/buf/private/pkg/slogapp" - "github.com/bufbuild/buf/private/pkg/storage" - "github.com/bufbuild/buf/private/pkg/storage/storageos" - "github.com/bufbuild/modules/private/bufpkg/bufstate" + "github.com/bufbuild/modules/internal/bufcasdiff" "github.com/spf13/pflag" ) @@ -107,121 +102,22 @@ func run( container appext.Container, flags *flags, ) error { - format, ok := formatsNamesToValues[flags.format] + f, ok := formatsNamesToValues[flags.format] if !ok { return fmt.Errorf("unsupported format %s", flags.format) } from, to := container.Arg(0), container.Arg(1) //nolint:varnamelen // from/to used symmetrically - if from == to { - return printDiff(newManifestDiff(), format) - } - // first, attempt to match from/to as module references in a state file in the same directory - // where the command is run - bucket, err := storageos.NewProvider().NewReadWriteBucket(".") - if err != nil { - return fmt.Errorf("new rw bucket: %w", err) - } - moduleStateReader, err := bucket.Get(ctx, bufstate.ModStateFileName) - if err != nil { - if !errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("read module state file: %w", err) - } - // if the state file does not exist, we assume we are in the cas directory, and that from/to are - // the manifest paths - mdiff, err := calculateDiffFromCASDirectory(ctx, bucket, from, to) - if err != nil { - return fmt.Errorf("calculate cas diff: %w", err) - } - return printDiff(mdiff, format) - } - // state file was found, attempt to parse it and match from/to with its references - stateRW, err := bufstate.NewReadWriter() - if err != nil { - return fmt.Errorf("new state rw: %w", err) - } - moduleState, err := stateRW.ReadModStateFile(moduleStateReader) - if err != nil { - return fmt.Errorf("read module state: %w", err) - } - var ( - fromManifestPath string - toManifestPath string - ) - for _, ref := range moduleState.GetReferences() { - if ref.GetName() == from { - fromManifestPath = ref.GetDigest() - if toManifestPath != "" { - break - } - } else if ref.GetName() == to { - toManifestPath = ref.GetDigest() - if fromManifestPath != "" { - break - } - } - } - if fromManifestPath == "" { - return fmt.Errorf("from reference %s not found in the module state file", from) - } - if toManifestPath == "" { - return fmt.Errorf("to reference %s not found in the module state file", to) - } - if fromManifestPath == toManifestPath { - return printDiff(newManifestDiff(), format) - } - casBucket, err := storageos.NewProvider().NewReadWriteBucket("cas") - if err != nil { - return fmt.Errorf("new rw cas bucket: %w", err) - } - mdiff, err := calculateDiffFromCASDirectory(ctx, casBucket, fromManifestPath, toManifestPath) + mdiff, err := bufcasdiff.DiffModuleDirectory(ctx, ".", from, to) if err != nil { - return fmt.Errorf("calculate cas diff from state references: %w", err) + return fmt.Errorf("calculate diff: %w", err) } - return printDiff(mdiff, format) -} - -// calculateDiffFromCASDirectory takes the cas bucket, and the from/to manifest paths to calculate a -// diff. -func calculateDiffFromCASDirectory( - ctx context.Context, - casBucket storage.ReadBucket, - fromManifestPath string, - toManifestPath string, -) (*manifestDiff, error) { - if fromManifestPath == toManifestPath { - return newManifestDiff(), nil - } - fromManifest, err := readManifest(ctx, casBucket, fromManifestPath) - if err != nil { - return nil, fmt.Errorf("read manifest from: %w", err) - } - toManifest, err := readManifest(ctx, casBucket, toManifestPath) - if err != nil { - return nil, fmt.Errorf("read manifest to: %w", err) - } - return buildManifestDiff(ctx, fromManifest, toManifest, casBucket) -} - -func readManifest(ctx context.Context, bucket storage.ReadBucket, manifestPath string) (cas.Manifest, error) { - data, err := storage.ReadPath(ctx, bucket, manifestPath) - if err != nil { - return nil, fmt.Errorf("read path: %w", err) - } - m, err := cas.ParseManifest(string(data)) - if err != nil { - return nil, fmt.Errorf("parse manifest: %w", err) - } - return m, nil -} - -func printDiff(mdiff *manifestDiff, format format) error { - switch format { + switch f { case formatText: - mdiff.printText() + fmt.Print(mdiff.AsText()) case formatMarkdown: - mdiff.printMarkdown() + fmt.Print(mdiff.AsMarkdown()) default: - return fmt.Errorf("format %s not supported", format.String()) + return fmt.Errorf("format %s not supported", f.String()) } return nil } diff --git a/internal/bufcasdiff/bufcasdiff.go b/internal/bufcasdiff/bufcasdiff.go new file mode 100644 index 00000000..eb5a64b9 --- /dev/null +++ b/internal/bufcasdiff/bufcasdiff.go @@ -0,0 +1,130 @@ +// Copyright 2021-2025 Buf Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package bufcasdiff + +import ( + "context" + "errors" + "fmt" + "io/fs" + "path/filepath" + + "github.com/bufbuild/buf/private/pkg/cas" + "github.com/bufbuild/buf/private/pkg/storage" + "github.com/bufbuild/buf/private/pkg/storage/storageos" + "github.com/bufbuild/modules/private/bufpkg/bufstate" +) + +// DiffModuleDirectory computes the diff between two refs or two digests in the module directory at +// dirPath. +// +// If a state.json file is present, from/to are resolved as ref names against it, otherwise, dirPath +// is treated as a CAS directory and from/to are manifest filenames directly. +func DiffModuleDirectory( + ctx context.Context, + dirPath string, + from string, + to string, +) (*ManifestDiff, error) { + if from == to { + return newManifestDiff(), nil + } + bucket, err := storageos.NewProvider().NewReadWriteBucket(dirPath) + if err != nil { + return nil, fmt.Errorf("new rw bucket: %w", err) + } + moduleStateReader, err := bucket.Get(ctx, bufstate.ModStateFileName) + if err != nil { + if !errors.Is(err, fs.ErrNotExist) { + return nil, fmt.Errorf("read module state file: %w", err) + } + // No state.json — dirPath is a CAS directory and from/to are manifest filenames. + return calculateDiffFromCASDirectory(ctx, bucket, from, to) + } + // state file was found, attempt to parse it and match from/to with its references + stateRW, err := bufstate.NewReadWriter() + if err != nil { + return nil, fmt.Errorf("new state rw: %w", err) + } + moduleState, err := stateRW.ReadModStateFile(moduleStateReader) + if err != nil { + return nil, fmt.Errorf("read module state: %w", err) + } + var ( + fromManifestPath string + toManifestPath string + ) + for _, ref := range moduleState.GetReferences() { + if ref.GetName() == from { + fromManifestPath = ref.GetDigest() + if toManifestPath != "" { + break + } + } else if ref.GetName() == to { + toManifestPath = ref.GetDigest() + if fromManifestPath != "" { + break + } + } + } + if fromManifestPath == "" { + return nil, fmt.Errorf("from reference %s not found in the module state file", from) + } + if toManifestPath == "" { + return nil, fmt.Errorf("to reference %s not found in the module state file", to) + } + if fromManifestPath == toManifestPath { + return newManifestDiff(), nil + } + casBucket, err := storageos.NewProvider().NewReadWriteBucket(filepath.Join(dirPath, "cas")) + if err != nil { + return nil, fmt.Errorf("new rw cas bucket: %w", err) + } + return calculateDiffFromCASDirectory(ctx, casBucket, fromManifestPath, toManifestPath) +} + +// calculateDiffFromCASDirectory takes the cas bucket, and the from/to manifest paths to calculate a +// diff. +func calculateDiffFromCASDirectory( + ctx context.Context, + casBucket storage.ReadBucket, + fromManifestPath string, + toManifestPath string, +) (*ManifestDiff, error) { + if fromManifestPath == toManifestPath { + return newManifestDiff(), nil + } + fromManifest, err := readManifest(ctx, casBucket, fromManifestPath) + if err != nil { + return nil, fmt.Errorf("read manifest from: %w", err) + } + toManifest, err := readManifest(ctx, casBucket, toManifestPath) + if err != nil { + return nil, fmt.Errorf("read manifest to: %w", err) + } + return buildManifestDiff(ctx, fromManifest, toManifest, casBucket) +} + +func readManifest(ctx context.Context, bucket storage.ReadBucket, manifestPath string) (cas.Manifest, error) { + data, err := storage.ReadPath(ctx, bucket, manifestPath) + if err != nil { + return nil, fmt.Errorf("read path: %w", err) + } + m, err := cas.ParseManifest(string(data)) + if err != nil { + return nil, fmt.Errorf("parse manifest: %w", err) + } + return m, nil +} diff --git a/internal/bufcasdiff/manifest_diff.go b/internal/bufcasdiff/manifest_diff.go index 57d84648..3bb813f4 100644 --- a/internal/bufcasdiff/manifest_diff.go +++ b/internal/bufcasdiff/manifest_diff.go @@ -39,7 +39,7 @@ type fileDiff struct { diff string } -func NewManifestDiff() *ManifestDiff { +func newManifestDiff() *ManifestDiff { return &ManifestDiff{ pathsAdded: make(map[string]cas.FileNode), pathsRenamed: make(map[string]fileDiff), @@ -48,14 +48,14 @@ func NewManifestDiff() *ManifestDiff { } } -func BuildManifestDiff( +func buildManifestDiff( ctx context.Context, from cas.Manifest, to cas.Manifest, //nolint:varnamelen // from/to used symmetrically bucket storage.ReadBucket, ) (*ManifestDiff, error) { var ( - diff = NewManifestDiff() + diff = newManifestDiff() digestToAddedPaths = make(map[string][]string) digestToRemovedPaths = make(map[string][]string) ) diff --git a/internal/bufcasdiff/manifest_diff_test.go b/internal/bufcasdiff/manifest_diff_test.go index 2f39ea21..0ec11601 100644 --- a/internal/bufcasdiff/manifest_diff_test.go +++ b/internal/bufcasdiff/manifest_diff_test.go @@ -31,7 +31,7 @@ func TestManifestDiff(t *testing.T) { t.Parallel() ctx := t.Context() casBucket, mFrom, mTo := prepareDiffCASBucket(ctx, t) - mdiff, err := BuildManifestDiff(ctx, mFrom, mTo, casBucket) + mdiff, err := buildManifestDiff(ctx, mFrom, mTo, casBucket) require.NoError(t, err) require.NotNil(t, mdiff) t.Run("removed", func(t *testing.T) { From 512c38d98159f23e8d9ae8272abf4a4d74e39291 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 11:54:07 -0500 Subject: [PATCH 3/7] Merge output funcs --- cmd/casdiff/casdiff.go | 4 +- internal/bufcasdiff/manifest_diff.go | 155 ++++++++++------------ internal/bufcasdiff/manifest_diff_test.go | 33 +++++ 3 files changed, 107 insertions(+), 85 deletions(-) diff --git a/cmd/casdiff/casdiff.go b/cmd/casdiff/casdiff.go index 6997944e..21703bc3 100644 --- a/cmd/casdiff/casdiff.go +++ b/cmd/casdiff/casdiff.go @@ -113,9 +113,9 @@ func run( } switch f { case formatText: - fmt.Print(mdiff.AsText()) + fmt.Print(mdiff.String(bufcasdiff.ManifestDiffOutputFormatText)) case formatMarkdown: - fmt.Print(mdiff.AsMarkdown()) + fmt.Print(mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown)) default: return fmt.Errorf("format %s not supported", f.String()) } diff --git a/internal/bufcasdiff/manifest_diff.go b/internal/bufcasdiff/manifest_diff.go index 3bb813f4..b6b8c407 100644 --- a/internal/bufcasdiff/manifest_diff.go +++ b/internal/bufcasdiff/manifest_diff.go @@ -26,6 +26,15 @@ import ( "github.com/bufbuild/buf/private/pkg/storage" ) +// ManifestDiffOutputFormat is the format in which a manifest diff can output its results. +type ManifestDiffOutputFormat int + +const ( + ManifestDiffOutputFormatText = iota + 1 + ManifestDiffOutputFormatMarkdown +) + +// ManifestDiff represents a change in between two CAS manifests. type ManifestDiff struct { pathsAdded map[string]cas.FileNode pathsRenamed map[string]fileDiff @@ -130,110 +139,90 @@ func buildManifestDiff( return diff, nil } -// FIXME: define a single `.String()` output with a list of formats. -func (d *ManifestDiff) AsText() string { +// String returns the diff output in the given format. On invalid or unknown format, this function +// defaults to ManifestDiffOutputFormatText. +func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { var b bytes.Buffer - fmt.Fprintf( - &b, - "%d files changed: %d removed, %d renamed, %d added, %d changed content\n", - len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), - len(d.pathsRemoved), - len(d.pathsRenamed), - len(d.pathsAdded), - len(d.pathsChangedContent), - ) - if len(d.pathsRemoved) > 0 { - b.WriteString("\n") - b.WriteString("Files removed:\n\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRemoved) - for _, path := range sortedPaths { - b.WriteString("- " + d.pathsRemoved[path].String() + "\n") - } - } - if len(d.pathsRenamed) > 0 { - b.WriteString("\n") - b.WriteString("Files renamed:\n\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRenamed) - for _, path := range sortedPaths { - b.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") - b.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") - } - } - if len(d.pathsAdded) > 0 { - b.WriteString("\n") - b.WriteString("Files added:\n\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsAdded) - for _, path := range sortedPaths { - b.WriteString("+ " + d.pathsAdded[path].String() + "\n") - } - } - if len(d.pathsChangedContent) > 0 { - b.WriteString("\n") - b.WriteString("Files changed content:\n\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsChangedContent) - for _, path := range sortedPaths { - fnDiff := d.pathsChangedContent[path] - b.WriteString(fnDiff.diff + "\n") - } + isMarkdown := format == ManifestDiffOutputFormatMarkdown + if isMarkdown { + fmt.Fprintf( + &b, + "> _%d files changed: %d removed, %d renamed, %d added, %d changed content_\n", + len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), + len(d.pathsRemoved), + len(d.pathsRenamed), + len(d.pathsAdded), + len(d.pathsChangedContent), + ) + } else { + fmt.Fprintf( + &b, + "%d files changed: %d removed, %d renamed, %d added, %d changed content\n", + len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), + len(d.pathsRemoved), + len(d.pathsRenamed), + len(d.pathsAdded), + len(d.pathsChangedContent), + ) } - return b.String() -} - -func (d *ManifestDiff) AsMarkdown() string { - var b bytes.Buffer - fmt.Fprintf( - &b, - "> _%d files changed: %d removed, %d renamed, %d added, %d changed content_\n", - len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), - len(d.pathsRemoved), - len(d.pathsRenamed), - len(d.pathsAdded), - len(d.pathsChangedContent), - ) if len(d.pathsRemoved) > 0 { b.WriteString("\n") - b.WriteString("# Files removed:\n\n") - b.WriteString("```diff\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRemoved) - for _, path := range sortedPaths { + if isMarkdown { + b.WriteString("# Files removed:\n\n```diff\n") + } else { + b.WriteString("Files removed:\n\n") + } + for _, path := range xslices.MapKeysToSortedSlice(d.pathsRemoved) { b.WriteString("- " + d.pathsRemoved[path].String() + "\n") } - b.WriteString("```\n") + if isMarkdown { + b.WriteString("```\n") + } } if len(d.pathsRenamed) > 0 { b.WriteString("\n") - b.WriteString("# Files renamed:\n\n") - b.WriteString("```diff\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsRenamed) - for _, path := range sortedPaths { + if isMarkdown { + b.WriteString("# Files renamed:\n\n```diff\n") + } else { + b.WriteString("Files renamed:\n\n") + } + for _, path := range xslices.MapKeysToSortedSlice(d.pathsRenamed) { b.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") b.WriteString("+ " + d.pathsRenamed[path].to.String() + "\n") } - b.WriteString("```\n") + if isMarkdown { + b.WriteString("```\n") + } } if len(d.pathsAdded) > 0 { b.WriteString("\n") - b.WriteString("# Files added:\n\n") - b.WriteString("```diff\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsAdded) - for _, path := range sortedPaths { + if isMarkdown { + b.WriteString("# Files added:\n\n```diff\n") + } else { + b.WriteString("Files added:\n\n") + } + for _, path := range xslices.MapKeysToSortedSlice(d.pathsAdded) { b.WriteString("+ " + d.pathsAdded[path].String() + "\n") } - b.WriteString("```\n") + if isMarkdown { + b.WriteString("```\n") + } } if len(d.pathsChangedContent) > 0 { b.WriteString("\n") - b.WriteString("# Files changed content:\n\n") - sortedPaths := xslices.MapKeysToSortedSlice(d.pathsChangedContent) - for _, path := range sortedPaths { + if isMarkdown { + b.WriteString("# Files changed content:\n\n") + } else { + b.WriteString("Files changed content:\n\n") + } + for _, path := range xslices.MapKeysToSortedSlice(d.pathsChangedContent) { fdiff := d.pathsChangedContent[path] - // the path we use here can be from/to, is the same, what changed was the content. - b.WriteString("## `" + fdiff.from.Path() + "`:\n") - b.WriteString( - "```diff\n" + - fdiff.diff + "\n" + - "```\n", - ) + if isMarkdown { + b.WriteString("## `" + fdiff.from.Path() + "`:\n") + b.WriteString("```diff\n" + fdiff.diff + "\n```\n") + } else { + b.WriteString(fdiff.diff + "\n") + } } } return b.String() diff --git a/internal/bufcasdiff/manifest_diff_test.go b/internal/bufcasdiff/manifest_diff_test.go index 0ec11601..fa15a8fd 100644 --- a/internal/bufcasdiff/manifest_diff_test.go +++ b/internal/bufcasdiff/manifest_diff_test.go @@ -108,6 +108,39 @@ func TestManifestDiff(t *testing.T) { }) } +func TestManifestDiffString(t *testing.T) { + t.Parallel() + ctx := t.Context() + casBucket, mFrom, mTo := prepareDiffCASBucket(ctx, t) + mdiff, err := buildManifestDiff(ctx, mFrom, mTo, casBucket) + require.NoError(t, err) + require.NotNil(t, mdiff) + + t.Run("text", func(t *testing.T) { + t.Parallel() + out := mdiff.String(ManifestDiffOutputFormatText) + assert.Contains(t, out, "files changed:") + assert.Contains(t, out, "Files removed:") + assert.Contains(t, out, "Files added:") + assert.Contains(t, out, "Files renamed:") + assert.Contains(t, out, "Files changed content:") + assert.NotContains(t, out, "```") + assert.NotContains(t, out, "> _") + }) + t.Run("markdown", func(t *testing.T) { + t.Parallel() + out := mdiff.String(ManifestDiffOutputFormatMarkdown) + assert.Contains(t, out, "> _") + assert.Contains(t, out, "files changed:") + assert.Contains(t, out, "# Files removed:") + assert.Contains(t, out, "# Files added:") + assert.Contains(t, out, "# Files renamed:") + assert.Contains(t, out, "# Files changed content:") + assert.Contains(t, out, "```diff") + assert.Contains(t, out, "```") + }) +} + func prepareDiffCASBucket(ctx context.Context, t *testing.T) ( storage.ReadBucket, cas.Manifest, From ad0753fb9a12af79c461fa01f1b6b7d79f88f2f4 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 12:11:48 -0500 Subject: [PATCH 4/7] Lint --- .golangci.yaml | 37 +++++++-------- cmd/casdiff/casdiff.go | 7 +-- internal/bufcasdiff/manifest_diff.go | 68 ++++++++++++++-------------- 3 files changed, 54 insertions(+), 58 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 720b2e12..183a02fb 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,21 +2,22 @@ version: "2" linters: default: all disable: - - cyclop # covered by gocyclo - - depguard # requires configuration for all non-stdlib deps - - exhaustruct # irrelevant for modules - - funlen # rely on code review to limit function length - - gocognit # dubious "cognitive overhead" quantification - - ireturn # "accept interfaces, return structs" isn't ironclad - - lll # don't want hard limits for line length - - maintidx # covered by gocyclo - - mnd # some unnamed constants are okay - - nlreturn # generous whitespace violates house style - - noinlineerr # inline error handling is idiomatic here - - testpackage # internal tests are fine - - wrapcheck # don't _always_ need to wrap errors - - wsl # generous whitespace violates house style - - wsl_v5 # generous whitespace violates house style + - cyclop # covered by gocyclo + - depguard # requires configuration for all non-stdlib deps + - exhaustruct # irrelevant for modules + - funlen # rely on code review to limit function length + - gocognit # dubious "cognitive overhead" quantification + - ireturn # "accept interfaces, return structs" isn't ironclad + - lll # don't want hard limits for line length + - maintidx # covered by gocyclo + - mnd # some unnamed constants are okay + - nlreturn # generous whitespace violates house style + - noinlineerr # inline error handling is idiomatic here + - testpackage # internal tests are fine + - varnamelen # too strict for common from/to and format f variables + - wrapcheck # don't _always_ need to wrap errors + - wsl # generous whitespace violates house style + - wsl_v5 # generous whitespace violates house style settings: errcheck: check-type-assertions: true @@ -32,12 +33,6 @@ linters: # temporary hacks, and use godox to prevent committing them. keywords: - FIXME - varnamelen: - ignore-decls: - - T any - - i int - - wg sync.WaitGroup - - tc testCase exclusions: generated: lax presets: diff --git a/cmd/casdiff/casdiff.go b/cmd/casdiff/casdiff.go index 21703bc3..f2497c99 100644 --- a/cmd/casdiff/casdiff.go +++ b/cmd/casdiff/casdiff.go @@ -17,6 +17,7 @@ package main import ( "context" "fmt" + "os" "strconv" "buf.build/go/app/appcmd" @@ -106,16 +107,16 @@ func run( if !ok { return fmt.Errorf("unsupported format %s", flags.format) } - from, to := container.Arg(0), container.Arg(1) //nolint:varnamelen // from/to used symmetrically + from, to := container.Arg(0), container.Arg(1) mdiff, err := bufcasdiff.DiffModuleDirectory(ctx, ".", from, to) if err != nil { return fmt.Errorf("calculate diff: %w", err) } switch f { case formatText: - fmt.Print(mdiff.String(bufcasdiff.ManifestDiffOutputFormatText)) + fmt.Fprint(os.Stdout, mdiff.String(bufcasdiff.ManifestDiffOutputFormatText)) case formatMarkdown: - fmt.Print(mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown)) + fmt.Fprint(os.Stdout, mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown)) default: return fmt.Errorf("format %s not supported", f.String()) } diff --git a/internal/bufcasdiff/manifest_diff.go b/internal/bufcasdiff/manifest_diff.go index b6b8c407..6ed91371 100644 --- a/internal/bufcasdiff/manifest_diff.go +++ b/internal/bufcasdiff/manifest_diff.go @@ -60,7 +60,7 @@ func newManifestDiff() *ManifestDiff { func buildManifestDiff( ctx context.Context, from cas.Manifest, - to cas.Manifest, //nolint:varnamelen // from/to used symmetrically + to cas.Manifest, bucket storage.ReadBucket, ) (*ManifestDiff, error) { var ( @@ -145,32 +145,25 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { var b bytes.Buffer isMarkdown := format == ManifestDiffOutputFormatMarkdown if isMarkdown { - fmt.Fprintf( - &b, - "> _%d files changed: %d removed, %d renamed, %d added, %d changed content_\n", - len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), - len(d.pathsRemoved), - len(d.pathsRenamed), - len(d.pathsAdded), - len(d.pathsChangedContent), - ) - } else { - fmt.Fprintf( - &b, - "%d files changed: %d removed, %d renamed, %d added, %d changed content\n", - len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), - len(d.pathsRemoved), - len(d.pathsRenamed), - len(d.pathsAdded), - len(d.pathsChangedContent), - ) + b.WriteString("> ") } + fmt.Fprintf( + &b, + "%d files changed: %d removed, %d renamed, %d added, %d changed content\n", + len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), + len(d.pathsRemoved), + len(d.pathsRenamed), + len(d.pathsAdded), + len(d.pathsChangedContent), + ) if len(d.pathsRemoved) > 0 { b.WriteString("\n") if isMarkdown { - b.WriteString("# Files removed:\n\n```diff\n") - } else { - b.WriteString("Files removed:\n\n") + b.WriteString("# ") + } + b.WriteString("Files removed:\n\n") + if isMarkdown { + b.WriteString("```diff\n") } for _, path := range xslices.MapKeysToSortedSlice(d.pathsRemoved) { b.WriteString("- " + d.pathsRemoved[path].String() + "\n") @@ -182,9 +175,11 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { if len(d.pathsRenamed) > 0 { b.WriteString("\n") if isMarkdown { - b.WriteString("# Files renamed:\n\n```diff\n") - } else { - b.WriteString("Files renamed:\n\n") + b.WriteString("# ") + } + b.WriteString("Files renamed:\n\n") + if isMarkdown { + b.WriteString("```diff\n") } for _, path := range xslices.MapKeysToSortedSlice(d.pathsRenamed) { b.WriteString("- " + d.pathsRenamed[path].from.String() + "\n") @@ -197,9 +192,11 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { if len(d.pathsAdded) > 0 { b.WriteString("\n") if isMarkdown { - b.WriteString("# Files added:\n\n```diff\n") - } else { - b.WriteString("Files added:\n\n") + b.WriteString("# ") + } + b.WriteString("Files added:\n\n") + if isMarkdown { + b.WriteString("```diff\n") } for _, path := range xslices.MapKeysToSortedSlice(d.pathsAdded) { b.WriteString("+ " + d.pathsAdded[path].String() + "\n") @@ -208,17 +205,20 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { b.WriteString("```\n") } } - if len(d.pathsChangedContent) > 0 { + if len(d.pathsChangedContent) > 0 { //nolint:nestif // Markdown vs text small differences. b.WriteString("\n") if isMarkdown { - b.WriteString("# Files changed content:\n\n") - } else { - b.WriteString("Files changed content:\n\n") + b.WriteString("# ") } + b.WriteString("Files changed content:\n\n") for _, path := range xslices.MapKeysToSortedSlice(d.pathsChangedContent) { fdiff := d.pathsChangedContent[path] if isMarkdown { b.WriteString("## `" + fdiff.from.Path() + "`:\n") + } else { + b.WriteString(fdiff.from.Path() + ":\n") + } + if isMarkdown { b.WriteString("```diff\n" + fdiff.diff + "\n```\n") } else { b.WriteString(fdiff.diff + "\n") @@ -231,7 +231,7 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { func calculateFileNodeDiff( ctx context.Context, from cas.FileNode, - to cas.FileNode, //nolint:varnamelen // from/to used symmetrically + to cas.FileNode, bucket storage.ReadBucket, ) (string, error) { if from.Path() == to.Path() && cas.DigestEqual(from.Digest(), to.Digest()) { From 3c1cc446ac3fa8d5c702158c5f5f1fc7642ed59b Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 12:20:52 -0500 Subject: [PATCH 5/7] Update output tests --- internal/bufcasdiff/manifest_diff_test.go | 57 +++++++++++-------- .../manifest_diff/from/to_rename_bar/1.txt | 2 +- .../manifest_diff/from/to_rename_bar/2.txt | 2 +- .../manifest_diff/from/to_rename_bar/3.txt | 2 +- .../manifest_diff/from/to_rename_foo/1.txt | 2 +- .../manifest_diff/from/to_rename_foo/2.txt | 2 +- .../manifest_diff/from/to_rename_foo/3.txt | 2 +- .../testdata/manifest_diff/markdown.golden.md | 42 ++++++++++++++ .../testdata/manifest_diff/text.golden.txt | 34 +++++++++++ .../manifest_diff/to/renamed_bar/1.txt | 2 +- .../manifest_diff/to/renamed_bar/2.txt | 2 +- .../manifest_diff/to/renamed_bar/3.txt | 2 +- .../manifest_diff/to/renamed_foo/1.txt | 2 +- .../manifest_diff/to/renamed_foo/2.txt | 2 +- .../manifest_diff/to/renamed_foo/3.txt | 2 +- 15 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md create mode 100644 internal/bufcasdiff/testdata/manifest_diff/text.golden.txt diff --git a/internal/bufcasdiff/manifest_diff_test.go b/internal/bufcasdiff/manifest_diff_test.go index fa15a8fd..21c57941 100644 --- a/internal/bufcasdiff/manifest_diff_test.go +++ b/internal/bufcasdiff/manifest_diff_test.go @@ -17,6 +17,9 @@ package bufcasdiff import ( "context" "encoding/hex" + "flag" + "os" + "path/filepath" "testing" "github.com/bufbuild/buf/private/pkg/cas" @@ -27,6 +30,8 @@ import ( "github.com/stretchr/testify/require" ) +var update = flag.Bool("update", false, "update golden test files") + func TestManifestDiff(t *testing.T) { t.Parallel() ctx := t.Context() @@ -116,29 +121,35 @@ func TestManifestDiffString(t *testing.T) { require.NoError(t, err) require.NotNil(t, mdiff) - t.Run("text", func(t *testing.T) { - t.Parallel() - out := mdiff.String(ManifestDiffOutputFormatText) - assert.Contains(t, out, "files changed:") - assert.Contains(t, out, "Files removed:") - assert.Contains(t, out, "Files added:") - assert.Contains(t, out, "Files renamed:") - assert.Contains(t, out, "Files changed content:") - assert.NotContains(t, out, "```") - assert.NotContains(t, out, "> _") - }) - t.Run("markdown", func(t *testing.T) { - t.Parallel() - out := mdiff.String(ManifestDiffOutputFormatMarkdown) - assert.Contains(t, out, "> _") - assert.Contains(t, out, "files changed:") - assert.Contains(t, out, "# Files removed:") - assert.Contains(t, out, "# Files added:") - assert.Contains(t, out, "# Files renamed:") - assert.Contains(t, out, "# Files changed content:") - assert.Contains(t, out, "```diff") - assert.Contains(t, out, "```") - }) + type testCase struct { + name string + format ManifestDiffOutputFormat + extension string + } + for _, tc := range []testCase{ + { + name: "text", + format: ManifestDiffOutputFormatText, + extension: ".txt", + }, + { + name: "markdown", + format: ManifestDiffOutputFormatMarkdown, + extension: ".md", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := mdiff.String(tc.format) + golden := filepath.Join("testdata", "manifest_diff", tc.name+".golden"+tc.extension) + if *update { + require.NoError(t, os.WriteFile(golden, []byte(got), 0600)) + } + want, err := os.ReadFile(golden) + require.NoError(t, err) + assert.Equal(t, string(want), got) + }) + } } func prepareDiffCASBucket(ctx context.Context, t *testing.T) ( diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt index ada65e61..d6768d64 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/1.txt @@ -1 +1 @@ -content bar to rename +content 1 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt index ada65e61..a3ee791f 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/2.txt @@ -1 +1 @@ -content bar to rename +content 2 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt index ada65e61..ddad5fbd 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_bar/3.txt @@ -1 +1 @@ -content bar to rename +content 3 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt index 479d73fd..102f504d 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/1.txt @@ -1 +1 @@ -content foo to rename +content 1 foo to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt index 479d73fd..4ff6ccbf 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/2.txt @@ -1 +1 @@ -content foo to rename +content 2 foo to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt index 479d73fd..987cb83d 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/from/to_rename_foo/3.txt @@ -1 +1 @@ -content foo to rename +content 3 foo to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md b/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md new file mode 100644 index 00000000..d6e53bff --- /dev/null +++ b/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md @@ -0,0 +1,42 @@ +> 9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content + +# Files removed: + +```diff +- shake256:c577f2df6a8f0a68975087beae771183dfe90008a052a86a6dc02bc3d1d958fc4d5e6ec94a884341073880d914088e015452ef0aa06e824618a1572cd2d95007 to_remove.txt +``` + +# Files renamed: + +```diff +- shake256:be106224d4fd69d388e9a2377c213c2e61e90ef1bee0358c3b9682f51aaad9bd8b91587c0e07651d02c7097cf3529456144db92051b19fc601454279aead75ea to_rename_bar/1.txt ++ shake256:be106224d4fd69d388e9a2377c213c2e61e90ef1bee0358c3b9682f51aaad9bd8b91587c0e07651d02c7097cf3529456144db92051b19fc601454279aead75ea renamed_bar/1.txt +- shake256:5f49a1c832bd2a18d6afd913355ddedaa3a99f781015dc5ffbeac3e6c13019f495a1a2b7b8f5b6074f87af6cb19666416da2f8052f429a971326a39816ed50bb to_rename_bar/2.txt ++ shake256:5f49a1c832bd2a18d6afd913355ddedaa3a99f781015dc5ffbeac3e6c13019f495a1a2b7b8f5b6074f87af6cb19666416da2f8052f429a971326a39816ed50bb renamed_bar/2.txt +- shake256:a05a325f8252642e5313a30246d3bb4f88da4a57b6388533eab4697889c9cb6ddb011c65b543c2f3ae540c0da407980c2dbbcfbaf5f1450edf472ed0f5fd168b to_rename_bar/3.txt ++ shake256:a05a325f8252642e5313a30246d3bb4f88da4a57b6388533eab4697889c9cb6ddb011c65b543c2f3ae540c0da407980c2dbbcfbaf5f1450edf472ed0f5fd168b renamed_bar/3.txt +- shake256:5e26886b08a3a92fc9bb6a07af17ea9b6b36d906a021557efc476658c6bf08925684b870851c5c7dc6683c5918b3b15b00223111ae2eb4517352f309f6950b4a to_rename_foo/1.txt ++ shake256:5e26886b08a3a92fc9bb6a07af17ea9b6b36d906a021557efc476658c6bf08925684b870851c5c7dc6683c5918b3b15b00223111ae2eb4517352f309f6950b4a renamed_foo/1.txt +- shake256:5f4b4c026a5f29c0823823d33469402424e197aa0c8d01bb111eef6cac172ade225116033d3b94b50b76292e42e95ff98e664902cabfbc958695a6600c36fd65 to_rename_foo/2.txt ++ shake256:5f4b4c026a5f29c0823823d33469402424e197aa0c8d01bb111eef6cac172ade225116033d3b94b50b76292e42e95ff98e664902cabfbc958695a6600c36fd65 renamed_foo/2.txt +- shake256:eb654b95971e4b90515a6456a6018b4f17b2e04fba3d3280e73c7b327be4505ef4a37416895941f3e4ec2aee83c75c37c27614654c1af313b054f37a7122895a to_rename_foo/3.txt ++ shake256:eb654b95971e4b90515a6456a6018b4f17b2e04fba3d3280e73c7b327be4505ef4a37416895941f3e4ec2aee83c75c37c27614654c1af313b054f37a7122895a renamed_foo/3.txt +``` + +# Files added: + +```diff ++ shake256:4b68d58714b638200c19c1f5dd421e5bfc90551775ee3e3c68953d3f7d3095938f2797403884abb21981f96cb771e3361374be819d3dfab83e1a303f8f2039dc added.txt +``` + +# Files changed content: + +## `changes.txt`: +```diff +--- shake256:497141e7e8fd76c38063d741b1c8acdece5f17b204f542326e4d4630cdc898640d8bf4578d5eedd508c6c2d2ebcf7894a872068f537ac4101582436bc4cf738e changes.txt ++++ shake256:315358331b5a9bb1f88cbb8a4675089d2d2574fd567d4ac42f38a23bde83fbbe5ecc928d196d747743cf201f66dc30a41ba02ea0a98bbd1230e60fe9525fece6 changes.txt +@@ -1 +1 @@ +-content to change ++content changed + +``` diff --git a/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt b/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt new file mode 100644 index 00000000..0b42b003 --- /dev/null +++ b/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt @@ -0,0 +1,34 @@ +9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content + +Files removed: + +- shake256:c577f2df6a8f0a68975087beae771183dfe90008a052a86a6dc02bc3d1d958fc4d5e6ec94a884341073880d914088e015452ef0aa06e824618a1572cd2d95007 to_remove.txt + +Files renamed: + +- shake256:be106224d4fd69d388e9a2377c213c2e61e90ef1bee0358c3b9682f51aaad9bd8b91587c0e07651d02c7097cf3529456144db92051b19fc601454279aead75ea to_rename_bar/1.txt ++ shake256:be106224d4fd69d388e9a2377c213c2e61e90ef1bee0358c3b9682f51aaad9bd8b91587c0e07651d02c7097cf3529456144db92051b19fc601454279aead75ea renamed_bar/1.txt +- shake256:5f49a1c832bd2a18d6afd913355ddedaa3a99f781015dc5ffbeac3e6c13019f495a1a2b7b8f5b6074f87af6cb19666416da2f8052f429a971326a39816ed50bb to_rename_bar/2.txt ++ shake256:5f49a1c832bd2a18d6afd913355ddedaa3a99f781015dc5ffbeac3e6c13019f495a1a2b7b8f5b6074f87af6cb19666416da2f8052f429a971326a39816ed50bb renamed_bar/2.txt +- shake256:a05a325f8252642e5313a30246d3bb4f88da4a57b6388533eab4697889c9cb6ddb011c65b543c2f3ae540c0da407980c2dbbcfbaf5f1450edf472ed0f5fd168b to_rename_bar/3.txt ++ shake256:a05a325f8252642e5313a30246d3bb4f88da4a57b6388533eab4697889c9cb6ddb011c65b543c2f3ae540c0da407980c2dbbcfbaf5f1450edf472ed0f5fd168b renamed_bar/3.txt +- shake256:5e26886b08a3a92fc9bb6a07af17ea9b6b36d906a021557efc476658c6bf08925684b870851c5c7dc6683c5918b3b15b00223111ae2eb4517352f309f6950b4a to_rename_foo/1.txt ++ shake256:5e26886b08a3a92fc9bb6a07af17ea9b6b36d906a021557efc476658c6bf08925684b870851c5c7dc6683c5918b3b15b00223111ae2eb4517352f309f6950b4a renamed_foo/1.txt +- shake256:5f4b4c026a5f29c0823823d33469402424e197aa0c8d01bb111eef6cac172ade225116033d3b94b50b76292e42e95ff98e664902cabfbc958695a6600c36fd65 to_rename_foo/2.txt ++ shake256:5f4b4c026a5f29c0823823d33469402424e197aa0c8d01bb111eef6cac172ade225116033d3b94b50b76292e42e95ff98e664902cabfbc958695a6600c36fd65 renamed_foo/2.txt +- shake256:eb654b95971e4b90515a6456a6018b4f17b2e04fba3d3280e73c7b327be4505ef4a37416895941f3e4ec2aee83c75c37c27614654c1af313b054f37a7122895a to_rename_foo/3.txt ++ shake256:eb654b95971e4b90515a6456a6018b4f17b2e04fba3d3280e73c7b327be4505ef4a37416895941f3e4ec2aee83c75c37c27614654c1af313b054f37a7122895a renamed_foo/3.txt + +Files added: + ++ shake256:4b68d58714b638200c19c1f5dd421e5bfc90551775ee3e3c68953d3f7d3095938f2797403884abb21981f96cb771e3361374be819d3dfab83e1a303f8f2039dc added.txt + +Files changed content: + +changes.txt: +--- shake256:497141e7e8fd76c38063d741b1c8acdece5f17b204f542326e4d4630cdc898640d8bf4578d5eedd508c6c2d2ebcf7894a872068f537ac4101582436bc4cf738e changes.txt ++++ shake256:315358331b5a9bb1f88cbb8a4675089d2d2574fd567d4ac42f38a23bde83fbbe5ecc928d196d747743cf201f66dc30a41ba02ea0a98bbd1230e60fe9525fece6 changes.txt +@@ -1 +1 @@ +-content to change ++content changed + diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt index ada65e61..d6768d64 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/1.txt @@ -1 +1 @@ -content bar to rename +content 1 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt index ada65e61..a3ee791f 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/2.txt @@ -1 +1 @@ -content bar to rename +content 2 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt index ada65e61..ddad5fbd 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_bar/3.txt @@ -1 +1 @@ -content bar to rename +content 3 bar to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt index 479d73fd..102f504d 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/1.txt @@ -1 +1 @@ -content foo to rename +content 1 foo to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt index 479d73fd..4ff6ccbf 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/2.txt @@ -1 +1 @@ -content foo to rename +content 2 foo to rename diff --git a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt index 479d73fd..987cb83d 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/to/renamed_foo/3.txt @@ -1 +1 @@ -content foo to rename +content 3 foo to rename From a1cfedff3bb29091204d3b534857d4ed55e2909b Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 12:34:42 -0500 Subject: [PATCH 6/7] Call casdiff pkg instead of shelling out --- cmd/commentprcasdiff/casdiff_runner.go | 35 +++++++------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index af666ac1..b9656942 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -15,13 +15,13 @@ package main import ( - "bytes" "context" "fmt" "os" - "os/exec" "path/filepath" "sync" + + "github.com/bufbuild/modules/internal/bufcasdiff" ) // casDiffResult contains the result of running casdiff for a transition. @@ -31,36 +31,19 @@ type casDiffResult struct { err error } -// runCASDiff executes casdiff command in the module directory. +// runCASDiff computes the CAS diff for a transition and returns its markdown output. func runCASDiff(ctx context.Context, transition stateTransition) casDiffResult { - result := casDiffResult{ - transition: transition, - } + result := casDiffResult{transition: transition} repoRoot, err := os.Getwd() if err != nil { result.err = fmt.Errorf("get working directory: %w", err) return result } - - // Run casdiff in the module directory. casdiff reads state.json from "." so it must run from the - // module directory. We use an absolute path to the package to avoid path resolution issues when - // cmd.Dir is set. - cmd := exec.CommandContext( //nolint:gosec - ctx, - "go", "run", filepath.Join(repoRoot, "cmd", "casdiff"), - transition.fromRef, - transition.toRef, - "--format=markdown", - ) - cmd.Dir = filepath.Join(repoRoot, transition.modulePath) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - result.err = fmt.Errorf("casdiff failed: %w (stderr: %s)", err, stderr.String()) + moduleDirPath := filepath.Join(repoRoot, transition.modulePath) + mdiff, err := bufcasdiff.DiffModuleDirectory(ctx, moduleDirPath, transition.fromRef, transition.toRef) + if err != nil { + result.err = fmt.Errorf("calculate casdiff: %w", err) return result } @@ -68,7 +51,7 @@ func runCASDiff(ctx context.Context, transition stateTransition) casDiffResult { "```sh\n$ casdiff %s %s --format=markdown\n```\n\n%s", transition.fromRef, transition.toRef, - stdout.String(), + mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown), ) return result } From 773129de88ef9a8034c86e21381b4054b4022432 Mon Sep 17 00:00:00 2001 From: Julian Figueroa Date: Mon, 30 Mar 2026 12:50:03 -0500 Subject: [PATCH 7/7] lint --- internal/bufcasdiff/manifest_diff_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/bufcasdiff/manifest_diff_test.go b/internal/bufcasdiff/manifest_diff_test.go index 21c57941..b8be0cc4 100644 --- a/internal/bufcasdiff/manifest_diff_test.go +++ b/internal/bufcasdiff/manifest_diff_test.go @@ -30,7 +30,7 @@ import ( "github.com/stretchr/testify/require" ) -var update = flag.Bool("update", false, "update golden test files") +var update = flag.Bool("update", false, "update golden test files") //nolint:gochecknoglobals // only used for testing when updating golden files. func TestManifestDiff(t *testing.T) { t.Parallel()