Export: trimming enhancements - #3530

Open

DuBento wants to merge 121 commits into

thought-machine:masterfrom

DuBento:export-from-package-metadata

DuBento commented May 1, 2026 •

edited

Loading

Contributor

Enhancements to plz export, moving from a basic target-level trimming (using gc.RewriteFile) to build statement-level trimming, including only the required build rules and subincludes.
For consistency, we format all the exported BUILD files.

Changelog:

Introduced PackageMetadata to track the relationship between BUILD file statements, the targets they generate, and the subincludes they require. Made optional to avoid the overhead for most ops and enabled for the export.
Introduced ScopeMetadata to track object origins (subincluded labels) and statement tracking during the interpreter phase. Made optional to avoid the overhead for most ops and enabled for the export.
Refactored src/export/export.go to enforce better separation of the DefaultExporter (for trimming) and NoTrimExporter.
Added logic to parse BUILD files and selectively write back statements based on whether the generated targets are part of the export set.
Implemented "minimal subinclude" generation, which rewrites subinclude() calls to only include labels actually used by the exported targets.
Update and added some of the e2e to reflect the changes in implementation.
Moved trimming from a GC-based target removal to a full AST statement walk.

duarte added 30 commits

April 29, 2026 19:28


          Package level metadata that includes a mapping of build statements to…

0ef91bc

… build targets


          export using BuildFileMetadata logic

63f1b0d


          register caller scope for callstack-like traversal

419bd56


          bufio writer for build statements

8e1bd00


          rewrite export logic into explicit flow

258b170


          export targets related to build statement


          enrich target with subincludes by looping though all scopes

a4252c9


          separate buildstmt register from adding target

82f117f


          select and write subincludes

eb657de

setting subincludes at package level instead of at target level


          skip statement for preloaded subincludes


          test: trim subincludes

8e6c9c0


          rename file

9f0b4fe


          use slices.collect and sort interface

0e597c6


          rewrite export with 2 concrete interfaces: default and notrim

de76e2c


          notrim: full copy BUILD file and visit sources

03ed361


          double new lines between targets

717aba1


          suppress diff output when enforcing repo differences

94160bf


          export by filtering the original BUILD file

582319e

- register subinclude statements in the package metadata
- filter subincludes label
- export all non build_target related statements


          subincludes use label short string

e6a5293


          Simplify package filtering method into a more explicit "switch" case

ea329c3


          skip subrepos and internal packages when writing package file

1a47c22


          fix reusing err var resulted in failed filtering

a35119a


          doc strings for package metadata

9c1522c


          export missing doc strings

9395a59


          test: named go_repo and change testify for slimmer UUID

3c3d3d0


          export dependencies of subrepos - 13/14 tests passing

13e95cb


          test: internal repo test using temp directory to avoid stale data

28b5441


          fix: 0 label subinclude

781af3a


          adjusting dependency lookup and adjacent target test to include secon…

2f8355c

…dary build def with sources

the updated test uses non-standard child naming to validate new trimming logic


          test: add custom tool to native test

3e89c85

DuBento added 24 commits

June 17, 2026 18:28


          rename active to required subincludes

51cbb35


          imp: skip scope and package metadata tracking for external subrepos

d491c84


          remove doc reference to required target naming for successful export

8a1a702


          Only pop symbols from stack in package scope + rework newScopeMetadat…

131b489

…a to also avoid tracking inside subincludes


          lazy initialize symbol origins

f437f8f


          Close package wait channels on error. Introduced a deadlock when Keep…

b0f200e

…Parser is set.

This wasn't an issue before since we would force close all the channels and fail during the first parse, but now we will attempt to wait for parsed package during an export.


          Support method calls as arguments by popping the symbol stack only at…

9c6cf87

… package level


          refactor: record all statements and respective origins for all statem…

349e3f8

…ents (including non-target generators).

The reason for pivoting is that the previous implementation failed to identify necessary subincludes for variable declarations or other builtin methods (string join)


          Package level scope metadata; Tracking done at package level only.

1c67f8b


          Package metadata lookup table in interpreter to avoid recursive calle…

7d6ba12

…r traversal.


          Trimmer: always keep for expression but trim stmts inside

0af4d97


          glob statement: register files required by statements

1e09063


          Checkpoint and restore symbol and file stack to support different cal…

…ls to interpretStatements with the package level scope (e.g. for loop)


          move to BuildLabels in export and metadata to avoid pressure on GC

e6c8fee


          track interpreted statements (mark as 0 targets) to correctly identif…

e4aa82b

…y if blocks that have interpreted stmts (var redeclaration)


          subinclude tracking for dynamic subincludes

8022ad5


          track inner level globs.

b89bc92


          feat(cmd): query print metadata - print package metadata

e72ef50


          track inner statements (e.g. if clauses) and trim clauses based on if…

70f4701

… they were interpreted oir not


          lint suggestions: remove hashBuildTarget and initialise labels slice …

0e7a3b2

…with cap


          feat(cmd): metadata json formatting

d3f27bf


          progress monitor for export


          metadata: drop json printing and optional hidden targets

cae0433


          maintain subinclude label order

75390e8

toastwaffle requested changes

View reviewed changes

