Skip to content

Commit

Permalink
OptionsBootstrapper is provided via a new SessionValues facility rath…
Browse files Browse the repository at this point in the history
…er than a Param (#10827)

### Problem

As described on #10062: any change to options values (including the CLI specs and passthrough args) currently completely invalidate all `@rules` that consume `Subsystem`s, because the "identities" (memoization keys) of the involved `@rules` change. As more heavy lifting has begun to depend on options, this has become more obvious and problematic.

### Solution

As sketched in #10062 (comment), move to providing the `OptionsBootstrapper` (and in future, perhaps much smaller wrappers around the "args" and "env" instead) via a new uncacheable `SessionValues` intrinsic.

More generally, the combination of `Params` for values that _should_ affect the identity of a `@rule`, and `SessionValues` for values that should _not_ affect the identity of a `@rule` seems to be a sufficient solution to the problem described on #6845. The vast majority of values consumed by `@rule`s should be computed from `Params`, so it's possible that the env/args will be the only values we ever provide via `SessionValues`: TBD.

### Result

The case described in #10062 (comment) no longer invalidates the consumers of `Subsystem`s, and in general, only the `Subsystem`s that are affected by an option change should be invalidated in memory (although: #10834).

Fixes #10062 and fixes #6845.

[ci skip-build-wheels]
  • Loading branch information
stuhood authored Sep 22, 2020
1 parent 74eda85 commit f4763dd
Show file tree
Hide file tree
Showing 48 changed files with 252 additions and 177 deletions.
3 changes: 1 addition & 2 deletions pants-plugins/internal_plugins/rules_for_testing/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from pants.engine.console import Console
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.rules import QueryRule, collect_rules, goal_rule
from pants.option.options_bootstrapper import OptionsBootstrapper


class ListAndDieForTestingSubsystem(GoalSubsystem):
Expand All @@ -29,5 +28,5 @@ def rules():
return [
*collect_rules(),
# NB: Would be unused otherwise.
QueryRule(ListAndDieForTestingSubsystem, [OptionsBootstrapper]),
QueryRule(ListAndDieForTestingSubsystem, []),
]
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.core.util_rules.pants_environment import PantsEnvironment
from pants.engine.addresses import Address
from pants.engine.fs import DigestContents
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -26,9 +25,7 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*awslambda_python_rules(),
QueryRule(
CreatedAWSLambda, (PythonAwsLambdaFieldSet, OptionsBootstrapper, PantsEnvironment)
),
QueryRule(CreatedAWSLambda, (PythonAwsLambdaFieldSet, PantsEnvironment)),
],
target_types=[PythonAWSLambda, PythonLibrary],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
HydrateSourcesRequest,
WrappedTarget,
)
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.source.source_root import NoSourceRootError
from pants.testutil.external_tool_test_base import ExternalToolTestBase
from pants.testutil.option_util import create_options_bootstrapper
Expand All @@ -39,8 +38,8 @@ def rules(cls):
*additional_fields.rules(),
*source_files.rules(),
*stripped_source_files.rules(),
QueryRule(HydratedSources, (HydrateSourcesRequest, OptionsBootstrapper)),
QueryRule(GeneratedSources, (GeneratePythonFromProtobufRequest, OptionsBootstrapper)),
QueryRule(HydratedSources, (HydrateSourcesRequest,)),
QueryRule(GeneratedSources, (GeneratePythonFromProtobufRequest,)),
)

