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]

The proof of concept #20913 motivating this discussion exposed a builtin_goals plugin hook for that effect. So the next steps would be to iterate on the proof of concept to have this new base class and expose it via a plugin hook not named builtin_goals.

[cognifloyd]

What about using the term Engine since this wraps the pants "engine"? (vs exposing the terms "scheduler" or "session")

• EngineGoal
• RootEngineGoal
• EngineManagerGoal <- I think I like this one best
• EngiineControllerGoal
• EngineControllingGoal
• EngineWrapperGoal
• SingeltonEngineGoal
• ExclusiveEngineGoal

[kaos]

I don't see this use case distinguished from any other goal rule in terms of "wrapping the engine".. but more how we interact with the engine. A regular goal is invoked by the engine as one of possibly many goals, and the goal rule implementation is limited to only invoke regular rules and then return.

This new goal is more like a daemon mode goal, I think. It could be long running, take over stdin/stdout/stderr, invoke any goals as well as regular rules, control the session(s) etc. So, how about calling it DaemonGoal?

[sureshjoshi]

Daemon tends to imply that it'll be backgrounded, outside of user interaction - which works in many cases, but something like help is a BuiltinGoal 🤷🏽

[kaos]

I didn't get the connection with the help goal being a BuiltinGoal. But yea, that's a little what I was getting at/hoping for.. the new goal being a long running task that can be "in the background" of other stuff going on.