Judge an ask by the builder's say the walk read, never a second read (issue 1886, review round 1)
A second read that failed, or that turned true in between, refused the ask and failed the whole plan.
This commit is contained in:
@@ -181,3 +181,43 @@ func TestTheReferenceWritersAreAskedOfBuildAgentWhateverTheBuilderSays(t *testin
|
||||
t.Fatalf("build-agent was asked %v", *asked)
|
||||
}
|
||||
}
|
||||
|
||||
// 1886 review round 1 (ADR 0522 §2): the guard of the one ask function judges by the say askModule read, never a second
|
||||
// read: a second read that fails, or that says otherwise, fails no plan.
|
||||
func TestTheAskGuardUsesTheSayTheWalkRead(t *testing.T) {
|
||||
was := askedModuleOf
|
||||
t.Cleanup(func() { askedModuleOf = was })
|
||||
askedModuleOf = func(_ context.Context, _ buildSource, path string) (string, error) {
|
||||
return strings.TrimPrefix(path, "modules/"), nil
|
||||
}
|
||||
reads := 0
|
||||
wasSays := builderSaysReferences
|
||||
t.Cleanup(func() { builderSaysReferences = wasSays })
|
||||
for _, second := range []struct {
|
||||
name string
|
||||
says bool
|
||||
err error
|
||||
}{{"a second read that fails", false, errors.New("the bus did not answer")}, {"a say that turned true", true, nil}} {
|
||||
reads = 0
|
||||
builderSaysReferences = func(context.Context) (bool, error) {
|
||||
reads++
|
||||
if reads == 1 {
|
||||
return false, nil
|
||||
}
|
||||
return second.says, second.err
|
||||
}
|
||||
wasAsk := askABuild
|
||||
askABuild = func(ctx context.Context, source buildSource, path, _ string) (string, error) {
|
||||
if err := onlyTheBuildersOwn(ctx, source, path); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return "build-9", nil
|
||||
}
|
||||
p, byName := aWalkOfOne("sudo")
|
||||
askModule(t.Context(), p, "sudo", byName)
|
||||
askABuild = wasAsk
|
||||
if s := p.Modules["sudo"]; p.State == inventory.PlanFailed || s.Build != "build-9" || s.Key {
|
||||
t.Errorf("%s: the walk %s, the module %+v", second.name, p.State, s)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,6 +51,20 @@ type planKeyReads interface {
|
||||
SaysReferences(ctx context.Context) (bool, error)
|
||||
}
|
||||
|
||||
// builderSayKey carries, on an ask's context, the builder's say the walk read before asking (withBuilderSay).
|
||||
type builderSayKey struct{}
|
||||
|
||||
// withBuilderSay hands the say a walk read to the ask's guard, so the guard never reads it a second time.
|
||||
func withBuilderSay(ctx context.Context, says bool) context.Context {
|
||||
return context.WithValue(ctx, builderSayKey{}, says)
|
||||
}
|
||||
|
||||
// builderSayOf is the say a walk handed the ask, and whether it handed one.
|
||||
func builderSayOf(ctx context.Context) (says, handed bool) {
|
||||
says, handed = ctx.Value(builderSayKey{}).(bool)
|
||||
return says, handed
|
||||
}
|
||||
|
||||
// builderSaysReferences is whether a running builder instance says, in its own bucket of instances, that the keys it
|
||||
// writes name references (issue 1886). Until one does, a walk asks build-agent for every module, as before R4e: the
|
||||
// keys the running builder writes could never be delivered from. A variable so a test needs no bus.
|
||||
|
||||
@@ -457,8 +457,16 @@ func onlyTheBuildersOwn(ctx context.Context, source buildSource, path string) er
|
||||
if isBuildersOwn(module) {
|
||||
return nil
|
||||
}
|
||||
// **While no running builder says it writes references, build-agent builds every module** (issue 1886).
|
||||
if says, err := builderSaysReferences(ctx); err == nil && !says {
|
||||
// **While no running builder says it writes references, build-agent builds every module** (issue 1886): judged by the
|
||||
// say the walk read when it has one, read here only for an ask that carries none.
|
||||
says, read := builderSayOf(ctx)
|
||||
if !read {
|
||||
var err error
|
||||
if says, err = builderSaysReferences(ctx); err != nil {
|
||||
says = true // not known: refused below, as before the say existed
|
||||
}
|
||||
}
|
||||
if !says {
|
||||
return nil
|
||||
}
|
||||
if module == "" {
|
||||
@@ -547,6 +555,8 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s
|
||||
takeFromKey(p, state, e, now)
|
||||
return
|
||||
}
|
||||
// The ask's guard judges by this say, never by a second read that could fail or differ (review of issue 1886).
|
||||
ctx = withBuilderSay(ctx, says)
|
||||
}
|
||||
source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat}
|
||||
fmt.Printf(" tier %d: ", p.Tier)
|
||||
|
||||
Reference in New Issue
Block a user