Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not leak non-goal subsystems as goals to the command line int… #12337

Merged
merged 3 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/python/pants/engine/goal.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from typing_extensions import final

from pants.option.scope import ScopeInfo
from pants.option.subsystem import Subsystem
from pants.util.meta import classproperty

Expand Down Expand Up @@ -37,6 +38,10 @@ def list(console: Console, list_subsystem: ListSubsystem) -> List:
# it should declare the union types that must have members.
required_union_implementations: Tuple[Type, ...] = ()

@classmethod
def create_scope_info(cls, **scope_info_kwargs) -> ScopeInfo:
return super().create_scope_info(is_goal=True, **scope_info_kwargs)

@classproperty
@abstractmethod
def name(cls):
Expand Down
12 changes: 12 additions & 0 deletions src/python/pants/engine/goal_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pants.engine.console import Console
from pants.engine.goal import Goal, GoalSubsystem, LineOriented
from pants.engine.rules import goal_rule
from pants.option.scope import ScopeInfo
from pants.testutil.option_util import create_goal_subsystem, create_options_bootstrapper
from pants.testutil.rule_runner import mock_console, run_rule_with_mocks

Expand Down Expand Up @@ -33,3 +34,14 @@ def output_rule(console: Console, options: OutputtingGoalOptions) -> OutputtingG
)
assert result.exit_code == 0
assert stdio_reader.get_stdout() == "output...line oriented\n"


def test_goal_scope_flag() -> None:
class DummyGoal(GoalSubsystem):
name = "dummy"

dummy = create_goal_subsystem(DummyGoal)
assert dummy.get_scope_info().is_goal is True
assert dummy.known_scope_infos() == {
ScopeInfo(scope="dummy", optionable_cls=DummyGoal, is_goal=True)
}
6 changes: 5 additions & 1 deletion src/python/pants/option/arg_splitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def __init__(self, known_scope_infos: Iterable[ScopeInfo], buildroot: str) -> No
"help-advanced",
"help-all",
}
self._known_goal_scopes = {si.scope for si in known_scope_infos if si.is_goal}
self._unconsumed_args: List[
str
] = [] # In reverse order, for efficient popping off the end.
Expand Down Expand Up @@ -171,7 +172,10 @@ def assign_flag_to_scope(flg: str, default_scope: str) -> None:
while scope:
if not self._check_for_help_request(scope.lower()):
add_scope(scope)
goals.add(scope.partition(".")[0])
if scope in self._known_goal_scopes:
goals.add(scope.partition(".")[0])
else:
unknown_scopes.append(scope)
for flag in flags:
assign_flag_to_scope(flag, scope)
scope, flags = self._consume_scope()
Expand Down
24 changes: 15 additions & 9 deletions src/python/pants/option/arg_splitter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,18 @@

class ArgSplitterTest(unittest.TestCase):
_known_scope_infos = [
ScopeInfo("compile"),
ScopeInfo("compile.java"),
ScopeInfo("compile.scala"),
ScopeInfo("jvm"),
ScopeInfo("jvm.test.junit"),
ScopeInfo("reporting"),
ScopeInfo("test"),
ScopeInfo("test.junit"),
]
ScopeInfo(scope, is_goal=True)
for scope in [
"compile",
"compile.java",
"compile.scala",
"jvm",
"jvm.test.junit",
"reporting",
"test",
"test.junit",
]
] + [ScopeInfo("hidden", is_goal=False)]

@staticmethod
def assert_valid_split(
Expand Down Expand Up @@ -384,3 +387,6 @@ def test_no_goal_detection(self) -> None:
splitter = ArgSplitter(ArgSplitterTest._known_scope_infos, buildroot=os.getcwd())
splitter.split_args(shlex.split("./pants foo/bar:baz"))
self.assertTrue(isinstance(splitter.help_request, NoGoalHelp))

def test_hidden_scope_is_unknown_goal(self) -> None:
self.assert_unknown_goal("./pants hidden", ["hidden"])
7 changes: 6 additions & 1 deletion src/python/pants/option/optionable.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,17 @@ def validate_scope_name_component(cls, s: str) -> None:
f"dash-separated-words, with words consisting only of lower-case letters and digits."
)

@classmethod
def create_scope_info(cls, **scope_info_kwargs) -> ScopeInfo:
"""One place to create scope info, to allow subclasses to inject custom scope args."""
return ScopeInfo(**scope_info_kwargs)

@classmethod
def get_scope_info(cls) -> ScopeInfo:
"""Returns a ScopeInfo instance representing this Optionable's options scope."""
if cls.options_scope is None:
raise OptionsError(f"{cls.__name__} must set options_scope.")
return ScopeInfo(scope=cast(str, cls.options_scope), optionable_cls=cls)
return cls.create_scope_info(scope=cast(str, cls.options_scope), optionable_cls=cls)

@classmethod
def subscope(cls, scope) -> str:
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/option/options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def global_scope() -> ScopeInfo:


def task(scope: str) -> ScopeInfo:
return ScopeInfo(scope)
return ScopeInfo(scope, is_goal=True)


def intermediate(scope: str) -> ScopeInfo:
Expand Down
3 changes: 3 additions & 0 deletions src/python/pants/option/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ class ScopeInfo:
removal_version: Optional[str] = None
removal_hint: Optional[str] = None

# command line goal scope flag
is_goal: bool = False

@property
def description(self) -> str:
return cast(str, getattr(self.optionable_cls, "help"))
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/option/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def get_scope_info(cls, subscope=None) -> ScopeInfo:
if subscope is None:
return super().get_scope_info()
else:
return ScopeInfo(cls.subscope(subscope), cls)
return cls.create_scope_info(scope=cls.subscope(subscope), optionable_cls=cls)

def __init__(self, scope: str, options: OptionValueContainer) -> None:
super().__init__()
Expand Down Expand Up @@ -191,9 +191,9 @@ def collect_scope_infos(optionable_cls, scoped_to, removal_version=None, removal
if scoped_to == GLOBAL_SCOPE
else optionable_cls.subscope(scoped_to)
)
scope_info = ScopeInfo(
scope,
optionable_cls,
scope_info = cls.create_scope_info(
scope=scope,
optionable_cls=optionable_cls,
removal_version=removal_version,
removal_hint=removal_hint,
)
Expand Down