Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid pruning unused dependencies for buf dep update #2966

Merged
merged 15 commits into from
May 15, 2024
Merged
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
11 changes: 10 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,14 @@ func run(
if err != nil {
return err
}
return internal.Prune(ctx, container.Logger(), controller, configuredDepModuleKeys, workspaceDepManager, dirPath)
return internal.Prune(
ctx,
container.Logger(),
controller,
container,
doriable marked this conversation as resolved.
Show resolved Hide resolved
configuredDepModuleKeys,
workspaceDepManager,
dirPath,
false, // keepUnused
)
}
11 changes: 10 additions & 1 deletion private/buf/cmd/buf/command/dep/depupdate/depupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,5 +149,14 @@ func run(
// 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)
return internal.Prune(
ctx,
logger,
controller,
container,
configuredDepModuleKeys,
workspaceDepManager,
dirPath,
true, // keepUnused
)
}
40 changes: 37 additions & 3 deletions private/buf/cmd/buf/command/dep/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ func ModuleKeysAndTransitiveDepModuleKeysForModuleRefs(

// Prune prunes the buf.lock.
//
// Used by both mod prune and mod update.
// Used by both dep/mod prune and dep/mod update.
doriable marked this conversation as resolved.
Show resolved Hide resolved
func Prune(
ctx context.Context,
logger *zap.Logger,
controller bufctl.Controller,
container appext.Container,
// Contains all the Modules and their transitive dependencies based on the buf.yaml.
//
// All dependencies must be within this group from RemoteDepsForModuleSet. If a dependency
Expand All @@ -69,6 +70,9 @@ func Prune(
bufYAMLBasedDepModuleKeys []bufmodule.ModuleKey,
workspaceDepManager bufworkspace.WorkspaceDepManager,
dirPath string,
// For some use cases, such as dep/mod update, we want to keep unused declared dependencies
// in buf.lock (and only prune dependencies that are no longer declared) when set to true.
doriable marked this conversation as resolved.
Show resolved Hide resolved
keepUnused bool,
) error {
workspace, err := controller.GetWorkspace(ctx, dirPath, bufctl.WithIgnoreAndDisallowV1BufWorkYAMLs())
if err != nil {
Expand All @@ -88,18 +92,20 @@ func Prune(
if err != nil {
return err
}
var unusedModuleRefs []bufmodule.ModuleRef
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(),
malformedDep.ModuleRef().ModuleFullName(),
)
unusedModuleRefs = append(unusedModuleRefs, malformedDep.ModuleRef())
default:
return fmt.Errorf("unknown MalformedDepType: %v", t)
}
}
// Sep that actual computed remote dependencies based on imports. These are all
// Step that actual computed remote dependencies based on imports. These are all
doriable marked this conversation as resolved.
Show resolved Hide resolved
// that is needed for buf.lock.
depModules, err := bufmodule.RemoteDepsForModuleSet(workspace)
if err != nil {
Expand All @@ -114,6 +120,34 @@ func Prune(
if err != nil {
return err
}
// If keepUnused is set to true, we add the unused declared dependencies to depModuleKeys.
if keepUnused {
moduleKeyProvider, err := bufcli.NewModuleKeyProvider(container)
if err != nil {
return err
}
unusedModuleKeys, err := moduleKeyProvider.GetModuleKeysForModuleRefs(
ctx,
unusedModuleRefs,
workspaceDepManager.BufLockFileDigestType(),
)
if err != nil {
return err
}
foundDepModuleKeys := copy(make([]bufmodule.ModuleKey, len(depModuleKeys)), depModuleKeys)
depModuleKeys = append(depModuleKeys, unusedModuleKeys...)
doriable marked this conversation as resolved.
Show resolved Hide resolved
doriable marked this conversation as resolved.
Show resolved Hide resolved
// Check that `depModuleKeys` and `unusedModuleKeys` do not overlap, since `depModuleKeys`
// should be based on used deps.
// Return a syserror if an overlap is found.
dedupedDepModuleKeys := slicesext.DeduplicateAny(
depModuleKeys,
func(moduleKey bufmodule.ModuleKey) string { return moduleKey.ModuleFullName().String() },
)
if len(depModuleKeys) != len(dedupedDepModuleKeys) {
return syserror.Newf("unexpected overlap found between depModuleKeys and unsuedModuleKeys: %v, %v", foundDepModuleKeys, unusedModuleKeys)
}
}

if err := validateModuleKeysContains(bufYAMLBasedDepModuleKeys, depModuleKeys); err != nil {
return err
}
Expand Down