Move casdiff code to its own internal pkg

.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:

cmd/casdiff/casdiff.go

@@ -16,19 +16,15 @@ package main
 
 import (
 	"context"
-	"errors"
 	"fmt"
-	"io/fs"
+	"os"
 	"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 +103,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)
+	from, to := container.Arg(0), container.Arg(1)
+	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.Fprint(os.Stdout, mdiff.String(bufcasdiff.ManifestDiffOutputFormatText))
 	case formatMarkdown:
-		mdiff.printMarkdown()
+		fmt.Fprint(os.Stdout, mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown))
 	default:
-		return fmt.Errorf("format %s not supported", format.String())
+		return fmt.Errorf("format %s not supported", f.String())
 	}
 	return nil
 }

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,44 +31,27 @@ 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
 	}
 
 	result.output = fmt.Sprintf(
 		"```sh\n$ casdiff %s %s --format=markdown\n```\n\n%s",
 		transition.fromRef,
 		transition.toRef,
-		stdout.String(),
+		mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown),
 	)
 	return result
 }