Skip to content

Commit

Permalink
Simplify OptionsBootstrapper (#21784)
Browse files Browse the repository at this point in the history
Now that options parsing is done in Rust, much of the logic
in the OptionsBootstrapper was unneeded. This change
removes much of that defunct logic, particularly around
config file discovery, and creation of intermediate 
"bootstrap options".

Unfortunately this logic was still pretty entangled into the
bootstrap sequence, so this change is slightly intricate.
  • Loading branch information
benjyw authored Dec 26, 2024
1 parent a2a0652 commit 9fd0472
Show file tree
Hide file tree
Showing 16 changed files with 301 additions and 416 deletions.
9 changes: 4 additions & 5 deletions pants-plugins/internal_plugins/releases/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from pants.engine.rules import Get, collect_rules, goal_rule, rule
from pants.engine.target import Target
from pants.engine.unions import UnionRule
from pants.option.config import Config
from pants.option.options_bootstrapper import OptionsBootstrapper
from pants.util.frozendict import FrozenDict
from pants.util.strutil import softwrap
from pants.version import VERSION

Expand Down Expand Up @@ -131,10 +131,9 @@ async def check_default_tools(
SessionValues(
{
OptionsBootstrapper: OptionsBootstrapper(
tuple(),
("./pants",),
args,
Config(tuple()),
args=args,
env=FrozenDict(),
allow_pantsrc=False,
)
}
),
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/bin/daemon_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def single_daemonized_run(
start_time = float(env_start_time) if env_start_time else time.time()

options_bootstrapper = OptionsBootstrapper.create(
env=env, args=args, allow_pantsrc=True
args=args, env=env, allow_pantsrc=True
)

# Run using the pre-warmed Session.
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/bin/local_pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def create(

# Verify configs.
if global_bootstrap_options.verify_config:
options.verify_configs(options_bootstrapper.config)
options.verify_configs()

# If we're running with the daemon, we'll be handed a warmed Scheduler, which we use
# to initialize a session here.
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/bin/pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def run(self, start_time: float) -> ExitCode:
self.scrub_pythonpath()

options_bootstrapper = OptionsBootstrapper.create(
env=self.env, args=self.args, allow_pantsrc=True
args=self.args, env=self.env, allow_pantsrc=True
)
with warnings.catch_warnings(record=True):
bootstrap_options = options_bootstrapper.bootstrap_options
Expand Down
7 changes: 2 additions & 5 deletions src/python/pants/help/help_info_extracter_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
from pants.engine.target import IntField, RegisteredTargetTypes, StringField, Target
from pants.engine.unions import UnionMembership
from pants.help.help_info_extracter import HelpInfoExtracter, pretty_print_type_hint, to_help_str
from pants.option.config import Config
from pants.option.global_options import GlobalOptions, LogLevelOption
from pants.option.native_options import NativeOptionParser
from pants.option.option_types import BoolOption, IntListOption, OptionInfo, StrListOption
Expand Down Expand Up @@ -260,12 +259,10 @@ class BazLibrary(Target):
config_path = "pants.test.toml"
config_source = SimpleNamespace(path=config_path, content=b"[GLOBAL]\nopt1 = '+[99]'")
options = Options.create(
args=["./pants", "--backend-packages=['internal_plugins.releases']"],
env={"PANTS_OPT1": "88"},
config=Config.load([config_source]),
native_options_config_discovery=False,
config_sources=[config_source],
known_scope_infos=[Global.get_scope_info(), Foo.get_scope_info(), Bar.get_scope_info()],
args=["./pants", "--backend-packages=['internal_plugins.releases']"],
bootstrap_option_values=None,
include_derivation=True,
)
Global.register_options_on_scope(options, UnionMembership({}))
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/init/options_initializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def _initialize_build_configuration(
2. is expensive to call, because it might resolve plugins from the network
"""

bootstrap_options = options_bootstrapper.get_bootstrap_options().for_global_scope()
bootstrap_options = options_bootstrapper.bootstrap_options.for_global_scope()

# Add any extra paths to python path (e.g., for loading extra source backends).
for path in bootstrap_options.pythonpath:
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/option/global_options_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ def test_invalidation_globs() -> None:
# empty entry (i.e.: a relative path for the current directory) doesn't cause an error.
suffix = "something-ridiculous"
ob = OptionsBootstrapper.create(
env={}, args=[f"--pythonpath=../{suffix}", "--pythonpath="], allow_pantsrc=False
args=[f"--pythonpath=../{suffix}", "--pythonpath="], env={}, allow_pantsrc=False
)
globs = GlobalOptions.compute_pantsd_invalidation_globs(
get_buildroot(), ob.bootstrap_options.for_global_scope()
Expand Down
49 changes: 18 additions & 31 deletions src/python/pants/option/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from pants.base.build_environment import get_buildroot
from pants.base.deprecated import warn_or_error
from pants.option.arg_splitter import ArgSplitter
from pants.option.config import Config
from pants.option.config import ConfigSource
from pants.option.errors import (
ConfigValidationError,
MutuallyExclusiveOptionError,
Expand Down Expand Up @@ -116,26 +116,26 @@ def complete_scopes(cls, scope_infos: Iterable[ScopeInfo]) -> FrozenOrderedSet[S
@classmethod
def create(
cls,
env: Mapping[str, str],
config: Config,
known_scope_infos: Iterable[ScopeInfo],
*,
args: Sequence[str],
bootstrap_option_values: OptionValueContainer | None = None,
env: Mapping[str, str],
config_sources: Sequence[ConfigSource] | None,
known_scope_infos: Sequence[ScopeInfo],
extra_specs: Sequence[str] = tuple(),
allow_unknown_options: bool = False,
native_options_config_discovery: bool = True,
allow_pantsrc: bool = True,
include_derivation: bool = False,
) -> Options:
"""Create an Options instance.
:param args: a list of cmd-line args; defaults to `sys.argv` if None is supplied.
:param env: a dict of environment variables.
:param config: data from a config file.
:param config_sources: sources of config data.
:param known_scope_infos: ScopeInfos for all scopes that may be encountered.
:param args: a list of cmd-line args; defaults to `sys.argv` if None is supplied.
:param bootstrap_option_values: An optional namespace containing the values of bootstrap
options. We can use these values when registering other options.
:param extra_specs: Extra specs to add to those specified in the args (e.g., from --spec-files).
:param allow_unknown_options: Whether to ignore or error on unknown cmd-line flags.
:param native_options_config_discovery: Whether to discover config files in the native
parser or use the ones supplied.
:param allow_pantsrc: Whether to read config from local .rc files. Typically
disabled in tests, for hermeticity.
:param include_derivation: Whether to gather option value derivation information.
"""
# We need registrars for all the intermediate scopes, so inherited option values
Expand All @@ -150,19 +150,19 @@ def create(
scope: registrar.known_scoped_args for scope, registrar in registrar_by_scope.items()
}

config_to_pass = None if native_options_config_discovery else config.sources()
native_parser = NativeOptionParser(
args[1:], # The native parser expects args without the sys.argv[0] binary name.
env,
config_sources=config_to_pass,
allow_pantsrc=True,
config_sources=config_sources,
allow_pantsrc=allow_pantsrc,
include_derivation=include_derivation,
known_scopes_to_flags=known_scope_to_flags,
)

splitter = ArgSplitter(complete_known_scope_infos, get_buildroot())
# We take the cli alias-expanded args[1:] from the native parser.
split_args = splitter.split_args([args[0], *native_parser.get_args()])
split_args.specs.extend(extra_specs)

if split_args.passthru and len(split_args.goals) > 1:
raise cls.AmbiguousPassthroughError(
Expand All @@ -177,15 +177,6 @@ def create(
)
)

if bootstrap_option_values:
spec_files = bootstrap_option_values.spec_files
if spec_files:
for spec_file in spec_files:
with open(spec_file) as f:
split_args.specs.extend(
[line for line in [line.strip() for line in f] if line]
)

return cls(
builtin_or_auxiliary_goal=split_args.builtin_or_auxiliary_goal,
goals=split_args.goals,
Expand Down Expand Up @@ -278,7 +269,7 @@ def known_scope_to_scoped_args(self) -> dict[str, frozenset[str]]:
def scope_to_flags(self) -> dict[str, list[str]]:
return self._scope_to_flags

def verify_configs(self, global_config: Config) -> None:
def verify_configs(self) -> None:
"""Verify all loaded configs have correct scopes and options."""

section_to_valid_options = {}
Expand All @@ -300,7 +291,6 @@ def verify_configs(self, global_config: Config) -> None:
"""
)
)
global_config.verify(section_to_valid_options)

def get_args(self) -> tuple[str, ...]:
return self._native_parser.get_args()
Expand Down Expand Up @@ -459,14 +449,11 @@ def for_scope(
)
)
setattr(builder, dest, RankedValue(rank, val))
native_values = builder.build()

# Check for any deprecation conditions, which are evaluated using `self._flag_matchers`.
if check_deprecations:
native_values_builder = native_values.to_builder()
self._check_and_apply_deprecations(scope, native_values_builder)
native_values = native_values_builder.build()
return native_values
self._check_and_apply_deprecations(scope, builder)
return builder.build()

def get_fingerprintable_for_scope(
self,
Expand Down
Loading

0 comments on commit 9fd0472

Please sign in to comment.