Export: trimming enhancements by DuBento · Pull Request #3530 · thought-machine/please

DuBento · 2026-05-01T14:45:16Z

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.

… build targets

setting subincludes at package level instead of at target level

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

…dary build def with sources the updated test uses non-standard child naming to validate new trimming logic

previous implementation was append only meaning that it would mark a subinclude as required if it was visited before even if not exported.

…a to also avoid tracking inside subincludes

…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.

… package level

…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)

…r traversal.

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

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

… they were interpreted oir not

…with cap

toastwaffle

Nearly there!

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

toastwaffle · 2026-07-08T14:03:52Z

 		keys = append(keys, k)
+		size += len(vals)
 	}
 	sort.Strings(keys)


Non-actionable: Why do we sort here?

toastwaffle · 2026-07-08T14:03:57Z


-func (target *BuildTarget) allBuildInputs(unnamed []BuildInput, named map[string][]BuildInput) []BuildInput {
-	ret := unnamed
+func (target *BuildTarget) selectBuildInputs(unnamed []BuildInput, named map[string][]BuildInput) []BuildInput {


Given you've touched it and presumably understand why this exists, could you add a doc comment here?

I'm not sure "select" is the right word, maybe "combined"?

Why is this even a method on BuildTarget? I think this can be a plain func

toastwaffle · 2026-07-08T14:09:38Z

 	return int64(bs.Start)
 }

+// hashBuildStatement mixes the Start and End byte coordinates to produce a unique 64-bit hash.


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?

toastwaffle · 2026-07-08T14:14:43Z

+	// dependencies. Dependencies identify the subincluded targets required for a successful
+	// interpretation of the statement, i.e. targets that provide the required symbols (variables or
+	// methods). The files argument identify the files required when interpreting that statement, for
+	// example using glob(),


I don't follow "for example using glob"

toastwaffle · 2026-07-08T14:15:42Z

-	RegisterSubincludeStatement(label BuildLabel, stmtProvider BuildStatementProvider)
+	RegisterStatementTarget(target BuildLabel, stmtProvider BuildStatementProvider)
+	// RegisterSubincludeStatement records the given build statement (provided by stmtProvider)
+	// as being a subincluded statement. This should only be called for statements in BUILD files.


subincluded -> subinclude? otherwise I don't follow

toastwaffle · 2026-07-11T20:22:45Z

@@ -0,0 +1,2 @@
+[parse]
+BuildFileName = BUILD_FILE
