Skip to content

Commit

Permalink
Merge branch 'main' into ed/enablePluginsByDefault
Browse files Browse the repository at this point in the history
  • Loading branch information
bufdev authored Sep 18, 2024
2 parents eacf8f6 + 43df40a commit d21a3ec
Show file tree
Hide file tree
Showing 5 changed files with 258 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- Add support for custom lint and breaking change plugins. See
[our launch blog post](https://buf.build/blog/buf-custom-lint-breaking-change-plugins)
for more details!
- Add `buf dep graph --format` flag that defaults to `dot`, and adds the option `json`, to print
the dependency graph in JSON format.
- Fix bugs in `buf format` where trailing comments on commas in message literals were not properly
propagated to the formatted proto, empty message literals were not properly indented, and
compound strings in options added an extra newline before trailing commas.
Expand Down
18 changes: 18 additions & 0 deletions private/buf/bufworkspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,24 @@ func testBasic(t *testing.T, subDirPath string, isV2 bool) {
graph,
bufmodule.Module.OpaqueID,
)
graphRemoteOnly, err := bufmodule.ModuleSetToDAG(workspace, bufmodule.ModuleSetToDAGWithRemoteOnly())
require.NoError(t, err)
dagtest.RequireGraphEqual(
t,
[]dagtest.ExpectedNode[string]{
{
Key: "buf.testing/acme/extension",
},
{
Key: "buf.testing/acme/date",
Outbound: []string{
"buf.testing/acme/extension",
},
},
},
graphRemoteOnly,
bufmodule.Module.OpaqueID,
)
module = workspace.GetModuleForOpaqueID("buf.testing/acme/bond")
require.NotNil(t, module)
_, err = module.StatFileInfo(ctx, "acme/bond/real/v1/bond.proto")
Expand Down
176 changes: 168 additions & 8 deletions private/buf/cmd/buf/command/dep/depgraph/depgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@ package depgraph

import (
"context"
"encoding/json"
"fmt"
"slices"

"github.com/bufbuild/buf/private/buf/bufcli"
"github.com/bufbuild/buf/private/buf/bufctl"
"github.com/bufbuild/buf/private/bufpkg/bufanalysis"
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/dag"
"github.com/bufbuild/buf/private/pkg/slicesext"
"github.com/bufbuild/buf/private/pkg/stringutil"
"github.com/bufbuild/buf/private/pkg/uuidutil"
"github.com/spf13/pflag"
Expand All @@ -32,6 +36,17 @@ import (
const (
errorFormatFlagName = "error-format"
disableSymlinksFlagName = "disable-symlinks"
formatFlagName = "format"

dotFormatString = "dot"
jsonFormatString = "json"
)

var (
allGraphFormatStrings = []string{
dotFormatString,
jsonFormatString,
}
)

// NewCommand returns a new Command.
Expand All @@ -42,7 +57,7 @@ func NewCommand(
flags := newFlags()
return &appcmd.Command{
Use: name + " <input>",
Short: "Print the dependency graph in DOT format",
Short: "Print the dependency graph",
Long: `As an example, if module in directory "src/proto" depends on module "buf.build/foo/bar"
from the BSR with commit "12345", and "buf.build/foo/bar:12345" depends on module "buf.build/foo/baz"
from the BSR with commit "67890", the following will be printed:
Expand All @@ -66,7 +81,6 @@ brew install graphviz
You can easily visualize a dependency graph using the dot tool:
buf dep graph | dot -Tpng >| graph.png && open graph.png
` + bufcli.GetSourceOrModuleLong(`the source or module to print the dependency graph for`),
Args: appcmd.MaximumNArgs(1),
Run: builder.NewRunFunc(
Expand All @@ -83,6 +97,7 @@ type flags struct {
DisableSymlinks bool
// special
InputHashtag string
Format string
}

func newFlags() *flags {
Expand All @@ -101,6 +116,15 @@ func (f *flags) Bind(flagSet *pflag.FlagSet) {
stringutil.SliceToString(bufanalysis.AllFormatStrings),
),
)
flagSet.StringVar(
&f.Format,
formatFlagName,
dotFormatString,
fmt.Sprintf(
"The format to print graph as. Must be one of %s",
stringutil.SliceToString(allGraphFormatStrings),
),
)
}

func run(
Expand Down Expand Up @@ -128,20 +152,156 @@ func run(
if err != nil {
return err
}
dotString, err := graph.DOTString(moduleToString)
if err != nil {
return err
var graphString string
switch flags.Format {
case dotFormatString:
dotString, err := graph.DOTString(moduleToString)
if err != nil {
return err
}
graphString = dotString
case jsonFormatString:
// We traverse each module (node) in the graph and populate the deps (outbound nodes).
// We keep track of every module we have seen so we can update their d
moduleFullNameOrOpaqueIDToExternalModule := make(map[string]externalModule)
if err := graph.WalkNodes(
func(module bufmodule.Module, _ []bufmodule.Module, deps []bufmodule.Module) error {
moduleFullNameOrOpaqueID := moduleFullNameOrOpaqueID(module)
// We have already populated this node through deps, we can skip module.
if _, ok := moduleFullNameOrOpaqueIDToExternalModule[moduleFullNameOrOpaqueID]; ok {
return nil
}
// We first scaffold a module with no deps populated yet.
externalModule, err := externalModuleNoDepsForModule(module)
if err != nil {
return err
}
if err := externalModule.addDeps(deps, graph, moduleFullNameOrOpaqueIDToExternalModule, flags); err != nil {
return err
}
// Sort the deps alphabetically before adding our external module.
sortExternalModules(externalModule.Deps)
moduleFullNameOrOpaqueIDToExternalModule[moduleFullNameOrOpaqueID] = externalModule
return nil
},
); err != nil {
return err
}
externalModules := slicesext.MapValuesToSlice(moduleFullNameOrOpaqueIDToExternalModule)
// Sort all modules alphabetically.
sortExternalModules(externalModules)
data, err := json.Marshal(externalModules)
if err != nil {
return err
}
graphString = string(data)
default:
return appcmd.NewInvalidArgumentErrorf("invalid value for --%s: %s", formatFlagName, flags.Format)
}
_, err = fmt.Fprintln(container.Stdout(), dotString)
_, err = fmt.Fprintln(container.Stdout(), graphString)
return err
}

func moduleToString(module bufmodule.Module) string {
if moduleFullName := module.ModuleFullName(); moduleFullName != nil {
if commitID := module.CommitID(); !commitID.IsNil() {
return moduleFullName.String() + ":" + uuidutil.ToDashless(commitID)
commitID := dashlessCommitIDStringForModule(module)
if commitID != "" {
return moduleFullName.String() + ":" + commitID
}
return moduleFullName.String()
}
return module.OpaqueID()
}

// moduleFullNameOrOpaqueID returns the ModuleFullName for a module if available, otherwise
// it returns the OpaqueID.
func moduleFullNameOrOpaqueID(module bufmodule.Module) string {
if moduleFullName := module.ModuleFullName(); moduleFullName != nil {
return moduleFullName.String()
}
return module.OpaqueID()
}

// dashlessCommitIDStringForModule returns the dashless UUID for the commit. If no commit
// is set, we return an empty string.
func dashlessCommitIDStringForModule(module bufmodule.Module) string {
if commitID := module.CommitID(); !commitID.IsNil() {
return uuidutil.ToDashless(commitID)
}
return ""
}

type externalModule struct {
// ModuleFullName if remote, OpaqueID if no ModuleFullName
Name string `json:"name,omitempty" yaml:"name,omitempty"`
// Dashless
Commit string `json:"commit,omitempty" yaml:"commit,omitempty"`
Digest string `json:"digest,omitempty" yaml:"digest,omitempty"`
Deps []externalModule `json:"deps,omitempty" yaml:"deps,omitempty"`
Local bool `json:"local,omitempty" yaml:"local,omitempty"`
}

func (e *externalModule) addDeps(
deps []bufmodule.Module,
graph *dag.Graph[string, bufmodule.Module],
moduleFullNameOrOpaqueIDToExternalModule map[string]externalModule,
flags *flags,
) error {
for _, dep := range deps {
depModuleFullNameOrOpaqueID := moduleFullNameOrOpaqueID(dep)
depExternalModule, ok := moduleFullNameOrOpaqueIDToExternalModule[depModuleFullNameOrOpaqueID]
if ok {
// If this dependency has already been seen, we can simply update our current module
// and return early.
e.Deps = append(e.Deps, depExternalModule)
return nil
}
// 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
// handled.
depExternalModule, err := externalModuleNoDepsForModule(dep)
if err != nil {
return err
}
transitiveDeps, err := graph.OutboundNodes(dep.OpaqueID())
if err != nil {
return err
}
if err := depExternalModule.addDeps(transitiveDeps, graph, moduleFullNameOrOpaqueIDToExternalModule, flags); err != nil {
return err
}
moduleFullNameOrOpaqueIDToExternalModule[depModuleFullNameOrOpaqueID] = depExternalModule
e.Deps = append(e.Deps, depExternalModule)
}
return nil
}

// externalModuleNoDepsForModule returns an externalModule for the given bufmodule.Module
// without populating the deps. This is because we want to populate the deps from the graph,
// so we handle it outside of this function.
func externalModuleNoDepsForModule(module bufmodule.Module) (externalModule, error) {
// We always calculate the b5 digest here, we do not check the digest type that is stored
// in buf.lock.
digest, err := module.Digest(bufmodule.DigestTypeB5)
if err != nil {
return externalModule{}, err
}
return externalModule{
Name: moduleFullNameOrOpaqueID(module),
Commit: dashlessCommitIDStringForModule(module),
Digest: digest.String(),
Local: module.IsLocal(),
}, nil
}

func sortExternalModules(externalModules []externalModule) {
slices.SortFunc(
externalModules,
func(a externalModule, b externalModule) int {
if a.Name > b.Name {
return 1
}
return -1
},
)
}
29 changes: 29 additions & 0 deletions private/bufpkg/bufmodule/bufmodule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,35 @@ func TestBasic(t *testing.T) {
graph,
bufmodule.Module.OpaqueID,
)
graphRemoteOnly, err := bufmodule.ModuleSetToDAG(moduleSet, bufmodule.ModuleSetToDAGWithRemoteOnly())
require.NoError(t, err)
dagtest.RequireGraphEqual(
t,
[]dagtest.ExpectedNode[string]{
{
Key: "buf.build/foo/extdep1",
Outbound: []string{},
},
{
Key: "buf.build/foo/extdep3",
Outbound: []string{
"buf.build/foo/extdep4",
},
},
{
Key: "buf.build/foo/extdep4",
Outbound: []string{},
},
{
Key: "buf.build/foo/extdep2",
Outbound: []string{
"buf.build/foo/extdep1",
},
},
},
graphRemoteOnly,
bufmodule.Module.OpaqueID,
)
remoteDeps, err := bufmodule.RemoteDepsForModuleSet(moduleSet)
require.NoError(t, err)
require.Equal(
Expand Down
46 changes: 41 additions & 5 deletions private/bufpkg/bufmodule/module_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,31 @@ func ModuleSetRemoteOpaqueIDs(moduleSet ModuleSet) []string {
//
// This only starts at target Modules. If a Module is not part of a graph
// with a target Module as a source, it will not be added.
func ModuleSetToDAG(moduleSet ModuleSet) (*dag.Graph[string, Module], error) {
func ModuleSetToDAG(moduleSet ModuleSet, options ...ModuleSetToDAGOption) (*dag.Graph[string, Module], error) {
moduleSetToDAGOptions := newModuleSetToDAGOptions()
for _, option := range options {
option(moduleSetToDAGOptions)
}
graph := dag.NewGraph[string, Module](Module.OpaqueID)
for _, module := range ModuleSetTargetModules(moduleSet) {
if err := moduleSetToDAGRec(module, graph); err != nil {
if err := moduleSetToDAGRec(module, graph, moduleSetToDAGOptions.remoteOnly); err != nil {
return nil, err
}
}
return graph, nil
}

// ModuleSetToDAGOption is an option for ModuleSetToDAG.
type ModuleSetToDAGOption func(*moduleSetToDAGOptions)

// ModuleSetToDAGWithRemoteOnly returns a new ModuleSetToDAGOption that specifies the graph
// should be built with only remote modules.
func ModuleSetToDAGWithRemoteOnly() ModuleSetToDAGOption {
return func(moduleSetToDAGOptions *moduleSetToDAGOptions) {
moduleSetToDAGOptions.remoteOnly = true
}
}

// *** PRIVATE ***

// moduleSet
Expand Down Expand Up @@ -401,6 +416,14 @@ func (m *moduleSet) getModuleForFilePathUncached(ctx context.Context, filePath s

func (*moduleSet) isModuleSet() {}

type moduleSetToDAGOptions struct {
remoteOnly bool
}

func newModuleSetToDAGOptions() *moduleSetToDAGOptions {
return &moduleSetToDAGOptions{}
}

// utils

func moduleSetTargetLocalModulesAndTransitiveLocalDepsRec(
Expand Down Expand Up @@ -440,15 +463,28 @@ func moduleSetTargetLocalModulesAndTransitiveLocalDepsRec(
func moduleSetToDAGRec(
module Module,
graph *dag.Graph[string, Module],
remoteOnly bool,
) error {
graph.AddNode(module)
// If remoteOnly is set, then we only want to add remote modules as nodes. However, we do
// not want to return necessarily, because we want to capture all remote dependencies.
//
// We do not return early and ignore local modules entirely, because we still want to check
// for remote dependencies from local modules.
if !remoteOnly || !module.IsLocal() {
graph.AddNode(module)
}
directModuleDeps, err := ModuleDirectModuleDeps(module)
if err != nil {
return err
}
for _, directModuleDep := range directModuleDeps {
graph.AddEdge(module, directModuleDep)
if err := moduleSetToDAGRec(directModuleDep, graph); err != nil {
// If remoteOnly is set, then we only want to add the edge if both the module _and_ the
// dependency are remote.
if !remoteOnly || (!module.IsLocal() && !directModuleDep.IsLocal()) {
graph.AddEdge(module, directModuleDep)
}
// We still want to check all transitive dependencies for all modules.
if err := moduleSetToDAGRec(directModuleDep, graph, remoteOnly); err != nil {
return err
}
}
Expand Down

0 comments on commit d21a3ec

Please sign in to comment.