Validate that [tailor].build_file_name is compatible with `[GLOBAL]…

….build_patterns` (#13420)

Closes #13419.

For example:

> ./pants tailor --build-file-name=bad
> ...
> ValueError: The option `[tailor].build_file_name` is set to `bad`, which is not compatible with `[GLOBAL].build_patterns`: ['BUILD', 'BUILD.*']. This means that generated BUILD files would be ignored.
>
> To fix, please update the options so that they are compatible.

Note that we don't look at `--build-ignore`. If a user wants to ignore the top-level `BUILD`, that does not imply they generally don't want to use the pattern `BUILD`.

[ci skip-rust]

src/python/pants/core/goals/tailor.py

@@ -31,6 +31,7 @@
     Workspace,
 )
 from pants.engine.goal import Goal, GoalSubsystem
+from pants.engine.internals.build_files import BuildFileOptions
 from pants.engine.internals.selectors import Get, MultiGet
 from pants.engine.rules import collect_rules, goal_rule, rule
 from pants.engine.target import (
@@ -41,6 +42,7 @@
     UnexpandedTargets,
 )
 from pants.engine.unions import UnionMembership, union
+from pants.source.filespec import Filespec, matches_filespec
 from pants.util.docutil import doc_url
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
@@ -252,7 +254,10 @@ def register_options(cls, register):
             advanced=True,
             type=str,
             default="BUILD",
-            help="The name to use for generated BUILD files.",
+            help=(
+                "The name to use for generated BUILD files.\n\n"
+                "This must be compatible with `[GLOBAL].build_patterns`."
+            ),
         )
 
         register(
@@ -297,6 +302,19 @@ def alias_for(self, standard_type: str) -> str | None:
         # This cast suffices to avoid typecheck errors.
         return cast(str, self.options.alias_mapping.get(standard_type))
 
+    def validate_build_file_name(self, build_file_patterns: tuple[str, ...]) -> None:
+        """Check that the specified BUILD file name works with the repository's BUILD file
+        patterns."""
+        filespec = Filespec(includes=list(build_file_patterns))
+        if not bool(matches_filespec(filespec, paths=[self.build_file_name])):
+            raise ValueError(
+                f"The option `[{self.options_scope}].build_file_name` is set to "
+                f"`{self.build_file_name}`, which is not compatible with "
+                f"`[GLOBAL].build_patterns`: {sorted(build_file_patterns)}. This means that "
+                "generated BUILD files would be ignored.\n\n"
+                "To fix, please update the options so that they are compatible."
+            )
+
 
 class Tailor(Goal):
     subsystem_cls = TailorSubsystem
@@ -415,9 +433,6 @@ async def restrict_conflicting_sources(ptgt: PutativeTarget) -> DisjointSourcePu
 @dataclass(frozen=True)
 class EditBuildFilesRequest:
     putative_targets: PutativeTargets
-    name: str
-    header: str | None
-    indent: str
 
 
 @dataclass(frozen=True)
@@ -438,8 +453,12 @@ def make_content_str(
 
 
 @rule(desc="Edit BUILD files with new targets", level=LogLevel.DEBUG)
-async def edit_build_files(req: EditBuildFilesRequest) -> EditedBuildFiles:
-    ptgts_by_build_file = group_by_build_file(req.name, req.putative_targets)
+async def edit_build_files(
+    req: EditBuildFilesRequest, tailor_subsystem: TailorSubsystem
+) -> EditedBuildFiles:
+    ptgts_by_build_file = group_by_build_file(
+        tailor_subsystem.build_file_name, req.putative_targets
+    )
     # There may be an existing *directory* whose name collides with that of a BUILD file
     # we want to create. This is more likely on a system with case-insensitive paths,
     # such as MacOS. We detect such cases and use an alt BUILD file name to fix.
@@ -458,9 +477,13 @@ async def edit_build_files(req: EditBuildFilesRequest) -> EditedBuildFiles:
     def make_content(bf_path: str, pts: Iterable[PutativeTarget]) -> FileContent:
         existing_content_bytes = existing_build_files_contents_by_path.get(bf_path)
         existing_content = (
-            req.header if existing_content_bytes is None else existing_content_bytes.decode()
+            tailor_subsystem.build_file_header
+            if existing_content_bytes is None
+            else existing_content_bytes.decode()
         )
-        new_content_bytes = make_content_str(existing_content, req.indent, pts).encode()
+        new_content_bytes = make_content_str(
+            existing_content, tailor_subsystem.build_file_indent, pts
+        ).encode()
         return FileContent(bf_path, new_content_bytes)
 
     new_digest = await Get(
@@ -509,14 +532,14 @@ async def tailor(
     workspace: Workspace,
     union_membership: UnionMembership,
     specs: Specs,
+    build_file_options: BuildFileOptions,
 ) -> Tailor:
+    tailor_subsystem.validate_build_file_name(build_file_options.patterns)
+
     search_paths = PutativeTargetsSearchPaths(specs_to_dirs(specs))
-    putative_target_request_types: Iterable[type[PutativeTargetsRequest]] = union_membership[
-        PutativeTargetsRequest
-    ]
     putative_targets_results = await MultiGet(
         Get(PutativeTargets, PutativeTargetsRequest, req_type(search_paths))
-        for req_type in putative_target_request_types
+        for req_type in union_membership[PutativeTargetsRequest]
     )
     putative_targets = PutativeTargets.merge(putative_targets_results)
     putative_targets = PutativeTargets(
@@ -531,13 +554,7 @@ async def tailor(
 
     if ptgts:
         edited_build_files = await Get(
-            EditedBuildFiles,
-            EditBuildFilesRequest(
-                PutativeTargets(ptgts),
-                tailor_subsystem.build_file_name,
-                tailor_subsystem.build_file_header,
-                tailor_subsystem.build_file_indent,
-            ),
+            EditedBuildFiles, EditBuildFilesRequest(PutativeTargets(ptgts))
         )
         updated_build_files = set(edited_build_files.updated_paths)
         workspace.write_digest(edited_build_files.digest)

src/python/pants/core/goals/tailor_test.py

@@ -35,10 +35,12 @@
 )
 from pants.core.util_rules import source_files
 from pants.engine.fs import EMPTY_DIGEST, DigestContents, FileContent, Workspace
+from pants.engine.internals.build_files import BuildFileOptions, extract_build_file_options
 from pants.engine.rules import QueryRule, rule
 from pants.engine.target import MultipleSourcesField, Target
 from pants.engine.unions import UnionMembership, UnionRule
 from pants.testutil.option_util import create_goal_subsystem
+from pants.testutil.pytest_util import no_exception
 from pants.testutil.rule_runner import MockGet, RuleRunner, mock_console, run_rule_with_mocks
 
 
@@ -92,6 +94,7 @@ def rule_runner() -> RuleRunner:
         rules=[
             *tailor.rules(),
             *source_files.rules(),
+            extract_build_file_options,
             infer_fortran_module_dependency,
             UnionRule(PutativeTargetsRequest, MockPutativeFortranModuleRequest),
             QueryRule(PutativeTargets, (MockPutativeFortranModuleRequest,)),
@@ -274,9 +277,12 @@ def test_edit_build_files(rule_runner: RuleRunner, name: str) -> None:
                 ),
             ]
         ),
-        name=name,
-        header="Copyright © 2021 FooCorp.",
-        indent="    ",
+    )
+    rule_runner.set_options(
+        [
+            f"--tailor-build-file-name={name}",
+            "--tailor-build-file-header=Copyright © 2021 FooCorp.",
+        ]
     )
     edited_build_files = rule_runner.request(EditedBuildFiles, [req])
 
@@ -339,9 +345,6 @@ def test_edit_build_files_without_header_text(rule_runner: RuleRunner) -> None:
                 ),
             ]
         ),
