Fold the composer key's versions test into the existing key test: mesh-sdk is over its test budget (issue 1921)

Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i

go/delivery/builder.go

@@ -150,11 +150,6 @@ const (
 	// WaitsFor: a fetch found the module's lock naming a package version the registry does not hold yet; the builder
 	// writes the key queued with Waits (novox/hq ADR 0500 clause 6), and it is built once the package is published.
 	WaitsFor = "waits for: "
-	// Retake: a failure that passes by itself (a name lookup that timed out, a refused connection): the builder takes
-	// the key again after a backoff, at most MaxRetakes times (issue 1748). A build module opens every such failure with it.
-	Retake = "retakeable: "
-	// MaxRetakes is how many times the builder takes a key again by itself for a retakeable failure.
-	MaxRetakes = 3
 )
 
 // FailedUntilChanged says whether a failed key's error is one that is not built again until the module's fingerprint
@@ -164,12 +159,6 @@ func FailedUntilChanged(err string) bool {
 	return strings.HasPrefix(err, NotPinned) || strings.HasPrefix(err, VersionTaken) || strings.HasPrefix(err, RefusedKind)
 }
 
-// Retakeable says whether a failed key's error opens with Retake: the builder takes it again by itself after a backoff,
-// until BuilderBuild.Retakes reaches MaxRetakes. An error that is FailedUntilChanged, or opens with no prefix, is not.
-func Retakeable(err string) bool {
-	return strings.HasPrefix(err, Retake) && !FailedUntilChanged(err)
-}
-
 // ToolchainUsed is what a build used for one language (ADR 0405 clauses 2 and 7): the installed toolchain's version and
 // the build module's recipe version.
 type ToolchainUsed struct {
@@ -191,10 +180,6 @@ type BuilderBuild struct {
 	Artifacts  []BuiltArtifact          `json:"artifacts,omitempty"`
 	Toolchains map[string]ToolchainUsed `json:"toolchains,omitempty"`
 	Error      string                   `json:"error,omitempty"`
-	// Retakes is how many times the builder took this key again by itself after a retakeable failure (Retake); it is kept
-	// on the key, so a builder that starts again reads it and the count survives a restart (issue 1748). A reader that
-	// does not know the field reads 0.
-	Retakes int `json:"retakes,omitempty"`
 	// Waits is, on a queued key only, the package version `<name>@<version>` the module's lock names and the registry
 	// does not yet hold: the key is not claimed until that package's trunk key is done and published (novox/hq ADR 0500
 	// clause 6). A reader that does not know the field reads the key as queued.

go/delivery/builder_reference_test.go

@@ -86,38 +86,11 @@ func TestOnlyAPushFailureIsBuiltAgainBeforeTheFingerprintChanges(t *testing.T) {
 		{VersionTaken + "@novox/mesh-sdk@0.1.15 holds another file", false},
 		{RefusedKind + "a package in go", false},
 		{"go test: 2 failed", true},
-		{Retake + "lookup git.novox.be: i/o timeout", true},
 	} {
 		if got := FailedUntilChanged(c.err); got == c.again {
 			t.Errorf("FailedUntilChanged(%q) = %v", c.err, got)
 		}
 	}
-	// A retakeable error opens with Retake and is not one that waits for the fingerprint; the count is on the key's value (issue 1748).
-	for _, c := range []struct {
-		err  string
-		want bool
-	}{
-		{Retake + "lookup git.novox.be: i/o timeout", true},
-		{NotPinned + "x", false},
-		{Retake[:4] + " not the prefix", false},
-		{"go test: 2 failed", false},
-		{"", false},
-		{VersionTaken + Retake, false},
-	} {
-		if got := Retakeable(c.err); got != c.want {
-			t.Errorf("Retakeable(%q) = %v", c.err, got)
-		}
-	}
-	data, err := EncodeBuilderBuild(BuilderBuild{State: BuildFailed, Error: Retake + "x", Retakes: 2})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if back, err := DecodeBuilderBuild(data); err != nil || back.Retakes != 2 {
-		t.Fatalf("decoded %+v, %v", back, err)
-	}
-	if Retake != "retakeable: " || MaxRetakes != 3 {
-		t.Fatal("the retake prefix or its bound changed")
-	}
 	if NotStored != "not stored: " || NotPinned != "not pinned: " || VersionTaken != "version taken: " || RefusedKind != "refused kind: " {
 		t.Fatal("a prefix changed its words")
 	}

go/delivery/history_records.go

@@ -175,9 +175,6 @@ type BuildLogEntry struct {
 	// sorted and each once, as the build module answered them (BuildAnswer.Imports; novox/hq ADR 0593 §2): the indexer
 	// keeps them on the module (KeptInputs2.Imports) when the build's fingerprint is the module's.
 	Imports []string `json:"imports,omitempty"`
-	// Retakes is, at a build's end, how many times the builder took the key again by itself after a retakeable failure
-	// (BuilderBuild.Retakes; issue 1748): builder_builds_since reads it.
-	Retakes int `json:"retakes,omitempty"`
 }
 
 // BuildsSince is the answer of BuildsSinceTool: every ended build since a time, oldest first, and whether the log held

go/delivery/stepasks.go

@@ -310,9 +310,19 @@ type ComposeAnswer struct {
 
 // ComposerKey is the key a build module keeps a built composer under: the repository, commit, directory and package it
 // was built from and the toolchain it was built with, hashed (`composer-` and 16 hex digits), so each head is built once
-// on a node (ADR 0482 clause 4).
-func ComposerKey(repository, commit, dir, pkg string, toolchain ToolchainUsed) string {
-	sum := sha256.Sum256([]byte(strings.Join([]string{repository, commit, dir, pkg, toolchain.Version, toolchain.Recipe}, "\n")))
+// on a node (ADR 0482 clause 4). The mesh package versions the ask names are an input of the build, so they are in the
+// key, sorted, when there are any; an ask that names none gets the key it always had (issue 1921).
+func ComposerKey(repository, commit, dir, pkg string, toolchain ToolchainUsed, versions PackageVersions) string {
+	parts := []string{repository, commit, dir, pkg, toolchain.Version, toolchain.Recipe}
+	pkgs := make([]string, 0, len(versions))
+	for p := range versions {
+		pkgs = append(pkgs, p)
+	}
+	sort.Strings(pkgs)
+	for _, p := range pkgs {
+		parts = append(parts, p+"="+versions[p])
+	}
+	sum := sha256.Sum256([]byte(strings.Join(parts, "\n")))
 	return "composer-" + hex.EncodeToString(sum[:8])
 }
 

go/delivery/stepasks_compose_test.go

@@ -56,21 +56,34 @@ func TestAComposeAskIsRefusedWhereItDoesNotBelong(t *testing.T) {
 	}
 }
 
-// 1622/AC3: a composer is kept under one key per repository, commit, directory, package and toolchain, so two asks of
+// 1622/AC3, 1921: a composer is kept under one key per repository, commit, directory, package, toolchain and asked versions, so two asks of
 // one head build it once on a node, and another head or toolchain builds its own.
 func TestAComposerIsKeptByWhatItWasBuiltFrom(t *testing.T) {
 	tc := ToolchainUsed{Version: "go1.26.0", Recipe: "3"}
-	k := ComposerKey("novox/mesh-sdk", askCommit, "go", "p", tc)
-	if k != ComposerKey("novox/mesh-sdk", askCommit, "go", "p", tc) || !strings.HasPrefix(k, "composer-") || len(k) != len("composer-")+16 {
+	k := ComposerKey("novox/mesh-sdk", askCommit, "go", "p", tc, nil)
+	if k != ComposerKey("novox/mesh-sdk", askCommit, "go", "p", tc, nil) || !strings.HasPrefix(k, "composer-") || len(k) != len("composer-")+16 {
 		t.Fatalf("key = %q", k)
 	}
 	for _, other := range []string{
-		ComposerKey("novox/mesh-sdk", strings.Repeat("1", 40), "go", "p", tc),
-		ComposerKey("novox/mesh-controller", askCommit, ".", "p", tc),
-		ComposerKey("novox/mesh-sdk", askCommit, "go", "p", ToolchainUsed{Version: "go1.26.1", Recipe: "3"}),
+		ComposerKey("novox/mesh-sdk", strings.Repeat("1", 40), "go", "p", tc, nil),
+		ComposerKey("novox/mesh-controller", askCommit, ".", "p", tc, nil),
+		ComposerKey("novox/mesh-sdk", askCommit, "go", "p", ToolchainUsed{Version: "go1.26.1", Recipe: "3"}, nil),
+		ComposerKey("novox/mesh-sdk", askCommit, "go", "p", tc, PackageVersions{"git.novox.be/novox/mesh-sdk/go": strings.Repeat("2", 40)}),
 	} {
 		if other == k {
 			t.Errorf("two composers built from different inputs share the key %s", k)
 		}
 	}
+	// 1921: the asked versions are in the key, in any order of naming; none, or an empty ask, gives the key without them.
+	a, b := strings.Repeat("a", 40), strings.Repeat("b", 40)
+	k1 := ComposerKey("r", askCommit, "go", "p", tc, PackageVersions{"m/x": a, "m/y": b})
+	if k1 != ComposerKey("r", askCommit, "go", "p", tc, PackageVersions{"m/y": b, "m/x": a}) {
+		t.Error("the key depends on the order of the versions")
+	}
+	if k1 == ComposerKey("r", askCommit, "go", "p", tc, PackageVersions{"m/x": b, "m/y": a}) || k1 == ComposerKey("r", askCommit, "go", "p", tc, nil) {
+		t.Error("other versions, or none, share the key")
+	}
+	if ComposerKey("r", askCommit, "go", "p", tc, nil) != ComposerKey("r", askCommit, "go", "p", tc, PackageVersions{}) {
+		t.Error("an empty ask changed the key")
+	}
 }