From 0c37f5a07f1c44b4a84638b85eac0abadd5e1402 Mon Sep 17 00:00:00 2001 From: Christian Gregg Date: Mon, 30 Mar 2026 20:41:25 +0100 Subject: [PATCH 1/2] Fix buf dep graph --format json silently dropping dependencies The addDeps method had a `return nil` that exited the entire loop after encountering the first already-seen dependency, causing all subsequent dependencies to be silently dropped from the JSON output. Changed to `continue` so the loop processes all dependencies. Adds a regression test with a three-module workspace (alpha, common, zeta) where zeta depends on both alpha and common. Because modules are walked alphabetically, both of zeta's deps are already in the seen map when zeta is processed, which triggered the early return. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../internal/command/dep/depgraph/depgraph.go | 4 +- .../command/dep/depgraph/depgraph_test.go | 89 +++++++++++++++++++ .../shared_dep_workspace/alpha/alpha.proto | 9 ++ .../testdata/shared_dep_workspace/buf.yaml | 5 ++ .../shared_dep_workspace/common/common.proto | 7 ++ .../shared_dep_workspace/zeta/zeta.proto | 11 +++ 6 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 cmd/buf/internal/command/dep/depgraph/depgraph_test.go create mode 100644 cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/alpha/alpha.proto create mode 100644 cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/buf.yaml create mode 100644 cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/common/common.proto create mode 100644 cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/zeta/zeta.proto diff --git a/cmd/buf/internal/command/dep/depgraph/depgraph.go b/cmd/buf/internal/command/dep/depgraph/depgraph.go index eaa3d5f516..6a3ae08b94 100644 --- a/cmd/buf/internal/command/dep/depgraph/depgraph.go +++ b/cmd/buf/internal/command/dep/depgraph/depgraph.go @@ -253,9 +253,9 @@ func (e *externalModule) addDeps( depExternalModule, ok := moduleFullNameOrOpaqueIDToExternalModule[depFullNameOrOpaqueID] if ok { // If this dependency has already been seen, we can simply update our current module - // and return early. + // and continue to the next dependency. e.Deps = append(e.Deps, depExternalModule) - return nil + continue } // Otherwise, we create a new external module for our direct dependency. However, we do // not add it to our map yet, we only add it once all transitive dependencies have been diff --git a/cmd/buf/internal/command/dep/depgraph/depgraph_test.go b/cmd/buf/internal/command/dep/depgraph/depgraph_test.go new file mode 100644 index 0000000000..92ab452805 --- /dev/null +++ b/cmd/buf/internal/command/dep/depgraph/depgraph_test.go @@ -0,0 +1,89 @@ +// Copyright 2020-2026 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 depgraph + +import ( + "bytes" + "encoding/json" + "path/filepath" + "testing" + + "buf.build/go/app/appcmd" + "buf.build/go/app/appcmd/appcmdtesting" + "buf.build/go/app/appext" + "github.com/bufbuild/buf/cmd/buf/internal/internaltesting" + "github.com/stretchr/testify/require" +) + +// TestJSONFormatSharedDep is a regression test for a bug where addDeps had +// a "return nil" instead of "continue" when a dependency was already in the +// seen map, causing all subsequent dependencies to be silently dropped. +// +// The workspace has three local modules: alpha -> common, zeta -> {alpha, common}. +// Because modules are walked alphabetically, both of zeta's deps are already +// in the seen map when zeta is processed, which triggered the early return. +func TestJSONFormatSharedDep(t *testing.T) { + t.Parallel() + var stdout bytes.Buffer + appcmdtesting.Run( + t, + func(name string) *appcmd.Command { + return NewCommand(name, appext.NewBuilder(name)) + }, + appcmdtesting.WithExpectedExitCode(0), + appcmdtesting.WithEnv(internaltesting.NewEnvFunc(t)), + appcmdtesting.WithStdout(&stdout), + appcmdtesting.WithArgs( + "--format", "json", + filepath.Join("testdata", "shared_dep_workspace"), + ), + ) + var got []externalModule + require.NoError(t, json.Unmarshal(stdout.Bytes(), &got)) + clearDigests(got) + expected := []externalModule{ + { + Name: "alpha", + Local: true, + Deps: []externalModule{ + {Name: "common", Local: true}, + }, + }, + { + Name: "common", + Local: true, + }, + { + Name: "zeta", + Local: true, + Deps: []externalModule{ + {Name: "alpha", Local: true, Deps: []externalModule{ + {Name: "common", Local: true}, + }}, + {Name: "common", Local: true}, + }, + }, + } + require.Equal(t, expected, got) +} + +// clearDigests zeros out Digest fields so we can compare structures without +// depending on content hashes. +func clearDigests(modules []externalModule) { + for i := range modules { + modules[i].Digest = "" + clearDigests(modules[i].Deps) + } +} diff --git a/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/alpha/alpha.proto b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/alpha/alpha.proto new file mode 100644 index 0000000000..73e3da7eca --- /dev/null +++ b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/alpha/alpha.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +package alpha.v1; + +import "common.proto"; + +message Foo { + common.v1.Shared shared = 1; +} diff --git a/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/buf.yaml b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/buf.yaml new file mode 100644 index 0000000000..4707f3c390 --- /dev/null +++ b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/buf.yaml @@ -0,0 +1,5 @@ +version: v2 +modules: + - path: alpha + - path: common + - path: zeta diff --git a/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/common/common.proto b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/common/common.proto new file mode 100644 index 0000000000..4aed79e720 --- /dev/null +++ b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/common/common.proto @@ -0,0 +1,7 @@ +syntax = "proto3"; + +package common.v1; + +message Shared { + string value = 1; +} diff --git a/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/zeta/zeta.proto b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/zeta/zeta.proto new file mode 100644 index 0000000000..e3aaf92789 --- /dev/null +++ b/cmd/buf/internal/command/dep/depgraph/testdata/shared_dep_workspace/zeta/zeta.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package zeta.v1; + +import "common.proto"; +import "alpha.proto"; + +message Bar { + common.v1.Shared shared = 1; + alpha.v1.Foo foo = 2; +} From f2f856369317b91146f2faec8f7ae0a8af499c90 Mon Sep 17 00:00:00 2001 From: Edward McFarlane Date: Tue, 31 Mar 2026 18:09:51 +0200 Subject: [PATCH 2/2] Add CHANGELOG entry for buf dep graph --format json fix --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a8f37a1489..e2427909dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- Fix `buf dep graph --format json` silently dropping dependencies when a dependency was already seen. - Add support for `--rbs_out` as a `protoc_builtin` plugin (requires protoc v34.0+). - Add relevant links from CEL LSP hover documentation to either or - Skip writing unchanged output files in `buf generate` to preserve modification times