-        name="BUILD",
-        header=None,
-        indent="    ",
     )
     edited_build_files = rule_runner.request(EditedBuildFiles, [req])
 
@@ -379,10 +382,9 @@ def test_build_file_lacks_leading_whitespace(rule_runner: RuleRunner, header: st
                 ),
             ]
         ),
-        name="BUILD",
-        header=header,
-        indent="    ",
     )
+    if header:
+        rule_runner.set_options([f"--tailor-build-file-header={header}"])
     edited_build_files = rule_runner.request(EditedBuildFiles, [req])
 
     assert edited_build_files.created_paths == ("src/fortran/baz/BUILD.pants",)
@@ -482,6 +484,7 @@ def test_tailor_rule(rule_runner: RuleRunner) -> None:
                 workspace,
                 union_membership,
                 specs,
+                BuildFileOptions(patterns=("BUILD",), ignores=()),
             ],
             mock_gets=[
                 MockGet(
@@ -583,3 +586,21 @@ def test_target_type_with_no_sources_field(rule_runner: RuleRunner) -> None:
         "but this target type does not have a `sources` field."
     )
     assert str(excinfo.value) == expected_msg
+
+
+@pytest.mark.parametrize(
+    "include,file_name,raises",
+    (
+        ("BUILD", "BUILD", False),
+        ("BUILD.*", "BUILD.foo", False),
+        ("BUILD.foo", "BUILD", True),
+    ),
+)
+def test_validate_build_file_name(include: str, file_name: str, raises: bool) -> None:
+    tailor_subsystem = create_goal_subsystem(TailorSubsystem, build_file_name=file_name)
+    if raises:
+        with pytest.raises(ValueError):
+            tailor_subsystem.validate_build_file_name((include,))
+    else:
+        with no_exception():
+            tailor_subsystem.validate_build_file_name((include,))

src/python/pants/option/global_options.py

@@ -1355,9 +1355,11 @@ def register_options(cls, register):
             type=list,
             default=["BUILD", "BUILD.*"],
             help=(
-                "The naming scheme for BUILD files, i.e. where you define targets. This only sets "
-                "the naming scheme, not the directory paths to look for. To add ignore"
-                "patterns, use the option `--build-ignore`."
+                "The naming scheme for BUILD files, i.e. where you define targets.\n\n"
+                "This only sets the naming scheme, not the directory paths to look for. To add "
+                "ignore patterns, use the option `[GLOBAL].build_ignore`.\n\n"
+                "You may also need to update the option `[tailor].build_file_name` so that it is "
+                "compatible with this option."
             ),
         )
         register(
@@ -1366,9 +1368,10 @@ def register_options(cls, register):
             type=list,
             default=[],
             help=(
-                "Paths to ignore when identifying BUILD files. This does not affect any other "
-                "filesystem operations; use `--pants-ignore` for that instead. Patterns use the "
-                "gitignore pattern syntax (https://git-scm.com/docs/gitignore)."
+                "Paths to ignore when identifying BUILD files.\n\n"
+                "This does not affect any other filesystem operations; use `--pants-ignore` for "
+                "that instead.\n\n"
+                "Patterns use the gitignore pattern syntax (https://git-scm.com/docs/gitignore)."
             ),
         )
         register(