diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 2b0d9e61..0849ba47 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -150,14 +150,15 @@ type Indexer struct { retrying atomic.Bool // a RetryUnread runs in the background (kickRetry) retryAgain atomic.Bool // an event asked for one more RetryUnread after the running one retryWG sync.WaitGroup // the background RetryUnread, for tests to wait on - fresh map[string]bool // modules whose import form is computed again by hand (FingerprintsAgain), "*" for every one; under recompute - recompute sync.Mutex // one Recompute at a time - untaken map[string]string // repository to the latest merge applied while the taken state was missing (taken.go) - filling atomic.Bool // a FillTaken runs in the background (KickFill) - fillAgain atomic.Bool // a kick arrived while it ran - atHeads map[string]map[string]meshcatalog.PackagesByModuleValue // a member head's published Go modules, by repository@head (candidatePublished) - saidAt map[string]string // the head of a repository whose failed read was said, by repository: said once per head - failedAt map[string]string // the head of a repository whose tree lacks a directory asked, by repository: not asked again until the head changes + fresh map[string]bool // modules whose import form is computed again by hand (FingerprintsAgain), "*" for every one; under freshMu + freshMu sync.Mutex + recompute sync.Mutex // one Recompute at a time + untaken map[string]string // repository to the latest merge applied while the taken state was missing (taken.go) + filling atomic.Bool // a FillTaken runs in the background (KickFill) + fillAgain atomic.Bool // a kick arrived while it ran + atHeads map[string]map[string]meshcatalog.PackagesByModuleValue // a member head's published Go modules, by repository@head (candidatePublished) + saidAt map[string]string // the head of a repository whose failed read was said, by repository: said once per head + failedAt map[string]string // the head of a repository whose tree lacks a directory asked, by repository: not asked again until the head changes // OwnBuild answers the fingerprint of the build this program runs, from the node-engine's seat; nil knows none. OwnBuild func(ctx context.Context) (string, error) underSeats bool // a dry run inside judge.WithSeats: its judge passes do not hold the seat list (verdicts.go) diff --git a/modules/mesh-delivery-indexer/internal/index/fingerprints.go b/modules/mesh-delivery-indexer/internal/index/fingerprints.go index f4fcdba1..a70ddf57 100644 --- a/modules/mesh-delivery-indexer/internal/index/fingerprints.go +++ b/modules/mesh-delivery-indexer/internal/index/fingerprints.go @@ -40,8 +40,6 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn out.Answer = "the indexer writes no fingerprints" return out } - ix.recompute.Lock() - defer ix.recompute.Unlock() before, err := ix.fingerprintsHeld(ctx) if err != nil { out.Answer = "the modules state cannot be read: " + err.Error() @@ -57,13 +55,22 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn if module == "all" { key = "*" } + ix.freshMu.Lock() ix.fresh = map[string]bool{key: true} - err = ix.recomputeLocked(ctx, nil, nil) + ix.freshMu.Unlock() + // The path of a merge's recompute: the modules, then the candidates their fingerprints reach (recomputeWith), then the + // pending merges a changed fingerprint settles (PendingAgain, as after a toolchain report). + err = ix.recomputeWith(ctx, nil, nil) + ix.freshMu.Lock() ix.fresh = nil + ix.freshMu.Unlock() if err != nil { out.Answer = "nothing was written: " + err.Error() return out } + if err := ix.PendingAgain(ctx); err != nil { + ix.logf("the pending merges are not evaluated again after the fingerprints were computed again: %v", err) + } after, err := ix.fingerprintsHeld(ctx) if err != nil { out.Answer = "computed, but the modules state cannot be read back: " + err.Error() @@ -82,6 +89,13 @@ func (ix *Indexer) FingerprintsAgain(ctx context.Context, module string) TakenIn return out } +// isFresh says the module's import form is computed again by hand in this pass. +func (ix *Indexer) isFresh(module string) bool { + ix.freshMu.Lock() + defer ix.freshMu.Unlock() + return ix.fresh[module] || ix.fresh["*"] +} + // fingerprintsHeld is each held module's fingerprint of the new form. func (ix *Indexer) fingerprintsHeld(ctx context.Context) (map[string]string, error) { keys, err := ix.Modules.Keys(ctx) @@ -113,7 +127,7 @@ func FingerprintsTools(ask func(key string, value []byte) (TakeInAsk, error), no "the import form is written for every module whose imports are all published, whatever fingerprint the module holds. For the " + "fingerprints a pass wrote while the packages state was unread (the version line over the import form: builds then fail on an " + "older package). Answers each module whose fingerprint moved, old and new; when the packages state cannot be read it writes " + - "nothing and says why. Acts first, then records the hand act through mesh-hand-acts; when that cannot be recorded the answer says so.", + "nothing and says why. Runs as a merge's recompute does (the modules, the candidates they reach, the pending merges they settle). Acts first, then records the hand act through mesh-hand-acts; when that cannot be recorded the answer says so.", Input: map[string]any{"type": "object", "required": []string{"why"}, "properties": map[string]any{ "module": map[string]any{"type": "string", "description": "the module's name, or all (the default)"}, "why": map[string]any{"type": "string", "description": "why it is computed again now, in plain words"}}}, diff --git a/modules/mesh-delivery-indexer/internal/index/fingerprints_test.go b/modules/mesh-delivery-indexer/internal/index/fingerprints_test.go deleted file mode 100644 index 82af8a7a..00000000 --- a/modules/mesh-delivery-indexer/internal/index/fingerprints_test.go +++ /dev/null @@ -1,45 +0,0 @@ -package index - -import ( - "errors" - "strings" - "testing" - "time" -) - -// indexer_fingerprints_again acts first and records the hand act after: a record that fails, or never answers, is said in -// the answer and never refuses or holds the recovery (mesh-hand-acts on a mesh with no ledger answers nothing). -func TestFingerprintsAgainActsBeforeItRecordsAndSaysAFailedRecord(t *testing.T) { - for _, c := range []struct { - name string - record func(what, why string) error - said string - }{ - {"recorded", func(string, string) error { return nil }, ""}, - {"record refused", func(string, string) error { return errors.New("no ledger") }, "not recorded: no ledger"}, - {"record never answers", func(string, string) error { select {} }, "not recorded within"}, - } { - t.Run(c.name, func(t *testing.T) { - var asked []string - ask := func(key string, _ []byte) (TakeInAsk, error) { - asked = append(asked, key) - return TakeInAsk{Result: &TakenIn{Module: "all", Stored: true}}, nil - } - tool := FingerprintsTools(ask, time.Now, c.record, 10*time.Millisecond)[0] - if _, err := tool.Run(map[string]any{"module": "avahi"}); err == nil { - t.Fatal("a hand act with no why was done") - } - if len(asked) != 0 { - t.Fatalf("asked the indexer without a why: %v", asked) - } - got, err := tool.Run(map[string]any{"module": "avahi", "why": "the packages read came back empty"}) - if err != nil || len(asked) != 1 || !strings.HasPrefix(asked[0], "fingerprints.avahi.") { - t.Fatalf("run: %v, asked %v", err, asked) - } - said, _ := got.(map[string]any)["hand-act"].(string) - if !strings.Contains(said, c.said) || (c.said == "") != (said == "") { - t.Fatalf("hand-act %q, want one saying %q", said, c.said) - } - }) - } -} diff --git a/modules/mesh-delivery-indexer/internal/index/imports_test.go b/modules/mesh-delivery-indexer/internal/index/imports_test.go index 880cf400..3d61e5f9 100644 --- a/modules/mesh-delivery-indexer/internal/index/imports_test.go +++ b/modules/mesh-delivery-indexer/internal/index/imports_test.go @@ -9,6 +9,7 @@ import ( "git.novox.be/novox/mesh-delivery/contracts/go/clients/meshcatalogclient" "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshcatalog" + "git.novox.be/novox/mesh-delivery/contracts/go/seats/meshdeliverychecker" "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/builds" ) @@ -52,6 +53,7 @@ func (m *missingDir) Error() string { return "no directory " + m.dir } func TestImportsReportedMoveTheFingerprintOnlyWithAnImportedPackage(t *testing.T) { t.Run("a build of an older fingerprint keeps no imports", buildOfAnOlderFingerprintKeepsNoImports) t.Run("a change of a sibling directory alone moves its importer", aSiblingDirectoryAloneMovesItsImporter) + t.Run("an unread packages state writes no fingerprint, and they are computed again by hand (issue 1966)", anUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain) t.Run("an accepted series' newest published version pins it, by that version's contents", anAcceptedSeriesPinsItsNewest) ctx := context.Background() ix, mods := newFormIndexer(t, mod{"avahi", catalogueManifest(t, "avahi")}) @@ -276,7 +278,7 @@ func anAcceptedSeriesPinsItsNewest(t *testing.T) { // answered no published package where held modules were computed from published ones, writes no fingerprint (the version // line over every import form is what an empty state computes); a version line written leaves no import form behind // that a later pass takes for the build's; and indexer_fingerprints_again computes the import form again by hand. -func TestAnUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain(t *testing.T) { +func anUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain(t *testing.T) { ctx := context.Background() // avahi at the import form, pinned to mesh-sdk at c3, as the test above leaves it. setup := func(t *testing.T) (*Indexer, *memStore, meshcatalog.ModulesByModuleValue) { @@ -359,6 +361,11 @@ func TestAnUnreadPackagesStateWritesNoFingerprintAndFingerprintsAreComputedAgain if now, _ := moduleOf(t, mods, "avahi"); now.Fp2 != good.Fp2 || ix.fresh != nil { t.Fatalf("avahi holds %q after it, want %q (fresh %v)", now.Fp2, good.Fp2, ix.fresh) } + // It runs a merge's recompute, candidates included: a candidate store that cannot be listed fails it, said. + ix.Store, ix.previews = &memStore{keysErr: errors.New("no bus")}, map[string]meshdeliverychecker.PreviewsByRepositoryHeadTrunkValue{"k": {}} + if r := ix.FingerprintsAgain(ctx, "avahi"); r.Stored || !strings.Contains(r.Answer, "listing the candidates") { + t.Fatalf("fingerprints again over candidates that cannot be listed: %+v", r) + } if r := ix.FingerprintsAgain(ctx, "nothing"); r.Stored { t.Fatalf("fingerprints again of a module not held answered stored: %+v", r) } diff --git a/modules/mesh-delivery-indexer/internal/index/pending_test.go b/modules/mesh-delivery-indexer/internal/index/pending_test.go index e7d52e72..0ecaef44 100644 --- a/modules/mesh-delivery-indexer/internal/index/pending_test.go +++ b/modules/mesh-delivery-indexer/internal/index/pending_test.go @@ -135,6 +135,40 @@ func sha(c byte) string { return strings.Repeat(string(c), 40) } func TestTakeIn(t *testing.T) { // 1585/T3-T13 (ADR 0633) ctx := context.Background() + t.Run("indexer_fingerprints_again acts before it records the hand act and says a failed record", func(t *testing.T) { // issue 1966 + for _, c := range []struct { + name string + record func(what, why string) error + said string + }{ + {"recorded", func(string, string) error { return nil }, ""}, + {"record refused", func(string, string) error { return errors.New("no ledger") }, "not recorded: no ledger"}, + {"record never answers", func(string, string) error { select {} }, "not recorded within"}, + } { + t.Run(c.name, func(t *testing.T) { + var asked []string + ask := func(key string, _ []byte) (TakeInAsk, error) { + asked = append(asked, key) + return TakeInAsk{Result: &TakenIn{Module: "all", Stored: true}}, nil + } + tool := FingerprintsTools(ask, time.Now, c.record, 10*time.Millisecond)[0] + if _, err := tool.Run(map[string]any{"module": "avahi"}); err == nil { + t.Fatal("a hand act with no why was done") + } + if len(asked) != 0 { + t.Fatalf("asked the indexer without a why: %v", asked) + } + got, err := tool.Run(map[string]any{"module": "avahi", "why": "the packages read came back empty"}) + if err != nil || len(asked) != 1 || !strings.HasPrefix(asked[0], "fingerprints.avahi.") { + t.Fatalf("run: %v, asked %v", err, asked) + } + said, _ := got.(map[string]any)["hand-act"].(string) + if !strings.Contains(said, c.said) || (c.said == "") != (said == "") { + t.Fatalf("hand-act %q, want one saying %q", said, c.said) + } + }) + } + }) t.Run("T6 a merge while the own build is unknown writes nothing", func(t *testing.T) { b := newTakeInBed(t) b.ix.ownFP, b.ix.ownFixed = "", false // this process is started again and asks the engine, which does not answer diff --git a/modules/mesh-delivery-indexer/internal/index/toolchains.go b/modules/mesh-delivery-indexer/internal/index/toolchains.go index 884d84a0..71044311 100644 --- a/modules/mesh-delivery-indexer/internal/index/toolchains.go +++ b/modules/mesh-delivery-indexer/internal/index/toolchains.go @@ -264,7 +264,7 @@ func (ix *Indexer) recomputeLocked(ctx context.Context, overlay map[string]meshc chain = append(chain, module) kept := *k.cur.Inputs2 kept.Bases, kept.Builder = nil, "" - if ix.fresh[module] || ix.fresh["*"] { // indexer_fingerprints_again: the import form is written again, the held one stands for nothing + if ix.isFresh(module) { // indexer_fingerprints_again: the import form is written again, the held one stands for nothing kept.ImportForm = "" } var missing []string