def assert_files_generated(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
from pants.backend.python.target_types import PythonLibrary, PythonRequirementLibrary
from pants.core.util_rules import stripped_source_files
from pants.engine.addresses import Address
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.util.frozendict import FrozenDict
Expand Down Expand Up @@ -75,9 +74,9 @@ def rule_runner() -> RuleRunner:
rules=[
*stripped_source_files.rules(),
*module_mapper_rules(),
QueryRule(FirstPartyModuleToAddressMapping, (OptionsBootstrapper,)),
QueryRule(ThirdPartyModuleToAddressMapping, (OptionsBootstrapper,)),
QueryRule(PythonModuleOwner, (PythonModule, OptionsBootstrapper)),
QueryRule(FirstPartyModuleToAddressMapping, ()),
QueryRule(ThirdPartyModuleToAddressMapping, ()),
QueryRule(PythonModuleOwner, (PythonModule,)),
],
target_types=[PythonLibrary, PythonRequirementLibrary],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from pants.engine.addresses import Address
from pants.engine.rules import SubsystemRule
from pants.engine.target import InferredDependencies
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -36,7 +35,7 @@ def test_infer_python_imports() -> None:
*module_mapper.rules(),
infer_python_dependencies,
SubsystemRule(PythonInference),
QueryRule(InferredDependencies, (InferPythonDependencies, OptionsBootstrapper)),
QueryRule(InferredDependencies, (InferPythonDependencies,)),
],
target_types=[PythonLibrary, PythonRequirementLibrary],
)
Expand Down Expand Up @@ -131,7 +130,7 @@ def test_infer_python_inits() -> None:
*ancestor_files.rules(),
infer_python_init_dependencies,
SubsystemRule(PythonInference),
QueryRule(InferredDependencies, (InferInitDependencies, OptionsBootstrapper)),
QueryRule(InferredDependencies, (InferInitDependencies,)),
],
target_types=[PythonLibrary],
)
Expand Down Expand Up @@ -174,7 +173,7 @@ def test_infer_python_conftests() -> None:
*ancestor_files.rules(),
infer_python_conftest_dependencies,
SubsystemRule(PythonInference),
QueryRule(InferredDependencies, (InferConftestDependencies, OptionsBootstrapper)),
QueryRule(InferredDependencies, (InferConftestDependencies,)),
],
target_types=[PythonTests],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import DigestContents, FileContent
from pants.engine.process import InteractiveRunner
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python27_and_python3_present
from pants.testutil.rule_runner import QueryRule, RuleRunner
Expand All @@ -45,10 +44,8 @@ def rule_runner() -> RuleRunner:
*binary.rules(),
*create_python_binary.rules(),
get_filtered_environment,
QueryRule(TestResult, (PythonTestFieldSet, OptionsBootstrapper, PantsEnvironment)),
QueryRule(
TestDebugRequest, (PythonTestFieldSet, OptionsBootstrapper, PantsEnvironment)
),
QueryRule(TestResult, (PythonTestFieldSet, PantsEnvironment)),
QueryRule(TestDebugRequest, (PythonTestFieldSet, PantsEnvironment)),
],
target_types=[PythonBinary, PythonLibrary, PythonTests, PythonRequirementLibrary],
)
Expand Down
11 changes: 5 additions & 6 deletions src/python/pants/backend/python/goals/setup_py_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
from pants.engine.rules import rule
from pants.engine.target import Targets
from pants.engine.unions import UnionRule
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand Down Expand Up @@ -96,7 +95,7 @@ def chroot_rule_runner() -> RuleRunner:
*python_sources.rules(),
setup_kwargs_plugin,
UnionRule(SetupKwargsRequest, PluginSetupKwargsRequest),
QueryRule(SetupPyChroot, (SetupPyChrootRequest, OptionsBootstrapper)),
QueryRule(SetupPyChroot, (SetupPyChrootRequest,)),
]
)

Expand Down Expand Up @@ -261,7 +260,7 @@ def test_get_sources() -> None:
rules=[
get_sources,
*python_sources.rules(),
QueryRule(SetupPySources, (SetupPySourcesRequest, OptionsBootstrapper)),
QueryRule(SetupPySources, (SetupPySourcesRequest,)),
]
)

