diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index bb003573..6eaa3cb7 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -1,6 +1,7 @@ package index import ( + "bytes" "cmp" "context" "errors" @@ -387,6 +388,10 @@ func (ix *Indexer) applyOpen(ctx context.Context, f forgemsg.Forge) error { // write changes one key of a store by compare-and-set: compute is given the value read (found says there is one) and // answers the new value, or del to remove the key, or neither for no change. A conflict reads again, up to writeTries. // read, when given, is set to the revision of the value compute was last given. +// +// **A value whose bytes are the bytes held is not written** (issue 1966, 2026-11-08: a compare of two map types that never +// matched rewrote every module on every recompute, each write a views event, and a reader of the views took each as a gap +// and asked its whole first view again): whatever compute decides, a write that changes nothing but the revision is none. func write(ctx context.Context, st Store, key string, compute func(value []byte, found bool) (next []byte, del bool, err error), read ...*uint64) error { for try := 0; try < writeTries; try++ { value, revision, found, err := st.Get(ctx, key) @@ -401,7 +406,7 @@ func write(ctx context.Context, st Store, key string, compute func(value []byte, *r = revision } switch { - case next == nil && !del: + case next == nil && !del, unchanged(next, del, value, found): return nil case del: err = st.Delete(ctx, key, revision) @@ -421,6 +426,11 @@ func write(ctx context.Context, st Store, key string, compute func(value []byte, return fmt.Errorf("%s changed under every one of %d tries", key, writeTries) } +// unchanged says that compute's answer next is the value held: write writes nothing then (write). +func unchanged(next []byte, del bool, held []byte, found bool) bool { + return found && !del && next != nil && bytes.Equal(next, held) +} + // next is the group after the message; nil when the group is left with no pull request. // // A pull message that carries manifests (issue 1448) gives each module it touches, in the pull request's Fps, the digest of diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index 0ab8bc43..738427a5 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -324,7 +324,7 @@ func (ix *Indexer) recomputeLocked(ctx context.Context, overlay map[string]meshc finals[key] = final continue } - if !stale[key] && k.cur.Fp2 == r.fp && k.cur.None2 == r.none && reflect.DeepEqual(k.cur.Toolchains, r.tcs) && reflect.DeepEqual(k.cur.Packages, r.pkgs) { + if !stale[key] && k.cur.Fp2 == r.fp && k.cur.None2 == r.none && sameUsed(k.cur, r.tcs, r.pkgs) { continue } digest, inputs, paths := k.cur.Manifest, k.cur.Inputs2, k.cur.Paths @@ -337,7 +337,7 @@ func (ix *Indexer) recomputeLocked(ctx context.Context, overlay map[string]meshc if err != nil || cur.Manifest != digest || cur.Inputs2 == nil { return nil, false, nil // a merge changed it meanwhile; its own recompute follows } - if cur.Fp2 == r.fp && cur.None2 == r.none && reflect.DeepEqual(cur.Toolchains, r.tcs) && reflect.DeepEqual(cur.Packages, r.pkgs) { + if cur.Fp2 == r.fp && cur.None2 == r.none && sameUsed(cur, r.tcs, r.pkgs) { return nil, false, nil } if cur.Fp2 != r.fp { @@ -350,7 +350,7 @@ func (ix *Indexer) recomputeLocked(ctx context.Context, overlay map[string]meshc cur.Fp2, cur.None2, cur.Toolchains, cur.Packages = r.fp, r.none, r.tcs, r.pkgs cur.Inputs2, cur.Paths = inputs, paths data, err := meshcatalogclient.EncodeModule(cur) - wrote = err == nil + wrote = err == nil && !unchanged(data, false, value, found) return data, false, err }) if err != nil { @@ -408,7 +408,7 @@ func (ix *Indexer) writeOverlay(ctx context.Context, overlay, finals map[string] w.taken(next.Manifest) // a new module, or a changed manifest: fresh, judged in full (ADR 0579 ยง1) } data, err := meshcatalogclient.EncodeModule(next) - wrote = err == nil + wrote = err == nil && !unchanged(data, false, value, found) return data, false, err }) if err != nil { @@ -469,6 +469,14 @@ func (ix *Indexer) rejudged(ctx context.Context, m catalogue.Manifest, cur meshc return cur, true } +// sameUsed says whether the toolchains and the package versions a recompute gave are the ones the value holds. The value's +// package versions are the contract's map and the recompute's are sdkstep.PackageVersions: compared as maps, never by +// reflect.DeepEqual, which holds two map types unequal whatever they hold (issue 1966: every module was written again, with +// the same bytes, at every recompute). A nil map and an empty one are the same, as the value's encoding has it. +func sameUsed(cur meshcatalog.ModulesByModuleValue, tcs map[string]meshcatalog.ToolchainUsed, pkgs sdkstep.PackageVersions) bool { + return maps.Equal(cur.Toolchains, tcs) && maps.Equal(cur.Packages, map[string]string(pkgs)) +} + // hasContext is whether an artifact of the manifest reads another repository's tree. func hasContext(m catalogue.Manifest) bool { if m.Build == nil { diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go index 1e783939..762649d9 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains_test.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "maps" "reflect" "slices" "strings" @@ -378,14 +379,41 @@ func TestABuildHeardBeforeAnyToolchainIsReadDropsNoFingerprint(t *testing.T) { / } } -func TestAToolchainEntryChangedToTheSameValuesWritesNothing(t *testing.T) { // 1444/AC9 - ix, mods := newFormIndexer(t, builderModules(t)...) - reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") - before := mods.writes - time.Sleep(2 * time.Millisecond) - reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") // the same version and recipe, a later time - if mods.writes != before { - t.Errorf("modules written %d times after the same values, want none", mods.writes-before) +func TestAToolchainEntryChangedToTheSameValuesWritesNothing(t *testing.T) { // 1444/AC9; 1966: nothing moved writes nothing + ctx := context.Background() + for _, c := range []struct { + name string + do func(t *testing.T, ix *Indexer, mods *memStore) + }{ + {"the same version and recipe, a later time", func(t *testing.T, ix *Indexer, _ *memStore) { + time.Sleep(2 * time.Millisecond) + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") + }}, + // Issue 1966 (2026-11-08): the package versions compared by reflect.DeepEqual across two map types never matched, so + // every recompute wrote every module again with the bytes it held, each write a views event. + {"a recompute with nothing moved", func(t *testing.T, ix *Indexer, _ *memStore) { + if err := ix.Recompute(ctx); err != nil { + t.Fatal(err) + } + }}, + // write itself: an answer whose bytes are the bytes held is no write, whoever computed it. + {"a compute answering the bytes held", func(t *testing.T, ix *Indexer, mods *memStore) { + for _, key := range slices.Sorted(maps.Keys(mods.vals)) { + if err := write(ctx, ix.Modules, key, func(value []byte, _ bool) ([]byte, bool, error) { return slices.Clone(value), false, nil }); err != nil { + t.Fatal(err) + } + } + }}, + } { + t.Run(c.name, func(t *testing.T) { + ix, mods := newFormIndexer(t, builderModules(t)...) + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") + before := mods.writes + c.do(t, ix, mods) + if mods.writes != before { + t.Errorf("modules written %d times, want none", mods.writes-before) + } + }) } }