Skip to content

Commit

Permalink
tailor for Go does not add targets when no ancestor go.mod (#15750)
Browse files Browse the repository at this point in the history
Closes #15735.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Jun 6, 2022
1 parent 33b7b2e commit fb82ff8
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 17 deletions.
5 changes: 0 additions & 5 deletions pants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,6 @@ ignore_paths = [
]
ignore_adding_targets = [
"src/python/pants:__main__",
"src/python/pants/backend/go/go_sources:bin",
"src/python/pants/backend/go/go_sources:go_sources0",
"src/python/pants/backend/go/go_sources/analyze_package:bin",
"src/python/pants/backend/go/go_sources/embedcfg:bin",
"src/python/pants/backend/go/go_sources/generate_testmain:bin",
"src/python/pants/backend/docker/subsystems:dockerfile_wrapper_script",
"src/python/pants/backend/python/dependency_inference/scripts:dependency_parser0",
"src/python/pants/backend/terraform:hcl2_parser0",
Expand Down
31 changes: 22 additions & 9 deletions src/python/pants/backend/go/goals/tailor.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,24 @@ def has_package_main(content: bytes) -> bool:
return _package_main_re.search(content) is not None


def has_go_mod_ancestor(dirname: str, all_go_mod_dirs: set[str]) -> bool:
"""We shouldn't add package targets if there is no `go.mod`, as it will cause an error."""
return any(dirname.startswith(go_mod_dir) for go_mod_dir in all_go_mod_dirs)


@rule(level=LogLevel.DEBUG, desc="Determine candidate Go targets to create")
async def find_putative_go_targets(
request: PutativeGoTargetsRequest,
all_owned_sources: AllOwnedSources,
golang_subsystem: GolangSubsystem,
) -> PutativeTargets:
putative_targets = []
_all_go_mod_paths = await Get(Paths, PathGlobs, request.path_globs("go.mod"))
all_go_mod_files = set(_all_go_mod_paths.files)
all_go_mod_dirs = {os.path.dirname(fp) for fp in all_go_mod_files}

if golang_subsystem.tailor_go_mod_targets:
all_go_mod_files = await Get(Paths, PathGlobs, request.path_globs("go.mod"))
unowned_go_mod_files = set(all_go_mod_files.files) - set(all_owned_sources)
unowned_go_mod_files = all_go_mod_files - set(all_owned_sources)
for dirname, filenames in group_by_dir(unowned_go_mod_files).items():
putative_targets.append(
PutativeTarget.for_target_type(
Expand All @@ -70,11 +77,13 @@ async def find_putative_go_targets(
unowned_go_files = set(all_go_files.files) - set(all_owned_sources)
for dirname, filenames in group_by_dir(unowned_go_files).items():
# Ignore paths that have `testdata` or `vendor` in them.
# From `go help packages`: Note, however, that a directory named vendor that itself contains code
# is not a vendored package: cmd/vendor would be a command named vendor.
# From `go help packages`: Note, however, that a directory named vendor that itself
# contains code is not a vendored package: cmd/vendor would be a command named vendor.
dirname_parts = PurePath(dirname).parts
if "testdata" in dirname_parts or "vendor" in dirname_parts[0:-1]:
continue
if not has_go_mod_ancestor(dirname, all_go_mod_dirs):
continue
putative_targets.append(
PutativeTarget.for_target_type(
GoPackageTarget,
Expand All @@ -88,11 +97,15 @@ async def find_putative_go_targets(
all_go_files_digest_contents = await Get(
DigestContents, PathGlobs, request.path_globs("*.go")
)
main_package_dirs = [
os.path.dirname(file_content.path)
for file_content in all_go_files_digest_contents
if has_package_main(file_content.content)
]

main_package_dirs = []
for file_content in all_go_files_digest_contents:
dirname = os.path.dirname(file_content.path)
if has_package_main(file_content.content) and has_go_mod_ancestor(
dirname, all_go_mod_dirs
):
main_package_dirs.append(dirname)

existing_targets = await Get(
UnexpandedTargets,
RawSpecs(
Expand Down
38 changes: 35 additions & 3 deletions src/python/pants/backend/go/goals/tailor_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
import pytest

from pants.backend.go import target_type_rules
from pants.backend.go.goals.tailor import PutativeGoTargetsRequest, has_package_main
from pants.backend.go.goals.tailor import (
PutativeGoTargetsRequest,
has_go_mod_ancestor,
has_package_main,
)
from pants.backend.go.goals.tailor import rules as go_tailor_rules
from pants.backend.go.target_types import GoBinaryTarget, GoModTarget, GoPackageTarget
from pants.backend.go.util_rules import (
Expand Down Expand Up @@ -64,8 +68,10 @@ def test_find_go_mod_targets(rule_runner: RuleRunner) -> None:
def test_find_go_package_targets(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"unowned/go.mod": "",
"unowned/f.go": "",
"unowned/f1.go": "",
"owned/go.mod": "",
"owned/f.go": "",
"owned/BUILD": "go_package()",
# Any `.go` files under a `testdata` or `vendor` folder should be ignored.
Expand All @@ -74,15 +80,23 @@ def test_find_go_package_targets(rule_runner: RuleRunner) -> None:
"unowned/vendor/example.com/foo/bar.go": "",
# Except if `vendor` is the last directory.
"unowned/cmd/vendor/main.go": "",
"no_go_mod/f.go": "",
}
)
putative_targets = rule_runner.request(
PutativeTargets,
[
PutativeGoTargetsRequest(
("unowned", "owned", "unowned/testdata", "unowned/vendor", "unowned/cmd/vendor")
(
"unowned",
"owned",
"unowned/testdata",
"unowned/vendor",
"unowned/cmd/vendor",
"no_go_mod",
)
),
AllOwnedSources(["owned/f.go"]),
AllOwnedSources(["owned/f.go", "unowned/go.mod", "owned/go.mod"]),
],
)
assert putative_targets == PutativeTargets(
Expand All @@ -106,14 +120,19 @@ def test_find_go_package_targets(rule_runner: RuleRunner) -> None:
def test_find_go_binary_targets(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"missing_binary_tgt/go.mod": "",
"missing_binary_tgt/app.go": "package main",
"missing_binary_tgt/BUILD": "go_package()",
"tgt_already_exists/go.mod": "",
"tgt_already_exists/app.go": "package main",
"tgt_already_exists/BUILD": "go_binary(name='bin')\ngo_package()",
"missing_pkg_and_binary_tgt/go.mod": "",
"missing_pkg_and_binary_tgt/app.go": "package main",
"main_set_to_different_dir/go.mod": "",
"main_set_to_different_dir/subdir/app.go": "package main",
"main_set_to_different_dir/subdir/BUILD": "go_package()",
"main_set_to_different_dir/BUILD": "go_binary(main='main_set_to_different_dir/subdir')",
"no_go_mod/app.go": "package main",
}
)
putative_targets = rule_runner.request(
Expand All @@ -125,12 +144,17 @@ def test_find_go_binary_targets(rule_runner: RuleRunner) -> None:
"tgt_already_exists",
"missing_pkg_and_binary_tgt",
"main_set_to_different_dir",
"no_go_mod",
)
),
AllOwnedSources(
[
"missing_binary_tgt/go.mod",
"missing_binary_tgt/app.go",
"tgt_already_exists/go.mod",
"tgt_already_exists/app.go",
"missing_pkg_and_binary_tgt/go.mod",
"main_set_to_different_dir/go.mod",
"main_set_to_different_dir/subdir/app.go",
]
),
Expand Down Expand Up @@ -168,3 +192,11 @@ def test_has_package_main() -> None:
assert not has_package_main(b"package foo")
assert not has_package_main(b'var = "package main"')
assert not has_package_main(b" package main")


def test_has_go_mod_ancestor() -> None:
assert has_go_mod_ancestor("dir/subdir", {"dir/subdir"}) is True
assert has_go_mod_ancestor("dir/subdir", {"dir/subdir/child"}) is False
assert has_go_mod_ancestor("dir/subdir", {"dir/another"}) is False
assert has_go_mod_ancestor("dir/subdir", {""}) is True
assert has_go_mod_ancestor("dir/subdir", {"another", "dir/another", "dir"}) is True

0 comments on commit fb82ff8

Please sign in to comment.