diff --git a/private/buf/bufworkspace/malformed_dep.go b/private/buf/bufworkspace/malformed_dep.go index d9efd8acc9..520d2d3db4 100644 --- a/private/buf/bufworkspace/malformed_dep.go +++ b/private/buf/bufworkspace/malformed_dep.go @@ -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. @@ -105,7 +106,7 @@ func MalformedDepsForWorkspace(workspace Workspace) ([]MalformedDep, error) { malformedDeps = append( malformedDeps, newMalformedDep( - configuredDepModuleRef.ModuleFullName(), + configuredDepModuleRef, MalformedDepTypeUnused, ), ) @@ -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 @@ -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 { diff --git a/private/buf/bufworkspace/workspace_test.go b/private/buf/bufworkspace/workspace_test.go index 7956bedd10..540e366ddb 100644 --- a/private/buf/bufworkspace/workspace_test.go +++ b/private/buf/bufworkspace/workspace_test.go @@ -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()) } diff --git a/private/buf/cmd/buf/command/dep/depprune/depprune.go b/private/buf/cmd/buf/command/dep/depprune/depprune.go index 2902d6ebae..269c55268a 100644 --- a/private/buf/cmd/buf/command/dep/depprune/depprune.go +++ b/private/buf/cmd/buf/command/dep/depprune/depprune.go @@ -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, + ) } diff --git a/private/buf/cmd/buf/command/dep/depupdate/depupdate.go b/private/buf/cmd/buf/command/dep/depupdate/depupdate.go index db7bab1bda..5025761b85 100644 --- a/private/buf/cmd/buf/command/dep/depupdate/depupdate.go +++ b/private/buf/cmd/buf/command/dep/depupdate/depupdate.go @@ -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" @@ -104,7 +105,6 @@ func run( if err != nil { return err } - configuredDepModuleRefs, err := workspaceDepManager.ConfiguredDepModuleRefs(ctx) if err != nil { return err @@ -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) } diff --git a/private/buf/cmd/buf/command/dep/internal/internal.go b/private/buf/cmd/buf/command/dep/internal/internal.go index fc2bdde97e..f19291b24b 100644 --- a/private/buf/cmd/buf/command/dep/internal/internal.go +++ b/private/buf/cmd/buf/command/dep/internal/internal.go @@ -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, @@ -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 { @@ -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( diff --git a/private/buf/cmd/buf/workspace_test.go b/private/buf/cmd/buf/workspace_test.go index f0037ba4ae..2d5c4f9b73 100644 --- a/private/buf/cmd/buf/workspace_test.go +++ b/private/buf/cmd/buf/workspace_test.go @@ -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"), ) @@ -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"), ) diff --git a/private/bufpkg/bufmodule/errors.go b/private/bufpkg/bufmodule/errors.go index 5e625f5e1e..55105ecd84 100644 --- a/private/bufpkg/bufmodule/errors.go +++ b/private/bufpkg/bufmodule/errors.go @@ -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() }