From 504dff34c0c39ff40208de7741aa9cb38da15f8c Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Mon, 21 Sep 2026 18:46:08 +0100 Subject: [PATCH 01/11] Fix remote download order --- .circleci/rex_test.sh | 14 +++++++++++ src/plz/plz.go | 52 ++++++++++++++++++++++++++++++++++++++- src/remote/remote.go | 47 ----------------------------------- src/remote/remote_test.go | 3 +++ 4 files changed, 68 insertions(+), 48 deletions(-) diff --git a/.circleci/rex_test.sh b/.circleci/rex_test.sh index 35fa4b1a67..e36f7b353f 100755 --- a/.circleci/rex_test.sh +++ b/.circleci/rex_test.sh @@ -33,3 +33,17 @@ plz test -o build.passenv:PATH --profile ci_remote -p -v notice --colour //src/c # And run any tests we deem to be pertinent to remote execution echo "Testing anything labeled rex..." plz test -o build.passenv:PATH --profile ci_remote -p -v notice --colour -i rex + +# Check that a target's runtime data gets downloaded along with it. +# src/plz builds runtime & data deps in parallel with the target itself, so +# //test/remote_data:slow_data deliberately takes longer to build than the target that +# depends on it. Both are named explicitly with --rebuild so that slow_data really does +# execute rather than being served from the action cache, which would let it win the race. +echo "Testing download of runtime data..." +rm -rf plz-out/gen/test/remote_data +plz build -o build.passenv:PATH --profile ci_remote -p -v notice --colour --rebuild \ + //test/remote_data:needs_data //test/remote_data:slow_data +if [ ! -f plz-out/gen/test/remote_data/slow_data.txt ]; then + echo "Runtime data for //test/remote_data:needs_data wasn't downloaded" + exit 1 +fi diff --git a/src/plz/plz.go b/src/plz/plz.go index d1d1e209e7..6666d5912a 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -477,8 +477,58 @@ func (r *runner) buildDep(ctx context.Context, dep core.BuildLabel, target *core return nil } -// buildOne builds a single target (which cannot be a pseudo-label like :all) +// buildOne builds a single target (which cannot be a pseudo-label like :all) and downloads +// anything needed to run it, if we're building remotely and it's a target we're downloading. func (r *runner) buildOne(ctx context.Context, target *core.BuildTarget) error { + if err := r.buildTargetAndDeps(ctx, target); err != nil { + return err + } + return r.downloadRuntimeFiles(target) +} + +// downloadRuntimeFiles downloads the files needed to run a target (i.e. its run-time & data +// dependencies, transitively). It must run after those have been built, which is why it can't +// live alongside the download of the target's own outputs in src/build; they build in parallel +// with the target itself so they aren't ready when it finishes. +func (r *runner) downloadRuntimeFiles(target *core.BuildTarget) error { + state := r.state.ForTarget(target) + if state.RemoteClient == nil || !state.ShouldDownload(target) { + return nil + } + // Hold a worker slot for this; it's remote I/O of much the same sort as the build itself, + // and without it the downloads for every original target would run unthrottled. + limiter := r.limiter(r.anyRemote) + limiter.Acquire() + defer limiter.Release() + + // N.B. DownloadInputsIfNeeded logs the target as building again, which it has already been + // reported as finished once. We must therefore log a terminal result on every path out + // of here, or it'll hold a slot in the display and show as building forever. + if err := state.DownloadInputsIfNeeded(target, true); err != nil { + state.LogBuildError(target.Label, core.TargetBuildFailed, err, "Failed to download runtime files for %s: %s", target.Label, err) + return err + } + logAlreadyBuilt(state, target) + return nil +} + +// logAlreadyBuilt re-logs the result that src/build logged for a target, after we've put it +// back into an active state in order to download its runtime files. +func logAlreadyBuilt(state *core.BuildState, target *core.BuildTarget) { + switch target.State() { + case core.ReusedRemotely: + state.LogBuildResult(target, core.TargetBuilt, "Reused existing action") + case core.BuiltRemotely: + state.LogBuildResult(target, core.TargetBuilt, "Built remotely") + case core.Unchanged, core.Reused: + state.LogBuildResult(target, core.TargetCached, "Unchanged") + default: + state.LogBuildResult(target, core.TargetBuilt, "Built") + } +} + +// buildTargetAndDeps builds a single target and all of its dependencies. +func (r *runner) buildTargetAndDeps(ctx context.Context, target *core.BuildTarget) error { g, gctx := r.group(ctx) for dep := range target.BuildDependencyLabels() { g.Go(func() error { diff --git a/src/remote/remote.go b/src/remote/remote.go index a8beb7b7bf..f09dbe3051 100644 --- a/src/remote/remote.go +++ b/src/remote/remote.go @@ -345,56 +345,9 @@ func (c *Client) Build(target *core.BuildTarget) (*core.BuildMetadata, error) { } c.setOutputsFromMetadata(target, metadata) - - if c.state.ShouldDownload(target) { - c.state.LogBuildResult(target, core.TargetBuilding, "Downloading") - if err := c.Download(target); err != nil { - return metadata, err - } - // TODO(peterebden): Should these not just be part of Download()? - if err := c.downloadData(target); err != nil { - return metadata, err - } - if err := c.downloadRuntimeDependencies(target); err != nil { - return metadata, err - } - } return metadata, nil } -// downloadData downloads all the runtime data for a target, recursively. -func (c *Client) downloadData(target *core.BuildTarget) error { - var g errgroup.Group - for _, datum := range target.AllData() { - if l, ok := datum.Label(); ok { - t := c.state.Graph.TargetOrDie(l) - g.Go(func() error { - if err := c.Download(t); err != nil { - return err - } - return c.downloadData(t) - }) - } - } - return g.Wait() -} - -// downloadRuntimeDependencies downloads all the runtime dependencies for a target. -func (c *Client) downloadRuntimeDependencies(target *core.BuildTarget) error { - var g errgroup.Group - for runDep := range target.IterAllRuntimeDependencies(c.state.Graph) { - l, _ := runDep.Label() - t := c.state.Graph.TargetOrDie(l) - g.Go(func() error { - if err := c.Download(t); err != nil { - return err - } - return nil - }) - } - return g.Wait() -} - // Run runs a target on the remote executors. func (c *Client) Run(target *core.BuildTarget) error { if err := c.CheckInitialised(); err != nil { diff --git a/src/remote/remote_test.go b/src/remote/remote_test.go index 07a35f90be..df66397464 100644 --- a/src/remote/remote_test.go +++ b/src/remote/remote_test.go @@ -265,6 +265,9 @@ func TestOutDirsSetOutsOnTarget(t *testing.T) { c.state.Graph.AddTarget(outDirTarget) _, err := c.Build(outDirTarget) require.NoError(t, err) + // Build no longer downloads anything itself; src/build does that once any post-build + // function has had its chance to alter the outputs. + require.NoError(t, c.Download(outDirTarget)) assert.Len(t, outDirTarget.Outputs(c.state.Graph), 2) assert.ElementsMatch(t, []string{"foo.txt", "bar.txt"}, outDirTarget.Outputs(c.state.Graph)) From a5d0f3b3b33034ed85dcb266748575c3e36ba2ae Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Mon, 21 Sep 2026 18:57:23 +0100 Subject: [PATCH 02/11] Cleanup --- src/build/build_step.go | 3 +++ src/core/state.go | 1 - src/plz/plz.go | 20 +------------------- src/test/test_step.go | 3 +++ 4 files changed, 7 insertions(+), 20 deletions(-) diff --git a/src/build/build_step.go b/src/build/build_step.go index 0684301690..14d15975db 100644 --- a/src/build/build_step.go +++ b/src/build/build_step.go @@ -216,6 +216,9 @@ func buildTarget(state *core.BuildState, target *core.BuildTarget, runRemotely b state.LogBuildResult(target, core.TargetBuilding, "Preparing...") // Ensure we have downloaded any previous dependencies if that's relevant. + if state.RemoteClient != nil { + state.LogBuildResult(target, core.TargetBuilding, "Downloading inputs...") + } if err := state.DownloadInputsIfNeeded(target, false); err != nil { return err } diff --git a/src/core/state.go b/src/core/state.go index 0c24158fe7..320463a4d3 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -772,7 +772,6 @@ func (state *BuildState) GetPreloadedSubincludes() []BuildLabel { // DownloadInputsIfNeeded downloads all the inputs (or runtime files) for a target if we are building remotely. func (state *BuildState) DownloadInputsIfNeeded(target *BuildTarget, runtime bool) error { if state.RemoteClient != nil { - state.LogBuildResult(target, TargetBuilding, "Downloading inputs...") for input := range state.IterInputs(target, runtime) { if l, ok := input.Label(); ok { dep := state.Graph.TargetOrDie(l) diff --git a/src/plz/plz.go b/src/plz/plz.go index 6666d5912a..bdaea73542 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -501,32 +501,14 @@ func (r *runner) downloadRuntimeFiles(target *core.BuildTarget) error { limiter.Acquire() defer limiter.Release() - // N.B. DownloadInputsIfNeeded logs the target as building again, which it has already been - // reported as finished once. We must therefore log a terminal result on every path out - // of here, or it'll hold a slot in the display and show as building forever. + log.Debug("Downloading runtime files for %s", target) if err := state.DownloadInputsIfNeeded(target, true); err != nil { state.LogBuildError(target.Label, core.TargetBuildFailed, err, "Failed to download runtime files for %s: %s", target.Label, err) return err } - logAlreadyBuilt(state, target) return nil } -// logAlreadyBuilt re-logs the result that src/build logged for a target, after we've put it -// back into an active state in order to download its runtime files. -func logAlreadyBuilt(state *core.BuildState, target *core.BuildTarget) { - switch target.State() { - case core.ReusedRemotely: - state.LogBuildResult(target, core.TargetBuilt, "Reused existing action") - case core.BuiltRemotely: - state.LogBuildResult(target, core.TargetBuilt, "Built remotely") - case core.Unchanged, core.Reused: - state.LogBuildResult(target, core.TargetCached, "Unchanged") - default: - state.LogBuildResult(target, core.TargetBuilt, "Built") - } -} - // buildTargetAndDeps builds a single target and all of its dependencies. func (r *runner) buildTargetAndDeps(ctx context.Context, target *core.BuildTarget) error { g, gctx := r.group(ctx) diff --git a/src/test/test_step.go b/src/test/test_step.go index f38e44d87a..28df32828f 100644 --- a/src/test/test_step.go +++ b/src/test/test_step.go @@ -584,6 +584,9 @@ func runtimeHash(state *core.BuildState, target *core.BuildTarget, runRemotely b } if target.Local { // If the test is marked to run locally, download the inputs as we need these to calculate the runtime hash. + if state.RemoteClient != nil { + state.LogTestRunning(target, run, core.TargetTesting, "Downloading inputs...") + } if err := state.DownloadInputsIfNeeded(target, true); err != nil { return nil, err } From c4829ff0db5b9396c274e28ec5e2407ec920e0d9 Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Mon, 21 Sep 2026 19:00:30 +0100 Subject: [PATCH 03/11] Bit more pruning --- src/plz/plz.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/plz/plz.go b/src/plz/plz.go index bdaea73542..5b26027e1b 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -486,17 +486,12 @@ func (r *runner) buildOne(ctx context.Context, target *core.BuildTarget) error { return r.downloadRuntimeFiles(target) } -// downloadRuntimeFiles downloads the files needed to run a target (i.e. its run-time & data -// dependencies, transitively). It must run after those have been built, which is why it can't -// live alongside the download of the target's own outputs in src/build; they build in parallel -// with the target itself so they aren't ready when it finishes. +// downloadRuntimeFiles downloads the files needed to run a target (i.e. itself and its run-time & data dependencies). func (r *runner) downloadRuntimeFiles(target *core.BuildTarget) error { state := r.state.ForTarget(target) if state.RemoteClient == nil || !state.ShouldDownload(target) { return nil } - // Hold a worker slot for this; it's remote I/O of much the same sort as the build itself, - // and without it the downloads for every original target would run unthrottled. limiter := r.limiter(r.anyRemote) limiter.Acquire() defer limiter.Release() From 1d8cd0f4614f55169dbd9f280291565675537c05 Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Mon, 21 Sep 2026 19:05:54 +0100 Subject: [PATCH 04/11] Add test case --- test/remote_data/BUILD | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 test/remote_data/BUILD diff --git a/test/remote_data/BUILD b/test/remote_data/BUILD new file mode 100644 index 0000000000..e51382562e --- /dev/null +++ b/test/remote_data/BUILD @@ -0,0 +1,22 @@ +# Regression test for downloading a remotely built target's runtime data. +# +# These are only built by .circleci/rex_test.sh, hence "manual"; they're pointless +# (and slow) without remote execution configured. +# +# The data dependency must take longer to build than the target that depends on it. +# src/plz builds runtime & data deps in parallel with the target itself, so if the +# data is quicker it wins the race and the ordering never gets exercised. +genrule( + name = "slow_data", + outs = ["slow_data.txt"], + cmd = "sleep 5 && echo hello > $OUT", + labels = ["manual"], +) + +genrule( + name = "needs_data", + outs = ["needs_data.txt"], + cmd = "echo hello > $OUT", + data = [":slow_data"], + labels = ["manual"], +) From b30deeed91cf3a9d3f1469b9894b54c5c7e0770e Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Tue, 22 Sep 2026 08:55:59 +0100 Subject: [PATCH 05/11] Recover panics better --- src/cmap/cerrmap.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/cmap/cerrmap.go b/src/cmap/cerrmap.go index 849c54a2a6..cf00edd170 100644 --- a/src/cmap/cerrmap.go +++ b/src/cmap/cerrmap.go @@ -66,7 +66,7 @@ func (m *ErrMap[K, V]) GetOrSet(key K, f func() (V, error)) (V, error) { } else if first { defer func() { if r := recover(); r != nil { - m.m.Set(key, errV[V]{Err: fmt.Errorf("%s", r)}) + m.m.Set(key, errV[V]{Err: panicToErr(r)}) } }() val, err := f() @@ -84,6 +84,14 @@ func (m *ErrMap[K, V]) GetOrSet(key K, f func() (V, error)) (V, error) { return v.Val, v.Err } +// panicToErr converts a recovered panic value into an error, retaining its type if it already is an error. +func panicToErr(r any) error { + if e, ok := r.(error); ok { + return e + } + return fmt.Errorf("%v", r) +} + // GetOrSetCtx is like GetOrSet but accepts a context that can be cancelled. // If the called function panics, the panic will be recovered and treated as though it returned an error. func (m *ErrMap[K, V]) GetOrSetCtx(ctx context.Context, key K, f func() (V, error)) (V, error) { @@ -93,7 +101,7 @@ func (m *ErrMap[K, V]) GetOrSetCtx(ctx context.Context, key K, f func() (V, erro } else if first { defer func() { if r := recover(); r != nil { - m.m.Set(key, errV[V]{Err: fmt.Errorf("%s", r)}) + m.m.Set(key, errV[V]{Err: panicToErr(r)}) } }() val, err := f() From 024076840c008292b2246603f156ea1bc148c6cf Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Tue, 22 Sep 2026 09:48:10 +0100 Subject: [PATCH 06/11] Log a build error if we fail to resolve preloaded subincludes --- src/plz/plz.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plz/plz.go b/src/plz/plz.go index 5b26027e1b..3c6dbb99e4 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -179,6 +179,9 @@ func (r *runner) parse(ctx context.Context, label, dependent core.BuildLabel, qu return nil, err } if err := r.ensurePreloads(ctx, state); err != nil { + if !quiet { + r.state.LogBuildError(label, core.ParseFailed, err, "Failed to resolve preloaded subincludes") + } return nil, err } pkg, wait, first, err := r.state.Graph.PackageOrWait(label) From 913b7caa819ee51eb9e68a6179b77e16318e810f Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Tue, 22 Sep 2026 09:51:59 +0100 Subject: [PATCH 07/11] Make sure we download preloads --- src/plz/plz.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plz/plz.go b/src/plz/plz.go index 3c6dbb99e4..afaaae32ad 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -291,7 +291,7 @@ func (r *runner) registerPreloads(ctx context.Context, state *core.BuildState) e } // Queue them up asynchronously to feed the queues as quickly as possible g.Go(func() error { - if _, err := r.Build(gctx, inc, core.OriginalTarget); err != nil { + if _, err := r.BuildAndDownload(gctx, inc, core.OriginalTarget); err != nil { return err } return r.parser.PreloadSubinclude(gctx, inc) From 61aa208d6e330e750ec10e8f4244a01d42d734dd Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Tue, 22 Sep 2026 12:58:23 +0100 Subject: [PATCH 08/11] More recovery --- src/cmap/cerrmap.go | 14 ++++++++------ src/cmap/cerrmap_test.go | 10 ++++++++++ src/core/state.go | 10 ++++++++-- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/cmap/cerrmap.go b/src/cmap/cerrmap.go index cf00edd170..a46bec9553 100644 --- a/src/cmap/cerrmap.go +++ b/src/cmap/cerrmap.go @@ -59,17 +59,18 @@ func (m *ErrMap[K, V]) Get(key K) (V, error) { // GetOrSet returns the value if set, or an error if one has been set. // If nothing has been set for the key, it runs the given function to generate the value and then sets it. // If the called function panics, the panic will be recovered and treated as though it returned an error. -func (m *ErrMap[K, V]) GetOrSet(key K, f func() (V, error)) (V, error) { +func (m *ErrMap[K, V]) GetOrSet(key K, f func() (V, error)) (val V, err error) { v, wait, first := m.m.GetOrWait(key) if v.Err != nil { return v.Val, v.Err } else if first { defer func() { if r := recover(); r != nil { - m.m.Set(key, errV[V]{Err: panicToErr(r)}) + err = panicToErr(r) + m.m.Set(key, errV[V]{Err: err}) } }() - val, err := f() + val, err = f() m.m.Set(key, errV[V]{Val: val, Err: err}) return val, err } else if wait != nil { @@ -94,17 +95,18 @@ func panicToErr(r any) error { // GetOrSetCtx is like GetOrSet but accepts a context that can be cancelled. // If the called function panics, the panic will be recovered and treated as though it returned an error. -func (m *ErrMap[K, V]) GetOrSetCtx(ctx context.Context, key K, f func() (V, error)) (V, error) { +func (m *ErrMap[K, V]) GetOrSetCtx(ctx context.Context, key K, f func() (V, error)) (val V, err error) { v, wait, first := m.m.GetOrWait(key) if v.Err != nil { return v.Val, v.Err } else if first { defer func() { if r := recover(); r != nil { - m.m.Set(key, errV[V]{Err: panicToErr(r)}) + err = panicToErr(r) + m.m.Set(key, errV[V]{Err: err}) } }() - val, err := f() + val, err = f() m.m.Set(key, errV[V]{Val: val, Err: err}) return val, err } else if wait != nil { diff --git a/src/cmap/cerrmap_test.go b/src/cmap/cerrmap_test.go index d8951c7459..f2396c43f5 100644 --- a/src/cmap/cerrmap_test.go +++ b/src/cmap/cerrmap_test.go @@ -34,6 +34,16 @@ func TestErrWait(t *testing.T) { assert.Error(t, err) } +func TestErrMapGetOrSetPanicReturnsErrorToFirstCaller(t *testing.T) { + m := NewErrMap[int, int](DefaultShardCount, hashInts, nil) + v, err := m.GetOrSet(9, func() (int, error) { + panic(fmt.Errorf("it broke")) + }) + assert.Zero(t, v) + assert.Error(t, err) + assert.Equal(t, "it broke", err.Error()) +} + func TestValues(t *testing.T) { m := NewErrMap[int, int](DefaultShardCount, hashInts, nil) broke := fmt.Errorf("it broke") diff --git a/src/core/state.go b/src/core/state.go index 320463a4d3..77e322e330 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -756,9 +756,15 @@ func (state *BuildState) GetPreloadedSubincludes() []BuildLabel { done := map[BuildLabel]struct{}{} includes := make([]BuildLabel, 0, len(state.Config.Parse.PreloadSubincludes)+len(state.RepoConfig.Parse.PreloadSubincludes)) - is := append(state.Config.Parse.PreloadSubincludes, state.RepoConfig.Parse.PreloadSubincludes...) + for _, i := range state.Config.Parse.PreloadSubincludes { + if _, ok := done[i]; ok { + continue + } - for _, i := range is { + includes = append(includes, i) + done[i] = struct{}{} + } + for _, i := range state.RepoConfig.Parse.PreloadSubincludes { if _, ok := done[i]; ok { continue } From e0ab501609e4a3163e22baf549c5689fd7b91e52 Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Wed, 23 Sep 2026 10:06:50 +0100 Subject: [PATCH 09/11] Improve error reporting on panics --- src/cmap/cerrmap.go | 4 +++- src/core/build_target.go | 3 +++ src/core/state.go | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cmap/cerrmap.go b/src/cmap/cerrmap.go index a46bec9553..16694f674b 100644 --- a/src/cmap/cerrmap.go +++ b/src/cmap/cerrmap.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "iter" + "runtime/debug" ) // A Limiter is the interface that we use to release/acquire workers while waiting. @@ -90,7 +91,8 @@ func panicToErr(r any) error { if e, ok := r.(error); ok { return e } - return fmt.Errorf("%v", r) + // Maintain the stack; if something panicked that wasn't an error we will probably want it for debugging. + return fmt.Errorf("%v\n%s", r, debug.Stack()) } // GetOrSetCtx is like GetOrSet but accepts a context that can be cancelled. diff --git a/src/core/build_target.go b/src/core/build_target.go index 4ac6fd6a66..db0fb6a0e3 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -1422,6 +1422,9 @@ func (target *BuildTarget) AddDebugTool(tool BuildInput) { // AllTestTools returns all the test tool paths for this rule. func (target *BuildTarget) AllTestTools() []BuildInput { + if target.Test == nil { + return nil + } if target.Test.namedTools == nil { return target.Test.tools } diff --git a/src/core/state.go b/src/core/state.go index 77e322e330..8883346ff6 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -257,7 +257,7 @@ func (state *BuildState) Initialise(subrepo *Subrepo) (err error) { // handled for us already in plz.go if state.CurrentSubrepo != "" { state.RepoConfig = &Configuration{} - if err := readSubrepoConfig(state.RepoConfig, subrepo); err != nil { + if err = readSubrepoConfig(state.RepoConfig, subrepo); err != nil { return } if err = validateSubrepoNameAndPluginConfig(state.Config, state.RepoConfig, subrepo); err != nil { From d747f20293aac098838fa9a34fa665692c7b7fe4 Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Wed, 23 Sep 2026 11:51:42 +0100 Subject: [PATCH 10/11] Reorder arch check --- src/plz/plz.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/plz/plz.go b/src/plz/plz.go index afaaae32ad..210180eadc 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -325,6 +325,10 @@ func (r *runner) ensureSubrepo(ctx context.Context, label, dependent core.BuildL if r.state.Graph.Subrepo(label.Subrepo) != nil { return nil // The parse above defined it, we're done. } + if arch, ok := couldBeArch(label.Subrepo); ok { + r.state.Graph.MaybeAddSubrepo(core.SubrepoForArch(r.state, arch)) + return nil + } if sl.Subrepo != dependent.Subrepo { nested := sl nested.Subrepo = dependent.Subrepo @@ -337,11 +341,7 @@ func (r *runner) ensureSubrepo(ctx context.Context, label, dependent core.BuildL } } } - // Nothing defines it, so the only remaining possibility is an architecture subrepo. - if arch, ok := couldBeArch(label.Subrepo); ok { - r.state.Graph.MaybeAddSubrepo(core.SubrepoForArch(r.state, arch)) - return nil - } else if err != nil { + if err != nil { // This returns the missing build file error that we got from tryParse above. return err } From 0560d707810a679f5684c8359c15856c28589b7c Mon Sep 17 00:00:00 2001 From: Peter Ebden Date: Thu, 1 Oct 2026 11:30:04 +0100 Subject: [PATCH 11/11] update comment --- src/remote/remote_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_test.go b/src/remote/remote_test.go index df66397464..ffe2bc1274 100644 --- a/src/remote/remote_test.go +++ b/src/remote/remote_test.go @@ -265,7 +265,7 @@ func TestOutDirsSetOutsOnTarget(t *testing.T) { c.state.Graph.AddTarget(outDirTarget) _, err := c.Build(outDirTarget) require.NoError(t, err) - // Build no longer downloads anything itself; src/build does that once any post-build + // Build does not download anything itself; src/build does that once any post-build // function has had its chance to alter the outputs. require.NoError(t, c.Download(outDirTarget))