Skip to content

Commit

Permalink
Factor out much of Package.Builder into TargetDefinitionContext
Browse files Browse the repository at this point in the history
This CL creates a new base class of `Package.Builder`, `TargetDefinitionContext`, to hold all the API needed during Starlark evaluation of a package. It does not include operations that are only needed during setup / tear down of the package builder. For instance, `addRule()` belongs on this base class, whereas `finishBuild()` remains on the `Builder`.

Factoring in this manner makes it clearer what operations need to be supported to evaluate a piece of a package corresponding to a single symbolic macro (needed for lazy macro evaluation). In any case, anything that splits the API in a reasonably systematic way and allows us to siphon off lines of code from Package.java is a win.

A previous base class by the name of `TargetDefinitionContext` was deleted in 33a2bb7. I made it a point to first remove that file before working on this CL so that we could treat the new `TargetDefinitionContext.java` as a branch of Package.java for better code review and history preservation. All the stuff that's branched over is left in the same declaration order.

Other changes:
- Migrated fields are made `protected` if they are still directly referenced in `Package.java`.
- `setMakeVariable`, `mergePackageArgsFrom` (2 overloads), and `setWorkspaceName` are made void (avoids the `Builder` return type problem).
- `getRulesSnapshotView`, `getNonFinalizerInstantiatedRule`, and `addMacro` are moved but also overridden by the builder.

Work toward #19922.

PiperOrigin-RevId: 681006375
Change-Id: I2df816dc35e7ceea9e8704ae9df2a2ea6daeceda
  • Loading branch information
brandjon authored and copybara-github committed Oct 1, 2024
1 parent d42301b commit beee1bf
Show file tree
Hide file tree
Showing 4 changed files with 665 additions and 553 deletions.
Loading

0 comments on commit beee1bf

Please sign in to comment.