From 0ca23eb76c1b9a53f3abad5208705f2dc1815eb6 Mon Sep 17 00:00:00 2001 From: duarte Date: Mon, 28 Sep 2026 11:56:19 +0100 Subject: [PATCH 01/10] purge ForceParseEntirePackage dead code --- src/core/state.go | 5 ----- src/export/README.md | 2 +- src/export/export.go | 4 ++-- src/please.go | 7 +------ 4 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/core/state.go b/src/core/state.go index 0c24158fe..1adc6907f 100644 --- a/src/core/state.go +++ b/src/core/state.go @@ -217,11 +217,6 @@ type BuildState struct { NeedDebugDeps bool // ParseMetadata is true if we want to store BUILD file metadata ParseMetadata bool - // ForceParseEntirePackage is true if we want to force parse and activate all targets in every visited - // package during the parse phase. This is used by the export operation to ensure that the entire - // dependency graph of visited packages is parsed upfront, including co-defined related targets - // and sibling targets. - ForceParseEntirePackage bool // initOnce is used to control loading the subrepo .plzconfig initOnce *sync.Once diff --git a/src/export/README.md b/src/export/README.md index 96a508b32..326adc98c 100644 --- a/src/export/README.md +++ b/src/export/README.md @@ -83,7 +83,7 @@ statement details: To support package-level trimming, the exporter must have access to metadata for adjacent targets that normally (e.g., during a standard build operation) would not be parsed or resolved. -For this purpose, Please utilizes an upfront-parsing model. When `ForceParseEntirePackage` is set to +For this purpose, Please utilizes an upfront-parsing model. When `ParseMetadata` is set to `true` during the parse phase, the interpreter synchronously parses and resolves all targets in any visited package upfront. This guarantees that all package-level statement and target metadata is fully populated and available, eliminating the need for any dynamic or lazy parsing during the diff --git a/src/export/export.go b/src/export/export.go index b5c403bea..3ecd9462e 100644 --- a/src/export/export.go +++ b/src/export/export.go @@ -241,7 +241,7 @@ func (be *baseExporter) exportFiles(paths []string) { } // getTarget attempts to lookup a target in the build graph. -// This is a synchronous lookup and it assumes that [state.ForceParseEntirePackage] +// This is a synchronous lookup and it assumes that [state.ParseMetadata] // was enabled during the parse phase to guarantee that all required targets are pre-parsed. func (be *baseExporter) getTarget(label core.BuildLabel) (*core.BuildTarget, error) { target := be.state.Graph.Target(label) @@ -252,7 +252,7 @@ func (be *baseExporter) getTarget(label core.BuildLabel) (*core.BuildTarget, err } // getPackage attempts to lookup a package in the build graph. -// This is a synchronous lookup and it assumes that [state.ForceParseEntirePackage] +// This is a synchronous lookup and it assumes that [state.ParseMetadata] // was enabled during the parse phase to guarantee that all required packages are pre-parsed. func (be *baseExporter) getPackage(label core.BuildLabel) (*core.Package, error) { pkg := be.state.Graph.PackageByLabel(label) diff --git a/src/please.go b/src/please.go index 2551c9deb..f88fa5ebc 100644 --- a/src/please.go +++ b/src/please.go @@ -784,7 +784,7 @@ var buildFunctions = map[string]func() int{ return 0 }, "export": func() int { - success, state := runBuild(opts.Export.Args.Targets, buildOpts{ParseMetadata: true, ForceParseEntirePackage: true}) + success, state := runBuild(opts.Export.Args.Targets, buildOpts{ParseMetadata: true}) if success { export.Repo(state, opts.Export.Output, opts.Export.NoTrim, state.ExpandOriginalLabels()) @@ -1201,7 +1201,6 @@ func Please(targets []core.BuildLabel, config *core.Configuration, buildOpts bui state.NeedBuild = buildOpts.Build state.NeedTests = buildOpts.Test state.ParseMetadata = buildOpts.ParseMetadata - state.ForceParseEntirePackage = buildOpts.ForceParseEntirePackage state.NeedDebugDeps = debug // What outputs get downloaded in remote execution. @@ -1363,10 +1362,6 @@ type buildOpts struct { IsQuery bool // ParseMetadata is true if we want to store BUILD file metadata during parsing. ParseMetadata bool - // ForceParseEntirePackage is true if we want to force parse and activate all targets in every - // visited package. This is required to include adjacent targets in the build graph for operations - // like export. - ForceParseEntirePackage bool } // Runs the actual build From 665c6e63721d9b7072203bc645e4be4b8a0f24d7 Mon Sep 17 00:00:00 2001 From: duarte Date: Mon, 28 Sep 2026 16:41:59 +0100 Subject: [PATCH 02/10] regression test for parsing the entire package --- .../test_notrim_parse_entire_package/BUILD | 11 +++++++++++ .../expected_repo/.plzconfig | 3 +++ .../expected_repo/BUILD_FILE | 9 +++++++++ .../expected_repo/file1.txt | 1 + .../expected_repo/intermediate/BUILD_FILE | 17 +++++++++++++++++ .../expected_repo/intermediate/adjacent.txt | 1 + .../expected_repo/intermediate/lib.txt | 1 + .../expected_repo/other_package/BUILD_FILE | 7 +++++++ .../expected_repo/other_package/file3.txt | 1 + .../source_repo/.plzconfig | 3 +++ .../source_repo/BUILD_FILE | 9 +++++++++ .../source_repo/file1.txt | 1 + .../source_repo/intermediate/BUILD_FILE | 17 +++++++++++++++++ .../source_repo/intermediate/adjacent.txt | 1 + .../source_repo/intermediate/lib.txt | 1 + .../source_repo/other_package/BUILD_FILE | 7 +++++++ .../source_repo/other_package/file3.txt | 1 + 17 files changed, 91 insertions(+) create mode 100644 test/export/test_notrim_parse_entire_package/BUILD create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/.plzconfig create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/file1.txt create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/intermediate/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/intermediate/adjacent.txt create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/intermediate/lib.txt create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/other_package/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/expected_repo/other_package/file3.txt create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/.plzconfig create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/file1.txt create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/intermediate/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/intermediate/adjacent.txt create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/intermediate/lib.txt create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/other_package/BUILD_FILE create mode 100644 test/export/test_notrim_parse_entire_package/source_repo/other_package/file3.txt diff --git a/test/export/test_notrim_parse_entire_package/BUILD b/test/export/test_notrim_parse_entire_package/BUILD new file mode 100644 index 000000000..02b0a5abf --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/BUILD @@ -0,0 +1,11 @@ +subinclude("//test/export:export_e2e_test_build_def") + +# Test --notrim exports adjacent targets and dependencies from packages that would not have been +# included in the original parse. This is a regression test to make sure we enforce entire package +# parsing when using the export command. +please_export_e2e_test( + name = "export_notrim_parse_entire_package", + enforce_different_repos = False, + export_targets = ["//:target1"], + flags = "--notrim", +) diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/.plzconfig b/test/export/test_notrim_parse_entire_package/expected_repo/.plzconfig new file mode 100644 index 000000000..8e1ae5655 --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/.plzconfig @@ -0,0 +1,3 @@ +[Parse] + +BuildFileName = BUILD_FILE diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/BUILD_FILE b/test/export/test_notrim_parse_entire_package/expected_repo/BUILD_FILE new file mode 100644 index 000000000..5c4fb397a --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/BUILD_FILE @@ -0,0 +1,9 @@ +genrule( + name = "target1", + srcs = [ + "file1.txt", + "//intermediate:lib", + ], + outs = ["out1.txt"], + cmd = "cat $SRCS > $OUT", +) diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/file1.txt b/test/export/test_notrim_parse_entire_package/expected_repo/file1.txt new file mode 100644 index 000000000..e2129701f --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/file1.txt @@ -0,0 +1 @@ +file1 diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/BUILD_FILE b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/BUILD_FILE new file mode 100644 index 000000000..723c521bb --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/BUILD_FILE @@ -0,0 +1,17 @@ +genrule( + name = "lib", + srcs = ["lib.txt"], + outs = ["out_lib.txt"], + cmd = "cat $SRCS > $OUT", + visibility = ["PUBLIC"], +) + +genrule( + name = "adjacent", + srcs = [ + "adjacent.txt", + "//other_package:other_target", + ], + outs = ["out_adjacent.txt"], + cmd = "cat $SRCS > $OUT", +) diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/adjacent.txt b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/adjacent.txt new file mode 100644 index 000000000..ac9595f5e --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/adjacent.txt @@ -0,0 +1 @@ +adjacent content diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/lib.txt b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/lib.txt new file mode 100644 index 000000000..3cb05a1df --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/intermediate/lib.txt @@ -0,0 +1 @@ +lib content diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/other_package/BUILD_FILE b/test/export/test_notrim_parse_entire_package/expected_repo/other_package/BUILD_FILE new file mode 100644 index 000000000..bd7b76ddd --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/other_package/BUILD_FILE @@ -0,0 +1,7 @@ +genrule( + name = "other_target", + srcs = ["file3.txt"], + outs = ["out3.txt"], + cmd = "cat $SRCS > $OUT", + visibility = ["PUBLIC"], +) diff --git a/test/export/test_notrim_parse_entire_package/expected_repo/other_package/file3.txt b/test/export/test_notrim_parse_entire_package/expected_repo/other_package/file3.txt new file mode 100644 index 000000000..7c8ac2f8d --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/expected_repo/other_package/file3.txt @@ -0,0 +1 @@ +file3 diff --git a/test/export/test_notrim_parse_entire_package/source_repo/.plzconfig b/test/export/test_notrim_parse_entire_package/source_repo/.plzconfig new file mode 100644 index 000000000..8e1ae5655 --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/.plzconfig @@ -0,0 +1,3 @@ +[Parse] + +BuildFileName = BUILD_FILE diff --git a/test/export/test_notrim_parse_entire_package/source_repo/BUILD_FILE b/test/export/test_notrim_parse_entire_package/source_repo/BUILD_FILE new file mode 100644 index 000000000..5c4fb397a --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/BUILD_FILE @@ -0,0 +1,9 @@ +genrule( + name = "target1", + srcs = [ + "file1.txt", + "//intermediate:lib", + ], + outs = ["out1.txt"], + cmd = "cat $SRCS > $OUT", +) diff --git a/test/export/test_notrim_parse_entire_package/source_repo/file1.txt b/test/export/test_notrim_parse_entire_package/source_repo/file1.txt new file mode 100644 index 000000000..e2129701f --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/file1.txt @@ -0,0 +1 @@ +file1 diff --git a/test/export/test_notrim_parse_entire_package/source_repo/intermediate/BUILD_FILE b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/BUILD_FILE new file mode 100644 index 000000000..723c521bb --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/BUILD_FILE @@ -0,0 +1,17 @@ +genrule( + name = "lib", + srcs = ["lib.txt"], + outs = ["out_lib.txt"], + cmd = "cat $SRCS > $OUT", + visibility = ["PUBLIC"], +) + +genrule( + name = "adjacent", + srcs = [ + "adjacent.txt", + "//other_package:other_target", + ], + outs = ["out_adjacent.txt"], + cmd = "cat $SRCS > $OUT", +) diff --git a/test/export/test_notrim_parse_entire_package/source_repo/intermediate/adjacent.txt b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/adjacent.txt new file mode 100644 index 000000000..ac9595f5e --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/adjacent.txt @@ -0,0 +1 @@ +adjacent content diff --git a/test/export/test_notrim_parse_entire_package/source_repo/intermediate/lib.txt b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/lib.txt new file mode 100644 index 000000000..3cb05a1df --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/intermediate/lib.txt @@ -0,0 +1 @@ +lib content diff --git a/test/export/test_notrim_parse_entire_package/source_repo/other_package/BUILD_FILE b/test/export/test_notrim_parse_entire_package/source_repo/other_package/BUILD_FILE new file mode 100644 index 000000000..bd7b76ddd --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/other_package/BUILD_FILE @@ -0,0 +1,7 @@ +genrule( + name = "other_target", + srcs = ["file3.txt"], + outs = ["out3.txt"], + cmd = "cat $SRCS > $OUT", + visibility = ["PUBLIC"], +) diff --git a/test/export/test_notrim_parse_entire_package/source_repo/other_package/file3.txt b/test/export/test_notrim_parse_entire_package/source_repo/other_package/file3.txt new file mode 100644 index 000000000..7c8ac2f8d --- /dev/null +++ b/test/export/test_notrim_parse_entire_package/source_repo/other_package/file3.txt @@ -0,0 +1 @@ +file3 From 33261e8c4d40962f52d9646ed1490a67f17f5413 Mon Sep 17 00:00:00 2001 From: duarte Date: Mon, 28 Sep 2026 18:50:26 +0100 Subject: [PATCH 03/10] ensure parse of entire package and subincludes --- src/plz/plz.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/plz/plz.go b/src/plz/plz.go index d1d1e209e..82d264c3c 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -400,20 +400,22 @@ func (r *runner) recursiveParse(ctx context.Context, label, dependent core.Build }) } if r.state.ParseMetadata { - // parseTarget can return as soon as the target itself turns up, before the rest of its package - // (and hence its metadata) has been parsed, so we have to make sure the whole package is done here. + // parseTarget can't return as soon as the target itself turns up, before the rest of its package + // (and hence its metadata) has been parsed, so we have to make sure the whole package is done + // here - all targets and subincludes. pkg, err := r.Parse(ctx, target.Label, dependent) if err != nil { return err } if !pkg.Subrepo.IsExternal() { - related, err := pkg.Metadata.FindRelatedTargets(target.Label) - if err != nil { - return err + for _, t := range pkg.AllTargets() { + g.Go(func() error { + return r.recursiveParse(gctx, t.Label, target.Label) + }) } - for _, rel := range related { + for _, subinc := range pkg.AllSubincludes(r.state.Graph) { g.Go(func() error { - return r.recursiveParse(gctx, rel, target.Label) + return r.recursiveParse(gctx, subinc, target.Label) }) } } From 9593cfe461366ff50d706de5f8f95225bdd328f2 Mon Sep 17 00:00:00 2001 From: duarte Date: Tue, 29 Sep 2026 14:59:02 +0100 Subject: [PATCH 04/10] fix export monitor last print --- src/export/export.go | 88 +++++++++++++++++++++++++------------------- 1 file changed, 51 insertions(+), 37 deletions(-) diff --git a/src/export/export.go b/src/export/export.go index 3ecd9462e..220df85b9 100644 --- a/src/export/export.go +++ b/src/export/export.go @@ -102,9 +102,8 @@ type baseExporter struct { // run specifies the main steps when running an export. func (be *baseExporter) run(targets core.BuildLabels) { - stop := make(chan struct{}) - go be.startMonitor(stop) - defer close(stop) + stop := be.startMonitor() + defer stop() be.exportRepoConfig() be.strategy.exportPreloaded() @@ -112,46 +111,61 @@ func (be *baseExporter) run(targets core.BuildLabels) { be.strategy.writePackageFiles() } -func (be *baseExporter) startMonitor(stop chan struct{}) { - // this total is meant to provide some general idea of the progress but in reality we might - // visit more than the targets currently in the build graph (those will have to be parsed - // adhoc). - total := len(be.state.Graph.AllTargets()) - startTime := time.Now() - isTerminal := cli.StdErrIsATerminal - - if isTerminal { - cli.CurrentBackend.SetPassthrough(false, 1, false) - defer cli.CurrentBackend.SetPassthrough(true, 1, false) - } +func (be *baseExporter) startMonitor() func() { + stop, done := make(chan struct{}), make(chan struct{}) + + go func() { + defer close(done) + + // this total is meant to provide some general idea of the progress but in reality we might + // visit more than the targets currently in the build graph (those will have to be parsed + // adhoc). + total := len(be.state.Graph.AllTargets()) + startTime := time.Now() + isTerminal := cli.StdErrIsATerminal + + if isTerminal { + cli.CurrentBackend.SetPassthrough(false, 1, false) + defer cli.CurrentBackend.SetPassthrough(true, 1, false) + } - ticker := time.NewTicker(100 * time.Millisecond) - defer ticker.Stop() - - for { - select { - case <-stop: - log.Infof("Exported %d targets (%.1fs)...", be.targetCounter, time.Since(startTime).Seconds()) - return - case <-ticker.C: - done := be.targetCounter - elapsed := time.Since(startTime).Seconds() - stats := be.state.SystemStats() - - if isTerminal { - logs := cli.CurrentBackend.FlushOutput() - if len(logs) > 0 { - // Remove footer line before writing logs. - cli.Fprintf(os.Stderr, "${RESETLN}") - for _, line := range logs { - fmt.Fprintln(os.Stderr, line) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + for { + select { + case <-stop: + elapsed := time.Since(startTime).Seconds() + cli.Fprintf(os.Stderr, "${RESETLN}${BOLD_WHITE}Exported${RESET} ${BOLD_GREEN}%d${RESET} targets in ${BOLD_YELLOW}%.1fs${RESET}", + be.targetCounter, elapsed) + log.Infof("Exported %d targets (%.1fs)", be.targetCounter, time.Since(startTime).Seconds()) + return + case <-ticker.C: + done := be.targetCounter + elapsed := time.Since(startTime).Seconds() + stats := be.state.SystemStats() + + if isTerminal { + logs := cli.CurrentBackend.FlushOutput() + if len(logs) > 0 { + // Remove footer line before writing logs. + cli.Fprintf(os.Stderr, "${RESETLN}") + for _, line := range logs { + fmt.Fprintln(os.Stderr, line) + } } + cli.Fprintf(os.Stderr, "${RESETLN}${BOLD_WHITE}Exporting${RESET} [${BOLD_GREEN}%d${RESET}/${BOLD_GREEN}%d${RESET}, ${BOLD_YELLOW}%.1fs${RESET}]:: CPU: %5.1f%% Mem: %5.1f%% IO: %5.1f%%${RESET}", + done, total, elapsed, stats.CPU.Used, stats.Memory.UsedPercent, stats.CPU.IOWait) } - cli.Fprintf(os.Stderr, "${RESETLN}${BOLD_WHITE}Exporting${RESET} [${BOLD_GREEN}%d${RESET}/${BOLD_GREEN}%d${RESET}, ${BOLD_YELLOW}%.1fs${RESET}]:: CPU: %5.1f%% Mem: %5.1f%% IO: %5.1f%%${RESET}", - done, total, elapsed, stats.CPU.Used, stats.Memory.UsedPercent, stats.CPU.IOWait) } } + }() + + return func() { + close(stop) + <-done // Blocks until the monitor finishes the post instructions. } + } // exportRepoConfig exports the repository's configuration files (e.g., .gitignore, .plzconfig and From e2ef88732e4f9cccd73d0e88fc91ce6616cab810 Mon Sep 17 00:00:00 2001 From: duarte Date: Tue, 29 Sep 2026 15:03:44 +0100 Subject: [PATCH 05/10] yield on AllBuildInputs --- src/core/build_target.go | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/core/build_target.go b/src/core/build_target.go index 4ac6fd6a6..6d67df068 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -1596,18 +1596,29 @@ func (target *BuildTarget) getCommand(state *BuildState, commands map[string]str return highestCommand } -// AllBuildInputs returns all the inputs for this target. -func (target *BuildTarget) AllBuildInputs() []BuildInput { - srcs := target.AllSources() - data := target.AllData() - tools := target.AllTools() - - size := len(srcs) + len(data) + len(tools) - inputs := make([]BuildInput, 0, size) - inputs = append(inputs, srcs...) - inputs = append(inputs, data...) - inputs = append(inputs, tools...) - return inputs +// AllBuildInputs returns yields over the inputs for this target. +func (target *BuildTarget) AllBuildInputs() iter.Seq2[int, BuildInput] { + return func(yield func(int, BuildInput) bool) { + var idx int + for _, src := range target.AllSources() { + if !yield(idx, src) { + return + } + idx++ + } + for _, data := range target.AllData() { + if !yield(idx, data) { + return + } + idx++ + } + for _, tool := range target.AllTools() { + if !yield(idx, tool) { + return + } + idx++ + } + } } // AllSources returns all the sources of this rule. From aed960ca2d053c7ba2267116baf04122e1839430 Mon Sep 17 00:00:00 2001 From: duarte Date: Tue, 29 Sep 2026 17:11:55 +0100 Subject: [PATCH 06/10] Parse entire package if ParseMetadata is set --- src/core/build_label.go | 9 +++++ src/export/export.go | 1 - src/plz/plz.go | 73 ++++++++++++++++++++++------------------- 3 files changed, 49 insertions(+), 34 deletions(-) diff --git a/src/core/build_label.go b/src/core/build_label.go index f7e2755f6..7750a6d80 100644 --- a/src/core/build_label.go +++ b/src/core/build_label.go @@ -478,6 +478,15 @@ func HashBuildLabel(l BuildLabel) uint64 { return cmap.XXHashes(l.Subrepo, l.PackageName, l.Name) } +// PackageLabel returns a build label representing the enclosing package (i.e. //pkg:all). +func (label BuildLabel) PackageLabel() BuildLabel { + return BuildLabel{ + Subrepo: label.Subrepo, + PackageName: label.PackageName, + Name: "all", + } +} + // packageKey returns a key for this build label that only uses the subrepo and package parts. func (label BuildLabel) packageKey() packageKey { return packageKey{Name: label.PackageName, Subrepo: label.Subrepo} diff --git a/src/export/export.go b/src/export/export.go index 220df85b9..b5d02911b 100644 --- a/src/export/export.go +++ b/src/export/export.go @@ -165,7 +165,6 @@ func (be *baseExporter) startMonitor() func() { close(stop) <-done // Blocks until the monitor finishes the post instructions. } - } // exportRepoConfig exports the repository's configuration files (e.g., .gitignore, .plzconfig and diff --git a/src/plz/plz.go b/src/plz/plz.go index 82d264c3c..6fe9402e8 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -365,59 +365,66 @@ func inSamePackage(label, dependent core.BuildLabel) bool { // RecursiveParse is like Parse but recurses down into all dependencies of the target as well. func (r *runner) RecursiveParse(ctx context.Context, label, dependent core.BuildLabel) error { + if r.state.ParseMetadata && !r.isExternal(label) { + // If [state.ParseMetadata] is set, we need the entire package (and hence its metadata) to be + // parsed. + label = label.PackageLabel() + } if !label.IsAllTargets() { - return r.recursiveParse(ctx, label, dependent) + return r.recursiveParseTarget(ctx, label, dependent) } - pkg, err := r.Parse(ctx, label, dependent) + return r.recursiveParsePackage(ctx, label, dependent) +} + +// isExternal reports whether the label represents a target from an external repo. +func (r *runner) isExternal(label core.BuildLabel) bool { + if label.Subrepo == "" { + return false + } + return r.state.Graph.Subrepo(label.Subrepo).IsExternal() +} + +// recursiveParseTarget parses a target and, transitively, everything it depends on. +func (r *runner) recursiveParseTarget(ctx context.Context, label, dependent core.BuildLabel) error { + if !r.parseOnce.Add(label, struct{}{}) { + return nil // Someone else has this one; they're in the same errgroup so we needn't wait for them. + } + target, err := r.parseTarget(ctx, label, dependent) if err != nil { return err } g, gctx := r.group(ctx) - for _, target := range pkg.AllTargets() { - for dep := range target.DeclaredDependencies() { - g.Go(func() error { - // N.B. No need to deduplicate these; recursiveParse does that for the whole walk. - return r.recursiveParse(gctx, dep, target.Label) - }) - } + for dep := range target.DeclaredDependencies() { + g.Go(func() error { + return r.RecursiveParse(gctx, dep, target.Label) + }) } return g.Wait() } -// recursiveParse parses a target and, transitively, everything it depends on. -func (r *runner) recursiveParse(ctx context.Context, label, dependent core.BuildLabel) error { +// recursiveParsePackage parses all targets in a package. +func (r *runner) recursiveParsePackage(ctx context.Context, label, dependent core.BuildLabel) error { if !r.parseOnce.Add(label, struct{}{}) { return nil // Someone else has this one; they're in the same errgroup so we needn't wait for them. } - target, err := r.parseTarget(ctx, label, dependent) + pkg, err := r.Parse(ctx, label, dependent) if err != nil { return err } g, gctx := r.group(ctx) - for dep := range target.DeclaredDependencies() { + for _, target := range pkg.AllTargets() { g.Go(func() error { - return r.recursiveParse(gctx, dep, target.Label) + // N.B. No need to deduplicate these; recursiveParse does that for the whole walk. + return r.recursiveParseTarget(gctx, target.Label, label) }) } - if r.state.ParseMetadata { - // parseTarget can't return as soon as the target itself turns up, before the rest of its package - // (and hence its metadata) has been parsed, so we have to make sure the whole package is done - // here - all targets and subincludes. - pkg, err := r.Parse(ctx, target.Label, dependent) - if err != nil { - return err - } - if !pkg.Subrepo.IsExternal() { - for _, t := range pkg.AllTargets() { - g.Go(func() error { - return r.recursiveParse(gctx, t.Label, target.Label) - }) - } - for _, subinc := range pkg.AllSubincludes(r.state.Graph) { - g.Go(func() error { - return r.recursiveParse(gctx, subinc, target.Label) - }) - } + if r.state.ParseMetadata && !pkg.Subrepo.IsExternal() { + // For storing parse metadata we have to make sure the whole package is done here - all targets + // but also the subincludes. + for _, subinc := range pkg.AllSubincludes(r.state.Graph) { + g.Go(func() error { + return r.RecursiveParse(gctx, subinc, label) + }) } } return g.Wait() From eb346ef89fe5716f72f54004a8f71526fdc1f568 Mon Sep 17 00:00:00 2001 From: duarte Date: Tue, 29 Sep 2026 17:35:37 +0100 Subject: [PATCH 07/10] redundant target string in error --- src/export/export.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/export/export.go b/src/export/export.go index b5d02911b..420c903b2 100644 --- a/src/export/export.go +++ b/src/export/export.go @@ -197,7 +197,7 @@ func (be *baseExporter) exportTargets(labels core.BuildLabels) { } target, err := be.getTarget(l) if err != nil { - log.Errorf("Unable to lookup target %s: %s", l, err) + log.Errorf("Unable to lookup target: %w", err) continue } be.strategy.exportTarget(target) From f44560eb7ba94ed00d9af158bbaca08eb701b3e2 Mon Sep 17 00:00:00 2001 From: duarte Date: Wed, 30 Sep 2026 10:55:07 +0100 Subject: [PATCH 08/10] remove int form iterator --- src/core/build_target.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/core/build_target.go b/src/core/build_target.go index 6d67df068..b49c53509 100644 --- a/src/core/build_target.go +++ b/src/core/build_target.go @@ -1597,26 +1597,22 @@ func (target *BuildTarget) getCommand(state *BuildState, commands map[string]str } // AllBuildInputs returns yields over the inputs for this target. -func (target *BuildTarget) AllBuildInputs() iter.Seq2[int, BuildInput] { - return func(yield func(int, BuildInput) bool) { - var idx int +func (target *BuildTarget) AllBuildInputs() iter.Seq[BuildInput] { + return func(yield func(BuildInput) bool) { for _, src := range target.AllSources() { - if !yield(idx, src) { + if !yield(src) { return } - idx++ } for _, data := range target.AllData() { - if !yield(idx, data) { + if !yield(data) { return } - idx++ } for _, tool := range target.AllTools() { - if !yield(idx, tool) { + if !yield(tool) { return } - idx++ } } } From a1cf900d3edc553f357559c1a63b9659bb0ee944 Mon Sep 17 00:00:00 2001 From: duarte Date: Wed, 30 Sep 2026 11:44:46 +0100 Subject: [PATCH 09/10] clarifying comments --- src/plz/plz.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/plz/plz.go b/src/plz/plz.go index 6fe9402e8..102e930e2 100644 --- a/src/plz/plz.go +++ b/src/plz/plz.go @@ -366,8 +366,10 @@ func inSamePackage(label, dependent core.BuildLabel) bool { // RecursiveParse is like Parse but recurses down into all dependencies of the target as well. func (r *runner) RecursiveParse(ctx context.Context, label, dependent core.BuildLabel) error { if r.state.ParseMetadata && !r.isExternal(label) { - // If [state.ParseMetadata] is set, we need the entire package (and hence its metadata) to be - // parsed. + // If [state.ParseMetadata] is set, we need the entire package to be parsed and present in + // the graph so that all adjacent targets and their dependencies are available. Converting + // the label to its package pseudo-label (e.g. //pkg:target -> //pkg:all) routes execution + // to recursiveParsePackage below. label = label.PackageLabel() } if !label.IsAllTargets() { @@ -419,8 +421,13 @@ func (r *runner) recursiveParsePackage(ctx context.Context, label, dependent cor }) } if r.state.ParseMetadata && !pkg.Subrepo.IsExternal() { - // For storing parse metadata we have to make sure the whole package is done here - all targets - // but also the subincludes. + // When storing parse metadata we need to fully parse any packages of subincluded targets - + // including transitive subincludes - to have all their adjacent targets parsed as well. + // While `pkg`'s own parse already resolved the individual subinclude targets (e.g. + // //lib:a), adjacent targets in those packages (e.g. //lib:b) were not necessarily parsed. + // Passing each subinclude to RecursiveParse will expand it to its enclosing package label + // when ParseMetadata is set, ensuring those packages and their adjacent targets are fully + // parsed into the graph. for _, subinc := range pkg.AllSubincludes(r.state.Graph) { g.Go(func() error { return r.RecursiveParse(gctx, subinc, label) From 3b2914a659a83faca7a9b6e6c1fb0ec746e33fa2 Mon Sep 17 00:00:00 2001 From: duarte Date: Wed, 30 Sep 2026 11:54:07 +0100 Subject: [PATCH 10/10] fix AllBuildInputs single variable --- src/export/export.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/export/export.go b/src/export/export.go index 420c903b2..bf3137eec 100644 --- a/src/export/export.go +++ b/src/export/export.go @@ -212,7 +212,7 @@ func (be *baseExporter) exportDependencies(target *core.BuildTarget) { // exportSources exports all files required by the target. func (be *baseExporter) exportSources(target *core.BuildTarget) { - for _, src := range target.AllBuildInputs() { + for src := range target.AllBuildInputs() { if _, ok := src.Label(); ok { continue // These will be handled as dependencies later }