diff --git a/.github/workflows/auto-casdiff-comment.yaml b/.github/workflows/auto-casdiff-comment.yaml index 96e83864..a0fef49a 100644 --- a/.github/workflows/auto-casdiff-comment.yaml +++ b/.github/workflows/auto-casdiff-comment.yaml @@ -6,6 +6,7 @@ on: branches: - main paths: + - modules/sync/state.json - modules/sync/*/*/state.json permissions: diff --git a/cmd/commentprcasdiff/casdiff_runner.go b/cmd/commentprcasdiff/casdiff_runner.go index b9656942..b33971cf 100644 --- a/cmd/commentprcasdiff/casdiff_runner.go +++ b/cmd/commentprcasdiff/casdiff_runner.go @@ -47,12 +47,18 @@ func runCASDiff(ctx context.Context, transition stateTransition) casDiffResult { return result } - result.output = fmt.Sprintf( - "```sh\n$ casdiff %s %s --format=markdown\n```\n\n%s", - transition.fromRef, - transition.toRef, - mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown), - ) + cmd := fmt.Sprintf("```sh\n$ casdiff %s \\\n %s \\\n --format=markdown\n```", transition.fromRef, transition.toRef) + diffOutput := mdiff.String(bufcasdiff.ManifestDiffOutputFormatMarkdown) + if transition.isOverallTransition { + result.output = "### Overall transition\n\n" + cmd + "\n\n" + diffOutput + } else { + result.output = fmt.Sprintf( + "**Intermediate transition**\n\n%s\n
%s\n

\n\n%s\n

\n
", + cmd, + mdiff.Summary(), + diffOutput, + ) + } return result } diff --git a/cmd/commentprcasdiff/main.go b/cmd/commentprcasdiff/main.go index 15fccf44..67e8493f 100644 --- a/cmd/commentprcasdiff/main.go +++ b/cmd/commentprcasdiff/main.go @@ -147,6 +147,17 @@ func run(ctx context.Context, flags *flags) error { } } + overallTransitions, err := getOverallTransitions(ctx, stateRW, baseRef, headRef) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to get overall transitions from global state: %v\n", err) + } else if len(overallTransitions) > 0 { + fmt.Fprintf(os.Stdout, "Found %d overall transition(s) in global state:\n", len(overallTransitions)) + for _, t := range overallTransitions { + fmt.Fprintf(os.Stdout, " %s: %s -> %s\n", strings.TrimPrefix(t.modulePath, "modules/sync/"), t.fromRef, t.toRef) + } + allTransitions = append(allTransitions, overallTransitions...) + } + if len(allTransitions) == 0 { fmt.Fprintf(os.Stdout, "No digest transitions found\n") return nil diff --git a/cmd/commentprcasdiff/state_analyzer.go b/cmd/commentprcasdiff/state_analyzer.go index b9ee220e..456e3e71 100644 --- a/cmd/commentprcasdiff/state_analyzer.go +++ b/cmd/commentprcasdiff/state_analyzer.go @@ -30,13 +30,14 @@ import ( // stateTransition represents a digest change in a module's state.json file. type stateTransition struct { - modulePath string // e.g., "modules/sync/bufbuild/protovalidate" - filePath string // e.g., "modules/sync/bufbuild/protovalidate/state.json" - fromRef string // Last reference with old digest (e.g., "v1.1.0") - toRef string // First reference with new digest (e.g., "v1.2.0") - fromDigest string // Old digest - toDigest string // New digest - lineNumber int // Line in diff where new digest first appears + modulePath string // e.g., "modules/sync/bufbuild/protovalidate" + filePath string // The module where the transition happened, can be an individual module like "modules/sync/bufbuild/protovalidate/state.json" or the global state file "modules/sync/state.json" + fromRef string // Old git reference (e.g., "v1.1.0") + toRef string // New git reference (e.g., "v1.2.0") + fromDigest string // Old digest + toDigest string // New digest + lineNumber int // Line in diff where the new reference or digest appears. + isOverallTransition bool // True for overall transitions on the global state.json file. } // getStateFileTransitions reads state.json from base and head branches, compares the JSON arrays to @@ -99,13 +100,14 @@ func getStateFileTransitions( lineNumber = lineNumbers[i] } transitions = append(transitions, stateTransition{ - modulePath: modulePath, - filePath: filePath, - fromRef: currentRef, - toRef: appendedRef.GetName(), - fromDigest: currentDigest, - toDigest: appendedRef.GetDigest(), - lineNumber: lineNumber, + modulePath: modulePath, + filePath: filePath, + fromRef: currentRef, + toRef: appendedRef.GetName(), + fromDigest: currentDigest, + toDigest: appendedRef.GetDigest(), + lineNumber: lineNumber, + isOverallTransition: false, }) currentDigest = appendedRef.GetDigest() } @@ -151,6 +153,90 @@ func resolveAppendedRefs( return baseRefs[len(baseRefs)-1], headRefs[len(baseRefs):] } +// getOverallTransitions reads modules/sync/state.json from both base and head, compares the +// two, and returns one stateTransition per module whose latest_reference changed. Modules that were +// added or removed between base and head are ignored. +func getOverallTransitions( + ctx context.Context, + stateRW *bufstate.ReadWriter, + baseRef string, + headRef string, +) ([]stateTransition, error) { + const globalStatePath = "modules/sync/state.json" + + baseContent, err := readFileAtRef(ctx, globalStatePath, baseRef) + if err != nil { + return nil, fmt.Errorf("read base global state: %w", err) + } + headContent, err := readFileAtRef(ctx, globalStatePath, headRef) + if err != nil { + return nil, fmt.Errorf("read head global state: %w", err) + } + + baseGlobalState, err := stateRW.ReadGlobalState(io.NopCloser(bytes.NewReader(baseContent))) + if err != nil { + return nil, fmt.Errorf("parse base global state: %w", err) + } + headGlobalState, err := stateRW.ReadGlobalState(io.NopCloser(bytes.NewReader(headContent))) + if err != nil { + return nil, fmt.Errorf("parse head global state: %w", err) + } + + baseLatestRefs := make(map[string]string, len(baseGlobalState.GetModules())) + for _, mod := range baseGlobalState.GetModules() { + baseLatestRefs[mod.GetModuleName()] = mod.GetLatestReference() + } + + var transitions []stateTransition + for _, mod := range headGlobalState.GetModules() { + moduleName := mod.GetModuleName() + toRef := mod.GetLatestReference() + fromRef, existsInBase := baseLatestRefs[moduleName] + if !existsInBase || fromRef == toRef { + continue // it is a new module, or the reference did not change. + } + lineNumber, err := findLatestReferenceLineInGlobalState(headContent, moduleName) + if err != nil { + return nil, fmt.Errorf("find line number for %q: %w", moduleName, err) + } + transitions = append(transitions, stateTransition{ + modulePath: "modules/sync/" + moduleName, + filePath: globalStatePath, + fromRef: fromRef, + toRef: toRef, + lineNumber: lineNumber, + isOverallTransition: true, + }) + } + return transitions, nil +} + +// findLatestReferenceLineInGlobalState scans the raw JSON of modules/sync/state.json and returns +// the 1-based line number of the "latest_reference" field for the given module. +func findLatestReferenceLineInGlobalState(content []byte, moduleName string) (int, error) { + quotedName := `"` + moduleName + `"` + scanner := bufio.NewScanner(bytes.NewReader(content)) + var ( + lineNum int + foundModule bool + ) + for scanner.Scan() { + lineNum++ + line := scanner.Text() + if !foundModule { + if strings.Contains(line, `"module_name"`) && strings.Contains(line, quotedName) { + foundModule = true + } + } else if strings.Contains(line, `"latest_reference"`) { + return lineNum, nil + } + } + if err := scanner.Err(); err != nil { + return 0, fmt.Errorf("scan global state: %w", err) + } + return 0, fmt.Errorf("latest_reference for module %q not found in global state", moduleName) +} + // readFileAtRef reads a file's content at a specific git ref using git show. func readFileAtRef(ctx context.Context, filePath string, ref string) ([]byte, error) { cmd := exec.CommandContext(ctx, "git", "show", fmt.Sprintf("%s:%s", ref, filePath)) //nolint:gosec diff --git a/internal/bufcasdiff/manifest_diff.go b/internal/bufcasdiff/manifest_diff.go index 6ed91371..befc0f38 100644 --- a/internal/bufcasdiff/manifest_diff.go +++ b/internal/bufcasdiff/manifest_diff.go @@ -19,6 +19,7 @@ import ( "context" "encoding/hex" "fmt" + "strings" "buf.build/go/standard/xslices" "github.com/bufbuild/buf/private/pkg/cas" @@ -139,6 +140,20 @@ func buildManifestDiff( return diff, nil } +// Summary returns a manifest diff summary in the shape of: +// +// %d files changed: %d removed, %d renamed, %d added, %d changed content. +func (d *ManifestDiff) Summary() string { + return fmt.Sprintf( + "%d files changed: %d removed, %d renamed, %d added, %d changed content.", + len(d.pathsRemoved)+len(d.pathsRenamed)+len(d.pathsAdded)+len(d.pathsChangedContent), + len(d.pathsRemoved), + len(d.pathsRenamed), + len(d.pathsAdded), + len(d.pathsChangedContent), + ) +} + // 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 { @@ -147,15 +162,7 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { if isMarkdown { 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), - ) + b.WriteString(d.Summary() + "\n") if len(d.pathsRemoved) > 0 { b.WriteString("\n") if isMarkdown { @@ -219,7 +226,7 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { b.WriteString(fdiff.from.Path() + ":\n") } if isMarkdown { - b.WriteString("```diff\n" + fdiff.diff + "\n```\n") + b.WriteString(markdownFencedDiff(fdiff.diff)) } else { b.WriteString(fdiff.diff + "\n") } @@ -228,6 +235,16 @@ func (d *ManifestDiff) String(format ManifestDiffOutputFormat) string { return b.String() } +// markdownFencedDiff wraps content in a ```diff code fence, using a longer fence if the content +// itself contains backtick runs that would break the fence. +func markdownFencedDiff(content string) string { + fence := "```" + for strings.Contains(content, fence) { + fence += "`" + } + return fence + "diff\n" + content + "\n" + fence + "\n" +} + func calculateFileNodeDiff( ctx context.Context, from cas.FileNode, diff --git a/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md b/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md index d6e53bff..30b4215c 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md +++ b/internal/bufcasdiff/testdata/manifest_diff/markdown.golden.md @@ -1,4 +1,4 @@ -> 9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content +> 9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content. # Files removed: diff --git a/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt b/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt index 0b42b003..8f11e8bc 100644 --- a/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt +++ b/internal/bufcasdiff/testdata/manifest_diff/text.golden.txt @@ -1,4 +1,4 @@ -9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content +9 files changed: 1 removed, 6 renamed, 1 added, 1 changed content. Files removed: