allow plugins to provide early goals (i.e., the current BuiltinGoal)

[tdyas]

Context: The core rules for Pants' support of Build Server Protocol ("BSP") are always loaded because the BSP rules need to provide a BuiltinGoal implementation (i.e., BSPGoal) so that the BSP server can take over stdin and stdout and respond to requests on them as per the BSP specification. This means loading rules (and the experimental-bsp goal) even if the users do not use the JVM backends which are the only (current) users of BSP.

#20913 is a draft PR which splits the BSP rules out of the core rules by turning BSP into a proper Pants backend.

But the BSP server still needs to register its BuiltinGoal implementation. The draft PR accomplishes this via a new builtin_goals plugin hook. This would be a new plugin API.

1. Are there alternative ways for the BSP server to take over stdin/stdout without needing to be a BuiltinGoal?
2. Is it acceptable to allow plugins to register BuiltinGoal implementations?
3. If so, maybe BuiltinGoal should be renamed to EarlyGoal which better fits what BuiltinGoal is actually used for? (Then the plugin hook would be early_goals.)

Longer term (ulterior motive): I may want to write a LSP server for Pants which can run a server similar to how the BSP server operates. It would need to take over stdin/stdout just like the BSP server and likely would need to be a BuiltinGoal (baring some other way to implement the server).

[sureshjoshi]

I'm interested about this LSP idea too? What would Pants serve over the LSP - as the VSCode plugin could make use of this too.

In the Slack chat, where we landed on as a short-term solution was that I've used pyright to support our BUILD files, and then maybe a json schema (or a toml LSP for pants.toml)

Aside: I also want to play around with the BSP later on - as right now, I'm using raw pants commands to interact with pants + vscode plugin but that feels... Blah.

[tdyas]

I'm interested about this LSP idea too? What would Pants serve over the LSP - as the VSCode plugin could make use of this too.

Basically an LSP for the Pants BUILD files and configuration files. Thus, an IDE plugin (e.g., VSCode, etc.) wold just query the running Pants instance for this information.

[tdyas]

Also, there are helper libraries for VSCode which handle the boilerplate of talking to an LSP server. That would make a VSCode plugin much easier to maintain.

[sureshjoshi]

Yep, pyright :)

RobotPajamas/suspenders@main...27-lsp

It works pretty well, I just haven't auto-generated the builtins.pyi

The general technique is that people tend to fork pyright and mod it (e.g. we would disallow imports and stuff).

[tdyas]

Nice!

[sureshjoshi]

Just to grab my comment from the PR that raised this discussion:

But we should probably carve out a ticket/discussion about this - as once this lands, BuiltIn goal becomes part of the usable public API, need docs around naming, why to use this vs goal_rule, etc etc blah blah.

Re: 2

Is it acceptable to allow plugins to register BuiltinGoal implementations?

I don't see why not, if the name was something different. I think in an ideal world, it would be nice to have everything filter through the goal_rule and rule APIs, but if BuiltInGoal has a particular need that can't be filled by the existing APIs, then it seems reasonable to expose it.

I don't know enough about the technical background about why they were split up like that, nor the implications (re: performance, customization, etc) to have any strong opinions otherwise.

Re: 3

If so, maybe BuiltinGoal should be renamed to EarlyGoal which better fits what BuiltinGoal is actually used for?

I don't particularly like "early_goal" as I feel like an API user would need a boatload of context just to understand why they would use it, why they wouldn't, and having a temporal prefix feels weird. Like, would there ever be a LateGoal - I can't imagine there would be, but when I see pre-, I usually expect an equivalent post-.

One item that struck me was more that BuiltinGoals aren't chainable/composable, so maybe there is a way to riff off that?

Or even, could goal_rule get a modifier to function as a BuiltinGoal?

Since @goal_rules always run in order to produce side effects (generally: console output),
they are not cacheable, and the Goal product of a @goal_rule contains only an exit_code
value to indicate whether the rule exited cleanly.

That seems to fit the spirit of the BSP to some degree, no?

[tdyas]

The primary reason for using BuiltinGoal was that the BSP server implementation takes over the Pants client's sys.stdin and sys.stdout entirely. (BSP servers are generally spawned by IDEs and communicate with the IDE on stdin/stdout as per the BSP server discovery spec.)

Rule code cannot do that currently. Even goal rules must interact with the external environment indirectly through loggers and the Workspace type (e.g., for writing out a Digest to the filesystem).

In contrast, the Pants BSP server logic controls all event processing itself and calls into the rule engine only as necessary. See BSPConnection.

[tdyas]

I don't particularly like "early_goal" as I feel like an API user would need a boatload of context just to understand why they would use it, why they wouldn't, and having a temporal prefix feels weird. Like, would there ever be a LateGoal - I can't imagine there would be, but when I see pre-, I usually expect an equivalent post-.

I am fine not using the word "early", I would prefer a word which conveys the fact that the BuiltinGoal instance is essentially side stepping all of the ordinary goal processing logic.

[sureshjoshi]

Just following up on some missing context @tdyas

Is the intention that the BSP would run "in"/"with" pantsd? What is the expectation if the user is running --no-pantsd?

I feel like that might help clarify some usage.

[tdyas]

At least for BSP, according to the spec, the IDE invokes Pants directly. Currently that is done as pants experimental-bsp --server which lets BSPGoal take over and run a BSP server on stdin/stdout to communicate with the IDE. I do not believe pantsd is involved at all. I haven't really delved into how Pants client requests are sent to pantsd, and whether this remains true for builtin goals.

[cognifloyd]

BuiltinGoal/EarlyGoal renaming / bikeshedding:

The interface for BuiltinGoal.run is:

pants/src/python/pants/goal/builtin_goal.py

