Skip to content

Commit 2f8dc61

Browse files
committed
[internal] Don't download Go third-party dependencies multiple times (pantsbuild#13352)
Turns out that pantsbuild#13339 didn't actually work - we are redownloading the same modules several times with Go! Downloads happen when: 1. determining `GoModInfo` (once per `go.mod`) 2. `AllDownloadedModules` (once per `go.mod`) 3. Determining metadata for each third-party package (once per third-party module) 4. Determining metadata for each first-party package (once per first-party package/directory) This PR fixes it so that we only download modules a single time, once per `go.mod`. To fix this, we stop treating third-party modules like first-party modules, i.e. we stop `cd`-ing into its downloaded directory and running `go list` directly in it, using its own `go.mod` and `go.sum`. That requires that the chroot has all of the module's transitive dependencies present, and it also resulted in issues like pantsbuild#13138. Instead, the much simpler thing to do is run `go list '...'` to do all third-party analysis in a single swoop. That gets us all the analysis we need. We also extract the relevant `.go` files from all of the downloaded `GOPATH`, i.e. all the downloaded modules. For compilation, all we need is the `.go` files + the metadata we had earlier calculated. Compilation doesn't need access to anything else like other package's. For first-party analysis, we copy the whole `GOPATH` into the chroot. (This is really slow! We need something like pantsbuild#12716 to fix this.) ## Benchmark Running in https://github.com/toolchainlabs/remote-api-tools. Before: ``` ❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::' Time (mean ± σ): 36.467 s ± 0.603 s [User: 41.109 s, System: 38.095 s] Range (min … max): 35.518 s … 37.137 s 5 runs ``` Fixing only third-party analysis: ``` ❯ hyperfine -r 5 --show-output './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::' Time (mean ± σ): 29.880 s ± 0.901 s [User: 29.564 s, System: 15.281 s] Range (min … max): 28.835 s … 31.312 s 5 runs ``` Fixing everything: ``` ❯ hyperfine -r 5 './pants_from_sources --no-process-execution-local-cache --no-pantsd package ::' Time (mean ± σ): 26.633 s ± 2.283 s [User: 24.115 s, System: 30.453 s] Range (min … max): 24.570 s … 30.037 s 5 runs ``` [ci skip-rust] [ci skip-build-wheels]
1 parent c9afb6e commit 2f8dc61

12 files changed

+677
-868
lines changed

src/python/pants/backend/go/target_type_rules.py

