Pants: v2 goals must shadow names of v1 goals

[illicitonion]

The v1 goals list is currently used to work out what goals should be run; v2 @console_tasks with non-overlapping names will not be invoked.

[illicitonion]

Further, a v1 Task needs to have been registered for that goal name, because goal detection is done matching against option scope: https://github.com/pantsbuild/pants/blob/57bb47811f6cc96a130a4e1bf0c2ef355d0d78d3/src/python/pants/option/arg_splitter.py#L192-L193

Additionally, this check doesn't actually do anything in this case, because self._options.goals is empty: https://github.com/pantsbuild/pants/blob/57bb47811f6cc96a130a4e1bf0c2ef355d0d78d3/src/python/pants/bin/local_pants_runner.py#L183-L186

[stuhood]

Looking at this as part of #6880.

[stuhood]

Quick thoughts on this. The behaviour we want is both to:
1) be able to completely replace a goal
2) but also to be able to have two implementations of some other goal simultaneously.

To do this, I think that we can partition all goals by whether they are v1, v2, or both, and then:

• If neither --[no-]v1 nor --[no-]v2 is manually specified, for any goals that are unambiguously v1 or v2, run the unambiguous implementation.
• For any goals that are ambiguous (ie, have both a v1 and v2 impl), rely on the flags, regardless of whether they were specified.
• If the flags are manually specified, obey them.

[stuhood]

Thoughts after looking a bit deeper.

In ~all cases, it should be possible to install a Subsystem to act as the aggregator for options for a @console_rule, and to register its scope. One potentially tricky case that I think just works out is the ScopeInfo.GOAL type: it looks like this can be replaced with a Subsystem as well.

This would mean that our approach to identifying console rules would be to _require_ that they have an associated Subsystem installed, even if it had empty options. And it should be possible to remove most/all of the boilerplate involved with this. At that point we should be able to assume that all @console_rules will have associated known_scopes, and parsing should work fine.

There is one more interesting case, which is the case where you want to have both a v1 and v2 implementation of a Goal (note, not Task: that's easier) installed at once: for example, list in v1 and v2. This allows for incremental ports of Tasks to @console_rules. To do this, I believe that the strategy should be to move all options to a Subsystem at the existing scope (list in this example), and to then install the v1 Task below that scope (ie, something like list.legacy). Both v1 and v2 would then depend on the Subsystem for their options and their help.

[illicitonion]

That last update sounds great :)

It would be good to have some sugar so that the subsystem creation for v2 goals can be implicit, as it's a rather weird rule-writer-facing coupling, but otherwise, sounds great :)

[stuhood]

My reason for making it slightly more explicit is that the "goal" concept is just one reason why someone might make and consume a Subsystem: @rules should be able to consume them directly deeper in the graph, and so I'm hoping for some amount of uniformity.

But I agree that actually using ScopeInfo.GOAL in #6880 is likely a good idea. Just not quite sure how to unify them. cc @benjyw

[benjyw]

To do this, I believe that the strategy should be to move all options to a Subsystem at the existing scope (list in this example), and to then install the v1 Task below that scope (ie, something like list.legacy). Both v1 and v2 would then depend on the Subsystem for their options and their help.

That's pretty much what popped into my mind while reading the issue.

[benjyw]

To help me clarify my understanding: So today a goal couples two things that should be unrelated: 1. A user goal ("compile these targets"), and 2. An options scope. It may be true in many cases that we want options that affect the outcome of a goal to be in (or under) a scope that has the same name as the goal, but I'm not sure we want to enforce that coupling at the conceptual level. For example, today it forces us to have "goals" that no user cares about or will run directly (jvm-platform-validate anyone?), just so that various implementation tasks have an options scope to be under. These still appear, e.g., in the ./pants goals output, which is noisy and confusing to users.

My understanding is that this is not the case in the new engine? At least in that there is no concept of registering tasks under goals?

[illicitonion]

My reason for making it slightly more explicit is that the "goal" concept is just one reason why someone might make and consume a Subsystem: @rules should be able to consume them directly deeper in the graph, and so I'm hoping for some amount of uniformity.

