From 45b0a49a4b51078ca6a2bb5e62de2a39bed055c7 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 1 Oct 2021 14:57:24 -0400 Subject: [PATCH 1/2] [internal] Use `DownloadedExternalModules` when computing `ExternalModulePkgImportPaths` # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/go/target_type_rules.py | 2 +- .../backend/go/target_type_rules_test.py | 6 +-- .../backend/go/util_rules/external_module.py | 19 ++++---- .../go/util_rules/external_module_test.py | 16 +++++-- .../pants/backend/go/util_rules/go_mod.py | 44 ++++++++++++------- src/python/pants/backend/go/util_rules/sdk.py | 26 ++++++++++- 6 files changed, 80 insertions(+), 33 deletions(-) diff --git a/src/python/pants/backend/go/target_type_rules.py b/src/python/pants/backend/go/target_type_rules.py index cca7d2fe880..cd9a987e0f2 100644 --- a/src/python/pants/backend/go/target_type_rules.py +++ b/src/python/pants/backend/go/target_type_rules.py @@ -244,7 +244,7 @@ async def generate_go_external_package_targets( ExternalModulePkgImportPathsRequest( module_path=module_descriptor.path, version=module_descriptor.version, - go_sum_digest=go_mod_info.go_sum_stripped_digest, + go_mod_stripped_digest=go_mod_info.stripped_digest, ), ) for module_descriptor in go_mod_info.modules diff --git a/src/python/pants/backend/go/target_type_rules_test.py b/src/python/pants/backend/go/target_type_rules_test.py index aaeb0816456..34ee0ae66e4 100644 --- a/src/python/pants/backend/go/target_type_rules_test.py +++ b/src/python/pants/backend/go/target_type_rules_test.py @@ -91,13 +91,12 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None: module go.example.com/foo go 1.17 - require ( - github.com/google/go-cmp v0.4.0 - ) + require github.com/google/go-cmp v0.4.0 """ ), "foo/go.sum": textwrap.dedent( """\ + github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= @@ -165,6 +164,7 @@ def test_generate_go_external_package_targets(rule_runner: RuleRunner) -> None: ), "src/go/go.sum": textwrap.dedent( """\ + github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4= github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/uuid v1.2.0 h1:qJYtXnJRWmpe7m/3XlyhrsLrEURqHRM2kxzoxXqyUDs= github.com/google/uuid v1.2.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= diff --git a/src/python/pants/backend/go/util_rules/external_module.py b/src/python/pants/backend/go/util_rules/external_module.py index 5f0f0f61107..38b5ee5aadb 100644 --- a/src/python/pants/backend/go/util_rules/external_module.py +++ b/src/python/pants/backend/go/util_rules/external_module.py @@ -259,12 +259,12 @@ async def resolve_external_go_package( class ExternalModulePkgImportPathsRequest: """Request the import paths for all packages belonging to an external Go module. - The `go_sum_digest` must have a `go.sum` file that includes the module. + The module must be included in the input `go.mod`/`go.sum`. """ module_path: str version: str - go_sum_digest: Digest + go_mod_stripped_digest: Digest class ExternalModulePkgImportPaths(DeduplicatedCollection[str]): @@ -277,18 +277,21 @@ class ExternalModulePkgImportPaths(DeduplicatedCollection[str]): async def compute_package_import_paths_from_external_module( request: ExternalModulePkgImportPathsRequest, ) -> ExternalModulePkgImportPaths: - downloaded_module = await Get( - DownloadedExternalModule, - DownloadExternalModuleRequest(request.module_path, request.version, request.go_sum_digest), + # TODO: Extract the module we care about, rather than using everything. We also don't need the + # root `go.sum` and `go.mod`. + downloaded_modules = await Get( + DownloadedExternalModules, DownloadExternalModulesRequest(request.go_mod_stripped_digest) ) json_result = await Get( ProcessResult, GoSdkProcess( - input_digest=downloaded_module.digest, + input_digest=downloaded_modules.digest, # "-find" skips determining dependencies and imports for each package. - command=("list", "-find", "-json", "./..."), + command=("list", "-find", "-mod=readonly", "-json", "./..."), + working_dir=f"gopath/pkg/mod/{request.module_path}@{request.version}", + env={"GOPROXY": "off"}, description=( - "Determine import paths in Go external module " + "Determine packages belonging to Go external module " f"{request.module_path}@{request.version}" ), ), diff --git a/src/python/pants/backend/go/util_rules/external_module_test.py b/src/python/pants/backend/go/util_rules/external_module_test.py index 7361f0f8716..4861c259d03 100644 --- a/src/python/pants/backend/go/util_rules/external_module_test.py +++ b/src/python/pants/backend/go/util_rules/external_module_test.py @@ -286,8 +286,18 @@ def test_determine_external_package_info(rule_runner: RuleRunner) -> None: def test_determine_external_module_package_import_paths(rule_runner: RuleRunner) -> None: - go_sum_digest = rule_runner.make_snapshot( + input_digest = rule_runner.make_snapshot( { + "go.mod": dedent( + """\ + module example.com/external-module + go 1.17 + require ( + github.com/google/go-cmp v0.5.6 + golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect + ) + """ + ), "go.sum": dedent( """\ github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= @@ -295,12 +305,12 @@ def test_determine_external_module_package_import_paths(rule_runner: RuleRunner) golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= """ - ) + ), } ).digest result = rule_runner.request( ExternalModulePkgImportPaths, - [ExternalModulePkgImportPathsRequest("github.com/google/go-cmp", "v0.5.6", go_sum_digest)], + [ExternalModulePkgImportPathsRequest("github.com/google/go-cmp", "v0.5.6", input_digest)], ) assert result == ExternalModulePkgImportPaths( [ diff --git a/src/python/pants/backend/go/util_rules/go_mod.py b/src/python/pants/backend/go/util_rules/go_mod.py index 959b4e26d62..2945fc937eb 100644 --- a/src/python/pants/backend/go/util_rules/go_mod.py +++ b/src/python/pants/backend/go/util_rules/go_mod.py @@ -4,6 +4,7 @@ import json import logging +import os from dataclasses import dataclass import ijson @@ -12,7 +13,8 @@ from pants.backend.go.util_rules.sdk import GoSdkProcess from pants.base.specs import AddressSpecs, AscendantAddresses from pants.build_graph.address import Address -from pants.engine.fs import Digest, DigestSubset, PathGlobs, RemovePrefix +from pants.engine.engine_aware import EngineAwareParameter +from pants.engine.fs import Digest, RemovePrefix from pants.engine.process import ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import ( @@ -43,14 +45,17 @@ class GoModInfo: # Digest containing the full paths to `go.mod` and `go.sum`. digest: Digest - # Digest containing only the `go.sum` with no leading directory prefix. - go_sum_stripped_digest: Digest + # Digest containing `go.mod` and `go.sum` with no path prefixes. + stripped_digest: Digest @dataclass(frozen=True) -class GoModInfoRequest: +class GoModInfoRequest(EngineAwareParameter): address: Address + def debug_hint(self) -> str: + return self.address.spec + def parse_module_descriptors(raw_json: bytes) -> list[ModuleDescriptor]: """Parse the JSON output of `go list -m`.""" @@ -76,33 +81,35 @@ async def determine_go_mod_info( ) -> GoModInfo: wrapped_target = await Get(WrappedTarget, Address, request.address) sources_field = wrapped_target.target[GoModSourcesField] + go_mod_path = sources_field.go_mod_path + go_mod_dir = os.path.dirname(go_mod_path) # Get the `go.mod` (and `go.sum`) and strip so the file has no directory prefix. hydrated_sources = await Get(HydratedSources, HydrateSourcesRequest(sources_field)) - sources_without_prefix = await Get( - Digest, RemovePrefix(hydrated_sources.snapshot.digest, request.address.spec_path) - ) - go_sum_digest_get = Get(Digest, DigestSubset(sources_without_prefix, PathGlobs(["go.sum"]))) + sources_digest = hydrated_sources.snapshot.digest mod_json_get = Get( ProcessResult, GoSdkProcess( command=("mod", "edit", "-json"), - input_digest=sources_without_prefix, - description=f"Parse {sources_field.go_mod_path}", + input_digest=sources_digest, + working_dir=go_mod_dir, + description=f"Parse {go_mod_path}", ), ) list_modules_get = Get( ProcessResult, GoSdkProcess( - input_digest=sources_without_prefix, command=("list", "-m", "-json", "all"), - description=f"List modules in {sources_field.go_mod_path}", + input_digest=sources_digest, + working_dir=go_mod_dir, + description=f"List modules in {go_mod_path}", ), ) - mod_json, list_modules, go_sum_digest = await MultiGet( - mod_json_get, list_modules_get, go_sum_digest_get + stripped_source_get = Get(Digest, RemovePrefix(sources_digest, go_mod_dir)) + mod_json, list_modules, stripped_sources = await MultiGet( + mod_json_get, list_modules_get, stripped_source_get ) module_metadata = json.loads(mod_json.stdout) @@ -110,8 +117,8 @@ async def determine_go_mod_info( return GoModInfo( import_path=module_metadata["Module"]["Path"], modules=FrozenOrderedSet(modules), - digest=hydrated_sources.snapshot.digest, - go_sum_stripped_digest=go_sum_digest, + digest=sources_digest, + stripped_digest=stripped_sources, ) @@ -122,9 +129,12 @@ class OwningGoModRequest: @dataclass(frozen=True) -class OwningGoMod: +class OwningGoMod(EngineAwareParameter): address: Address + def debug_hint(self) -> str: + return self.address.spec + @rule async def find_nearest_go_mod(request: OwningGoModRequest) -> OwningGoMod: diff --git a/src/python/pants/backend/go/util_rules/sdk.py b/src/python/pants/backend/go/util_rules/sdk.py index 9effb0cdab6..2319ebc6f76 100644 --- a/src/python/pants/backend/go/util_rules/sdk.py +++ b/src/python/pants/backend/go/util_rules/sdk.py @@ -6,6 +6,7 @@ import shlex import textwrap from dataclasses import dataclass +from typing import Iterable, Mapping from pants.backend.go.subsystems import golang from pants.backend.go.subsystems.golang import GoRoot @@ -13,18 +14,41 @@ from pants.engine.internals.selectors import Get from pants.engine.process import BashBinary, Process from pants.engine.rules import collect_rules, rule +from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel +from pants.util.meta import frozen_after_init -@dataclass(frozen=True) +@frozen_after_init +@dataclass(unsafe_hash=True) class GoSdkProcess: command: tuple[str, ...] description: str + env: FrozenDict[str, str] input_digest: Digest = EMPTY_DIGEST working_dir: str | None = None output_files: tuple[str, ...] = () output_directories: tuple[str, ...] = () + def __init__( + self, + command: Iterable[str], + *, + description: str, + env: Mapping[str, str] | None = None, + input_digest: Digest = EMPTY_DIGEST, + working_dir: str | None = None, + output_files: Iterable[str] = (), + output_directories: Iterable[str] = (), + ) -> None: + self.command = tuple(command) + self.description = description + self.env = FrozenDict(env or {}) + self.input_digest = input_digest + self.working_dir = working_dir + self.output_files = tuple(output_files) + self.output_directories = tuple(output_directories) + @rule async def setup_go_sdk_process(request: GoSdkProcess, goroot: GoRoot, bash: BashBinary) -> Process: From 33704d235b60cc9505fd46ec7bbd4e7046a9e1c3 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 1 Oct 2021 15:14:56 -0400 Subject: [PATCH 2/2] Refactor # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../pants/backend/go/util_rules/external_module.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/python/pants/backend/go/util_rules/external_module.py b/src/python/pants/backend/go/util_rules/external_module.py index 38b5ee5aadb..bbdc8d61a27 100644 --- a/src/python/pants/backend/go/util_rules/external_module.py +++ b/src/python/pants/backend/go/util_rules/external_module.py @@ -41,6 +41,11 @@ class DownloadedExternalModules: digest: Digest + @staticmethod + def module_dir(module_path: str, version: str) -> str: + """The path to the module's directory.""" + return f"gopath/pkg/mod/{module_path}@{version}" + @dataclass(frozen=True) class DownloadExternalModulesRequest: @@ -82,7 +87,6 @@ async def download_external_modules( f"{contents[1].content.decode()}\n\n" ) - # TODO: strip `gopath` and other paths? # TODO: stop including irrelevant files like the `.zip` files. download_snapshot = await Get(Snapshot, Digest, download_result.output_digest) @@ -91,13 +95,10 @@ async def download_external_modules( # To analyze each module via `go list`, we need a `go.mod` in each module's directory. If the # module does not natively use Go modules, then Go will generate a `go.mod` for us, but we # need to relocate the file to the correct location. - module_dirs_with_go_mod = [] generated_go_mods_to_module_dirs = {} for module_metadata in ijson.items(download_result.stdout, "", multiple_values=True): download_dir = strip_v2_chroot_path(module_metadata["Dir"]) - if f"{download_dir}/go.mod" in all_downloaded_files: - module_dirs_with_go_mod.append(download_dir) - else: + if f"{download_dir}/go.mod" not in all_downloaded_files: generated_go_mod = strip_v2_chroot_path(module_metadata["GoMod"]) generated_go_mods_to_module_dirs[generated_go_mod] = download_dir @@ -288,7 +289,7 @@ async def compute_package_import_paths_from_external_module( input_digest=downloaded_modules.digest, # "-find" skips determining dependencies and imports for each package. command=("list", "-find", "-mod=readonly", "-json", "./..."), - working_dir=f"gopath/pkg/mod/{request.module_path}@{request.version}", + working_dir=downloaded_modules.module_dir(request.module_path, request.version), env={"GOPROXY": "off"}, description=( "Determine packages belonging to Go external module "