Bootstrap scheduler ignores invalid BUILD file symbols when parsing (p…

…antsbuild#16661)

Part of the environment redesign from pantsbuild#7735.

We decided (for now, subject to change) to model each environment's metadata via targets. This creates a chicken-and-egg problem with Pants's bootstrapping with `PluginResolver`. The `PluginResolver` runs a `VenvPex`, and thus needs to access several config files like `[GLOBAL].ca_certs_path` and `[python-bootstrap]`. To do that, we will need to load what `local_environment` targets the user has. That works fine, except that we will not yet have loaded most backends and any plugins, so it's very likely BUILD files will fail to parse due to unrecognized symbols.

The solution is to simply ignore those symbols at the bootstrap stage. As shown by this PR's tests, any problematic symbols will be skipped; the valid targets still work. After the bootstrap stage, those BUILD files will be re-parsed and we will properly error on unrecognized symbols.

The alternative approach I considered is creating a convention that environments should only be defined in specific BUILD files, without other targets. However, that is arbitrary to limit our users, and it would be hard to pull off because our code is based on `AddressFamilyDir`, which looks at all BUILD files in that directory, not only one of them.

This does not yet properly handle `__defaults__`: pantsbuild#16669

[ci skip-rust]
[ci skip-build-wheels]

src/python/pants/engine/internals/build_files_test.py

@@ -47,7 +47,12 @@ def test_parse_address_family_empty() -> None:
     optional_af = run_rule_with_mocks(
         parse_address_family,
         rule_args=[
-            Parser(build_root="", target_type_aliases=[], object_aliases=BuildFileAliases()),
+            Parser(
+                build_root="",
+                target_type_aliases=[],
+                object_aliases=BuildFileAliases(),
+                ignore_unrecognized_symbols=False,
+            ),
             BuildFileOptions(("BUILD",)),
             BuildFilePreludeSymbols(FrozenDict()),
             AddressFamilyDir("/dev/null"),

src/python/pants/engine/internals/mapper_test.py

@@ -8,6 +8,7 @@
 import pytest
 
 from pants.backend.project_info.filter_targets import FilterSubsystem, TargetGranularity
+from pants.base.exceptions import MappingError
 from pants.build_graph.address import Address
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.engine.internals.defaults import BuildFileDefaults, BuildFileDefaultsParserState
@@ -18,17 +19,22 @@
     DuplicateNameError,
     SpecsFilter,
 )
-from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser
+from pants.engine.internals.parser import BuildFilePreludeSymbols, Parser, _unrecognized_symbol_func
 from pants.engine.internals.target_adaptor import TargetAdaptor
 from pants.engine.target import RegisteredTargetTypes, Tags, Target
 from pants.engine.unions import UnionMembership
 from pants.testutil.option_util import create_goal_subsystem
 from pants.util.frozendict import FrozenDict
 
 
-def parse_address_map(build_file: str) -> AddressMap:
+def parse_address_map(build_file: str, *, ignore_unrecognized_symbols: bool = False) -> AddressMap:
     path = "/dev/null"
-    parser = Parser(build_root="", target_type_aliases=["thing"], object_aliases=BuildFileAliases())
+    parser = Parser(
+        build_root="",
+        target_type_aliases=["thing"],
+        object_aliases=BuildFileAliases(),
+        ignore_unrecognized_symbols=ignore_unrecognized_symbols,
+    )
     address_map = AddressMap.parse(
         path,
         build_file,
@@ -64,6 +70,32 @@ def test_address_map_parse() -> None:
     } == address_map.name_to_target_adaptor
 
 
+def test_address_map_unrecognized_symbol() -> None:
+    build_file = dedent(
+        """
+        thing(name="one")
+        thing(name="bad", age=fake)
+        thing(name="two")
+        another_fake()
+        yet_another()
+        thing(name="three")
+        and_finally(age=fakey)
+        """
+    )
+    address_map = parse_address_map(build_file, ignore_unrecognized_symbols=True)
+    assert {
+        "one": TargetAdaptor(type_alias="thing", name="one"),
+        "bad": TargetAdaptor(type_alias="thing", name="bad", age=_unrecognized_symbol_func),
+        "two": TargetAdaptor(type_alias="thing", name="two"),
+        "three": TargetAdaptor(
+            type_alias="thing",
+            name="three",
+        ),
+    } == address_map.name_to_target_adaptor
+    with pytest.raises(MappingError):
+        parse_address_map(build_file, ignore_unrecognized_symbols=False)
+
+
 def test_address_map_duplicate_names() -> None:
     with pytest.raises(DuplicateNameError):
         parse_address_map("thing(name='one')\nthing(name='one')")

src/python/pants/engine/internals/parser.py

@@ -4,6 +4,7 @@
 from __future__ import annotations
 
 import os.path
+import re
 import threading
 import tokenize
 from dataclasses import dataclass
@@ -19,6 +20,7 @@
 from pants.engine.internals.target_adaptor import TargetAdaptor
 from pants.util.docutil import doc_url
 from pants.util.frozendict import FrozenDict
+from pants.util.strutil import softwrap
 
 
 @dataclass(frozen=True)
@@ -81,10 +83,12 @@ def __init__(
         build_root: str,
         target_type_aliases: Iterable[str],
         object_aliases: BuildFileAliases,
+        ignore_unrecognized_symbols: bool,
     ) -> None:
         self._symbols, self._parse_state = self._generate_symbols(
             build_root, target_type_aliases, object_aliases
         )
+        self.ignore_unrecognized_symbols = ignore_unrecognized_symbols
 
     @staticmethod
     def _generate_symbols(
@@ -163,15 +167,30 @@ def parse(
                 v.__globals__.update(global_symbols)
             global_symbols[k] = v
 
+        if self.ignore_unrecognized_symbols:
+            while True:
+                try:
+                    exec(build_file_content, global_symbols)
+                except NameError as e:
+                    bad_symbol = _extract_symbol_from_name_error(e)
+                    global_symbols[bad_symbol] = _unrecognized_symbol_func
+                    self._parse_state.reset(rel_path=os.path.dirname(filepath), defaults=defaults)
+                    continue
+                break
+
+            error_on_imports(build_file_content, filepath)
+            return self._parse_state.parsed_targets()
+
         try:
             exec(build_file_content, global_symbols)
         except NameError as e:
             valid_symbols = sorted(s for s in global_symbols.keys() if s != "__builtins__")
             original = e.args[0].capitalize()
-            help_str = (
-                "If you expect to see more symbols activated in the below list,"
-                f" refer to {doc_url('enabling-backends')} for all available"
-                " backends to activate."
+            help_str = softwrap(
+                f"""
+                If you expect to see more symbols activated in the below list, refer to
+                {doc_url('enabling-backends')} for all available backends to activate.
+                """
             )
 
             candidates = get_close_matches(build_file_content, valid_symbols)
@@ -188,7 +207,6 @@ def parse(
             )
 
         error_on_imports(build_file_content, filepath)
-
         return self._parse_state.parsed_targets()
 
 
@@ -210,3 +228,23 @@ def error_on_imports(build_file_content: str, filepath: str) -> None:
             f"\n\nInstead, consider writing a macro ({doc_url('macros')}) or "
             f"writing a plugin ({doc_url('plugins-overview')}."
         )
+
+
+def _extract_symbol_from_name_error(err: NameError) -> str:
+    result = re.match(r"^name '(\w*)'", err.args[0])
+    if result is None:
+        raise AssertionError(
+            softwrap(
+                f"""
+                Failed to extract symbol from NameError: {err}
+
+                Please open a bug at https://github.com/pantsbuild/pants/issues/new/choose
+                """
+            )
+        )
+    return result.group(1)
+
+
+def _unrecognized_symbol_func(**kwargs):
+    """Allows us to not choke on unrecognized symbols, including when they're called as
+    functions."""

src/python/pants/engine/internals/parser_test.py

@@ -7,9 +7,15 @@
 
 from pants.build_graph.build_file_aliases import BuildFileAliases
 from pants.engine.internals.defaults import BuildFileDefaults, BuildFileDefaultsParserState
-from pants.engine.internals.parser import BuildFilePreludeSymbols, ParseError, Parser
+from pants.engine.internals.parser import (
+    BuildFilePreludeSymbols,
+    ParseError,
+    Parser,
+    _extract_symbol_from_name_error,
+)
 from pants.engine.target import RegisteredTargetTypes
 from pants.engine.unions import UnionMembership
+from pants.testutil.pytest_util import no_exception
 from pants.util.docutil import doc_url
 from pants.util.frozendict import FrozenDict
 
@@ -22,7 +28,12 @@ def defaults_parser_state() -> BuildFileDefaultsParserState:
 
 
 def test_imports_banned(defaults_parser_state: BuildFileDefaultsParserState) -> None:
-    parser = Parser(build_root="", target_type_aliases=[], object_aliases=BuildFileAliases())
+    parser = Parser(
+        build_root="",
+        target_type_aliases=[],
+        object_aliases=BuildFileAliases(),
+        ignore_unrecognized_symbols=False,
+    )
     with pytest.raises(ParseError) as exc:
         parser.parse(
             "dir/BUILD",
@@ -34,15 +45,17 @@ def test_imports_banned(defaults_parser_state: BuildFileDefaultsParserState) ->
 
 
 def test_unrecognized_symbol(defaults_parser_state: BuildFileDefaultsParserState) -> None:
-    def perform_test(extra_targets: list[str], dym: str) -> None:
+    build_file_aliases = BuildFileAliases(
+        objects={"obj": 0},
+        context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
+    )
 
+    def perform_test(extra_targets: list[str], dym: str) -> None:
         parser = Parser(
             build_root="",
             target_type_aliases=["tgt", *extra_targets],
-            object_aliases=BuildFileAliases(
-                objects={"obj": 0},
-                context_aware_object_factories={"caof": lambda parse_context: lambda _: None},
-            ),
+            object_aliases=build_file_aliases,
+            ignore_unrecognized_symbols=False,
         )
         prelude_symbols = BuildFilePreludeSymbols(FrozenDict({"prelude": 0}))
         fmt_extra_sym = str(extra_targets)[1:-1] + (", ") if len(extra_targets) != 0 else ""
@@ -62,6 +75,20 @@ def perform_test(extra_targets: list[str], dym: str) -> None:
             "'obj', 'prelude', 'tgt']"
         )
 
+        with no_exception():
+            parser = Parser(
+                build_root="",
+                target_type_aliases=["tgt", *extra_targets],
+                object_aliases=build_file_aliases,
+                ignore_unrecognized_symbols=True,
+            )
+            parser.parse(
+                "dir/BUILD",
+                "fake",
+                prelude_symbols,
+                defaults_parser_state,
+            )
+
     test_targs = ["fake1", "fake2", "fake3", "fake4", "fake5"]
 
     perform_test([], "")
@@ -71,3 +98,8 @@ def perform_test(extra_targets: list[str], dym: str) -> None:
     perform_test(test_targs[:2], dym_two)
     dym_many = "Did you mean fake5, fake4, or fake3?\n\n"
     perform_test(test_targs, dym_many)
+
+
+@pytest.mark.parametrize("symbol", ["a", "bad", "BAD", "a___b_c", "a231", "áç"])
+def test_extract_symbol_from_name_error(symbol: str) -> None:
+    assert _extract_symbol_from_name_error(NameError(f"name '{symbol}' is not defined")) == symbol

src/python/pants/init/engine_initializer.py

@@ -178,6 +178,7 @@ def setup_graph(
         build_configuration: BuildConfiguration,
         dynamic_remote_options: DynamicRemoteOptions,
         executor: PyExecutor | None = None,
+        ignore_unrecognized_build_file_symbols: bool = False,
     ) -> GraphScheduler:
         build_root = get_buildroot()
         executor = executor or GlobalOptions.create_py_executor(bootstrap_options)
@@ -197,6 +198,7 @@ def setup_graph(
             include_trace_on_error=bootstrap_options.print_stacktrace,
             engine_visualize_to=bootstrap_options.engine_visualize_to,
             watch_filesystem=bootstrap_options.watch_filesystem,
+            ignore_unrecognized_build_file_symbols=ignore_unrecognized_build_file_symbols,
         )
 
     @staticmethod
@@ -215,6 +217,7 @@ def setup_graph_extended(
         include_trace_on_error: bool = True,
         engine_visualize_to: str | None = None,
         watch_filesystem: bool = True,
+        ignore_unrecognized_build_file_symbols: bool = False,
     ) -> GraphScheduler:
         build_root_path = build_root or get_buildroot()
 
@@ -230,6 +233,7 @@ def parser_singleton() -> Parser:
                 build_root=build_root_path,
                 target_type_aliases=registered_target_types.aliases,
                 object_aliases=build_configuration.registered_aliases,
+                ignore_unrecognized_symbols=ignore_unrecognized_build_file_symbols,
             )
 
         @rule

src/python/pants/init/options_initializer.py

@@ -76,6 +76,7 @@ def create_bootstrap_scheduler(
             bc_builder.create(),
             DynamicRemoteOptions.disabled(),
             executor,
+            ignore_unrecognized_build_file_symbols=True,
         ).scheduler
     )