Lines 38 to 46 in bef63e9

	def run(
	self,
	*,
	build_config: BuildConfiguration,
	graph_session: GraphSession,
	options: Options,
	specs: Specs,
	union_membership: UnionMembership,
	) -> ExitCode:

The interface for @goal_rule is wide open, accepting variable args like other rules. A @goal_rule is composable, where each matching goals runs serially, unlike BuiltinGoal where only one BuiltinGoal can run at a time. The docstring for BuiltinGoal is informative here:

pants/src/python/pants/goal/builtin_goal.py

Lines 19 to 28 in bef63e9

	class BuiltinGoal(ABC, GoalSubsystem):
	"""Builtin goals have precedence over regular goal rules.
	Only a single builtin goal is executed per run, any remaining goals/arguments are passed
	unaltered to the builtin goal.
	When multiple builtin goals are presented, the first builtin goal will be used unless there is a
	builtin goal that begin with a hyphen (`-`), in which case the last such "option goal" will be
	prioritized. This is to support things like `./pants some-builtin-goal --help`.
	"""

As "only a single builtin goal is executed per run", maybe we should use the term "Singleton" instead of "Builtin"? So, maybe one of these?

• SingletonGoal
• ExclusiveGoal

Or we play off of the interface, which includes a GraphSession which is a lightweight wrapper around the scheduler, and highlight that this kind of goal can run other goals:

• GraphManagerGoal
• GoalManager
• SchedulerControllingGoal

Other variations of these themes:

• RootGoal
• LeaderGoal
• SuperGoal
• CoreGoal

[tdyas]

What about these goal names (as suggested to me by the Claude AI):

• BypassGoal
• AuxiliaryGoal
• IndependentGoal
• AlternateGoal

I like the first two (BypassGoal and AuxillaryGoal). Both seems reasonable for "daemon" goals and one-off goals whcih bypass the engine.

[tdyas]

I asked Claude to argue in favor of one:

I am inclined to the suggestions of BypassGoal and AuxiliaryGoal. Please argue in favor of one of those suggestions.

Response:

I'll argue in favor of AuxiliaryGoal as the base class name:

1. Accuracy: AuxiliaryGoal accurately describes the nature of these goals. They provide additional functionality that supports the main application but operates independently of the core engine. This name doesn't imply any negative connotation or suggest that these goals are circumventing something they shouldn't.

2. Flexibility: The term "auxiliary" is broad enough to encompass both the original "builtin" goals and the new daemon server goals. It doesn't limit the potential types of goals that could inherit from this base class in the future.

3. Clarity: For developers unfamiliar with the codebase, AuxiliaryGoal immediately suggests that these are supplementary processes, which helps in understanding the overall architecture of the application.

4. Positive connotation: Unlike "bypass," which might imply avoiding or working around something, "auxiliary" has a positive connotation of providing additional support or functionality.

5. Common usage: The term "auxiliary" is widely used in software engineering to describe components that provide supporting functionality, making it intuitive for other developers.

6. Scalability: As your application grows, you might add more types of goals that operate outside the engine. AuxiliaryGoal can comfortably serve as a base class for a wide variety of such goals.

7. Relationship to main system: It clearly indicates a relationship with the main system (in this case, the engine) while also implying a degree of independence, which accurately reflects the nature of these goals.

[tdyas]

And prompting for further argument for AuxiliaryGoal:

Please compare AuxiliaryGoal more directly to BypassGoal and also any other previous suggestions which you find to be relevant.

Response:

Certainly. I'll compare AuxiliaryGoal to BypassGoal and a few other relevant suggestions:

1. AuxiliaryGoal vs BypassGoal:

   • AuxiliaryGoal suggests additional, supporting functionality, while BypassGoal implies circumvention.
   • AuxiliaryGoal has a more positive connotation, whereas BypassGoal might suggest avoiding proper channels.
   • AuxiliaryGoal is more flexible for future expansion, while BypassGoal is more specific to the current situation.

2. AuxiliaryGoal vs IndependentGoal (from earlier suggestions):

   • Both imply operation outside the main engine.
   • AuxiliaryGoal suggests a supporting role, while IndependentGoal emphasizes complete separation.
   • AuxiliaryGoal might better represent the goals' relationship to the overall application.

3. AuxiliaryGoal vs ParallelGoal (from earlier suggestions):

   • Both indicate operation alongside the main engine.
   • AuxiliaryGoal implies a supporting role, whereas ParallelGoal simply suggests concurrent operation.
   • AuxiliaryGoal might better capture the nature of these goals as additional, rather than just simultaneous.

4. AuxiliaryGoal vs SupplementaryGoal (from earlier suggestions):

   • These are quite similar in meaning.
   • AuxiliaryGoal is slightly more concise.
   • AuxiliaryGoal might be more commonly used in software contexts.

In summary, AuxiliaryGoal stands out for its positive connotation, flexibility, and accurate representation of the goals' role in the application. It avoids the potential negative implications of BypassGoal, is more specific than IndependentGoal or ParallelGoal, and is slightly more concise than SupplementaryGoal.

Would you like me to elaborate on any specific aspect of this comparison?

[cognifloyd]

Fascinating use of AI. AuxiliaryGoal works for me.

[sureshjoshi]

I've tried using ChatGPT to help with naming, and I generally hate what it comes up with. I think I'm in the same boat with Claude 😆

I'm not too involved with the naming of this to care too much, but I think if it's in this vein at all, then "SupplementalGoal" or "SupplementaryGoal" fits typical English usage a bit better than Auxiliary - but it all feels like $10 words to say "not BuiltInGoal" 😄

Makes me question whether "Goal" is even the correct word for what this is trying to do, but at this point, I'd rather have the feature than the bike shed 😆