Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Two enhancements to tailor. #11625

Merged
merged 2 commits into from Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/python/pants/core/goals/tailor.py
Expand Up @@ -268,6 +268,10 @@ async def rename_conflicting_targets(ptgts: PutativeTargets) -> UniquelyNamedPut
for ptgt in ptgts:
idx = 0
possibly_renamed_ptgt = ptgt
# Targets in root-level BUILD files must be named explicitly.
if possibly_renamed_ptgt.path == "" and possibly_renamed_ptgt.kwargs.get("name") is None:
possibly_renamed_ptgt = possibly_renamed_ptgt.rename("root")
# Eliminate any address collisions.
while possibly_renamed_ptgt.address.spec in existing_addrs:
possibly_renamed_ptgt = ptgt.rename(f"{ptgt.name}{idx}")
idx += 1
Expand Down Expand Up @@ -342,6 +346,16 @@ def make_content_str(
@rule(desc="Edit BUILD files with new targets")
async def edit_build_files(req: EditBuildFilesRequest) -> EditedBuildFiles:
ptgts_by_build_file = group_by_build_file(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.
existing_paths = await Get(Paths, PathGlobs(ptgts_by_build_file.keys()))
existing_dirs = set(existing_paths.dirs)
# Technically there could be a dir named "BUILD.pants" as well, but that's pretty unlikely.
ptgts_by_build_file = {
(f"{bf}.pants" if bf in existing_dirs else bf): pts
for bf, pts in ptgts_by_build_file.items()
}
existing_build_files_contents = await Get(DigestContents, PathGlobs(ptgts_by_build_file.keys()))
existing_build_files_contents_by_path = {
ebfc.path: ebfc.content for ebfc in existing_build_files_contents
Expand Down
27 changes: 25 additions & 2 deletions src/python/pants/core/goals/tailor_test.py
Expand Up @@ -141,6 +141,28 @@ def test_rename_conflicting_targets(rule_runner: RuleRunner) -> None:
)


def test_root_targets_are_explicitly_named(rule_runner: RuleRunner) -> None:
rule_runner.create_file("foo.f90", "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to waste CI time fixing this, but FYI the content takes a default arg and you can leave that off.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to be explicit anyway.

ptgt = PutativeTarget("", "", "fortran_library", ["foo.f90"], FortranLibrarySources.default)
unpts = rule_runner.request(UniquelyNamedPutativeTargets, [PutativeTargets([ptgt])])
ptgts = unpts.putative_targets
assert (
PutativeTargets(
[
PutativeTarget(
"",
"root",
"fortran_library",
["foo.f90"],
FortranLibrarySources.default,
kwargs={"name": "root"},
)
]
)
== ptgts
)


def test_restrict_conflicting_sources(rule_runner: RuleRunner) -> None:
dir_structure = {
"src/fortran/foo/BUILD": "fortran_library(sources=['bar/baz1.f90'])",
Expand Down Expand Up @@ -173,6 +195,7 @@ def test_restrict_conflicting_sources(rule_runner: RuleRunner) -> None:

def test_edit_build_files(rule_runner: RuleRunner) -> None:
rule_runner.create_file("src/fortran/foo/BUILD", 'fortran_library(sources=["bar1.f90"])')
rule_runner.create_dir("src/fortran/baz/BUILD") # NB: A directory, not a file.
req = EditBuildFilesRequest(
PutativeTargets(
[
Expand Down Expand Up @@ -200,12 +223,12 @@ def test_edit_build_files(rule_runner: RuleRunner) -> None:
)
edited_build_files = rule_runner.request(EditedBuildFiles, [req])

assert edited_build_files.created_paths == ("src/fortran/baz/BUILD",)
assert edited_build_files.created_paths == ("src/fortran/baz/BUILD.pants",)
assert edited_build_files.updated_paths == ("src/fortran/foo/BUILD",)

contents = rule_runner.request(DigestContents, [edited_build_files.digest])
expected = [
FileContent("src/fortran/baz/BUILD", "fortran_library()\n".encode()),
FileContent("src/fortran/baz/BUILD.pants", "fortran_library()\n".encode()),
FileContent(
"src/fortran/foo/BUILD",
textwrap.dedent(
Expand Down