Skip to content

Commit

Permalink
Merge branch 'dev' into BSR-3747-BSR-3751-git-metadata
Browse files Browse the repository at this point in the history
  • Loading branch information
bufdev committed May 15, 2024
2 parents 2623430 + 4a0f6b4 commit 483d1ea
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 39 deletions.
24 changes: 14 additions & 10 deletions private/buf/bufworkspace/malformed_dep.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ const (
type MalformedDepType int

// MalformedDep is a dep that was malformed in some way in the buf.yaml.
// It provides the module ref information and the malformed dep type.
type MalformedDep interface {
// ModuleFullName returns the full name of the malformed dep.
// ModuleRef is the module ref information of the malformed dep.
//
// Always present.
ModuleFullName() bufmodule.ModuleFullName
ModuleRef() bufmodule.ModuleRef
// Type is why this dep was malformed.
//
// Always present.
Expand Down Expand Up @@ -105,7 +106,7 @@ func MalformedDepsForWorkspace(workspace Workspace) ([]MalformedDep, error) {
malformedDeps = append(
malformedDeps,
newMalformedDep(
configuredDepModuleRef.ModuleFullName(),
configuredDepModuleRef,
MalformedDepTypeUnused,
),
)
Expand All @@ -114,8 +115,8 @@ func MalformedDepsForWorkspace(workspace Workspace) ([]MalformedDep, error) {
sort.Slice(
malformedDeps,
func(i int, j int) bool {
return malformedDeps[i].ModuleFullName().String() <
malformedDeps[j].ModuleFullName().String()
return malformedDeps[i].ModuleRef().ModuleFullName().String() <
malformedDeps[j].ModuleRef().ModuleFullName().String()
},
)
return malformedDeps, nil
Expand All @@ -124,19 +125,22 @@ func MalformedDepsForWorkspace(workspace Workspace) ([]MalformedDep, error) {
// *** PRIVATE ***

type malformedDep struct {
moduleFullName bufmodule.ModuleFullName
moduleRef bufmodule.ModuleRef
malformedDepType MalformedDepType
}

func newMalformedDep(moduleFullName bufmodule.ModuleFullName, malformedDepType MalformedDepType) *malformedDep {
func newMalformedDep(
moduleRef bufmodule.ModuleRef,
malformedDepType MalformedDepType,
) *malformedDep {
return &malformedDep{
moduleFullName: moduleFullName,
moduleRef: moduleRef,
malformedDepType: malformedDepType,
}
}

func (m *malformedDep) ModuleFullName() bufmodule.ModuleFullName {
return m.moduleFullName
func (m *malformedDep) ModuleRef() bufmodule.ModuleRef {
return m.moduleRef
}

func (m *malformedDep) Type() MalformedDepType {
Expand Down
4 changes: 2 additions & 2 deletions private/buf/bufworkspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,9 @@ func TestUnusedDep(t *testing.T) {
malformedDeps, err := MalformedDepsForWorkspace(workspace)
require.NoError(t, err)
require.Equal(t, 2, len(malformedDeps))
require.Equal(t, "buf.testing/acme/date", malformedDeps[0].ModuleFullName().String())
require.Equal(t, "buf.testing/acme/date", malformedDeps[0].ModuleRef().ModuleFullName().String())
require.Equal(t, MalformedDepTypeUnused, malformedDeps[0].Type())
require.Equal(t, "buf.testing/acme/extension", malformedDeps[1].ModuleFullName().String())
require.Equal(t, "buf.testing/acme/extension", malformedDeps[1].ModuleRef().ModuleFullName().String())
require.Equal(t, MalformedDepTypeUnused, malformedDeps[1].Type())
}

Expand Down
9 changes: 8 additions & 1 deletion private/buf/cmd/buf/command/dep/depprune/depprune.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,12 @@ func run(
if err != nil {
return err
}
return internal.Prune(ctx, container.Logger(), controller, configuredDepModuleKeys, workspaceDepManager, dirPath)
return internal.Prune(
ctx,
container.Logger(),
controller,
configuredDepModuleKeys,
workspaceDepManager,
dirPath,
)
}
23 changes: 16 additions & 7 deletions private/buf/cmd/buf/command/dep/depupdate/depupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"

"github.com/bufbuild/buf/private/buf/bufcli"
"github.com/bufbuild/buf/private/buf/bufctl"
"github.com/bufbuild/buf/private/buf/cmd/buf/command/dep/internal"
"github.com/bufbuild/buf/private/bufpkg/bufmodule"
"github.com/bufbuild/buf/private/pkg/app/appcmd"
Expand Down Expand Up @@ -104,7 +105,6 @@ func run(
if err != nil {
return err
}

configuredDepModuleRefs, err := workspaceDepManager.ConfiguredDepModuleRefs(ctx)
if err != nil {
return err
Expand Down Expand Up @@ -143,11 +143,20 @@ func run(
if err := workspaceDepManager.UpdateBufLockFile(ctx, configuredDepModuleKeys); err != nil {
return err
}
// Prune the buf.lock. This also verifies the workspace builds again.
workspace, err := controller.GetWorkspace(ctx, dirPath, bufctl.WithIgnoreAndDisallowV1BufWorkYAMLs())
if err != nil {
return err
}
// Validate that the workspace builds.
// Building also has the side effect of doing tamper-proofing.
//
// Note that the check to make sure configuredDepModuleKeys is a superset of the remote deps should be a no-op
// in this case, they should be equivalent based on the updates we just did, but we do the check
// anyways to triple verify.
return internal.Prune(ctx, logger, controller, configuredDepModuleKeys, workspaceDepManager, dirPath)
if _, err := controller.GetImageForWorkspace(
ctx,
workspace,
// This is a performance optimization - we don't need source code info.
bufctl.WithImageExcludeSourceInfo(true),
); err != nil {
return err
}
// Log warnings for users on unused configured deps.
return internal.LogUnusedConfiguredDepsForWorkspace(workspace, logger)
}
42 changes: 27 additions & 15 deletions private/buf/cmd/buf/command/dep/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func ModuleKeysAndTransitiveDepModuleKeysForModuleRefs(

// Prune prunes the buf.lock.
//
// Used by both mod prune and mod update.
// Used by dep/mod prune.
func Prune(
ctx context.Context,
logger *zap.Logger,
Expand Down Expand Up @@ -84,22 +84,10 @@ func Prune(
}
// Compute those dependencies that are in buf.yaml that are not used at all, and warn
// about them.
malformedDeps, err := bufworkspace.MalformedDepsForWorkspace(workspace)
if err != nil {
if err := LogUnusedConfiguredDepsForWorkspace(workspace, logger); err != nil {
return err
}
for _, malformedDep := range malformedDeps {
switch t := malformedDep.Type(); t {
case bufworkspace.MalformedDepTypeUnused:
logger.Sugar().Warnf(
`Module %[1]s is declared in your buf.yaml deps but is unused. This command only modifies buf.lock files, not buf.yaml files. Please remove %[1]s from your buf.yaml deps if it is not needed.`,
malformedDep.ModuleFullName(),
)
default:
return fmt.Errorf("unknown MalformedDepType: %v", t)
}
}
// Sep that actual computed remote dependencies based on imports. These are all
// Step that actually computes remote dependencies based on imports. These are all
// that is needed for buf.lock.
depModules, err := bufmodule.RemoteDepsForModuleSet(workspace)
if err != nil {
Expand All @@ -120,6 +108,30 @@ func Prune(
return workspaceDepManager.UpdateBufLockFile(ctx, depModuleKeys)
}

// LogUnusedConfiugredDepsForWorkspace takes a workspace and logs the unused configured
// dependencies as warnings to the user.
func LogUnusedConfiguredDepsForWorkspace(
workspace bufworkspace.Workspace,
logger *zap.Logger,
) error {
malformedDeps, err := bufworkspace.MalformedDepsForWorkspace(workspace)
if err != nil {
return err
}
for _, malformedDep := range malformedDeps {
switch t := malformedDep.Type(); t {
case bufworkspace.MalformedDepTypeUnused:
logger.Sugar().Warnf(
`Module %[1]s is declared in your buf.yaml deps but is unused. This command only modifies buf.lock files, not buf.yaml files. Please remove %[1]s from your buf.yaml deps if it is not needed.`,
malformedDep.ModuleRef().ModuleFullName(),
)
default:
return fmt.Errorf("unknown MalformedDepType: %v", t)
}
}
return nil
}

// moduleKeysAndTransitiveDepModuleKeysForModuleKeys returns the ModuleKeys
// and all the transitive dependencies.
func moduleKeysAndTransitiveDepModuleKeysForModuleKeys(
Expand Down
4 changes: 2 additions & 2 deletions private/buf/cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ func TestWorkspaceNotExistFail(t *testing.T) {
nil,
1,
``,
filepath.FromSlash(`Failure: module notexist had no .proto files`),
filepath.FromSlash(`Failure: "notexist" had no .proto files`),
"build",
filepath.Join("testdata", "workspace", "fail", "notexist"),
)
Expand All @@ -1071,7 +1071,7 @@ func TestWorkspaceNotExistFail(t *testing.T) {
nil,
1,
``,
filepath.FromSlash(`Failure: module notexist had no .proto files`),
filepath.FromSlash(`Failure: "notexist" had no .proto files`),
"build",
filepath.Join("testdata", "workspace", "fail", "v2", "notexist"),
)
Expand Down
4 changes: 2 additions & 2 deletions private/bufpkg/bufmodule/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,10 @@ func (n *NoProtoFilesError) Error() string {
return ""
}
var builder strings.Builder
_, _ = builder.WriteString(`module `)
_, _ = builder.WriteString(`"`)
// Writing even if the error is malformed via d.OpaqueID being empty.
_, _ = builder.WriteString(n.OpaqueID)
_, _ = builder.WriteString(` had no .proto files`)
_, _ = builder.WriteString(`" had no .proto files`)
return builder.String()
}

Expand Down

0 comments on commit 483d1ea

Please sign in to comment.