Skip to content

Commit

Permalink
Fix handling of Go version for third-party packages (pantsbuild#14457)
Browse files Browse the repository at this point in the history
We weren't following the spec properly. Closes pantsbuild#14446.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano committed Feb 12, 2022
1 parent 11189f3 commit 0a75688
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 5 deletions.
20 changes: 19 additions & 1 deletion src/python/pants/backend/go/subsystems/golang.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ class GoRoot:
"""Path to the Go installation (the `GOROOT`)."""

path: str
version: str

def is_compatible_version(self, version: str) -> bool:
"""Can this Go compiler handle the target version?
Inspired by
https://github.com/golang/go/blob/30501bbef9fcfc9d53e611aaec4d20bb3cdb8ada/src/cmd/go/internal/work/exec.go#L429-L445.
Input expected in the form `1.17`.
"""
if version == "1.0":
return True

def parse(v: str) -> tuple[int, int]:
major, minor = v.split(".", maxsplit=1)
return int(major), int(minor)

return parse(version) <= parse(self.version)


@rule(desc="Find Go binary", level=LogLevel.DEBUG)
Expand Down Expand Up @@ -163,7 +181,7 @@ async def setup_goroot(golang_subsystem: GolangSubsystem) -> GoRoot:
),
)
goroot = env_result.stdout.decode("utf-8").strip()
return GoRoot(goroot)
return GoRoot(goroot, version)

logger.debug(
f"Go binary at {binary_path.path} has version {version}, but this "
Expand Down
10 changes: 10 additions & 0 deletions src/python/pants/backend/go/subsystems/golang_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,13 @@ def test_no_valid_versions(rule_runner: RuleRunner) -> None:
exc = e.value.wrapped_exceptions[0]
assert isinstance(exc, BinaryNotFoundError)
assert "Cannot find a `go` binary with the expected version" in str(exc)


def test_valid_go_version() -> None:
go_root = GoRoot("", "1.15")
for v in range(16):
assert go_root.is_compatible_version(f"1.{v}") is True
for v in range(17, 40):
assert go_root.is_compatible_version(f"1.{v}") is False
for v in range(2, 4):
assert go_root.is_compatible_version(f"{v}.0") is False
14 changes: 10 additions & 4 deletions src/python/pants/backend/go/util_rules/build_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os.path
from dataclasses import dataclass

from pants.backend.go.subsystems.golang import GoRoot
from pants.backend.go.util_rules.assembly import (
AssemblyPostCompilation,
AssemblyPostCompilationRequest,
Expand Down Expand Up @@ -198,7 +199,9 @@ class RenderedEmbedConfig:
# 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)
async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPackage:
async def build_go_package(
request: BuildGoPackageRequest, go_root: GoRoot
) -> FallibleBuiltGoPackage:
maybe_built_deps = await MultiGet(
Get(FallibleBuiltGoPackage, BuildGoPackageRequest, build_request)
for build_request in request.direct_dependencies
Expand Down Expand Up @@ -255,11 +258,14 @@ async def build_go_package(request: BuildGoPackageRequest) -> FallibleBuiltGoPac
request.import_path,
"-importcfg",
import_config.CONFIG_PATH,
# See https://github.com/golang/go/blob/f229e7031a6efb2f23241b5da000c3b3203081d6/src/cmd/go/internal/work/gc.go#L79-L100
# for why Go sets the default to 1.16.
f"-lang=go{request.minimum_go_version or '1.16'}",
]

# See https://github.com/golang/go/blob/f229e7031a6efb2f23241b5da000c3b3203081d6/src/cmd/go/internal/work/gc.go#L79-L100
# for where this logic comes from.
go_version = request.minimum_go_version or "1.16"
if go_root.is_compatible_version(go_version):
compile_args.append(f"-lang=go{go_version}")

if symabis_path:
compile_args.extend(["-symabis", symabis_path])

Expand Down

0 comments on commit 0a75688

Please sign in to comment.