toastwaffle left a comment

Contributor

Nearly there!

As ever, comments phrased as questions probably imply a need for code comments

src/core/build_target.go

src/core/build_target.go Outdated

src/core/package_metadata.go Outdated

               	return int64(bs.Start)
               }
+              // hashBuildStatement mixes the Start and End byte coordinates to produce a unique 64-bit hash.

toastwaffle Jul 8, 2026

Contributor

I feel like for amusement we should state that "This does introduce a 4,294,967,296 byte size limit on BUILD files processed by Please"

Also, is it at all concerning that our hash is not uniformly distributed? What's worse - a non-uniform distribution, or doing more work to make it uniform?

src/core/package_metadata.go Outdated

src/core/package_metadata.go Outdated

test/export/test_subinclude_unused/source_repo/.plzconfig

		@@ -0,0 +1,2 @@
		[parse]
		BuildFileName = BUILD_FILE No newline at end of file

toastwaffle Jul 11, 2026

Contributor

supernit: missing trailing newline

test/export/test_subinclude_unused/source_repo/BUILD_FILE

+              custom_file(
+                  name = "unneeded",
+                  src = "unneeded.txt",
+              )

                
                    No newline at end of file

toastwaffle Jul 11, 2026

Contributor

supernit: missing trailing newline

test/export/test_subrepo_subtarget/expected_repo/.plzconfig

		@@ -0,0 +1,2 @@
		[parse]
		BuildFileName = BUILD_FILE No newline at end of file

toastwaffle Jul 11, 2026

Contributor

supernit: missing trailing newline

test/export/test_subrepo_subtarget/source_repo/subrepo/BUILD_FILE

+                  name = "_foo#sub",
+                  srcs = ["BUILD_FILE"],
+                  visibility = ["PUBLIC"],
+              )

                
                    No newline at end of file

toastwaffle Jul 11, 2026

Contributor

supernit: missing trailing newline

test/export/test_subrepo_subtarget/source_repo/.plzconfig

		@@ -0,0 +1,2 @@
		[parse]
		BuildFileName = BUILD_FILE No newline at end of file

toastwaffle Jul 11, 2026

Contributor

supernit: missing trailing newline

duarte added 3 commits

August 5, 2026 12:53


          combined build inputs

41d0bfd


          Pseudo uniform distribution

1cc61c8


          Package metadata renaming and doc comment fixes

3f140a1

peterebden requested changes

View reviewed changes

peterebden left a comment

Collaborator

I have some worries about crossing package responsibilities here - I get there's a lot more information we need to store to support this, and we can work through a bunch of that, but I think some of these changes like wanting to request parses from code post the actual build is a line we shouldn't cross (and I think maybe we don't have to).

src/core/state.go

+              	// is when exporting dependencies of targets that are not explicitly used but adjacent/related.
+              	KeepParserRunning bool
+              	// WaitForDisplay is a function that blocks until the display thread has finished.
+              	WaitForDisplay func()

peterebden Aug 13, 2026

Collaborator

This feels very out of place here. I don't think the build state should need to know about this.

src/core/state.go

+              	// calls to the parser. This is needed to support the export operation since the export logic will
+              	// attempt to export targets that have not been parsed during the normal build phase. An example
+              	// is when exporting dependencies of targets that are not explicitly used but adjacent/related.
+              	KeepParserRunning bool

peterebden Aug 13, 2026

Collaborator

I'm really uncomfortable about this. It feels very strange to keep things around post the actual build running. I'm also not clear how it can really work, since in general the parser needs to be able to request further builds, so you don't just need the parser running, you need the whole build 'engine' ready to go again.

IIUC the requirement is for export to be able to request additional parses of things 'alongside' in the same BUILD file but weren't original targets. I think we could solve this more simply by make it parse all those things in the first place - the faster version is to convert all initial labels to :all, the dumber version is to just parse everything for export in the first place, so we know the graph is complete when we get there.

src/core/package_metadata.go

Comment on lines +12 to +15

+              // BuildStatement represents the start and end byte positions of a parsed statement in a BUILD file.
+              type BuildStatement struct {
+              	Start, End int
+              }

peterebden Aug 13, 2026

Collaborator

This isn't fundamentally bad, but it does all feel like something that core doesn't currently deal with.

I guess we do need it sitting on Package really, but I wonder if it'd be better being an unimplemented interface in this package, and having other packages that know about BUILD file source stuff implement it.

peterebden reviewed

View reviewed changes

test/export/please_export_e2e_test.build_defs

                   ] + [f'plz --repo_root="{exported_repo}" {cmd}' for cmd in cmd_on_export]
-                  test_cmd = [cmd.replace("plz ", "$TOOLS_PLEASE ") for cmd in test_cmd]
+                  cache_args = '--override=cache.dir:"$($TOOLS_PLEASE query reporoot)"/plz-out/test-cache'

peterebden Aug 13, 2026

Collaborator

This is very fragile. I ran into corruption of this directory fairly quickly - presumably due to multiple tests running simultaneously that fought over it.

peterebden Aug 14, 2026

Collaborator

I think it might be reasonable to add some locking to the cache so separate processes exclude each other, which should fix this. I can look at that separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet