Skip to content

Commit

Permalink
go: cgo fixes plus add tests for each supported language (#17018)
Browse files Browse the repository at this point in the history
Fix various bugs remaining in the cgo support. Add tests demonstrating that C++, Objective-C, and Fortran support works (when the system supports the applicable compiler/libraries).

Fixes #16836
Fixes #16830
  • Loading branch information
Tom Dyas authored Sep 28, 2022
1 parent 9550caf commit 191ab4e
Show file tree
Hide file tree
Showing 10 changed files with 462 additions and 68 deletions.
4 changes: 2 additions & 2 deletions src/python/pants/backend/codegen/protobuf/go/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,8 @@ async def setup_full_package_build_request(
pkg_name=analysis.name,
digest=gen_sources.digest,
dir_path=analysis.dir_path,
go_file_names=analysis.go_files,
s_file_names=analysis.s_files,
go_files=analysis.go_files,
s_files=analysis.s_files,
direct_dependencies=dep_build_requests,
minimum_go_version=analysis.minimum_go_version,
),
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/go/go_sources/load_go_binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ async def setup_go_binary(request: LoadedGoBinaryRequest) -> LoadedGoBinary:
pkg_name="main",
dir_path="",
digest=source_digest,
go_file_names=tuple(fc.path for fc in file_contents),
s_file_names=(),
go_files=tuple(fc.path for fc in file_contents),
s_files=(),
direct_dependencies=(),
minimum_go_version=None,
),
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/backend/go/goals/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,8 +266,8 @@ def compilation_failure(exit_code: int, stdout: str | None, stderr: str | None)
pkg_name=f"{pkg_analysis.name}_test",
digest=pkg_digest.digest,
dir_path=pkg_analysis.dir_path,
go_file_names=pkg_analysis.xtest_go_files,
s_file_names=(), # TODO: Are there .s files for xtest?
go_files=pkg_analysis.xtest_go_files,
s_files=(), # TODO: Are there .s files for xtest?
direct_dependencies=tuple(direct_dependencies),
minimum_go_version=pkg_analysis.minimum_go_version,
embed_config=pkg_digest.xtest_embed_config,
Expand Down Expand Up @@ -313,8 +313,8 @@ def compilation_failure(exit_code: int, stdout: str | None, stderr: str | None)
pkg_name="main",
digest=testmain_input_digest,
dir_path="",
go_file_names=(GeneratedTestMain.TEST_MAIN_FILE, *coverage_setup_files),
s_file_names=(),
go_files=(GeneratedTestMain.TEST_MAIN_FILE, *coverage_setup_files),
s_files=(),
direct_dependencies=tuple(main_direct_deps),
minimum_go_version=pkg_analysis.minimum_go_version,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def test_build_invalid_package(rule_runner: RuleRunner) -> None:
import_path="example.com/assembly",
pkg_name="main",
dir_path="",
go_file_names=("add_amd64.go", "add_arm64.go"),
go_files=("add_amd64.go", "add_arm64.go"),
digest=rule_runner.make_snapshot(
{
"add_amd64.go": "package main\nfunc add(x, y int64) int64",
Expand All @@ -144,7 +144,7 @@ def test_build_invalid_package(rule_runner: RuleRunner) -> None:
"add_arm64.s": "INVALID!!!",
}
).digest,
s_file_names=("add_amd64.s", "add_arm64.s"),
s_files=("add_amd64.s", "add_arm64.s"),
direct_dependencies=(),
minimum_go_version=None,
)
Expand Down
114 changes: 91 additions & 23 deletions src/python/pants/backend/go/util_rules/build_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,19 @@
from pants.backend.go.util_rules.goroot import GoRoot
from pants.backend.go.util_rules.import_analysis import ImportConfig, ImportConfigRequest
from pants.backend.go.util_rules.sdk import GoSdkProcess, GoSdkToolIDRequest, GoSdkToolIDResult
from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior
from pants.engine.engine_aware import EngineAwareParameter, EngineAwareReturnType
from pants.engine.fs import EMPTY_DIGEST, AddPrefix, CreateDigest, Digest, FileContent, MergeDigests
from pants.engine.fs import (
EMPTY_DIGEST,
AddPrefix,
CreateDigest,
Digest,
DigestContents,
DigestSubset,
FileContent,
MergeDigests,
PathGlobs,
)
from pants.engine.process import FallibleProcessResult, ProcessResult
from pants.engine.rules import Get, MultiGet, collect_rules, rule, rule_helper
from pants.util.frozendict import FrozenDict
Expand All @@ -44,14 +55,14 @@ def __init__(
pkg_name: str,
digest: Digest,
dir_path: str,
go_file_names: tuple[str, ...],
s_file_names: tuple[str, ...],
go_files: tuple[str, ...],
s_files: tuple[str, ...],
direct_dependencies: tuple[BuildGoPackageRequest, ...],
minimum_go_version: str | None,
for_tests: bool = False,
embed_config: EmbedConfig | None = None,
coverage_config: GoCoverageConfig | None = None,
cgo_file_names: tuple[str, ...] = (),
cgo_files: tuple[str, ...] = (),
cgo_flags: CGoCompilerFlags | None = None,
c_files: tuple[str, ...] = (),
cxx_files: tuple[str, ...] = (),
Expand All @@ -68,14 +79,14 @@ def __init__(
self.pkg_name = pkg_name
self.digest = digest
self.dir_path = dir_path
self.go_file_names = go_file_names
self.s_file_names = s_file_names
self.go_files = go_files
self.s_files = s_files
self.direct_dependencies = direct_dependencies
self.minimum_go_version = minimum_go_version
self.for_tests = for_tests
self.embed_config = embed_config
self.coverage_config = coverage_config
self.cgo_file_names = cgo_file_names
self.cgo_files = cgo_files
self.cgo_flags = cgo_flags
self.c_files = c_files
self.cxx_files = cxx_files
Expand All @@ -87,14 +98,14 @@ def __init__(
self.pkg_name,
self.digest,
self.dir_path,
self.go_file_names,
self.s_file_names,
self.go_files,
self.s_files,
self.direct_dependencies,
self.minimum_go_version,
self.for_tests,
self.embed_config,
self.coverage_config,
self.cgo_file_names,
self.cgo_files,
self.cgo_flags,
self.c_files,
self.cxx_files,
Expand All @@ -112,14 +123,14 @@ def __repr__(self) -> str:
f"pkg_name={self.pkg_name}, "
f"digest={self.digest}, "
f"dir_path={self.dir_path}, "
f"go_file_names={self.go_file_names}, "
f"s_file_names={self.s_file_names}, "
f"go_files={self.go_files}, "
f"s_files={self.s_files}, "
f"direct_dependencies={[dep.import_path for dep in self.direct_dependencies]}, "
f"minimum_go_version={self.minimum_go_version}, "
f"for_tests={self.for_tests}, "
f"embed_config={self.embed_config}, "
f"coverage_config={self.coverage_config}, "
f"cgo_file_names={self.cgo_file_names}, "
f"cgo_files={self.cgo_files}, "
f"cgo_flags={self.cgo_flags}, "
f"c_files={self.c_files}, "
f"cxx_files={self.cxx_files}, "
Expand All @@ -140,13 +151,13 @@ def __eq__(self, other):
and self.pkg_name == other.pkg_name
and self.digest == other.digest
and self.dir_path == other.dir_path
and self.go_file_names == other.go_file_names
and self.s_file_names == other.s_file_names
and self.go_files == other.go_files
and self.s_files == other.s_files
and self.minimum_go_version == other.minimum_go_version
and self.for_tests == other.for_tests
and self.embed_config == other.embed_config
and self.coverage_config == other.coverage_config
and self.cgo_file_names == other.cgo_file_names
and self.cgo_files == other.cgo_files
and self.cgo_flags == other.cgo_flags
and self.c_files == other.c_files
and self.cxx_files == other.cxx_files
Expand Down Expand Up @@ -288,6 +299,51 @@ async def _add_objects_to_archive(
return pack_result


def _maybe_is_golang_assembly(data: bytes) -> bool:
"""Return true if `data` looks like it could be a Golang-format assembly language file.
This is used by the cgo rules as a heuristic to determine if the user is passing Golang assembly
format instead of gcc assembly format.
"""
return (
data.startswith(b"TEXT")
or b"\nTEXT" in data
or data.startswith(b"DATA")
or b"\nDATA" in data
or data.startswith(b"GLOBL")
or b"\nGLOBL" in data
)


@rule_helper
async def _any_file_is_golang_assembly(
digest: Digest, dir_path: str, s_files: Iterable[str]
) -> bool:
"""Return true if any of the given `s_files` look like it could be a Golang-format assembly
language file.
This is used by the cgo rules as a heuristic to determine if the user is passing Golang assembly
format instead of gcc assembly format.
"""
digest_contents = await Get(
DigestContents,
DigestSubset(
digest,
PathGlobs(
globs=[os.path.join(dir_path, s_file) for s_file in s_files],
glob_match_error_behavior=GlobMatchErrorBehavior.error,
description_of_origin="golang cgo rules",
),
),
)
for s_file in s_files:
for entry in digest_contents:
if entry.path == os.path.join(dir_path, s_file):
if _maybe_is_golang_assembly(entry.content):
return True
return False


# NB: We must have a description for the streaming of this rule to work properly
# (triggered by `FallibleBuiltGoPackage` subclassing `EngineAwareReturnType`).
@rule(desc="Compile with Go", level=LogLevel.DEBUG)
Expand Down Expand Up @@ -326,8 +382,9 @@ async def build_go_package(

# If coverage is enabled for this package, then replace the Go source files with versions modified to
# contain coverage code.
go_files = request.go_file_names
cgo_files = request.cgo_file_names
go_files = request.go_files
cgo_files = request.cgo_files
s_files = list(request.s_files)
go_files_digest = request.digest
cover_file_metadatas: tuple[FileCodeCoverageMetadata, ...] | None = None
if request.coverage_config:
Expand All @@ -350,6 +407,15 @@ async def build_go_package(

cgo_compile_result: CGoCompileResult | None = None
if cgo_files:
# Check if any assembly files contain gcc assembly, and not Go assembly. Raise an exception if any are
# likely in Go format since in cgo packages, assembly files are passed to gcc and must be in gcc format.
if s_files and await _any_file_is_golang_assembly(
request.digest, request.dir_path, s_files
):
raise ValueError(
f"Package {request.import_path} is a cgo package but contains Go assembly files."
)

assert request.cgo_flags is not None
cgo_compile_result = await Get(
CGoCompileResult,
Expand All @@ -361,13 +427,15 @@ async def build_go_package(
cgo_files=cgo_files,
cgo_flags=request.cgo_flags,
c_files=request.c_files,
s_files=tuple(s_files),
cxx_files=request.cxx_files,
objc_files=request.objc_files,
fortran_files=request.fortran_files,
),
)
assert cgo_compile_result is not None
unmerged_input_digests.append(cgo_compile_result.digest)
s_files = [] # Clear s_files since assembly has already been handled in cgo rules.

input_digest = await Get(
Digest,
Expand All @@ -376,12 +444,12 @@ async def build_go_package(

assembly_digests = None
symabis_path = None
if request.s_file_names:
if s_files:
assembly_setup = await Get(
FallibleAssemblyPreCompilation,
AssemblyPreCompilationRequest(
compilation_input=input_digest,
s_files=request.s_file_names,
s_files=tuple(s_files),
dir_path=request.dir_path,
import_path=request.import_path,
),
Expand Down Expand Up @@ -424,7 +492,7 @@ async def build_go_package(
if embedcfg.digest != EMPTY_DIGEST:
compile_args.extend(["-embedcfg", RenderedEmbedConfig.PATH])

if not request.s_file_names:
if not s_files:
# If there are no non-Go sources, then pass -complete flag which tells the compiler that the provided
# Go files are the entire package.
compile_args.append("-complete")
Expand Down Expand Up @@ -460,7 +528,7 @@ async def build_go_package(
AssemblyPostCompilationRequest(
compilation_result=compilation_digest,
assembly_digests=assembly_digests,
s_files=request.s_file_names,
s_files=tuple(s_files),
dir_path=request.dir_path,
),
)
Expand Down Expand Up @@ -568,7 +636,7 @@ async def compute_compile_action_id(
compile_tool_id = await Get(GoSdkToolIDResult, GoSdkToolIDRequest("compile"))
h.update(f"compile {compile_tool_id.tool_id}\n".encode())
# TODO: Add compiler flags as per `go`'s algorithm. Need to figure out
if bq.s_file_names:
if bq.s_files:
asm_tool_id = await Get(GoSdkToolIDResult, GoSdkToolIDRequest("asm"))
h.update(f"asm {asm_tool_id.tool_id}\n".encode())
# TODO: Add asm flags as per `go`'s algorithm.
Expand Down
14 changes: 7 additions & 7 deletions src/python/pants/backend/go/util_rules/build_pkg_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ async def setup_build_go_package_target_request(
embed_config = embed_config.merge(_first_party_pkg_digest.test_embed_config)
else:
embed_config = _first_party_pkg_digest.test_embed_config
s_file_names = _first_party_pkg_analysis.s_files
cgo_file_names = _first_party_pkg_analysis.cgo_files
s_files = _first_party_pkg_analysis.s_files
cgo_files = _first_party_pkg_analysis.cgo_files
cgo_flags = _first_party_pkg_analysis.cgo_flags
c_files = _first_party_pkg_analysis.c_files
cxx_files = _first_party_pkg_analysis.cxx_files
Expand All @@ -199,9 +199,9 @@ async def setup_build_go_package_target_request(
digest = _third_party_pkg_info.digest
minimum_go_version = _third_party_pkg_info.minimum_go_version
go_file_names = _third_party_pkg_info.go_files
s_file_names = _third_party_pkg_info.s_files
s_files = _third_party_pkg_info.s_files
embed_config = _third_party_pkg_info.embed_config
cgo_file_names = _third_party_pkg_info.cgo_files
cgo_files = _third_party_pkg_info.cgo_files
cgo_flags = _third_party_pkg_info.cgo_flags
c_files = _third_party_pkg_info.c_files
cxx_files = _third_party_pkg_info.cxx_files
Expand Down Expand Up @@ -239,9 +239,9 @@ async def setup_build_go_package_target_request(
import_path="main" if request.is_main else import_path,
pkg_name=pkg_name,
dir_path=dir_path,
go_file_names=go_file_names,
s_file_names=s_file_names,
cgo_file_names=cgo_file_names,
go_files=go_file_names,
s_files=s_files,
cgo_files=cgo_files,
cgo_flags=cgo_flags,
c_files=c_files,
cxx_files=cxx_files,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ async def generate_from_file(request: GoCodegenBuildFilesRequest) -> FallibleBui
pkg_name="gen",
digest=digest,
dir_path="codegen",
go_file_names=("f.go",),
s_file_names=(),
go_files=("f.go",),
s_files=(),
direct_dependencies=(thirdparty_dep.request,),
minimum_go_version=None,
),
Expand Down Expand Up @@ -139,8 +139,8 @@ def assert_pkg_target_built(
build_request = rule_runner.request(BuildGoPackageRequest, [BuildGoPackageTargetRequest(addr)])
assert build_request.import_path == expected_import_path
assert build_request.dir_path == expected_dir_path
assert build_request.go_file_names == tuple(expected_go_file_names)
assert not build_request.s_file_names
assert build_request.go_files == tuple(expected_go_file_names)
assert not build_request.s_files
assert [
dep.import_path for dep in build_request.direct_dependencies
] == expected_direct_dependency_import_paths
Expand Down
Loading

0 comments on commit 191ab4e

Please sign in to comment.