Debug bufmod calc

private/buf/bufworkspace/workspace_provider.go

@@ -441,6 +441,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2(
 			return nil, syserror.Newf("unknown FileVersion: %v", fileVersion)
 		}
 		for _, depModuleKey := range bufLockFile.DepModuleKeys() {
+			fmt.Println("workspace_provider.go v2 depModuleKey", depModuleKey)
 			// DepModuleKeys from a BufLockFile is expected to have all transitive dependencies,
 			// and we can rely on this property.
 			moduleSetBuilder.AddRemoteModule(

private/bufpkg/bufmodule/added_module.go

@@ -110,6 +110,7 @@ func (a *addedModule) ToModule(
 	// Else, get the remote Module.
 	getModuleData := sync.OnceValues(
 		func() (ModuleData, error) {
+			fmt.Println("?????")
 			moduleDatas, err := moduleDataProvider.GetModuleDatasForModuleKeys(
 				ctx,
 				[]ModuleKey{a.remoteModuleKey},
@@ -257,6 +258,7 @@ func (a *addedModule) ToModule(
 	// digest calculations. Within the Module, we say that if we get a remote Module, use the
 	// declared ModuleKeys instead of whatever Module we have resolved to for a given FullName.
 	getDeclaredDepModuleKeysB5 := func() ([]ModuleKey, error) {
+		fmt.Println("Getting declared dependencies")
 		moduleData, err := getModuleData()
 		if err != nil {
 			return nil, err

private/bufpkg/bufmodule/bufmoduleapi/module_data_provider.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
-	"sort"
 
 	modulev1 "buf.build/gen/go/bufbuild/registry/protocolbuffers/go/buf/registry/module/v1"
 	"github.com/bufbuild/buf/private/bufpkg/bufmodule"
@@ -122,13 +121,22 @@ func (a *moduleDataProvider) getIndexedModuleDatasForRegistryAndIndexedModuleKey
 	ctx context.Context,
 	v1ProtoModuleProvider *v1ProtoModuleProvider,
 	registry string,
-	indexedModuleKeys []slicesext.Indexed[bufmodule.ModuleKey],
+	indexedModuleKeys []slicesext.Indexed[bufmodule.ModuleKey], // [c1]
 	digestType bufmodule.DigestType,
 ) ([]slicesext.Indexed[bufmodule.ModuleData], error) {
+	// Is this graph with pruned dependencies.
+	//  c -> a2
+	//   \-> b1 -> a1 -> x1
+	//
+	//  c -> a2
+	//  \-> b1 -> a2
+	//
+	fmt.Println("indexedModuleKeys", indexedModuleKeys)
 	graph, err := a.graphProvider.GetGraphForModuleKeys(ctx, slicesext.IndexedToValues(indexedModuleKeys))
 	if err != nil {
 		return nil, err
 	}
+	// graph -> c -> a2, b1
 	commitIDToIndexedModuleKey, err := slicesext.ToUniqueValuesMapError(
 		indexedModuleKeys,
 		func(indexedModuleKey slicesext.Indexed[bufmodule.ModuleKey]) (uuid.UUID, error) {
@@ -148,64 +156,49 @@ func (a *moduleDataProvider) getIndexedModuleDatasForRegistryAndIndexedModuleKey
 	if err != nil {
 		return nil, err
 	}
+
+	// CAll graph with one commit -> c -> a2, b2
+
+	// Get the module deps for this commit.
 	indexedModuleDatas := make([]slicesext.Indexed[bufmodule.ModuleData], 0, len(indexedModuleKeys))
-	if err := graph.WalkNodes(
-		func(
-			moduleKey bufmodule.ModuleKey,
-			_ []bufmodule.ModuleKey,
-			_ []bufmodule.ModuleKey,
-		) error {
-			// TopoSort will get us both the direct and transitive dependencies for the key.
-			//
-			// The outgoing edge list is just the direct dependencies.
-			//
-			// There is definitely a better way to do this in one pass for all commits with
-			// memoization - this is algorithmically bad.
-			depModuleKeys, err := graph.TopoSort(bufmodule.ModuleKeyToRegistryCommitID(moduleKey))
-			if err != nil {
-				return err
-			}
-			depModuleKeys = depModuleKeys[:len(depModuleKeys)-1]
-			sort.Slice(
-				depModuleKeys,
-				func(i int, j int) bool {
-					return depModuleKeys[i].FullName().String() < depModuleKeys[j].FullName().String()
-				},
-			)
+	for _, indexedModuleKey := range indexedModuleKeys {
+		moduleKey := indexedModuleKey.Value
+		// TopoSort will get us both the direct and transitive dependencies for the key.
+		depModuleKeys, err := graph.TopoSort(bufmodule.ModuleKeyToRegistryCommitID(moduleKey)) // ALL transitive dependencies
+		if err != nil {
+			return nil, err
+		}
+		// Remove the final key, this module.
+		depModuleKeys = depModuleKeys[:len(depModuleKeys)-1]
+		fmt.Println("moduleKey", moduleKey)
+		fmt.Println("\tdepModuleKeys", depModuleKeys)
 
-			universalProtoContent, ok := commitIDToUniversalProtoContent[moduleKey.CommitID()]
-			if !ok {
-				// We only care to get content for a subset of the graph. If we have something
-				// in the graph without content, we just skip it.
-				return nil
-			}
-			indexedModuleKey, ok := commitIDToIndexedModuleKey[moduleKey.CommitID()]
-			if !ok {
-				return syserror.Newf("could not find indexed ModuleKey for commit ID %q", uuidutil.ToDashless(moduleKey.CommitID()))
-			}
-			indexedModuleData := slicesext.Indexed[bufmodule.ModuleData]{
-				Value: bufmodule.NewModuleData(
-					ctx,
-					moduleKey,
-					func() (storage.ReadBucket, error) {
-						return universalProtoFilesToBucket(universalProtoContent.Files)
-					},
-					func() ([]bufmodule.ModuleKey, error) { return depModuleKeys, nil },
-					func() (bufmodule.ObjectData, error) {
-						return universalProtoFileToObjectData(universalProtoContent.V1BufYAMLFile)
-					},
-					func() (bufmodule.ObjectData, error) {
-						return universalProtoFileToObjectData(universalProtoContent.V1BufLockFile)
-					},
-				),
-				Index: indexedModuleKey.Index,
-			}
-			indexedModuleDatas = append(indexedModuleDatas, indexedModuleData)
-			return nil
-		},
-	); err != nil {
-		return nil, err
+		universalProtoContent, ok := commitIDToUniversalProtoContent[moduleKey.CommitID()]
+		if !ok {
+			// We only care to get content for a subset of the graph. If we have something
+			// in the graph without content, we just skip it.
+			return nil, syserror.Newf("no content returned for commit ID %s", moduleKey.CommitID())
+		}
+		indexedModuleData := slicesext.Indexed[bufmodule.ModuleData]{
+			Value: bufmodule.NewModuleData(
+				ctx,
+				moduleKey,
+				func() (storage.ReadBucket, error) {
+					return universalProtoFilesToBucket(universalProtoContent.Files)
+				},
+				func() ([]bufmodule.ModuleKey, error) { return depModuleKeys, nil },
+				func() (bufmodule.ObjectData, error) {
+					return universalProtoFileToObjectData(universalProtoContent.V1BufYAMLFile)
+				},
+				func() (bufmodule.ObjectData, error) {
+					return universalProtoFileToObjectData(universalProtoContent.V1BufLockFile)
+				},
+			),
+			Index: indexedModuleKey.Index,
+		}
+		indexedModuleDatas = append(indexedModuleDatas, indexedModuleData)
 	}
+	fmt.Println("indexedModuleDatas", indexedModuleDatas)
 	return indexedModuleDatas, nil
 }
 

private/bufpkg/bufmodule/module.go

@@ -16,6 +16,7 @@ package bufmodule
 
 import (
 	"context"
+	"fmt"
 	"sync"
 
 	"github.com/bufbuild/buf/private/bufpkg/bufparse"
@@ -452,33 +453,46 @@ func newGetDigestFuncForModuleAndDigestType(module *module, digestType DigestTyp
 			}
 			return getB4Digest(module.ctx, bucket, v1BufYAMLObjectData, v1BufLockObjectData)
 		case DigestTypeB5:
-			moduleDeps, err := module.ModuleDeps()
+			fmt.Println("DEPS B5", module.FullName())
+			// Either the Module.Key with a lazy digest that does the recompute,
+			// or the declared deps, that is just the ModuleKey with already computed digest that we must use.
+			moduleDeps, err := module.ModuleDeps() // ALl transitive deps?
 			if err != nil {
 				return nil, err
 			}
-			// For remote modules to have consistent B5 digests, they must not change the digests of their
-			// dependencies based on the local workspace. Use the pruned b5 module keys from
-			// ModuleData.DeclaredDepModuleKeys to calculate the digest.
+			// c -> a2, b1
+
+			// c1, a2, b1
+
+			// b1 -> a2 (used module dep)
+
+			for _, dep := range moduleDeps {
+				digest, err := dep.Digest(digestType)
+				if err != nil {
+					panic(err)
+				}
+				fmt.Println("\t", dep.FullName(), digest)
+			}
+			// Remote modules may not have all their dependencies, or at the correct commits, in the local workspace.
+			// The declared dependencies are used to calculate the digest for remote modules.
+			// Local modules are always calculated from the current workspace state.
 			if !module.isLocal {
-				declaredDepModuleKeys, err := module.getDeclaredDepModuleKeysB5()
+				// Why prune?
+				// a2, b1
+				declaredDepModuleKeys, err := module.getDeclaredDepModuleKeysB5() // Includes transitive.
 				if err != nil {
 					return nil, err
 				}
-				moduleDepFullNames := make(map[string]struct{}, len(moduleDeps))
-				for _, dep := range moduleDeps {
-					fullName := dep.FullName()
-					if fullName == nil {
-						return nil, syserror.Newf("remote module dependencies should have full names")
-					}
-					moduleDepFullNames[fullName.String()] = struct{}{}
-				}
-				prunedDepModuleKeys := make([]ModuleKey, 0, len(declaredDepModuleKeys))
+				fmt.Println("DEPS B5 DECLARED", module.FullName())
 				for _, dep := range declaredDepModuleKeys {
-					if _, ok := moduleDepFullNames[dep.FullName().String()]; ok {
-						prunedDepModuleKeys = append(prunedDepModuleKeys, dep)
+					digest, err := dep.Digest()
+					if err != nil {
+						panic(err)
 					}
+					fmt.Println("\t", dep.FullName(), digest)
 				}
-				return getB5DigestForBucketAndDepModuleKeys(module.ctx, bucket, prunedDepModuleKeys)
+
+				return getB5DigestForBucketAndDepModuleKeys(module.ctx, bucket, declaredDepModuleKeys)
 			}
 			return getB5DigestForBucketAndModuleDeps(module.ctx, bucket, moduleDeps)
 		default:

private/bufpkg/bufmodule/module_data.go

@@ -16,6 +16,7 @@ package bufmodule
 
 import (
 	"context"
+	"fmt"
 	"sync"
 
 	"github.com/bufbuild/buf/private/pkg/storage"
@@ -112,6 +113,7 @@ func newModuleData(
 	}
 	moduleData.checkDigest = sync.OnceValue(
 		func() error {
+			fmt.Println("moduleData.checkDigest")
 			// We have to use the get.* functions so that we don't invoke checkDigest.
 			bucket, err := moduleData.getBucket()
 			if err != nil {
@@ -144,6 +146,14 @@ func newModuleData(
 				if err != nil {
 					return err
 				}
+				fmt.Println("DEPS:")
+				for _, dep := range declaredDepModuleKeys {
+					digest, err := dep.Digest()
+					if err != nil {
+						panic(err)
+					}
+					fmt.Printf("\t%s %s\n", dep.FullName(), digest)
+				}
 				// This isn't the Digest as computed by the Module exactly, as the Module uses
 				// file imports to determine what the dependencies are. However, this is checking whether
 				// or not the digest of the returned information matches the digest we expected, which is
@@ -162,6 +172,7 @@ func newModuleData(
 					return err
 				}
 			}
+			fmt.Printf("DIGEST %t\n\t%s %s\n\t-%s\n\t+%s\n", !DigestEqual(expectedDigest, actualDigest), moduleKey.FullName(), moduleKey.CommitID().String(), expectedDigest, actualDigest)
 			if !DigestEqual(expectedDigest, actualDigest) {
 				return &DigestMismatchError{
 					FullName:       moduleKey.FullName(),

private/bufpkg/bufmodule/module_set_builder.go

@@ -17,6 +17,7 @@ package bufmodule
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log/slog"
 	"sync/atomic"
 
@@ -89,7 +90,6 @@ type ModuleSetBuilder interface {
 	//
 	// Remote modules are rarely targets. However, if we are reading a ModuleSet from a
 	// ModuleProvider for example with a buf build buf.build/foo/bar call, then this
-
 	// specific Module will be targeted, while its dependencies will not be.
 	//
 	// Returns the same ModuleSetBuilder.
@@ -137,16 +137,31 @@ func NewModuleSetForRemoteModule(
 	options ...RemoteModuleOption,
 ) (ModuleSet, error) {
 	moduleSetBuilder := NewModuleSetBuilder(ctx, logger, moduleDataProvider, commitProvider)
-	moduleSetBuilder.AddRemoteModule(moduleKey, true, options...)
+	// This is a dependency aware graph.
+	//
+	// c1 -> a2,b1
+	//
 	graph, err := graphProvider.GetGraphForModuleKeys(ctx, []ModuleKey{moduleKey})
 	if err != nil {
 		return nil, err
 	}
 	if err := graph.WalkNodes(
-		func(node ModuleKey, _ []ModuleKey, _ []ModuleKey) error {
-			if node.CommitID() != moduleKey.CommitID() {
-				// Add the dependency ModuleKey with no path filters.
-				moduleSetBuilder.AddRemoteModule(node, false)
+		func(node ModuleKey, inbound []ModuleKey, outbound []ModuleKey) error {
+			digest, _ := node.Digest()
+			fmt.Println("Adding remote module", node.FullName(), digest)
+			fmt.Println("\tinbound:")
+			for _, inbound := range inbound {
+				fmt.Println("\t\t", inbound)
+			}
+			fmt.Println("\toutbound:")
+			for _, outbound := range outbound {
+				fmt.Println("\t\t", outbound)
+			}
+			// Add the remote module and its dependencies with with no options if it is not the target.
+			if isTarget := node.CommitID() == moduleKey.CommitID(); isTarget {
+				moduleSetBuilder.AddRemoteModule(node, isTarget, options...)
+			} else {
+				moduleSetBuilder.AddRemoteModule(node, isTarget)
 			}
 			return nil
 		},
@@ -355,6 +370,11 @@ func (b *moduleSetBuilder) AddLocalModule(
 		func() (ObjectData, error) { return localModuleOptions.v1BufYAMLObjectData, nil },
 		func() (ObjectData, error) { return localModuleOptions.v1BufLockObjectData, nil },
 		func() ([]ModuleKey, error) {
+			//moduleKeys
+			//for _, dep := range eachDep {
+			//	moduleKeys = NewModuleKey("", nil, dep.Digest)
+			//}
+
 			// See comment in added_module.go when we construct remote Modules for why
 			// we have this function in the first place.
 			return nil, syserror.Newf("getDeclaredDepModuleKeysB5 should never be called for a local Module")