Validate that option scopes and target aliases don't collide. #11004
Conversation
Also reserves some names that cannot be used as scopes or aliases
('global', 'targets', 'goals') so that `./pants help <name>` is
unambiguous.
Works towards addressing pantsbuild#10999.
Also removes some cruft from test_extension_loader.py.
[ci skip-rust]
[ci skip-build-wheels]
|
Note, also tested manually by changing names of various scopes to see what happens. |
| @dataclass(frozen=True) | ||
| class BuildConfiguration: | ||
| """Stores the types and helper functions exposed to BUILD files.""" | ||
|
|
||
| registered_aliases: BuildFileAliases | ||
| optionables: FrozenOrderedSet[Optionable] | ||
| optionables: FrozenOrderedSet[Type[Optionable]] |
There was a problem hiding this comment.
Note that this was always wrong.
There was a problem hiding this comment.
Thanks! Everything looks great modulo one comment.
With that comment implemented, I think you will need to rename #10999 (comment). I encourage doing that as a precursor PR for the sake of the changelog. This PR is "internal only" so left off, and we want a dedicated entry for "Deprecated scope Foo in favor of Bar"
| """Return all optionables in the system: global and those registered via rule usage.""" | ||
| return _GLOBAL_SUBSYSTEMS | self.optionables | ||
|
|
||
| def __post_init__(self) -> None: |
There was a problem hiding this comment.
I don't think this will catch collisions between the subsystem pex-binary and the target type pex_binary. As discussed, that's subtle and I think we do want to error here.
To fix, I think for subsystems you can add both my-scope and my_scope to the defaultdict? We know target types can only use _ due to Python restrictions.
There was a problem hiding this comment.
Was going to do that in a followup, along with actually allowing the normalized form in config files, but yeah I can add just the check here now.
I don't think this is internal only - it has external effect. If someone happened to have a custom target type or option with a colliding name, they will now get an error. |
True, but I think that would be "Plugin API Changes", vs. "User API changes" for the deprecations. |
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
…uild#11004) Also reserves some names that cannot be used as scopes or aliases ('global', 'targets', 'goals') so that `./pants help <name>` is unambiguous. Works towards addressing pantsbuild#10999. Also removes some cruft from test_extension_loader.py.
Also reserves some names that cannot be used as scopes or aliases
('global', 'targets', 'goals') so that
./pants help <name>isunambiguous.
Works towards addressing #10999.
Also removes some cruft from test_extension_loader.py.
[ci skip-rust]
[ci skip-build-wheels]