+8-27
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
GoImportPathField,
2020
GoModPackageSourcesField,
2121
GoModTarget,
22-
GoThirdPartyModulePathField,
23-
GoThirdPartyModuleVersionField,
2422
GoThirdPartyPackageDependenciesField,
2523
GoThirdPartyPackageTarget,
2624
)
@@ -32,8 +30,8 @@
3230
from pants.backend.go.util_rules.go_mod import GoModInfo, GoModInfoRequest
3331
from pants.backend.go.util_rules.import_analysis import GoStdLibImports
3432
from pants.backend.go.util_rules.third_party_pkg import (
35-
ThirdPartyModuleInfo,
36-
ThirdPartyModuleInfoRequest,
33+
AllThirdPartyPackages,
34+
AllThirdPartyPackagesRequest,
3735
ThirdPartyPkgInfo,
3836
ThirdPartyPkgInfoRequest,
3937
)
@@ -153,12 +151,7 @@ async def inject_go_third_party_package_dependencies(
153151
tgt = wrapped_target.target
154152
pkg_info = await Get(
155153
ThirdPartyPkgInfo,
156-
ThirdPartyPkgInfoRequest(
157-
module_path=tgt[GoThirdPartyModulePathField].value,
158-
version=tgt[GoThirdPartyModuleVersionField].value,
159-
import_path=tgt[GoImportPathField].value,
160-
go_mod_stripped_digest=go_mod_info.stripped_digest,
161-
),
154+
ThirdPartyPkgInfoRequest(tgt[GoImportPathField].value, go_mod_info.stripped_digest),
162155
)
163156

164157
inferred_dependencies = []
@@ -214,16 +207,9 @@ async def generate_targets_from_go_mod(
214207
request.generator[GoModPackageSourcesField].path_globs(files_not_found_behavior),
215208
),
216209
)
217-
all_module_info = await MultiGet(
218-
Get(
219-
ThirdPartyModuleInfo,
220-
ThirdPartyModuleInfoRequest(
221-
module_path=module_descriptor.path,
222-
version=module_descriptor.version,
223-
go_mod_stripped_digest=go_mod_info.stripped_digest,
224-
),
225-
)
226-
for module_descriptor in go_mod_info.modules
210+
all_third_party_packages = await Get(
211+
AllThirdPartyPackages,
212+
AllThirdPartyPackagesRequest(go_mod_info.stripped_digest),
227213
)
228214

229215
dir_to_filenames = group_by_dir(go_paths.files)
@@ -251,11 +237,7 @@ def create_first_party_package_tgt(dir: str) -> GoFirstPartyPackageTarget:
251237

252238
def create_third_party_package_tgt(pkg_info: ThirdPartyPkgInfo) -> GoThirdPartyPackageTarget:
253239
return GoThirdPartyPackageTarget(
254-
{
255-
GoThirdPartyModulePathField.alias: pkg_info.module_path,
256-
GoThirdPartyModuleVersionField.alias: pkg_info.version,
257-
GoImportPathField.alias: pkg_info.import_path,
258-
},
240+
{GoImportPathField.alias: pkg_info.import_path},
259241
# E.g. `src/go:mod#github.com/google/uuid`.
260242
generator_addr.create_generated(pkg_info.import_path),
261243
union_membership,
@@ -264,8 +246,7 @@ def create_third_party_package_tgt(pkg_info: ThirdPartyPkgInfo) -> GoThirdPartyP
264246

265247
third_party_pkgs = (
266248
create_third_party_package_tgt(pkg_info)
267-
for module_info in all_module_info
268-
for pkg_info in module_info.values()
249+
for pkg_info in all_third_party_packages.import_paths_to_pkg_info.values()
269250
)
270251
return GeneratedTargets(request.generator, (*first_party_pkgs, *third_party_pkgs))
271252

src/python/pants/backend/go/target_type_rules_test.py

+27-63
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
GoFirstPartyPackageTarget,
2525
GoImportPathField,
2626
GoModTarget,
27-
GoThirdPartyModulePathField,
28-
GoThirdPartyModuleVersionField,
2927
GoThirdPartyPackageTarget,
3028
)
3129
from pants.backend.go.util_rules import first_party_pkg, go_mod, sdk, third_party_pkg
@@ -79,23 +77,21 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None:
7977
module go.example.com/foo
8078
go 1.17
8179
82-
require github.com/google/go-cmp v0.4.0
80+
require github.com/google/uuid v1.3.0
8381
"""
8482
),
8583
"foo/go.sum": dedent(
8684
"""\
87-
github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
88-
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
89-
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
90-
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
91-
"""
85+
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
86+
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
87+
"""
9288
),
9389
"foo/pkg/foo.go": dedent(
9490
"""\
9591
package pkg
96-
import "github.com/google/go-cmp/cmp"
97-
func grok(left, right string) bool {
98-
return cmp.Equal(left, right)
92+
import "github.com/google/uuid"
93+
func Grok() string {
94+
return uuid.Foo()
9995
}
10096
"""
10197
),
@@ -126,7 +122,7 @@ def test_go_package_dependency_inference(rule_runner: RuleRunner) -> None:
126122
[InferGoPackageDependenciesRequest(tgt2[GoFirstPartyPackageSourcesField])],
127123
)
128124
assert inferred_deps2.dependencies == FrozenOrderedSet(
129-
[Address("foo", generated_name="github.com/google/go-cmp/cmp")]
125+
[Address("foo", generated_name="github.com/google/uuid")]
130126
)
131127

