pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.32k stars 636 forks source link

Introduce call-by-name syntax for `@rules` #19730

Open stuhood opened 1 year ago

stuhood commented 1 year ago

(as motivated in #18905)

Implementation:

  1. rule_graph crate changes:
    • [x] adjust the definition of Get (DependencyKey) for rule-graph solving to accept a fixed name/identifier for the rule to use, and some/all of its positional arguments
    • [x] adjust graph solving to use the explicitly specified arguments to skip solving
  2. syntax changes:
    • [x] adjust the rule visitor (AwaitableCollector) to support extracting directly called @rules, in addition to the existing support for "rule helpers"
    • [x] introduce an implicitly(..) builtin, which takes arguments similar to Get(TypeName, ..)
    • [x] add support for positional arguments to @rule calls
    • [x] implement support for call-by-name in MultiGet syntax
    • [x] expose intrinsics as call-by-name (https://github.com/pantsbuild/pants/pull/20874)
    • [ ] determine new @union-usage syntax (or keep it type driven)
  3. runtime changes:
    • [x] adjust the @rule decorator such that when it has been called directly (i.e., not called with some secret-handshake), it trampolines back out to fill the additional positional arguments and to begin memoization
      • applies to both async def and def @rules, since the act of memoizing can potentially mean blocking to wait for a result computed by another task
    • [x] Use the @rule ids introduced in #19755 to drive @rule solving
  4. deployment:
    • [x] #20572 - add a pants-builtin goal which will execute a rewrite of one or more specified plugins or files from await Get(TypeName, ..) -> await rule_name(.., **implicitly(..))
      • this requires rule-graph solving first (to select the @rule to use at each Get callsite), so the rewritten file must actually have been loaded
    • [ ] #21065 - migrate pants' code
    • [ ] Audit and change @rule names that are widely used in plugins/in-repo-backends in cases where they are not sufficiently descriptive.
    • [ ] Update docs for new syntax
stuhood commented 1 year ago

Have updated the TODO list in the description with some of the (unordered) next steps. In particular: we are not yet actually using the facility introduced by #19755 to drive solving when the syntax from #19797 is used.

stuhood commented 8 months ago

I've updated the description to include two new tasks:

stuhood commented 6 months ago

.@sureshjoshi took a huge step on this one via #20572. Huzzah!

I've updated the description.

huonw commented 1 week ago

Added #21065 to the todo list.