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 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; +}