Support adding to dict-valued options in config files. (pantsbuild#16481

)

Previously you could use the "+" prefix on a stringified dict to add to the value across different sources (flag, env var, config, default), but this did not work across multiple config files. Instead, the last config file would stomp the values in previous ones.

This change:

- Adds support for "+" across config files.
- Also supports using the ".add" affordance, similar to list-valued options, so that the value can be a toml map instead of a stringified python dict. Note that, unlike with lists, "add" could actually be a valid dict key, in which case we will do the wrong thing. In that case, the escape hatch is to use "+" with a stringified value. 
- Ensures that the above works for `cli.alias`, which we special-case in the options bootstrapper.
- Streamlines some of the config machinery. In particular, simplifies how we do interpolation. Instead of interpolating individual values in lists and then manually stringifying them, we interpolate after stringification. This is safe in practice, as for `%(name)s` to span two list items, `name` would have to contain forbidden characters like a quote or double-quote.
- As a byproduct of the above, adds support for interpolation inside dicts, following the same post-stringification principle.

Fixes pantsbuild#15922 

[ci skip-rust]

[ci skip-build-wheels]

src/python/pants/backend/docker/registries.py

@@ -43,7 +43,7 @@ def from_dict(cls, alias: str, d: dict[str, Any]) -> DockerRegistryOptions:
             extra_image_tags=tuple(
                 d.get("extra_image_tags", DockerRegistryOptions.extra_image_tags)
             ),
-            repository=Parser.to_value_type(d.get("repository"), str, None, None),
+            repository=Parser.to_value_type(d.get("repository"), str, None),
         )
 
     def register(self, registries: dict[str, DockerRegistryOptions]) -> None:

src/python/pants/engine/target.py

@@ -2450,10 +2450,9 @@ class Dependencies(StringSequenceField, AsyncFieldMixin):
         `{bin_name()} dependencies` or `{bin_name()} peek` on this target to get the final
         result.
 
-        See {doc_url('targets#target-addresses')} and {doc_url('targets#target-generation')} for
-        more about how addresses are formed, including for generated targets. You can also run
-        `{bin_name()} list ::` to find all addresses in your project, or
-        `{bin_name()} list dir:` to find all addresses defined in that directory.
+        See {doc_url('targets')} for more about how addresses are formed, including for generated
+        targets. You can also run `{bin_name()} list ::` to find all addresses in your project, or
+        `{bin_name()} list dir` to find all addresses defined in that directory.
 
         If the target is in the same BUILD file, you can leave off the BUILD file path, e.g.
         `:tgt` instead of `helloworld/subdir:tgt`. For generated first-party addresses, use

src/python/pants/option/config.py

@@ -8,7 +8,6 @@
 import os
 import re
 from dataclasses import dataclass
-from functools import partial
 from types import SimpleNamespace
 from typing import Any, Dict, Iterable, List, Mapping, Union, cast
 
@@ -23,7 +22,7 @@
 
 
 # A dict with optional override seed values for buildroot, pants_workdir, and pants_distdir.
-SeedValues = Dict[str, Value]
+SeedValues = Mapping[str, Value]
 
 
 class ConfigSource(Protocol):
@@ -163,7 +162,7 @@ def get_sources_for_option(self, section: str, option: str) -> list[str]:
 
 
 _TomlPrimitive = Union[bool, int, float, str]
-_TomlValue = Union[_TomlPrimitive, List[_TomlPrimitive]]
+_TomlValue = Union[_TomlPrimitive, List[_TomlPrimitive], Dict[str, _TomlPrimitive]]
 
 
 @dataclass(frozen=True)
@@ -173,16 +172,6 @@ class _ConfigValues:
     path: str
     section_to_values: dict[str, dict[str, Any]]
 
