diff --git a/pants-plugins/internal_plugins/releases/register.py b/pants-plugins/internal_plugins/releases/register.py index be8a87f6e15..b2c51cfeaa6 100644 --- a/pants-plugins/internal_plugins/releases/register.py +++ b/pants-plugins/internal_plugins/releases/register.py @@ -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 @@ -131,10 +131,9 @@ async def check_default_tools( SessionValues( { OptionsBootstrapper: OptionsBootstrapper( - tuple(), - ("./pants",), - args, - Config(tuple()), + args=args, + env=FrozenDict(), + allow_pantsrc=False, ) } ), diff --git a/src/python/pants/bin/daemon_pants_runner.py b/src/python/pants/bin/daemon_pants_runner.py index 815b2d03b6a..40e1765b275 100644 --- a/src/python/pants/bin/daemon_pants_runner.py +++ b/src/python/pants/bin/daemon_pants_runner.py @@ -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. diff --git a/src/python/pants/bin/local_pants_runner.py b/src/python/pants/bin/local_pants_runner.py index efb52993773..6a5880d569d 100644 --- a/src/python/pants/bin/local_pants_runner.py +++ b/src/python/pants/bin/local_pants_runner.py @@ -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. diff --git a/src/python/pants/bin/pants_runner.py b/src/python/pants/bin/pants_runner.py index 0f24f7e1b28..3cf16efa054 100644 --- a/src/python/pants/bin/pants_runner.py +++ b/src/python/pants/bin/pants_runner.py @@ -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 diff --git a/src/python/pants/help/help_info_extracter_test.py b/src/python/pants/help/help_info_extracter_test.py index 7dbd1580dfe..d198a1220a9 100644 --- a/src/python/pants/help/help_info_extracter_test.py +++ b/src/python/pants/help/help_info_extracter_test.py @@ -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 @@ -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({})) diff --git a/src/python/pants/init/options_initializer.py b/src/python/pants/init/options_initializer.py index 8faf7df7c63..346fb3d4536 100644 --- a/src/python/pants/init/options_initializer.py +++ b/src/python/pants/init/options_initializer.py @@ -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: diff --git a/src/python/pants/option/global_options_test.py b/src/python/pants/option/global_options_test.py index 2ee73c4f4e5..6abc3c842f8 100644 --- a/src/python/pants/option/global_options_test.py +++ b/src/python/pants/option/global_options_test.py @@ -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() diff --git a/src/python/pants/option/options.py b/src/python/pants/option/options.py index 24601ba5013..af3bbe8a7df 100644 --- a/src/python/pants/option/options.py +++ b/src/python/pants/option/options.py @@ -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, @@ -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 @@ -150,12 +150,11 @@ 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, ) @@ -163,6 +162,7 @@ def create( 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( @@ -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, @@ -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 = {} @@ -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() @@ -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, diff --git a/src/python/pants/option/options_bootstrapper.py b/src/python/pants/option/options_bootstrapper.py index cfd3251f0c2..631ebc7493b 100644 --- a/src/python/pants/option/options_bootstrapper.py +++ b/src/python/pants/option/options_bootstrapper.py @@ -3,255 +3,143 @@ from __future__ import annotations -import itertools import os -import warnings from dataclasses import dataclass -from pathlib import Path from typing import TYPE_CHECKING, Iterable, Mapping, Sequence -from pants.base.build_environment import get_buildroot, get_default_pants_config_file, pants_version +from pants.base.build_environment import get_buildroot, pants_version from pants.base.exceptions import BuildConfigurationError from pants.engine.unions import UnionMembership -from pants.option.config import Config -from pants.option.custom_types import ListValueComponent from pants.option.global_options import BootstrapOptions, GlobalOptions from pants.option.option_types import collect_options_info from pants.option.options import Options from pants.option.scope import GLOBAL_SCOPE, ScopeInfo -from pants.option.subsystem import Subsystem -from pants.util.dirutil import read_file +from pants.util.frozendict import FrozenDict from pants.util.memo import memoized_method, memoized_property -from pants.util.ordered_set import FrozenOrderedSet -from pants.util.strutil import ensure_text, softwrap +from pants.util.strutil import softwrap if TYPE_CHECKING: from pants.build_graph.build_configuration import BuildConfiguration +# TODO: Rebrand this. It isn't actually about "bootstrapping" any more (and the term +# "bootstrap options" now means just "options needed to create a Scheduler"). + + @dataclass(frozen=True) class OptionsBootstrapper: - """Holds the result of the first stage of options parsing, and assists with parsing full - options.""" + """Creates Options instances with appropriately registered options.""" - env_tuples: tuple[tuple[str, str], ...] - bootstrap_args: tuple[str, ...] args: tuple[str, ...] - config: Config + env: FrozenDict[str, str] + allow_pantsrc: bool def __repr__(self) -> str: - env = {pair[0]: pair[1] for pair in self.env_tuples} # Bootstrap args are included in `args`. We also drop the first argument, which is the path # to `pants_loader.py`. args = list(self.args[1:]) - return f"OptionsBootstrapper(args={args}, env={env}, config={self.config})" - - @staticmethod - def get_config_file_paths(env, args) -> list[str]: - """Get the location of the config files. - - The locations are specified by the --pants-config-files option. However we need to load the - config in order to process the options. This method special-cases --pants-config-files - in order to solve this chicken-and-egg problem. - - Note that, obviously, it's not possible to set the location of config files in a config file. - Doing so will have no effect. - """ - # This exactly mirrors the logic applied in Option to all regular options. Note that we'll - # also parse --pants-config as a regular option later, but there's no harm in that. In fact, - # it's preferable, so that any code that happens to want to know where we read config from - # can inspect the option. - flag = "--pants-config-files=" - evars = [ - "PANTS_GLOBAL_PANTS_CONFIG_FILES", - "PANTS_PANTS_CONFIG_FILES", - "PANTS_CONFIG_FILES", - ] - - path_list_values = [] - default = get_default_pants_config_file() - if Path(default).is_file(): - path_list_values.append(ListValueComponent.create(default)) - for var in evars: - if var in env: - path_list_values.append(ListValueComponent.create(env[var])) - break - - for arg in args: - # Technically this is very slightly incorrect, as we don't check scope. But it's - # very unlikely that any task or subsystem will have an option named --pants-config-files. - # TODO: Enforce a ban on options with a --pants- prefix outside our global options? - if arg.startswith(flag): - path_list_values.append(ListValueComponent.create(arg[len(flag) :])) - - return ListValueComponent.merge(path_list_values).val - - @staticmethod - def parse_bootstrap_options( - env: Mapping[str, str], args: Sequence[str], config: Config - ) -> Options: - bootstrap_options = Options.create( - env=env, - config=config, - known_scope_infos=[GlobalOptions.get_scope_info()], - args=args, - native_options_config_discovery=False, - ) - - for options_info in collect_options_info(BootstrapOptions): - # Only use of Options.register? - bootstrap_options.register(GLOBAL_SCOPE, *options_info.args, **options_info.kwargs) - - return bootstrap_options + return f"OptionsBootstrapper(args={args}, env={self.env})" @classmethod def create( - cls, env: Mapping[str, str], args: Sequence[str], *, allow_pantsrc: bool + cls, + *, + args: Sequence[str], + env: Mapping[str, str], + allow_pantsrc: bool = True, ) -> OptionsBootstrapper: """Parses the minimum amount of configuration necessary to create an OptionsBootstrapper. - :param env: An environment dictionary. :param args: An args array. + :param env: An environment dictionary. :param allow_pantsrc: True to allow pantsrc files to be used. Unless tests are expecting to consume pantsrc files, they should pass False in order to avoid reading files from absolute paths. Production use-cases should pass True to allow options values to make the decision of whether to respect pantsrc files. """ args = tuple(args) - with warnings.catch_warnings(record=True): - # We can't use pants.engine.fs.FileContent here because it would cause a circular dep. - @dataclass(frozen=True) - class FileContent: - path: str - content: bytes - - def filecontent_for(path: str) -> FileContent: - return FileContent( - ensure_text(path), - read_file(path, binary_mode=True), - ) - - bargs = cls._get_bootstrap_args(args) - - config_file_paths = cls.get_config_file_paths(env=env, args=args) - config_files_products = [filecontent_for(p) for p in config_file_paths] - pre_bootstrap_config = Config.load(config_files_products, env=env) - - initial_bootstrap_options = cls.parse_bootstrap_options( - env, bargs, pre_bootstrap_config - ) - bootstrap_option_values = initial_bootstrap_options.for_global_scope() - - # Now re-read the config, post-bootstrapping. Note the order: First whatever we bootstrapped - # from (typically pants.toml), then config override, then rcfiles. - full_config_sources = pre_bootstrap_config.sources() - if allow_pantsrc and bootstrap_option_values.pantsrc: - rcfiles = [ - os.path.expanduser(str(rcfile)) - for rcfile in bootstrap_option_values.pantsrc_files - ] - existing_rcfiles = [filecontent_for(p) for p in filter(os.path.exists, rcfiles)] - full_config_sources.extend(existing_rcfiles) - - post_bootstrap_config = Config.load( - full_config_sources, - seed_values=bootstrap_option_values.as_dict(), - env=env, - ) - - bargs = cls._get_bootstrap_args(args) - - # We need to set this env var to allow various static help strings to reference the - # right name (via `pants.util.docutil`), and we need to do it as early as possible to - # avoid needing to lazily import code to avoid chicken-and-egg-problems. This is the - # earliest place it makes sense to do so and is generically used by both the local and - # remote pants runners. - os.environ["__PANTS_BIN_NAME"] = munge_bin_name( - bootstrap_option_values.pants_bin_name, get_buildroot() - ) - - # TODO: We really only need the env vars starting with PANTS_, plus any env - # vars used in env.FOO-style interpolation in config files. - # Filtering to just those would allow OptionsBootstrapper to have a less - # unwieldy __str__. - # We used to filter all but PANTS_* (https://github.com/pantsbuild/pants/pull/7312), - # but we can't now because of env interpolation in the native config file parser. - # We can revisit this once the legacy python parser is no more, and we refactor - # the OptionsBootstrapper and/or convert it to Rust. - env_tuples = tuple(sorted(env.items())) - return cls( - env_tuples=env_tuples, - bootstrap_args=bargs, - args=args, - config=post_bootstrap_config, - ) + bootstrap_options = cls._create_bootstrap_options(args, env, allow_pantsrc) + + # We need to set this env var to allow various static help strings to reference the + # right name (via `pants.util.docutil`), and we need to do it as early as possible to + # avoid needing to lazily import code to avoid chicken-and-egg-problems. This is the + # earliest place it makes sense to do so and is generically used by both the local and + # remote pants runners. + os.environ["__PANTS_BIN_NAME"] = munge_bin_name( + bootstrap_options.for_global_scope().pants_bin_name, get_buildroot() + ) - @classmethod - def _get_bootstrap_args(cls, args: Sequence[str]) -> tuple[str, ...]: - # TODO(13244): there is a typing issue with `memoized_classmethod`. - options = GlobalOptions.get_options_flags() # type: ignore[call-arg] + # TODO: We really only need the env vars starting with PANTS_, plus any env + # vars used in env.FOO-style interpolation in config files. + # Filtering to just those would allow OptionsBootstrapper to have a less + # unwieldy __str__. + # We used to filter all but PANTS_* (https://github.com/pantsbuild/pants/pull/7312), + # but we can't now because of env interpolation in the native config file parser. + # We can revisit this once the legacy python parser is no more, and we refactor + # the OptionsBootstrapper and/or convert it to Rust. + return cls( + args=args, + env=FrozenDict(env), + allow_pantsrc=allow_pantsrc, + ) - def is_bootstrap_option(arg: str) -> bool: - components = arg.split("=", 1) - if components[0] in options.flags: - return True - for flag in options.short_flags: - if arg.startswith(flag): - return True - return False + @staticmethod + def _create_bootstrap_options( + args: Sequence[str], env: Mapping[str, str], allow_pantsrc: bool + ) -> Options: + """Create an Options instance containing just the bootstrap options. - # Take just the bootstrap args, so we don't choke on other global-scope args on the cmd line. - # Stop before '--' since args after that are pass-through and may have duplicate names to our - # bootstrap options. - bargs = ("",) + tuple( - filter(is_bootstrap_option, itertools.takewhile(lambda arg: arg != "--", args)) + These are the options needed to create a scheduler. + """ + ret = Options.create( + args=args, + env=env, + config_sources=None, + known_scope_infos=[GlobalOptions.get_scope_info()], + allow_unknown_options=True, + allow_pantsrc=allow_pantsrc, ) - return bargs - - @memoized_property - def env(self) -> dict[str, str]: - return dict(self.env_tuples) + for option_info in collect_options_info(BootstrapOptions): + ret.register(GLOBAL_SCOPE, *option_info.args, **option_info.kwargs) + return ret @memoized_property def bootstrap_options(self) -> Options: - """The post-bootstrap options, computed from the env, args, and fully discovered Config. + """An Options instance containing just the bootstrap options. - Re-computing options after Config has been fully expanded allows us to pick up bootstrap values - (such as backends) from a config override file, for example. - - Because this can be computed from the in-memory representation of these values, it is not part - of the object's identity. + These are the options needed to create a scheduler. """ - return self.parse_bootstrap_options(self.env, self.bootstrap_args, self.config) - - def get_bootstrap_options(self) -> Options: - """Returns an Options instance that only knows about the bootstrap options.""" - return self.bootstrap_options + return self._create_bootstrap_options(self.args, self.env, self.allow_pantsrc) @memoized_method def _full_options( self, - known_scope_infos: FrozenOrderedSet[ScopeInfo], + known_scope_infos: Sequence[ScopeInfo], union_membership: UnionMembership, allow_unknown_options: bool = False, ) -> Options: - bootstrap_option_values = self.get_bootstrap_options().for_global_scope() + extra_specs = [] + for spec_file in self.bootstrap_options.for_global_scope().spec_files: + with open(spec_file) as f: + extra_specs.extend([line for line in [line.strip() for line in f] if line]) + options = Options.create( - self.env, - self.config, - known_scope_infos, args=self.args, - bootstrap_option_values=bootstrap_option_values, + env=self.env, + config_sources=None, + known_scope_infos=known_scope_infos, + extra_specs=extra_specs, allow_unknown_options=allow_unknown_options, - native_options_config_discovery=False, + allow_pantsrc=self.allow_pantsrc, ) - distinct_subsystem_classes: set[type[Subsystem]] = set() + distinct_subsystem_classes = set() for ksi in known_scope_infos: - if not ksi.subsystem_cls or ksi.subsystem_cls in distinct_subsystem_classes: - continue - distinct_subsystem_classes.add(ksi.subsystem_cls) - ksi.subsystem_cls.register_options_on_scope(options, union_membership) + if ksi.subsystem_cls is not None: + if ksi.subsystem_cls in distinct_subsystem_classes: + continue + distinct_subsystem_classes.add(ksi.subsystem_cls) + ksi.subsystem_cls.register_options_on_scope(options, union_membership) return options @@ -261,14 +149,9 @@ def full_options_for_scopes( union_membership: UnionMembership, allow_unknown_options: bool = False, ) -> Options: - """Get the full Options instance bootstrapped by this object for the given known scopes. - - :param known_scope_infos: ScopeInfos for all scopes that may be encountered. - :returns: A bootstrapped Options instance that also carries options for all the supplied known - scopes. - """ + """Get the full Options instance bootstrapped by this object for the given known scopes.""" return self._full_options( - FrozenOrderedSet(sorted(known_scope_infos, key=lambda si: si.scope)), + tuple(sorted(set(known_scope_infos), key=lambda si: si.scope)), union_membership, allow_unknown_options=allow_unknown_options, ) @@ -276,17 +159,6 @@ def full_options_for_scopes( def full_options( self, build_configuration: BuildConfiguration, union_membership: UnionMembership ) -> Options: - global_bootstrap_options = self.get_bootstrap_options().for_global_scope() - if global_bootstrap_options.pants_version != pants_version(): - raise BuildConfigurationError( - softwrap( - f""" - Version mismatch: Requested version was {global_bootstrap_options.pants_version}, - our version is {pants_version()}. - """ - ) - ) - # Parse and register options. known_scope_infos = [ subsystem.get_scope_info() for subsystem in build_configuration.all_subsystems @@ -296,6 +168,17 @@ def full_options( union_membership, allow_unknown_options=build_configuration.allow_unknown_options, ) + + global_options = options.for_global_scope() + if global_options.pants_version != pants_version(): + raise BuildConfigurationError( + softwrap( + f""" + Version mismatch: Requested version was {global_options.pants_version}, + our version is {pants_version()}. + """ + ) + ) GlobalOptions.validate_instance(options.for_global_scope()) return options diff --git a/src/python/pants/option/options_bootstrapper_test.py b/src/python/pants/option/options_bootstrapper_test.py index 1ed4284e123..ccd9ee1c1cf 100644 --- a/src/python/pants/option/options_bootstrapper_test.py +++ b/src/python/pants/option/options_bootstrapper_test.py @@ -39,9 +39,9 @@ def assert_bootstrap_options( fp.write(f"{k} = {repr(v)}\n") fp.close() - args = [*self._config_path(fp.name), *(args or [])] - bootstrapper = OptionsBootstrapper.create(env=env or {}, args=args, allow_pantsrc=False) - vals = bootstrapper.get_bootstrap_options().for_global_scope() + args = ["pants", *self._config_path(fp.name), *(args or [])] + bootstrapper = OptionsBootstrapper.create(args=args, env=env or {}, allow_pantsrc=False) + vals = bootstrapper.bootstrap_options.for_global_scope() vals_dict = {k: getattr(vals, k) for k in expected_entries} assert expected_entries == vals_dict @@ -127,7 +127,7 @@ def test_create_bootstrapped_options(self) -> None: fp.close() args = ["pants", "--pants-workdir=/qux"] + self._config_path(fp.name) bootstrapper = OptionsBootstrapper.create( - env={"PANTS_DISTDIR": "/pear"}, args=args, allow_pantsrc=False + args=args, env={"PANTS_DISTDIR": "/pear"}, allow_pantsrc=False ) opts = bootstrapper.full_options_for_scopes( known_scope_infos=[ @@ -150,7 +150,9 @@ def test_bootstrapped_options_include_all_env(self) -> None: pants_option = "PANTS_DISTDIR" not_a_pants_option = "NON_PANTS_ENV" bootstrapper = OptionsBootstrapper.create( - env={not_a_pants_option: "pear", pants_option: "banana"}, args=[], allow_pantsrc=False + args=["pants"], + env={not_a_pants_option: "pear", pants_option: "banana"}, + allow_pantsrc=False, ) assert pants_option in bootstrapper.env # See https://github.com/pantsbuild/pants/pull/20956 for context. @@ -164,8 +166,8 @@ def test_create_bootstrapped_multiple_pants_config_files(self) -> None: def create_options_bootstrapper(*config_paths: str) -> OptionsBootstrapper: return OptionsBootstrapper.create( + args=["pants", *(f"--pants-config-files={cp}" for cp in config_paths)], env={}, - args=[f"--pants-config-files={cp}" for cp in config_paths], allow_pantsrc=False, ) @@ -230,8 +232,8 @@ def assert_config_read_correctly( def test_options_pantsrc_files(self) -> None: def create_options_bootstrapper(*config_paths: str) -> OptionsBootstrapper: return OptionsBootstrapper.create( + args=["pants", *(f"--pantsrc-files={cp}" for cp in config_paths)], env={}, - args=[f"--pantsrc-files={cp}" for cp in config_paths], allow_pantsrc=True, ) @@ -260,7 +262,7 @@ def create_options_bootstrapper(*config_paths: str) -> OptionsBootstrapper: def test_full_options_caching(self) -> None: with temporary_file_path() as config: args = self._config_path(config) - bootstrapper = OptionsBootstrapper.create(env={}, args=args, allow_pantsrc=False) + bootstrapper = OptionsBootstrapper.create(args=args, env={}, allow_pantsrc=False) opts1 = bootstrapper.full_options_for_scopes( known_scope_infos=[ @@ -303,12 +305,10 @@ def test_full_options_caching(self) -> None: def test_bootstrap_short_options(self) -> None: def parse_options(*args: str) -> OptionValueContainer: - full_args = [*args, *self._config_path(None)] - return ( - OptionsBootstrapper.create(env={}, args=full_args, allow_pantsrc=False) - .get_bootstrap_options() - .for_global_scope() - ) + full_args = ["pants", *args, *self._config_path(None)] + return OptionsBootstrapper.create( + args=full_args, env={}, allow_pantsrc=False + ).bootstrap_options.for_global_scope() # No short options passed - defaults presented. vals = parse_options() @@ -327,11 +327,9 @@ def parse_options(*args: str) -> OptionValueContainer: def test_bootstrap_options_passthrough_dup_ignored(self) -> None: def parse_options(*args: str) -> OptionValueContainer: full_args = [*args, *self._config_path(None)] - return ( - OptionsBootstrapper.create(env={}, args=full_args, allow_pantsrc=False) - .get_bootstrap_options() - .for_global_scope() - ) + return OptionsBootstrapper.create( + args=full_args, env={}, allow_pantsrc=False + ).bootstrap_options.for_global_scope() vals = parse_options("main", "args", "-lwarn", "--", "-lerror") assert LogLevel.WARN == vals.level @@ -339,60 +337,6 @@ def parse_options(*args: str) -> OptionValueContainer: vals = parse_options("main", "args", "--", "-lerror") assert LogLevel.INFO == vals.level - def test_bootstrap_options_explicit_config_path(self) -> None: - def config_path(*args, **env): - return OptionsBootstrapper.get_config_file_paths(env, args) - - assert ["/foo/bar/pants.toml"] == config_path( - "main", "args", "--pants-config-files=['/foo/bar/pants.toml']" - ) - - assert ["/from/env1", "/from/env2"] == config_path( - "main", "args", PANTS_CONFIG_FILES="['/from/env1', '/from/env2']" - ) - - assert ["/from/flag"] == config_path( - "main", - "args", - "-x", - "--pants-config-files=['/from/flag']", - "goal", - "--other-flag", - PANTS_CONFIG_FILES="['/from/env']", - ) - - # Test appending to the default. - assert [f"{get_buildroot()}/pants.toml", "/from/env", "/from/flag"] == config_path( - "main", - "args", - "-x", - "--pants-config-files=+['/from/flag']", - "goal", - "--other-flag", - PANTS_CONFIG_FILES="+['/from/env']", - ) - - # Test replacing the default, then appending. - assert ["/from/env", "/from/flag"] == config_path( - "main", - "args", - "-x", - "--pants-config-files=+['/from/flag']", - "goal", - "--other-flag", - PANTS_CONFIG_FILES="['/from/env']", - ) - - assert ["/from/flag"] == config_path( - "main", - "args", - "-x", - "--pants-config-files=['/from/flag']", - "goal", - "--other-flag", - PANTS_CONFIG_FILES="+['/from/env']", - ) - def test_setting_pants_config_in_config(self, tmp_path: Path) -> None: # Test that setting pants_config in the config file has no effect. @@ -402,9 +346,11 @@ def test_setting_pants_config_in_config(self, tmp_path: Path) -> None: config2.write_text("[DEFAULT]\nlogdir = 'logdir2'\n") ob = OptionsBootstrapper.create( - env={}, args=[f"--pants-config-files=['{config1.as_posix()}']"], allow_pantsrc=False + args=["pants", f"--pants-config-files=['{config1.as_posix()}']"], + env={}, + allow_pantsrc=False, ) - logdir = ob.get_bootstrap_options().for_global_scope().logdir + logdir = ob.bootstrap_options.for_global_scope().logdir assert "logdir1" == logdir @@ -422,3 +368,26 @@ def munge(bin_name: str) -> str: assert munge(os.path.join(build_root, "bin", "pants")) == "./bin/pants" assert munge("/foo/pants") == "pants" assert munge("/foo/bar/pants") == "pants" + + +def test_file_spec_args() -> None: + with temporary_file(binary_mode=False) as tmp: + tmp.write( + dedent( + """ + foo + bar + """ + ) + ) + tmp.flush() + args = ["pants", f"--spec-files={tmp.name}", "compile", "morx:tgt", "fleem:tgt"] + options_bootstrapper = OptionsBootstrapper.create(args=args, env={}, allow_pantsrc=False) + options = options_bootstrapper.full_options_for_scopes( + known_scope_infos=[ + ScopeInfo(""), + ], + union_membership=UnionMembership({}), + ) + sorted_specs = sorted(options.specs) + assert ["bar", "fleem:tgt", "foo", "morx:tgt"] == sorted_specs diff --git a/src/python/pants/option/options_test.py b/src/python/pants/option/options_test.py index f5ed413cf88..3b30da3ceb8 100644 --- a/src/python/pants/option/options_test.py +++ b/src/python/pants/option/options_test.py @@ -12,7 +12,7 @@ from functools import partial from pathlib import Path from textwrap import dedent -from typing import Any, Callable, Dict, cast +from typing import Any, Callable, Dict, List, cast import pytest import toml @@ -22,7 +22,7 @@ from pants.base.build_environment import get_buildroot from pants.base.deprecated import CodeRemovedError, warn_or_error from pants.engine.fs import FileContent -from pants.option.config import Config +from pants.option.config import ConfigSource from pants.option.custom_types import UnsetBool, file_option, shell_str, target_option from pants.option.errors import ( BooleanConversionError, @@ -43,7 +43,6 @@ from pants.option.native_options import parse_dest from pants.option.option_types import OptionInfo, StrOption from pants.option.options import Options -from pants.option.options_bootstrapper import OptionsBootstrapper from pants.option.options_fingerprinter import OptionEncoder from pants.option.ranked_value import Rank, RankedValue from pants.option.scope import GLOBAL_SCOPE, ScopeInfo @@ -79,11 +78,10 @@ def create_options( extra_scope_infos: list[ScopeInfo] | None = None, ) -> Options: options = Options.create( + args=["pants", *(args or ())], env=env or {}, - config=Config.load([FileContent("pants.toml", toml.dumps(config or {}).encode())]), + config_sources=[FileContent("pants.toml", toml.dumps(config or {}).encode())], known_scope_infos=[*(ScopeInfo(scope) for scope in scopes), *(extra_scope_infos or ())], - native_options_config_discovery=False, - args=["pants", *(args or ())], ) register_fn(options) return options @@ -453,13 +451,11 @@ def register(opts: Options) -> None: def _create_config( config: dict[str, dict[str, str]] | None = None, config2: dict[str, dict[str, str]] | None = None, -) -> Config: - return Config.load( - [ - FileContent("test_config.toml", toml.dumps(config or {}).encode()), - FileContent("test_config2.toml", toml.dumps(config2 or {}).encode()), - ] - ) +) -> List[ConfigSource]: + return [ + FileContent("test_config.toml", toml.dumps(config or {}).encode()), + FileContent("test_config2.toml", toml.dumps(config2 or {}).encode()), + ] def _parse( @@ -468,17 +464,15 @@ def _parse( env: dict[str, str] | None = None, config: dict[str, dict[str, Any]] | None = None, config2: dict[str, dict[str, Any]] | None = None, - bootstrap_option_values=None, allow_unknown_options=False, ) -> Options: args = ["pants", *shlex.split(flags)] + options = Options.create( + args=args, env=env or {}, - config=_create_config(config, config2), - native_options_config_discovery=False, + config_sources=_create_config(config, config2), known_scope_infos=_known_scope_infos, - args=args, - bootstrap_option_values=bootstrap_option_values, allow_unknown_options=allow_unknown_options, ) _register(options) @@ -630,7 +624,7 @@ def register_global(*args, **kwargs): def test_env_var_of_type_int() -> None: create_options_object = partial( Options.create, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=_known_scope_infos, args=shlex.split("pants"), ) @@ -1021,7 +1015,7 @@ def assert_error(expected_error, *args, **kwargs): options = Options.create( args=["pants"], env={}, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=[global_scope()], ) options.register(GLOBAL_SCOPE, *args, **kwargs) @@ -1042,10 +1036,10 @@ def assert_error(expected_error, *args, **kwargs): def test_shadowing() -> None: options = Options.create( + args=["pants"], env={}, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=[global_scope(), task("bar"), intermediate("foo"), task("foo.bar")], - args=["pants"], ) options.register("", "--opt1") options.register("foo", "--opt2") @@ -1058,35 +1052,13 @@ def test_is_known_scope() -> None: assert not options.is_known_scope("nonexistent_scope") -def test_file_spec_args() -> None: - with temporary_file(binary_mode=False) as tmp: - tmp.write( - dedent( - """ - foo - bar - """ - ) - ) - tmp.flush() - # Note that we prevent loading a real pants.toml during get_bootstrap_options(). - flags = f'--spec-files={tmp.name} --pants-config-files="[]" compile morx:tgt fleem:tgt' - bootstrapper = OptionsBootstrapper.create( - env={}, args=shlex.split(f"pants {flags}"), allow_pantsrc=False - ) - bootstrap_options = bootstrapper.bootstrap_options.for_global_scope() - options = _parse(flags=flags, bootstrap_option_values=bootstrap_options) - sorted_specs = sorted(options.specs) - assert ["bar", "fleem:tgt", "foo", "morx:tgt"] == sorted_specs - - def test_passthru_args_subsystems_and_goals(): # Test that passthrough args are applied. options = Options.create( + args=["pants", "test", "target", "--", "bar", "--baz", "@dont_fromfile_expand_me"], env={}, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=[global_scope(), task("test"), subsystem("passconsumer")], - args=["pants", "test", "target", "--", "bar", "--baz", "@dont_fromfile_expand_me"], ) options.register("passconsumer", "--passthing", passthrough=True, type=list, member_type=str) @@ -1098,10 +1070,10 @@ def test_passthru_args_subsystems_and_goals(): def test_at_most_one_goal_with_passthru_args(): with pytest.raises(Options.AmbiguousPassthroughError) as exc: Options.create( + args=["pants", "test", "fmt", "target", "--", "bar", "--baz"], env={}, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=[global_scope(), task("test"), task("fmt")], - args=["pants", "test", "fmt", "target", "--", "bar", "--baz"], ) assert ( "Specifying multiple goals (in this case: ['test', 'fmt']) along with passthrough args" @@ -1112,12 +1084,6 @@ def test_at_most_one_goal_with_passthru_args(): def test_passthru_args_not_interpreted(): # Test that passthrough args are not interpreted. options = Options.create( - env={}, - config=_create_config( - {"consumer": {"shlexed": ["from config"], "string": ["from config"]}} - ), - native_options_config_discovery=False, - known_scope_infos=[global_scope(), task("test"), subsystem("consumer")], args=[ "pants", "--consumer-shlexed=a", @@ -1127,6 +1093,11 @@ def test_passthru_args_not_interpreted(): "[bar]", "multi token from passthrough", ], + env={}, + config_sources=_create_config( + {"consumer": {"shlexed": ["from config"], "string": ["from config"]}} + ), + known_scope_infos=[global_scope(), task("test"), subsystem("consumer")], ) options.register("consumer", "--shlexed", passthrough=True, type=list, member_type=shell_str) options.register("consumer", "--string", passthrough=True, type=list, member_type=str) @@ -1166,20 +1137,17 @@ def test_alias() -> None: """ ) - config = Config.load( - [ - FileContent("config0", config0_content.encode()), - FileContent("config1", config1_content.encode()), - FileContent("config2", config2_content.encode()), - ] - ) + config_sources = [ + FileContent("config0", config0_content.encode()), + FileContent("config1", config1_content.encode()), + FileContent("config2", config2_content.encode()), + ] options = Options.create( + args=["pants", "pyupgrade", "green"], env={}, - config=config, - native_options_config_discovery=False, + config_sources=config_sources, known_scope_infos=[], - args=["pants", "pyupgrade", "green"], ) assert ( @@ -1192,11 +1160,10 @@ def test_alias() -> None: ) == options.get_args() options = Options.create( + args=["pants", "shell"], env={}, - config=config, - native_options_config_discovery=False, + config_sources=config_sources, known_scope_infos=[], - args=["pants", "shell"], ) assert ("repl",) == options.get_args() @@ -1208,11 +1175,9 @@ def test_alias_validation() -> None: foo = "fail_on_known_scope" """ ) - config = Config.load( - [ - FileContent("config", config_content.encode()), - ] - ) + config_sources = [ + FileContent("config", config_content.encode()), + ] with pytest.raises( ParseError, @@ -1222,11 +1187,10 @@ def test_alias_validation() -> None: ), ): Options.create( + args=["pants"], env={}, - config=config, - native_options_config_discovery=False, + config_sources=config_sources, known_scope_infos=[ScopeInfo("foo")], - args=["pants"], ) @@ -1606,10 +1570,10 @@ def test_pants_global_with_default() -> None: def test_double_registration() -> None: options = Options.create( + args=shlex.split("pants"), env={}, - config=_create_config(), + config_sources=_create_config(), known_scope_infos=_known_scope_infos, - args=shlex.split("pants"), ) options.register(GLOBAL_SCOPE, "--foo-bar") with pytest.raises(OptionAlreadyRegistered): diff --git a/src/python/pants/option/subsystem_test.py b/src/python/pants/option/subsystem_test.py index 2e52da0251d..b7bfd3aba2d 100644 --- a/src/python/pants/option/subsystem_test.py +++ b/src/python/pants/option/subsystem_test.py @@ -4,7 +4,6 @@ import pytest from pants.engine.unions import UnionMembership -from pants.option.config import Config from pants.option.errors import OptionsError from pants.option.option_types import BoolOption, StrListOption from pants.option.option_value_container import OptionValueContainer @@ -67,11 +66,10 @@ class GoodToGo(Subsystem): options_scope = "good-to-go" options = Options.create( + args=["./pants"], env={}, - config=Config.load([]), + config_sources=[], known_scope_infos=[GoodToGo.get_scope_info()], - args=["./pants"], - bootstrap_option_values=None, ) GoodToGo.register_options_on_scope(options, UnionMembership({})) @@ -89,11 +87,10 @@ class Blender: contents = StrListOption(help="brrrrr") options = Options.create( + args=["./pants"], env={}, - config=Config.load([]), + config_sources=[], known_scope_infos=[Electrical.get_scope_info()], - args=["./pants"], - bootstrap_option_values=None, ) Electrical.register_options_on_scope( options, diff --git a/src/python/pants/pantsd/pants_daemon.py b/src/python/pants/pantsd/pants_daemon.py index 752a63411b7..1ade4226bda 100644 --- a/src/python/pants/pantsd/pants_daemon.py +++ b/src/python/pants/pantsd/pants_daemon.py @@ -232,7 +232,7 @@ def launch_new_pantsd_instance(): """An external entrypoint that spawns a new pantsd instance.""" options_bootstrapper = OptionsBootstrapper.create( - env=os.environ, args=sys.argv, allow_pantsrc=True + args=sys.argv, env=os.environ, allow_pantsrc=True ) daemon = PantsDaemon.create(options_bootstrapper) daemon.run_sync() diff --git a/src/rust/engine/options/src/lib.rs b/src/rust/engine/options/src/lib.rs index 2fe88c3ef90..6fc885d110d 100644 --- a/src/rust/engine/options/src/lib.rs +++ b/src/rust/engine/options/src/lib.rs @@ -41,6 +41,7 @@ mod types; use std::any::Any; use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt::Debug; +use std::fs; use std::hash::Hash; use std::path::Path; use std::sync::Arc; @@ -380,11 +381,25 @@ impl OptionParser { let config_sources = match config_sources { Some(cs) => cs, None => { + // If a pants.toml exists at the build root, use it as the default config file + // if no config files were explicitly specified via --pants-config-files + // (or PANTS_CONFIG_FILES). + // If it doesn't exist, proceed with no config files. We don't need to error + // if no config file exists (we may error later if an option value is not + // provided). + // In regular usage there is always a config file in practice, but there may not + // be in some obscure test scenarios. let default_config_path = path_join(&buildroot_string, "pants.toml"); + let default_config_paths = + if fs::exists(Path::new(&default_config_path)).map_err(|e| e.to_string())? { + vec![default_config_path] + } else { + vec![] + }; let config_paths = parser .parse_string_list( &option_id!("pants", "config", "files"), - vec![default_config_path], + default_config_paths, )? .value; config_paths @@ -748,6 +763,17 @@ impl OptionParser { }) } + // Return the config files used by this parser. Useful for testing config file discovery. + pub fn get_config_file_paths(&self) -> Vec { + let mut ret = vec![]; + for source in self.sources.keys() { + if let Source::Config { ordinal: _, path } = source { + ret.push(path.to_owned()); + } + } + ret + } + pub fn get_args(&self) -> Result, String> { Ok(self.get_args_reader()?.get_args()) } diff --git a/src/rust/engine/options/src/tests.rs b/src/rust/engine/options/src/tests.rs index 7a8033fc4a4..e9c15100120 100644 --- a/src/rust/engine/options/src/tests.rs +++ b/src/rust/engine/options/src/tests.rs @@ -637,3 +637,66 @@ fn test_validate_config() { }, ); } + +#[test] +fn test_config_path_discovery() { + let buildroot = TempDir::new().unwrap(); + File::create(buildroot.path().join("pants.toml")).unwrap(); + + let config_path_buf = buildroot.path().join("pants.other.toml"); + let config_path = format!("{}", config_path_buf.display()); + File::create(config_path_buf).unwrap(); + + // Setting from flag. + assert_eq!( + vec!["pants.other.toml"], + OptionParser::new( + Args::new(vec![format!("--pants-config-files=['{}']", config_path)]), + Env { env: hashmap! {} }, + None, + false, + false, + Some(BuildRoot::find_from(buildroot.path()).unwrap()), + None, + ) + .unwrap() + .get_config_file_paths() + ); + + // Setting to empty and then appending from flags. + assert_eq!( + vec!["pants.other.toml"], + OptionParser::new( + Args::new(vec![ + "--pants-config-files=[]".to_string(), + format!("--pants-config-files={}", config_path) + ]), + Env { env: hashmap! {} }, + None, + false, + false, + Some(BuildRoot::find_from(buildroot.path()).unwrap()), + None, + ) + .unwrap() + .get_config_file_paths() + ); + + // Appending from env var. + assert_eq!( + vec!["pants.toml", "pants.other.toml"], + OptionParser::new( + Args::new([]), + Env { + env: hashmap! {"PANTS_CONFIG_FILES".to_string() => config_path.to_string()} + }, + None, + false, + false, + Some(BuildRoot::find_from(buildroot.path()).unwrap()), + None, + ) + .unwrap() + .get_config_file_paths() + ); +} diff --git a/tests/python/pants_test/init/test_options_initializer.py b/tests/python/pants_test/init/test_options_initializer.py index 964a1202ca0..60efc85d11c 100644 --- a/tests/python/pants_test/init/test_options_initializer.py +++ b/tests/python/pants_test/init/test_options_initializer.py @@ -13,8 +13,8 @@ class OptionsInitializerTest(unittest.TestCase): def test_invalid_version(self) -> None: options_bootstrapper = OptionsBootstrapper.create( - env={}, args=["--backend-packages=[]", "--pants-version=99.99.9999"], + env={}, allow_pantsrc=False, ) @@ -26,8 +26,8 @@ def test_invalid_version(self) -> None: def test_global_options_validation(self) -> None: # Specify an invalid combination of options. ob = OptionsBootstrapper.create( - env={}, args=["--backend-packages=[]", "--no-watch-filesystem", "--loop"], + env={}, allow_pantsrc=False, ) env = CompleteEnvironmentVars({})