132128
# Compilation failures should not blow up Pants.
@@ -154,6 +150,7 @@ def test_generate_package_targets(rule_runner: RuleRunner) -> None:
154150
require (
155151
github.com/google/go-cmp v0.4.0
156152
github.com/google/uuid v1.2.0
153+
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect
157154
)
158155
"""
159156
),
@@ -189,15 +186,9 @@ def gen_first_party_tgt(rel_dir: str, sources: list[str]) -> GoFirstPartyPackage
189186
residence_dir=os.path.join("src/go", rel_dir).rstrip("/"),
190187
)
191188

192-
def gen_third_party_tgt(
193-
mod_path: str, version: str, import_path: str
194-
) -> GoThirdPartyPackageTarget:
189+
def gen_third_party_tgt(import_path: str) -> GoThirdPartyPackageTarget:
195190
return GoThirdPartyPackageTarget(
196-
{
197-
GoImportPathField.alias: import_path,
198-
GoThirdPartyModulePathField.alias: mod_path,
199-
GoThirdPartyModuleVersionField.alias: version,
200-
},
191+
{GoImportPathField.alias: import_path},
201192
Address("src/go", generated_name=import_path),
202193
)
203194

@@ -207,44 +198,21 @@ def gen_third_party_tgt(
207198
gen_first_party_tgt("", ["hello.go"]),
208199
gen_first_party_tgt("subdir", ["subdir/f.go", "subdir/f2.go"]),
209200
gen_first_party_tgt("another_dir/subdir", ["another_dir/subdir/f.go"]),
210-
gen_third_party_tgt("github.com/google/uuid", "v1.2.0", "github.com/google/uuid"),
211-
gen_third_party_tgt(
212-
"github.com/google/go-cmp", "v0.4.0", "github.com/google/go-cmp/cmp"
213-
),
214-
gen_third_party_tgt(
215-
"github.com/google/go-cmp", "v0.4.0", "github.com/google/go-cmp/cmp/cmpopts"
216-
),
217-
gen_third_party_tgt(
218-
"github.com/google/go-cmp", "v0.4.0", "github.com/google/go-cmp/cmp/internal/diff"
219-
),
220-
gen_third_party_tgt(
221-
"github.com/google/go-cmp", "v0.4.0", "github.com/google/go-cmp/cmp/internal/flags"
222-
),
223-
gen_third_party_tgt(
224-
"github.com/google/go-cmp",
225-
"v0.4.0",
226-
"github.com/google/go-cmp/cmp/internal/function",
227-
),
228-
gen_third_party_tgt(
229-
"github.com/google/go-cmp",
230-
"v0.4.0",
231-
"github.com/google/go-cmp/cmp/internal/testprotos",
232-
),
233-
gen_third_party_tgt(
234-
"github.com/google/go-cmp",
235-
"v0.4.0",
236-
"github.com/google/go-cmp/cmp/internal/teststructs",
237-
),
238-
gen_third_party_tgt(
239-
"github.com/google/go-cmp", "v0.4.0", "github.com/google/go-cmp/cmp/internal/value"
240-
),
241-
gen_third_party_tgt(
242-
"golang.org/x/xerrors", "v0.0.0-20191204190536-9bdfabe68543", "golang.org/x/xerrors"
243-
),
244-
gen_third_party_tgt(
245-
"golang.org/x/xerrors",
246-
"v0.0.0-20191204190536-9bdfabe68543",
247-
"golang.org/x/xerrors/internal",
201+
*(
202+
gen_third_party_tgt(pkg)
203+
for pkg in (
204+
"github.com/google/uuid",
205+
"github.com/google/go-cmp/cmp",
206+
"github.com/google/go-cmp/cmp/cmpopts",
207+
"github.com/google/go-cmp/cmp/internal/diff",
208+
"github.com/google/go-cmp/cmp/internal/flags",
209+
"github.com/google/go-cmp/cmp/internal/function",
210+
"github.com/google/go-cmp/cmp/internal/testprotos",
211+
"github.com/google/go-cmp/cmp/internal/teststructs",
212+
"github.com/google/go-cmp/cmp/internal/value",
213+
"golang.org/x/xerrors",
214+
"golang.org/x/xerrors/internal",
215+
)
248216
),
249217
},
250218
)
@@ -261,11 +229,7 @@ def test_package_targets_cannot_be_manually_created() -> None:
261229
)
262230
with pytest.raises(InvalidTargetException):
263231
GoThirdPartyPackageTarget(
264-
{
265-
GoImportPathField.alias: "foo",
266-
GoThirdPartyModulePathField.alias: "foo",
267-
GoThirdPartyModuleVersionField.alias: "foo",
268-
},
232+
{GoImportPathField.alias: "foo"},
269233
Address("foo"),
270234
)
271235

src/python/pants/backend/go/target_types.py

+1-28
Original file line numberDiff line numberDiff line change
@@ -188,36 +188,9 @@ class GoThirdPartyPackageDependenciesField(Dependencies):
188188
pass
189189

190190

191-
class GoThirdPartyModulePathField(StringField):
192-
alias = "module_path"
193-
help = (
194-
"The module path of the third-party module this package comes from, "
195-
"e.g. `github.com/google/go-cmp`.\n\n"
196-
"This field should not be overridden; use the value from target generation."
197-
)
198-
required = True
199-
value: str
200-
201-
202-
class GoThirdPartyModuleVersionField(StringField):
203-
alias = "version"
204-
help = (
205-
"The version of the third-party module this package comes from, e.g. `v0.4.0`.\n\n"
206-
"This field should not be overridden; use the value from target generation."
207-
)
208-
required = True
209-
value: str
210-
211-
212191
class GoThirdPartyPackageTarget(Target):
213192
alias = "go_third_party_package"
214-
core_fields = (
215-
*COMMON_TARGET_FIELDS,
216-
GoThirdPartyPackageDependenciesField,
217-
GoThirdPartyModulePathField,
218-
GoThirdPartyModuleVersionField,
219-
GoImportPathField,
220-
)
193+
core_fields = (*COMMON_TARGET_FIELDS, GoThirdPartyPackageDependenciesField, GoImportPathField)
221194
help = (
222195
"A package from a third-party Go module.\n\n"
223196
"You should not explicitly create this target in BUILD files. Instead, add a `go_mod` "

src/python/pants/backend/go/util_rules/build_pkg.py

+5-12
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@
99
GoFirstPartyPackageSourcesField,
1010
GoFirstPartyPackageSubpathField,
1111
GoImportPathField,
12-
GoThirdPartyModulePathField,
13-
GoThirdPartyModuleVersionField,
12+
GoThirdPartyPackageDependenciesField,
1413
)
1514
from pants.backend.go.util_rules.assembly import (
1615
AssemblyPostCompilation,
@@ -32,7 +31,6 @@
3231
from pants.engine.process import FallibleProcessResult
3332
from pants.engine.rules import Get, MultiGet, collect_rules, rule
3433
from pants.engine.target import Dependencies, DependenciesRequest, UnexpandedTargets, WrappedTarget
35-
from pants.util.dirutil import fast_relpath
3634
from pants.util.frozendict import FrozenDict
3735
from pants.util.strutil import path_safe
3836

@@ -262,22 +260,17 @@ async def setup_build_go_package_target_request(
262260
go_file_names += _first_party_pkg_info.test_files
263261
s_file_names = _first_party_pkg_info.s_files
264262

265-
elif target.has_field(GoThirdPartyModulePathField):
266-
_module_path = target[GoThirdPartyModulePathField].value
267-
subpath = fast_relpath(import_path, _module_path)
268-
263+
elif target.has_field(GoThirdPartyPackageDependenciesField):
269264
_go_mod_address = target.address.maybe_convert_to_target_generator()
270265
_go_mod_info = await Get(GoModInfo, GoModInfoRequest(_go_mod_address))
271266
_third_party_pkg_info = await Get(
272267
ThirdPartyPkgInfo,
273268
ThirdPartyPkgInfoRequest(
274-
import_path=import_path,
275-
module_path=_module_path,
276-
version=target[GoThirdPartyModuleVersionField].value,
277-
go_mod_stripped_digest=_go_mod_info.stripped_digest,
269+
import_path=import_path, go_mod_stripped_digest=_go_mod_info.stripped_digest
278270
),
279271
)
280272

273+
subpath = _third_party_pkg_info.subpath
281274
digest = _third_party_pkg_info.digest
282275
go_file_names = _third_party_pkg_info.go_files
283276
s_file_names = _third_party_pkg_info.s_files
@@ -297,7 +290,7 @@ async def setup_build_go_package_target_request(
297290
for tgt in all_deps
298291
if (
299292
tgt.has_field(GoFirstPartyPackageSourcesField)
300-
or tgt.has_field(GoThirdPartyModulePathField)
293+
or tgt.has_field(GoThirdPartyPackageDependenciesField)
301294
)
302295
)
303296
direct_dependencies = []

src/python/pants/backend/go/util_rules/build_pkg_test.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ def test_build_third_party_pkg_target(rule_runner: RuleRunner) -> None:
293293
rule_runner,
294294
Address("", target_name="mod", generated_name=import_path),
295295
expected_import_path=import_path,
296-
expected_subpath="",
296+
expected_subpath="github.com/google/uuid@v1.3.0",
297297
expected_go_file_names=[
298298
"dce.go",
299299
"doc.go",
@@ -378,7 +378,7 @@ def test_build_target_with_dependencies(rule_runner: RuleRunner) -> None:
378378
rule_runner,
379379
Address("", target_name="mod", generated_name=xerrors_internal_import_path),
380380
expected_import_path=xerrors_internal_import_path,
381-
expected_subpath="internal",
381+
expected_subpath="golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543/internal",
382382
expected_go_file_names=["internal.go"],
383383
expected_direct_dependency_import_paths=[],
384384
expected_transitive_dependency_import_paths=[],
@@ -389,7 +389,7 @@ def test_build_target_with_dependencies(rule_runner: RuleRunner) -> None:
389389
rule_runner,
390390
Address("", target_name="mod", generated_name=xerrors_import_path),
391391
expected_import_path=xerrors_import_path,
392-
expected_subpath="",
392+
expected_subpath="golang.org/x/xerrors@v0.0.0-20191204190536-9bdfabe68543",
393393
expected_go_file_names=[
394394
"adaptor.go",
395395
"doc.go",

src/python/pants/backend/go/util_rules/first_party_pkg.py

+12-4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
)
1616
from pants.backend.go.util_rules.go_mod import GoModInfo, GoModInfoRequest
1717
from pants.backend.go.util_rules.sdk import GoSdkProcess
18+
from pants.backend.go.util_rules.third_party_pkg import (
19+
AllThirdPartyPackages,
20+
AllThirdPartyPackagesRequest,
21+
)
1822
from pants.build_graph.address import Address
1923
from pants.engine.engine_aware import EngineAwareParameter
2024
from pants.engine.fs import Digest, MergeDigests
@@ -80,11 +84,15 @@ async def compute_first_party_package_info(
8084
import_path = target[GoImportPathField].value
8185
subpath = target[GoFirstPartyPackageSubpathField].value
8286

83-
pkg_sources = await Get(
84-
HydratedSources, HydrateSourcesRequest(target[GoFirstPartyPackageSourcesField])
87+
pkg_sources, all_third_party_packages = await MultiGet(
88+
Get(HydratedSources, HydrateSourcesRequest(target[GoFirstPartyPackageSourcesField])),
89+
Get(AllThirdPartyPackages, AllThirdPartyPackagesRequest(go_mod_info.stripped_digest)),
8590
)
8691
input_digest = await Get(
87-
Digest, MergeDigests([pkg_sources.snapshot.digest, go_mod_info.digest])
92+
Digest,
93+
MergeDigests(
94+
[pkg_sources.snapshot.digest, go_mod_info.digest, all_third_party_packages.digest]
95+
),
8896
)
8997

9098
result = await Get(
@@ -93,7 +101,7 @@ async def compute_first_party_package_info(
93101
input_digest=input_digest,
94102
command=("list", "-json", f"./{subpath}"),
95103
description=f"Determine metadata for {request.address}",
96-
working_dir=request.address.spec_path,
104+
working_dir=request.address.spec_path, # i.e. the `go.mod`'s directory.
97105
),
98106
)
99107
if result.exit_code != 0:

0 commit comments

Comments
 (0)