Simplify subsystem option scoping.
Review Request #2380 - Created June 16, 2015 and submitted
|nhoward_tw, patricklaw, zundel|
This gets rid of the concept of "scope qualifiers", which was mostly just confusing.
Now subsystems just have scopes ('ivy', 'scala-platform' etc.) and task-specific
instances have options in the appropriate subscope (e.g., `cache.compile.java` instead of
`compile.java.cache` as it was previously). This has many advantages:
- Option value inheritance "just works": `cache.compile.java` will inherit values from
`cache`, in the usual way.
- Overriding values in the subscope will also work as it does today - by registering
the option as recursive=True.
- Code is simpler, with a net reduction in LoC: "82 insertions(+), 114 deletions(-)".
- Subsystems and tasks are now more uniform. For example, both just have an options_scope
property, instead of one having a scope_qualifier() method.
This change doesn't affect any existing pants.ini or flags because we don't have any
task-specific subsystem instances yet.
It does mean that the section name for a task-specific config would be
[cache.compile.java], not [compile.java.cache], but that doesn't seem like a big deal.
All unit tests pass locally.
CI passes here: https://travis-ci.org/pantsbuild/pants/builds/67110458.
BTW I'm looking into some metaclass magic for detecting when a class hasn't declared an options_scope, and throwing a sensible error, instead of letting None propagate. But that will be in a followup change. I don't want to turn options_scope into a method because we have dozens of tasks that declare it as a class variable, and I don't want to have to change that (not even for Subsystems, because I want those to be uniform with tasks).
Looks good! Will need to update the config/options migrator I suppose?
"until we kill this subsystem outright" is there are ticket/issue/plan for that somewhere?
can drop the
Can we imagine cases in which the global subsystem would need options that we wouldn't want inherited by the tasks?
setup implies that initializing a subsystem has side-effects, which I don't think we have ambitions for?
What the "main" scope is isn't defined anywhere, so using that words might be filler here.
Can you create a test where it demonstrates the new behavior for setting an option, "task-specific
instances have options in the appropriate subscope (e.g.,
compile.java.cacheas it was previously)"
Address code review comments.
Revision 2 (+82 -114)
Status: Closed (submitted)
Submitted as 3b38c063a3d7fe23bd0af9a4be9e1814eb294e77.