-    @staticmethod
-    def _is_an_option(option_value: _TomlValue | dict) -> bool:
-        """Determine if the value is actually an option belonging to that section.
-
-        This handles the special syntax of `my_list_option.add` and `my_list_option.remove`.
-        """
-        if isinstance(option_value, dict):
-            return "add" in option_value or "remove" in option_value
-        return True
-
     def _possibly_interpolate_value(
         self,
         raw_value: str,
@@ -193,10 +182,10 @@ def _possibly_interpolate_value(
     ) -> str:
         """For any values with %(foo)s, substitute it with the corresponding value from DEFAULT or
         the same section."""
+        # TODO(benjy): I wonder if we can abuse ConfigParser to do this for us?
 
         def format_str(value: str) -> str:
-            # Because dictionaries use the symbols `{}`, we must proactively escape the symbols so
-            # that .format() does not try to improperly interpolate.
+            # Escape embedded { and } characters, so that .format() does not act on them.
             escaped_str = value.replace("{", "{{").replace("}", "}}")
             new_style_format_str = re.sub(
                 pattern=r"%\((?P<interpolated>[a-zA-Z_0-9.]+)\)s",
@@ -224,82 +213,46 @@ def recursively_format_str(value: str) -> str:
 
         return recursively_format_str(raw_value)
 
-    def _stringify_val(
-        self,
-        raw_value: _TomlValue,
-        *,
-        option: str,
-        section: str,
-        section_values: dict,
-        interpolate: bool = True,
-        list_prefix: str | None = None,
-    ) -> str:
-        # We convert all values to strings, which allows us to treat them uniformly with
-        # env vars and cmd-line flags in parser.py.
-        possibly_interpolate = partial(
-            self._possibly_interpolate_value,
-            option=option,
-            section=section,
-            section_values=section_values,
-        )
-        if isinstance(raw_value, str):
-            return possibly_interpolate(raw_value) if interpolate else raw_value
-
-        if isinstance(raw_value, list):
-
-            def stringify_list_member(member: _TomlPrimitive) -> str:
-                if not isinstance(member, str):
-                    return str(member)
-                interpolated_member = possibly_interpolate(member) if interpolate else member
-                return f'"{interpolated_member}"'
-
-            list_members = ", ".join(stringify_list_member(member) for member in raw_value)
-            return f"{list_prefix or ''}[{list_members}]"
-
-        return str(raw_value)
-
-    def _stringify_val_without_interpolation(self, raw_value: _TomlValue) -> str:
-        return self._stringify_val(
-            raw_value,
-            option="",
-            section="",
-            section_values={},
-            interpolate=False,
-        )
-
     def get_value(self, section: str, option: str) -> str | None:
         section_values = self.section_to_values.get(section)
         if section_values is None:
             return None
         if option not in section_values:
             return None
 
-        stringify = partial(
-            self._stringify_val,
-            option=option,
-            section=section,
-            section_values=section_values,
-        )
+        def stringify(raw_val: _TomlValue, prefix: str = "") -> str:
+            string_val = self._possibly_interpolate_value(
+                raw_value=str(raw_val),
+                option=option,
+                section=section,
+                section_values=section_values or {},
+            )
+            return f"{prefix}{string_val}"
 
         option_value = section_values[option]
         if not isinstance(option_value, dict):
             return stringify(option_value)
 
-        # Handle dict options, along with the special `my_list_option.add` and
-        # `my_list_option.remove` syntax. We only treat `add` and `remove` as the special list
-        # syntax if the values are lists to reduce the risk of incorrectly special casing.
-        has_add = isinstance(option_value.get("add"), list)
-        has_remove = isinstance(option_value.get("remove"), list)
-        if not has_add and not has_remove:
-            return stringify(option_value)
-
-        add_val = stringify(option_value["add"], list_prefix="+") if has_add else "[]"
-        remove_val = stringify(option_value["remove"], list_prefix="-") if has_remove else "[]"
-        if has_add and has_remove:
-            return f"{add_val},{remove_val}"
-        if has_add:
-            return add_val
-        return remove_val
+        # Handle dict options, along with the special `my_dict_option.add`, `my_list_option.add` and
+        # `my_list_option.remove` syntax. We only treat `add` and `remove` as the special syntax
+        # if the values have the approprate type, to reduce the risk of incorrectly special casing.
+        has_add_dict = isinstance(option_value.get("add"), dict)
+        has_add_list = isinstance(option_value.get("add"), list)
+        has_remove_list = isinstance(option_value.get("remove"), list)
+
+        if has_add_dict:
+            return stringify(option_value["add"], prefix="+")
+
+        if has_add_list or has_remove_list:
+            add_val = stringify(option_value["add"], prefix="+") if has_add_list else "[]"
+            remove_val = stringify(option_value["remove"], prefix="-") if has_remove_list else "[]"
+            if has_add_list and has_remove_list:
+                return f"{add_val},{remove_val}"
+            if has_add_list:
+                return add_val
+            return remove_val
+
+        return stringify(option_value)
 
     @property
     def defaults(self) -> dict[str, Any]: