From 7b6f79faf49371418a0ad7376f6033e5a648b0ab Mon Sep 17 00:00:00 2001 From: Stefan VanBuren Date: Thu, 19 Feb 2026 09:52:41 -0500 Subject: [PATCH] Fix fetcher rendering empty fields In #2242, we added updating of deps, which involved yaml unmarshaling / remarshling. In the case of empty fields, we were adding those fields into the output. This reworks those updates to avoid the yaml marshal -> remarshal and instead just do string replacement, similar to the way this works in the rest of the fetcher, which avoids those empty fields. Noticed by @pkwarren. Ref: https://github.com/bufbuild/plugins/pull/2271/changes#diff-3f2a5e693e9637734da256045f6073b97b95a66e1b95c9051f8385f2b8d53b04R12-R22 --- internal/cmd/fetcher/main.go | 36 ++++++++++++----------------- internal/cmd/fetcher/main_test.go | 38 +++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/internal/cmd/fetcher/main.go b/internal/cmd/fetcher/main.go index ab948a3dc..327c18350 100644 --- a/internal/cmd/fetcher/main.go +++ b/internal/cmd/fetcher/main.go @@ -294,8 +294,8 @@ func runPluginTests(ctx context.Context, logger *slog.Logger, plugins []createdP } // updatePluginDeps updates plugin dependencies in a buf.plugin.yaml file to their latest versions. -// It parses the YAML content, finds any entries in the "deps:" section with "plugin:" fields, -// and updates them to use the latest available version from latestVersions map. +// It parses the YAML content to find deps entries, then uses text replacement to update +// version references in-place, preserving the original formatting and comments. // For example, if the YAML contains: // // deps: @@ -320,9 +320,10 @@ func updatePluginDeps(ctx context.Context, logger *slog.Logger, content []byte, return content, nil } - modified := false - for i := range config.Deps { - dep := &config.Deps[i] + // Use text replacement rather than re-marshaling the struct to avoid introducing + // empty fields from zero-value nested structs in ExternalConfig. + result := string(content) + for _, dep := range config.Deps { if dep.Plugin == "" { continue } @@ -334,27 +335,18 @@ func updatePluginDeps(ctx context.Context, logger *slog.Logger, content []byte, } // Look up the latest version for this plugin - if latestVersion, exists := latestVersions[pluginName]; exists && latestVersion != currentVersion { - oldPluginRef := dep.Plugin - newPluginRef := pluginName + ":" + latestVersion - dep.Plugin = newPluginRef - logger.InfoContext(ctx, "updating plugin dependency", slog.String("old", oldPluginRef), slog.String("new", newPluginRef)) - modified = true + latestVersion, exists := latestVersions[pluginName] + if !exists || latestVersion == currentVersion { + continue } - } - - if !modified { - // No changes made, return original content - return content, nil - } - // Marshal back to YAML - updatedContent, err := encoding.MarshalYAML(&config) - if err != nil { - return nil, fmt.Errorf("failed to marshal updated YAML: %w", err) + oldPluginRef := dep.Plugin + newPluginRef := pluginName + ":" + latestVersion + logger.InfoContext(ctx, "updating plugin dependency", slog.String("old", oldPluginRef), slog.String("new", newPluginRef)) + result = strings.ReplaceAll(result, oldPluginRef, newPluginRef) } - return updatedContent, nil + return []byte(result), nil } // pluginToCreate represents a plugin that needs a new version created. diff --git a/internal/cmd/fetcher/main_test.go b/internal/cmd/fetcher/main_test.go index e3d6b744c..231fd7667 100644 --- a/internal/cmd/fetcher/main_test.go +++ b/internal/cmd/fetcher/main_test.go @@ -103,6 +103,44 @@ plugin_version: v1.0.0 }, } + // Regression test: updating deps must not re-marshal the full config, which would + // introduce empty fields for zero-value nested structs and strip YAML comments. + t.Run("preserves formatting and comments without adding empty fields", func(t *testing.T) { + t.Parallel() + input := `version: v1 +name: buf.build/test/grpc-go +plugin_version: v1.5.1 +deps: + - plugin: buf.build/protocolbuffers/go:v1.35.0 +registry: + go: + # https://pkg.go.dev/google.golang.org/grpc + min_version: "1.21" + deps: + - module: google.golang.org/grpc + version: v1.70.0 + opts: + - paths=source_relative +` + latestVersions := map[string]string{ + "buf.build/protocolbuffers/go": "v1.36.11", + } + logger := slog.New(slog.NewTextHandler(testWriter{t}, &slog.HandlerOptions{Level: slog.LevelDebug})) + result, err := updatePluginDeps(t.Context(), logger, []byte(input), latestVersions) + require.NoError(t, err) + + output := string(result) + // Dep version should be updated. + assert.Contains(t, output, "buf.build/protocolbuffers/go:v1.36.11") + // Comment in the registry section should be preserved. + assert.Contains(t, output, "# https://pkg.go.dev/google.golang.org/grpc") + // No empty fields should have been introduced. + assert.NotContains(t, output, "npm:") + assert.NotContains(t, output, "maven:") + assert.NotContains(t, output, "source_url: \"\"") + assert.NotContains(t, output, "description: \"\"") + }) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel()