diff --git a/modules/mesh-delivery-builder/internal/build/claim.go b/modules/mesh-delivery-builder/internal/build/claim.go index c99779f..576a450 100644 --- a/modules/mesh-delivery-builder/internal/build/claim.go +++ b/modules/mesh-delivery-builder/internal/build/claim.go @@ -4,10 +4,12 @@ import ( "context" "errors" "fmt" - "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" + "sort" "strings" + "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" + "git.novox.be/novox/mesh-sdk/go/delivery/buildrun" ) // ErrConflict is what a Store answers when a Create finds the key or an Update finds another revision than the one given. @@ -66,23 +68,48 @@ type ClaimOptions struct { // ImageBuilder says whether a node runs a healthy image build module now: a key failed for NoImageBuilderPrefix is taken // again only then. Nil means none does. ImageBuilder func() bool - // Trunk is whether the trunk's record names this fingerprint. A done key whose image has no reference was built as a - // candidate's, which never pushes (ADR 0487 clause 12); on the trunk it is taken again to publish the image, when an image - // builder runs (ImageBuilder). + // Trunk is whether the trunk's record names this fingerprint. A done key whose image or package has no reference was + // built as a candidate's, which never pushes or publishes (ADR 0487 clause 12, ADR 0498 clauses 5 and 6); on the trunk it + // is taken again to publish them, an image only when an image builder runs (ImageBuilder). Trunk bool } -// Unpublished is whether a done build holds an image built without a push: it has no reference. -func Unpublished(b delivery.BuilderBuild) bool { +// Unpublished is whether a done build holds an image built without a push or a package built without a publish: it has +// no reference (a candidate's, ADR 0487 clause 12, ADR 0498 clauses 5 and 6). +func Unpublished(b delivery.BuilderBuild) bool { return len(UnpublishedModules(b)) > 0 } + +// UnpublishedModules are the build modules that publish what a done build holds unpublished: the image build module for +// an image, the Node build module for a package; sorted, none when it holds nothing unpublished. +func UnpublishedModules(b delivery.BuilderBuild) []string { if b.State != delivery.BuildDone { - return false + return nil } + var image, pkg bool for _, a := range b.Artifacts { - if a.Kind == "image" && a.Reference == "" { - return true + image = image || (a.Kind == catalogue.ArtifactImage && a.Reference == "") + pkg = pkg || (a.Kind == catalogue.ArtifactPackage && a.Reference == "") + } + var out []string + if image { + out = append(out, ImageBuildModule) + } + if pkg { + out = append(out, buildrun.NodeBuildModule) + } + sort.Strings(out) + return out +} + +// publishable are the build modules of UnpublishedModules that can publish now: the image build module only while a node +// runs it (imageBuilder). +func publishable(b delivery.BuilderBuild, imageBuilder func() bool) []string { + var out []string + for _, bm := range UnpublishedModules(b) { + if bm != ImageBuildModule || (imageBuilder != nil && imageBuilder()) { + out = append(out, bm) } } - return false + return out } // Unreferenced is whether a done build holds a bundle, an archive or an upstream copy that names no reference: a key done @@ -139,7 +166,7 @@ func (c Claimer) Claim(ctx context.Context, module, fingerprint, manifest string if modules := OneNodeModules(cur); len(modules) > 0 && opt.MoreNodes != nil && opt.MoreNodes(modules) { return &Claim{Key: key, Rev: rev, Build: cur}, nil } - if opt.Trunk && Unpublished(cur) && opt.ImageBuilder != nil && opt.ImageBuilder() { + if opt.Trunk && len(publishable(cur, opt.ImageBuilder)) > 0 { return &Claim{Key: key, Rev: rev, Build: cur}, nil } return nil, nil diff --git a/modules/mesh-delivery-builder/internal/build/r4e_test.go b/modules/mesh-delivery-builder/internal/build/r4e_test.go index a4b7379..3205bb7 100644 --- a/modules/mesh-delivery-builder/internal/build/r4e_test.go +++ b/modules/mesh-delivery-builder/internal/build/r4e_test.go @@ -200,3 +200,47 @@ func TestAVersionTakenFailsTheKeyUntilTheFingerprintChanges(t *testing.T) { // B t.Errorf("a version-taken key was built again (%d writes, was %d)", r.st.writes, writes) } } + +func TestACandidatesPackageIsPublishedOnceWhenTheTrunkNamesItsFingerprint(t *testing.T) { // B6, ADR 0498 clause 6 + r := newRig(t, tsPackage, tsBundle) + r.bm.packages["lib"] = "@novox/mesh-sdk@0.1.15" + trunkModule(t, r) + r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildDone { + t.Fatalf("candidate build = %+v", v) + } + rev := r.st.revs[r.key] + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) // the merge names the candidate's fingerprint + r.b.Wait() + v := valueOf(t, r.st, r.key) + if r.st.revs[r.key] <= rev { + t.Fatalf("the trunk did not build the unpublished package again (revision %d, was %d)", r.st.revs[r.key], rev) + } + if ok, missing := v.Referenced(); !ok { + t.Fatalf("build = %+v, missing references %v", v, missing) + } + rev = r.st.revs[r.key] + r.b.tried = map[string]bool{} + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Wait() + if r.st.revs[r.key] != rev { + t.Error("a published package was built again: once only") + } +} + +func TestAPublishThatFailsVersionTakenOnTheTrunkKeepsThePrefixFirst(t *testing.T) { // B6, ADR 0498 clauses 5 and 6 + r := newRig(t, tsPackage) + r.bm.packages["lib"] = "@novox/mesh-sdk@0.1.15" + trunkModule(t, r) + r.b.Candidate(context.Background(), "feat%2Fx", candidateValue(t, "feat/x", 7, headX)) + r.b.Wait() + for _, n := range []string{"ace", "novox"} { + r.bm.fails[n+"/build_node_build"] = delivery.VersionTaken + "@novox/mesh-sdk@0.1.15 is published with another packed file" + } + r.b.Module(context.Background(), "m", moduleValue(t, "modules/m")) + r.b.Wait() + if v := valueOf(t, r.st, r.key); v.State != delivery.BuildFailed || !strings.HasPrefix(v.Error, delivery.VersionTaken) { + t.Errorf("build = %+v, want failed opening %q", v, delivery.VersionTaken) + } +} diff --git a/modules/mesh-delivery-builder/internal/build/run.go b/modules/mesh-delivery-builder/internal/build/run.go index 64d8889..6754c17 100644 --- a/modules/mesh-delivery-builder/internal/build/run.go +++ b/modules/mesh-delivery-builder/internal/build/run.go @@ -316,9 +316,11 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { for _, bm := range OneNodeModules(cl.Build) { only[bm] = true } - if trunk && Unpublished(cl.Build) { - only[ImageBuildModule] = true - publishing = true + if trunk { + for _, bm := range publishable(cl.Build, b.imageBuilderRuns) { + only[bm] = true + publishing = true + } } } result := b.build(ctx, module, m, plan, planErr, prior, only) @@ -331,9 +333,9 @@ func (b *Builder) try(ctx context.Context, module string, m delivery.Module) { } result.By, result.Manifest = b.Claimer.By, m.Manifest if publishing && result.State == delivery.BuildFailed { - // The trunk's image was built only as a candidate's and its push failed: the key says so, so the builds view, the + // The trunk's image or package was built only as a candidate's and its push or publish failed: the key says so, so the builds view, the // checker and the failed-key retake see it, and a look does not build the image again for nothing. - result.Error = "publishing the image of the merged commit failed: " + result.Error + result.Error = keyError([]string{"publishing what the merged commit built failed: " + result.Error}) } else if cl.Build.State == delivery.BuildDone && result.State == delivery.BuildFailed && !strings.HasPrefix(result.Error, buildrun.DigestsDifferPrefix) { // A done key built again on a second node keeps its done value when the new build fails for another reason than // the digests: the first build stands, and a later change of the nodes looks again.