I think that if you need a re-usable subsystem, you should be able to create one, but I disagree that a goal _needs_ one to be explicitly created - it adds some pretty non-obvious boilerplate to goal creation, adding an entire concept you either need to understand, or be happy to ignore the fact that you don't understand, to do something we want to be easy.

[stuhood]

I think that if you need a re-usable subsystem, you should be able to create one, but I disagree that a goal needs one to be explicitly created

Agreed. But one challenge is supporting both "goals with no associated options" (which is the only thing we supported before #6880) and "goals with associated options". I don't think it's good to have a non-uniform experience where things are almost incredibly simple when you consume no options, and then there is suddenly a new way to declare your goal once you need to consume options.

Additionally, we'd need to find some other solution to give @console_rules help, since even if you have no options, you need help.

Finally, it seems very unlikely that _any_ of the goals we create will have no options at all. So it's unclear whether that case is worth optimizing for.

My understanding is that this is not the case in the new engine? At least in that there is no concept of registering tasks under goals?

That's correct. A @console_rule represents an entire goal, period. Composition of @rules that produce a uniform product type (with some sort of switch statement to unify them, as in #4535) is how multiple language backends would participate in the goal.

[illicitonion]

I think that if you need a re-usable subsystem, you should be able to create one, but I disagree that a goal needs one to be explicitly created

Agreed. But one challenge is supporting both "goals with no associated options" (which is the only thing we supported before #6880) and "goals with associated options". I don't think it's good to have a non-uniform experience where things are almost incredibly simple when you consume no options, and then there is suddenly a new way to declare your goal once you need to consume options.

Additionally, we'd need to find some other solution to give @console_rules help, since even if you have no options, you need help.

Finally, it seems very unlikely that _any_ of the goals we create will have no options at all. So it's unclear whether that case is worth optimizing for.

The case in trying to optimise for isn't "goals with no options" but "goals where the options are only consumed in the console rule, and don't need to be injected as a subsystem elsewhere in the graph".

We encourage people to write these often in v1. Right now we get push back from folks because they think they're too complex/fiddly to create, so I'm trying to make sure this use case is as simple as possible in v2 by removing dissonant concepts ("I don't know what a subsystem is, now I need to learn more things, this is scary, I give up") and boilerplate.

I think concretely what I'm asking for is:

1.

class Goal(Subsystem):
  options_scope = self.goal_name # or whatever the real syntax is for this

1. Implicitly register the global subsystem for any console_rule which is registered.

(And 3. to detect these with ScopeInfo.GOAL not ScopeInfo.SUBSYSTEM, for the clarity of pants devs, though this is less important)

Which doesn't seem too high a burden for the simplified experience?

[stuhood]

Which doesn't seem too high a burden for the simplified experience?

I agree that that seems reasonable. Sorry, I thought that this comment was suggesting that defining a Goal/Subsystem by any name was too much boilerplate. But I think that instead we thoroughly agree =)

[stuhood]

@benjyw : I agree with @illicitonion's suggestion above about stealing ScopeInfo.GOAL for this usecase. In order to do that, I think that maybe I should do a separate PR (as a prereq for #6880) to adjust the relationship between pants.goal.goal.Goal, pants.task.goal_options_registrar.GoalOptionsRegistrar, and Subsystem.

At first glance, pants.goal.goal.Goal has a lot attached to it that we probably don't want to bring along... my feeling is that we should be aiming for class Goal(Subsystem) as @illicitonion sketched above. That has me feeling that GoalOptionsRegistrar should maybe evolve or be renamed and steal the name Goal. But maybe it should continue to subclass Optionable rather than subclassing Subsystem?

Will maybe start this this afternoon, but I have jury duty, so: unknown.

[benjyw]

pants.goal.goal.Goal sounds like an Argentinian soccer announcer...

[benjyw]

We do today register options on GOAL scope in a few places. How would that be affected?

[stuhood]

We do today register options on GOAL scope in a few places. How would that be affected?

Not sure. Going to be busy for the afternoon, but if you have any thoughts on this they'd be appreciated.

[illicitonion]

We could also make GOAL_V2 a thing