From 32542ebc14c3a5b5c3477afc170ea947ae2b26e3 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 23 Oct 2026 22:17:09 +0200 Subject: [PATCH] Keep an unreadable manifest's inputs and judge its verdict again at start, so an upgraded SDK clears it (issue 1923) --- .../mesh-delivery-indexer/integration_test.go | 71 +++++++++++++++++ .../internal/index/inputs.go | 2 +- .../internal/index/modules.go | 26 +++++++ .../internal/index/stale_none2_test.go | 77 +++++++++++++++++++ .../internal/index/toolchains.go | 44 ++++++++--- 5 files changed, 208 insertions(+), 12 deletions(-) create mode 100644 modules/mesh-delivery-indexer/internal/index/stale_none2_test.go diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go index b4e7349..110ca94 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer/integration_test.go @@ -478,3 +478,74 @@ func TestAMergeOfMeshSDKChangingOnlyGoLeavesItsNamedTreeEqual(t *testing.T) { t.Errorf("the new form moved: %q/%q, then %q/%q", first.FP2, first.None2, second.FP2, second.None2) } } + +// TestIndexerJudgesAnUnreadableVerdictAgainAtStart: the modules bucket holds avahi as an older SDK wrote it, with the reason +// 'the manifest cannot be read', the inputs the merge carried (the tree, the lock hash by file name) and the digest of its +// manifest, which manifests. holds and the running SDK reads; the go toolchain is reported. Within 5 s of the start +// and with no repository merge the value carries a fingerprint and no reason (issue 1923). +func TestIndexerJudgesAnUnreadableVerdictAgainAtStart(t *testing.T) { // 1923/AC3 + manifest, err := os.ReadFile("../../internal/index/testdata/catalogue-a67f0196-avahi.module.json") + if err != nil { + t.Fatal(err) + } + const goSum = "3b5028f0c1e9bde04dd766abd63eb5931bc516bae695cf46f00abcd8510b0381" + digest := delivery.ManifestDigest(manifest) + _, ctx, _, modules := startIndexer(t, func(context.Context, jetstream.KeyValue) {}, func(ctx context.Context, js jetstream.JetStream) { + mkey, err := delivery.ManifestsKey(digest) + if err != nil { + t.Fatal(err) + } + mdata, err := delivery.EncodeManifest(delivery.Manifest{Module: "avahi", JSON: string(manifest)}) + if err != nil { + t.Fatal(err) + } + manifests, err := js.KeyValue(ctx, delivery.ManifestsBucket) + if err != nil { + t.Fatal(err) + } + if _, err := manifests.Put(ctx, mkey, mdata); err != nil { + t.Fatal(err) + } + stale, err := delivery.EncodeModule(delivery.Module{Repository: "novox/mesh-catalog", Commit: "89abcdef0123456789abcdef0123456789abcdef", Path: "modules/avahi", + Manifest: digest, None2: "the manifest cannot be read: claims node-build-go, which is a seat in the mesh's own namespace that it does not define", + Inputs2: &delivery.KeptInputs2{Tree: "tree-avahi", Locks: map[string]string{"go.sum": goSum}}}) + if err != nil { + t.Fatal(err) + } + held, err := js.KeyValue(ctx, delivery.ModulesBucket) + if err != nil { + t.Fatal(err) + } + if _, err := held.Put(ctx, "avahi", stale); err != nil { + t.Fatal(err) + } + kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: delivery.ToolchainsBucket("mesh-build-go"), History: 1}) + if err != nil { + t.Fatal(err) + } + key, err := delivery.ToolchainKey("n1", "go") + if err != nil { + t.Fatal(err) + } + value, err := delivery.EncodeToolchain(delivery.Toolchain{Language: "go", Version: "go1.27.1", Recipe: "1", At: time.Now()}) + if err != nil { + t.Fatal(err) + } + if _, err := kv.Put(ctx, key, value); err != nil { + t.Fatal(err) + } + }) + var got delivery.Module + for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(50 * time.Millisecond) { + if e, err := modules.Get(ctx, "avahi"); err == nil { + if m, err := delivery.DecodeModule(e.Value()); err == nil { + if got = m; got.FP2 != "" { + break + } + } + } + } + if got.FP2 == "" || got.None2 != "" { + t.Errorf("avahi = fp2 %q none2 %q within 5 s of the start, want a fingerprint and no reason", got.FP2, got.None2) + } +} diff --git a/modules/mesh-delivery-indexer/internal/index/inputs.go b/modules/mesh-delivery-indexer/internal/index/inputs.go index b562587..d2e54a2 100644 --- a/modules/mesh-delivery-indexer/internal/index/inputs.go +++ b/modules/mesh-delivery-indexer/internal/index/inputs.go @@ -69,7 +69,7 @@ func inputsOf(module, raw, tree string, locks map[string]string, base baseFP) mo func inputsOfNamed(module, raw, tree string, locks, objects map[string]string, base baseFP) moduleInputs { m, err := catalogue.ParseManifestForInputs([]byte(raw)) if err != nil { - return moduleInputs{None2: "the manifest cannot be read: " + err.Error()} + return moduleInputs{None2: None2Unreadable + err.Error()} } if m.Module != module { return moduleInputs{None2: fmt.Sprintf("the manifest is of module %q, not %q", m.Module, module)} diff --git a/modules/mesh-delivery-indexer/internal/index/modules.go b/modules/mesh-delivery-indexer/internal/index/modules.go index 6dda09b..b37c893 100644 --- a/modules/mesh-delivery-indexer/internal/index/modules.go +++ b/modules/mesh-delivery-indexer/internal/index/modules.go @@ -70,6 +70,11 @@ func (ix *Indexer) applyMerged(ctx context.Context, f delivery.Forge) error { } else { in := inputsOfNamed(name, raw, f.Trees[dir], f.Locks[dir], objects[dir], ix.baseFP(ctx)) next.Inputs2, next.None2, next.ArtifactPaths = in.Kept, in.None2, in.ArtifactPaths + if in.Kept == nil && strings.HasPrefix(in.None2, None2Unreadable) { + // The manifest cannot be read by this SDK; the inputs the merge carried are kept all the same, so a later SDK + // that reads it judges the module again at start with no merge (issue 1923). + next.Inputs2 = unreadInputs(f.Trees[dir], f.Locks[dir], f.Paths[dir]) + } if in.Kept == nil && namedErr[dir] != "" { next.None2 = in.None2 + ": the forge could not say them: " + namedErr[dir] } @@ -253,3 +258,24 @@ func (ix *Indexer) Reconcile(ctx context.Context) error { } return nil } + +// None2Unreadable begins the reason of a module whose manifest the running SDK cannot read. +const None2Unreadable = "the manifest cannot be read: " + +// unreadInputs are the inputs a merge carried for a module whose manifest cannot be read: the directory's tree, the lock +// hashes by file name (the artifacts that own them are not known without the manifest) and the objects of the named +// paths the message carried; nil when the message carried no tree. Judging the manifest again (rejudged) turns them into +// the inputs of the module (issue 1923). +func unreadInputs(tree string, locks, paths map[string]string) *delivery.KeptInputs2 { + if tree == "" { + return nil + } + in := &delivery.KeptInputs2{Tree: tree} + if len(locks) > 0 { + in.Locks = maps.Clone(locks) + } + if len(paths) > 0 { + in.Paths = maps.Clone(paths) + } + return in +} diff --git a/modules/mesh-delivery-indexer/internal/index/stale_none2_test.go b/modules/mesh-delivery-indexer/internal/index/stale_none2_test.go new file mode 100644 index 0000000..24bb25b --- /dev/null +++ b/modules/mesh-delivery-indexer/internal/index/stale_none2_test.go @@ -0,0 +1,77 @@ +package index + +import ( + "context" + "strings" + "testing" +) + +// unreadableAvahi is avahi's manifest with a field the running parser refuses: the shape of a manifest an older SDK could +// not read. +func unreadableAvahi(t *testing.T) string { + t.Helper() + raw := strings.Replace(catalogueManifest(t, "avahi"), `"version": "1",`, `"version": "1", "scope": "everything",`, 1) + if raw == catalogueManifest(t, "avahi") { + t.Fatal("the fixture has no version line") + } + return raw +} + +func TestAMergeOfAnUnreadableManifestKeepsTheInputsBesideTheReason(t *testing.T) { // 1923/AC1 + _, mods := newFormIndexer(t, mod{"avahi", unreadableAvahi(t)}) + m, ok := moduleOf(t, mods, "avahi") + if !ok || !strings.HasPrefix(m.None2, "the manifest cannot be read: ") || m.FP2 != "" { + t.Fatalf("module = %+v, want the reason 'the manifest cannot be read' and no fingerprint", m) + } + if m.Inputs2 == nil || m.Inputs2.Tree != "tree-avahi" || m.Inputs2.Locks["go.sum"] != avahiGoSum { + t.Errorf("inputs = %+v, want the directory's tree and its lock hashes kept", m.Inputs2) + } +} + +func TestAStaleUnreadableVerdictIsJudgedAgainAtStartAndAnotherReasonIsLeft(t *testing.T) { // 1923/AC2 + ctx := context.Background() + // The fingerprint a merge of the readable manifest gives. + ixGood, good := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")}) + reportToolchain(t, ixGood, "mesh-build-go", "novox", "go", "go1.27.1") + reportToolchain(t, ixGood, "mesh-build-go", "ace", "go", "go1.27.1") + want, _ := moduleOf(t, good, "avahi") + if want.FP2 == "" { + t.Fatalf("the readable manifest gives no fingerprint: %+v", want) + } + + // The same module as an older SDK wrote it: the reason, the inputs kept at the merge, the manifest stored by digest. + ix, mods := newFormIndexer(t, mod{"avahi", unreadableAvahi(t)}) + reportToolchain(t, ix, "mesh-build-go", "novox", "go", "go1.27.1") + reportToolchain(t, ix, "mesh-build-go", "ace", "go", "go1.27.1") + stale, _ := moduleOf(t, mods, "avahi") + stale.Manifest = want.Manifest + ix.Manifests.(*memStore).put(want.Manifest, good2(t, ixGood, want.Manifest)) + mods.put("avahi", mustEncode(t, stale)) + if err := ix.Recompute(ctx); err != nil { + t.Fatal(err) + } + got, _ := moduleOf(t, mods, "avahi") + if got.FP2 != want.FP2 || got.None2 != "" { + t.Errorf("after the start: fp2 %q none2 %q, want %q and no reason", got.FP2, got.None2, want.FP2) + } + + // A manifest still unreadable keeps its value as it was. + ix2, mods2 := newFormIndexer(t, mod{"avahi", unreadableAvahi(t)}) + before := string(mods2.vals["avahi"]) + if err := ix2.Recompute(ctx); err != nil { + t.Fatal(err) + } + if after := string(mods2.vals["avahi"]); after != before { + t.Errorf("a manifest still unreadable changed the value:\n%s\n%s", before, after) + } +} + +// good2 is the stored manifest value of the digest in the indexer that merged the readable manifest. +func good2(t *testing.T, ix *Indexer, digest string) []byte { + t.Helper() + v, _, found, err := ix.Manifests.Get(context.Background(), digest) + if err != nil || !found { + t.Fatalf("manifests.%s: %v %v", digest, found, err) + } + return v +} diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index 367e179..43db138 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -156,6 +156,7 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli } slices.Sort(keys) held := map[string]delivery.Module{} + stale := map[string]bool{} // values an SDK that could not read their manifest wrote, judged again (issue 1923) all := map[string]*known{} for _, key := range keys { cur, ok := overlay[key] @@ -176,11 +177,15 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli if cur.Manifest == "" || cur.Inputs2 == nil { continue } - m, err := ix.manifestOf(ctx, cur.Manifest) + raw, m, err := ix.manifestRaw(ctx, cur.Manifest) if err != nil { ix.logf("modules.%s: its manifest %s cannot be read, left: %v", key, cur.Manifest, err) continue } + if again, ok := ix.rejudged(ctx, m, cur, raw); ok { + cur, stale[key] = again, true + held[key] = cur + } all[key] = &known{key: key, cur: cur, languages: languagesOf(m), bases: basesOf(m), contexts: hasContext(m)} } tools := ix.agreed() @@ -275,10 +280,10 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli finals[key] = final continue } - if k.cur.FP2 == r.fp && k.cur.None2 == r.none && reflect.DeepEqual(k.cur.Toolchains, r.tcs) { + if !stale[key] && k.cur.FP2 == r.fp && k.cur.None2 == r.none && reflect.DeepEqual(k.cur.Toolchains, r.tcs) { continue } - digest, inputs := k.cur.Manifest, k.cur.Inputs2 + digest, inputs, paths := k.cur.Manifest, k.cur.Inputs2, k.cur.ArtifactPaths err := write(ctx, ix.Modules, key, func(value []byte, found bool) ([]byte, bool, error) { if !found { return nil, false, nil @@ -291,7 +296,7 @@ func (ix *Indexer) recomputeModules(ctx context.Context, overlay map[string]deli return nil, false, nil } cur.FP2, cur.None2, cur.Toolchains = r.fp, r.none, r.tcs - cur.Inputs2 = inputs + cur.Inputs2, cur.ArtifactPaths = inputs, paths data, err := delivery.EncodeModule(cur) return data, false, err }) @@ -326,24 +331,41 @@ func (ix *Indexer) writeOverlay(ctx context.Context, overlay, finals map[string] return nil } -// manifestOf is the parsed manifest manifests. holds. -func (ix *Indexer) manifestOf(ctx context.Context, digest string) (catalogue.Manifest, error) { +// manifestRaw is manifests.: the manifest as stored and parsed by the running SDK. +func (ix *Indexer) manifestRaw(ctx context.Context, digest string) (string, catalogue.Manifest, error) { key, err := delivery.ManifestsKey(digest) if err != nil { - return catalogue.Manifest{}, err + return "", catalogue.Manifest{}, err } value, _, found, err := ix.Manifests.Get(ctx, key) if err != nil { - return catalogue.Manifest{}, err + return "", catalogue.Manifest{}, err } if !found { - return catalogue.Manifest{}, fmt.Errorf("manifests.%s is not there", key) + return "", catalogue.Manifest{}, fmt.Errorf("manifests.%s is not there", key) } kept, err := delivery.DecodeManifest(value) if err != nil { - return catalogue.Manifest{}, err + return "", catalogue.Manifest{}, err } - return catalogue.ParseManifestForInputs([]byte(kept.JSON)) + m, err := catalogue.ParseManifestForInputs([]byte(kept.JSON)) + return kept.JSON, m, err +} + +// rejudged is a value an SDK that could not read its manifest wrote, as the running SDK judges it: its kept inputs (the +// tree, the lock hashes by file name, the named paths' objects) taken through the manifest as a merge takes them, and the +// reason that gives. It answers false for any other value, and for one whose manifest the running SDK still cannot make +// inputs of (issue 1923). +func (ix *Indexer) rejudged(ctx context.Context, m catalogue.Manifest, cur delivery.Module, raw string) (delivery.Module, bool) { + if cur.FP2 != "" || cur.Inputs2 == nil || !strings.HasPrefix(cur.None2, None2Unreadable) { + return cur, false + } + in := inputsOfNamed(m.Module, raw, cur.Inputs2.Tree, cur.Inputs2.Locks, cur.Inputs2.Paths, ix.baseFP(ctx)) + if in.Kept == nil { + return cur, false + } + cur.Inputs2, cur.None2, cur.ArtifactPaths = in.Kept, in.None2, in.ArtifactPaths + return cur, true } // hasContext is whether an artifact of the manifest reads another repository's tree. -- 2.54.0