Expand Down Expand Up @@ -371,7 +370,7 @@ def test_get_requirements() -> None:
get_requirements,
get_owned_dependencies,
get_exporting_owner,
QueryRule(ExportedTargetRequirements, (DependencyOwner, OptionsBootstrapper)),
QueryRule(ExportedTargetRequirements, (DependencyOwner,)),
]
)
rule_runner.add_to_build_file(
Expand Down Expand Up @@ -454,7 +453,7 @@ def test_owned_dependencies() -> None:
rules=[
get_owned_dependencies,
get_exporting_owner,
QueryRule(OwnedDependencies, (DependencyOwner, OptionsBootstrapper)),
QueryRule(OwnedDependencies, (DependencyOwner,)),
]
)
rule_runner.add_to_build_file(
Expand Down Expand Up @@ -543,7 +542,7 @@ def exporting_owner_rule_runner() -> RuleRunner:
return create_setup_py_rule_runner(
rules=[
get_exporting_owner,
QueryRule(ExportedTarget, (OwnedDependency, OptionsBootstrapper)),
QueryRule(ExportedTarget, (OwnedDependency,)),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import DigestContents, FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python27_and_python3_present
from pants.testutil.rule_runner import QueryRule, RuleRunner
Expand All @@ -24,7 +23,7 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*bandit_rules(),
QueryRule(LintResults, (BanditRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (BanditRequest, PantsEnvironment)),
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import CreateDigest, Digest, FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python38_present
from pants.testutil.rule_runner import QueryRule, RuleRunner
Expand All @@ -27,9 +26,9 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*black_rules(),
QueryRule(LintResults, (BlackRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(FmtResult, (BlackRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (BlackRequest, PantsEnvironment)),
QueryRule(FmtResult, (BlackRequest, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, PantsEnvironment)),
],
target_types=[PythonLibrary],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import CreateDigest, Digest, FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -25,9 +24,9 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*docformatter_rules(),
QueryRule(LintResults, (DocformatterRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(FmtResult, (DocformatterRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (DocformatterRequest, PantsEnvironment)),
QueryRule(FmtResult, (DocformatterRequest, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, PantsEnvironment)),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import DigestContents, FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python27_and_python3_present
from pants.testutil.rule_runner import QueryRule, RuleRunner
Expand All @@ -24,7 +23,7 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*flake8_rules(),
QueryRule(LintResults, (Flake8Request, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (Flake8Request, PantsEnvironment)),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import CreateDigest, Digest, FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -25,9 +24,9 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*isort_rules(),
QueryRule(LintResults, (IsortRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(FmtResult, (IsortRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (IsortRequest, PantsEnvironment)),
QueryRule(FmtResult, (IsortRequest, PantsEnvironment)),
QueryRule(SourceFiles, (SourceFilesRequest, PantsEnvironment)),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import FileContent
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python27_and_python3_present
from pants.testutil.rule_runner import QueryRule, RuleRunner
Expand All @@ -27,7 +26,7 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*pylint_rules(),
QueryRule(LintResults, (PylintRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(LintResults, (PylintRequest, PantsEnvironment)),
],
target_types=[PythonLibrary, PythonRequirementLibrary, PylintSourcePlugin],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from pants.engine.addresses import Address
from pants.engine.fs import CreateDigest, Digest, FileContent
from pants.engine.target import Targets
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -26,9 +25,7 @@ def rule_runner() -> RuleRunner:
format_python_target,
*black_rules(),
*isort_rules(),
QueryRule(
LanguageFmtResults, (PythonFmtTargets, OptionsBootstrapper, PantsEnvironment)
),
QueryRule(LanguageFmtResults, (PythonFmtTargets, PantsEnvironment)),
]
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
from pants.base.specs import AddressSpecs, DescendantAddresses, FilesystemSpecs, Specs
from pants.engine.addresses import Address
from pants.engine.target import Targets
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[QueryRule(Targets, (OptionsBootstrapper, Specs))],
rules=[QueryRule(Targets, (Specs,))],
target_types=[PythonRequirementLibrary, PythonRequirementsFile],
context_aware_object_factories={"pipenv_requirements": PipenvRequirements},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,14 @@
from pants.engine.addresses import Address
from pants.engine.internals.scheduler import ExecutionError
from pants.engine.target import Targets
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[QueryRule(Targets, (OptionsBootstrapper, Specs))],
rules=[QueryRule(Targets, (Specs,))],
target_types=[PythonRequirementLibrary, PythonRequirementsFile],
context_aware_object_factories={"python_requirements": PythonRequirements},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from pants.engine.fs import FileContent
from pants.engine.rules import QueryRule
from pants.engine.target import Target
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.python_interpreter_selection import skip_unless_python38_present
from pants.testutil.rule_runner import RuleRunner
Expand All @@ -30,7 +29,7 @@ def rule_runner() -> RuleRunner:
rules=[
*mypy_rules(),
*dependency_inference_rules.rules(), # Used for import inference.
QueryRule(TypecheckResults, (MyPyRequest, OptionsBootstrapper, PantsEnvironment)),
QueryRule(TypecheckResults, (MyPyRequest, PantsEnvironment)),
],
target_types=[PythonLibrary, PythonRequirementLibrary, MyPySourcePlugin],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
identify_missing_ancestor_files,
)
from pants.engine.fs import DigestContents
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.testutil.option_util import create_options_bootstrapper
from pants.testutil.rule_runner import QueryRule, RuleRunner

Expand All @@ -22,7 +21,7 @@ def rule_runner() -> RuleRunner:
return RuleRunner(
rules=[
*ancestor_files.rules(),
QueryRule(AncestorFiles, (AncestorFilesRequest, OptionsBootstrapper)),
QueryRule(AncestorFiles, (AncestorFilesRequest,)),
]
)

Expand Down
Loading

0 comments on commit f4763dd

Please sign in to comment.