Skip to content

Commit 7b6f79f

Browse files
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
1 parent d86578c commit 7b6f79f

File tree

2 files changed

+52
-22
lines changed

2 files changed

+52
-22
lines changed

internal/cmd/fetcher/main.go

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,8 @@ func runPluginTests(ctx context.Context, logger *slog.Logger, plugins []createdP
294294
}
295295

296296
// updatePluginDeps updates plugin dependencies in a buf.plugin.yaml file to their latest versions.
297-
// It parses the YAML content, finds any entries in the "deps:" section with "plugin:" fields,
298-
// and updates them to use the latest available version from latestVersions map.
297+
// It parses the YAML content to find deps entries, then uses text replacement to update
298+
// version references in-place, preserving the original formatting and comments.
299299
// For example, if the YAML contains:
300300
//
301301
// deps:
@@ -320,9 +320,10 @@ func updatePluginDeps(ctx context.Context, logger *slog.Logger, content []byte,
320320
return content, nil
321321
}
322322

323-
modified := false
324-
for i := range config.Deps {
325-
dep := &config.Deps[i]
323+
// Use text replacement rather than re-marshaling the struct to avoid introducing
324+
// empty fields from zero-value nested structs in ExternalConfig.
325+
result := string(content)
326+
for _, dep := range config.Deps {
326327
if dep.Plugin == "" {
327328
continue
328329
}
@@ -334,27 +335,18 @@ func updatePluginDeps(ctx context.Context, logger *slog.Logger, content []byte,
334335
}
335336

336337
// Look up the latest version for this plugin
337-
if latestVersion, exists := latestVersions[pluginName]; exists && latestVersion != currentVersion {
338-
oldPluginRef := dep.Plugin
339-
newPluginRef := pluginName + ":" + latestVersion
340-
dep.Plugin = newPluginRef
341-
logger.InfoContext(ctx, "updating plugin dependency", slog.String("old", oldPluginRef), slog.String("new", newPluginRef))
342-
modified = true
338+
latestVersion, exists := latestVersions[pluginName]
339+
if !exists || latestVersion == currentVersion {
340+
continue
343341
}
344-
}
345-
346-
if !modified {
347-
// No changes made, return original content
348-
return content, nil
349-
}
350342

351-
// Marshal back to YAML
352-
updatedContent, err := encoding.MarshalYAML(&config)
353-
if err != nil {
354-
return nil, fmt.Errorf("failed to marshal updated YAML: %w", err)
343+
oldPluginRef := dep.Plugin
344+
newPluginRef := pluginName + ":" + latestVersion
345+
logger.InfoContext(ctx, "updating plugin dependency", slog.String("old", oldPluginRef), slog.String("new", newPluginRef))
346+
result = strings.ReplaceAll(result, oldPluginRef, newPluginRef)
355347
}
356348

357-
return updatedContent, nil
349+
return []byte(result), nil
358350
}
359351

360352
// pluginToCreate represents a plugin that needs a new version created.

internal/cmd/fetcher/main_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,44 @@ plugin_version: v1.0.0
103103
},
104104
}
105105

106+
// Regression test: updating deps must not re-marshal the full config, which would
107+
// introduce empty fields for zero-value nested structs and strip YAML comments.
108+
t.Run("preserves formatting and comments without adding empty fields", func(t *testing.T) {
109+
t.Parallel()
110+
input := `version: v1
111+
name: buf.build/test/grpc-go
112+
plugin_version: v1.5.1
113+
deps:
114+
- plugin: buf.build/protocolbuffers/go:v1.35.0
115+
registry:
116+
go:
117+
# https://pkg.go.dev/google.golang.org/grpc
118+
min_version: "1.21"
119+
deps:
120+
- module: google.golang.org/grpc
121+
version: v1.70.0
122+
opts:
123+
- paths=source_relative
124+
`
125+
latestVersions := map[string]string{
126+
"buf.build/protocolbuffers/go": "v1.36.11",
127+
}
128+
logger := slog.New(slog.NewTextHandler(testWriter{t}, &slog.HandlerOptions{Level: slog.LevelDebug}))
129+
result, err := updatePluginDeps(t.Context(), logger, []byte(input), latestVersions)
130+
require.NoError(t, err)
131+
132+
output := string(result)
133+
// Dep version should be updated.
134+
assert.Contains(t, output, "buf.build/protocolbuffers/go:v1.36.11")
135+
// Comment in the registry section should be preserved.
136+
assert.Contains(t, output, "# https://pkg.go.dev/google.golang.org/grpc")
137+
// No empty fields should have been introduced.
138+
assert.NotContains(t, output, "npm:")
139+
assert.NotContains(t, output, "maven:")
140+
assert.NotContains(t, output, "source_url: \"\"")
141+
assert.NotContains(t, output, "description: \"\"")
142+
})
143+
106144
for _, tt := range tests {
107145
t.Run(tt.name, func(t *testing.T) {
108146
t.Parallel()

0 commit comments

Comments
 (0)