Skip to content

Commit f3029e2

Browse files
authored
Remove more hasattr/getattr calls in go_context (#4054)
**What type of PR is this?** starlark cleanup **What does this PR do? Why is it needed?** The current logic to grab all the providers is confusing; it runs the fallback logics even in the case when `go_context_data` is found. Easier to follow when we pass the data in explicitly, and a bit faster as well. I also considered making a `go_context_v2` api that doesn't have all the fallbacks and using that to power `go_context`, but didn't do it for now. **Which issues(s) does this PR fix?** Fixes # **Other notes for review**
1 parent 756d39c commit f3029e2

File tree

4 files changed

+57
-20
lines changed

4 files changed

+57
-20
lines changed

go/private/context.bzl

+25-17
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,8 @@ def go_context(
381381
importpath = None,
382382
importmap = None,
383383
embed = None,
384-
importpath_aliases = None):
384+
importpath_aliases = None,
385+
go_context_data = None):
385386
"""Returns an API used to build Go code.
386387
387388
See /go/toolchains.rst#go-context
@@ -390,26 +391,33 @@ def go_context(
390391
attr = ctx.attr
391392
toolchain = ctx.toolchains[GO_TOOLCHAIN]
392393
cgo_context_info = None
394+
go_context_info = None
393395
go_config_info = None
394396
stdlib = None
395-
coverdata = None
396-
nogo = None
397-
if hasattr(attr, "_go_context_data"):
398-
go_context_data = _flatten_possibly_transitioned_attr(attr._go_context_data)
397+
398+
if go_context_data == None:
399+
if hasattr(attr, "_go_context_data"):
400+
go_context_data = _flatten_possibly_transitioned_attr(attr._go_context_data)
401+
if CgoContextInfo in go_context_data:
402+
cgo_context_info = go_context_data[CgoContextInfo]
403+
go_config_info = go_context_data[GoConfigInfo]
404+
stdlib = go_context_data[GoStdLib]
405+
go_context_info = go_context_data[GoContextInfo]
406+
if getattr(attr, "_cgo_context_data", None) and CgoContextInfo in attr._cgo_context_data:
407+
cgo_context_info = attr._cgo_context_data[CgoContextInfo]
408+
if getattr(attr, "cgo_context_data", None) and CgoContextInfo in attr.cgo_context_data:
409+
cgo_context_info = attr.cgo_context_data[CgoContextInfo]
410+
if hasattr(attr, "_go_config"):
411+
go_config_info = attr._go_config[GoConfigInfo]
412+
if hasattr(attr, "_stdlib"):
413+
stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]
414+
else:
415+
go_context_data = _flatten_possibly_transitioned_attr(go_context_data)
399416
if CgoContextInfo in go_context_data:
400417
cgo_context_info = go_context_data[CgoContextInfo]
401418
go_config_info = go_context_data[GoConfigInfo]
402419
stdlib = go_context_data[GoStdLib]
403-
coverdata = go_context_data[GoContextInfo].coverdata
404-
nogo = go_context_data[GoContextInfo].nogo
405-
if getattr(attr, "_cgo_context_data", None) and CgoContextInfo in attr._cgo_context_data:
406-
cgo_context_info = attr._cgo_context_data[CgoContextInfo]
407-
if getattr(attr, "cgo_context_data", None) and CgoContextInfo in attr.cgo_context_data:
408-
cgo_context_info = attr.cgo_context_data[CgoContextInfo]
409-
if hasattr(attr, "_go_config"):
410-
go_config_info = attr._go_config[GoConfigInfo]
411-
if hasattr(attr, "_stdlib"):
412-
stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]
420+
go_context_info = go_context_data[GoContextInfo]
413421

414422
mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info)
415423

@@ -515,8 +523,8 @@ def go_context(
515523
importpath_aliases = importpath_aliases,
516524
pathtype = pathtype,
517525
cgo_tools = cgo_tools,
518-
nogo = nogo,
519-
coverdata = coverdata,
526+
nogo = go_context_info.nogo if go_context_info else None,
527+
coverdata = go_context_info.coverdata if go_context_info else None,
520528
coverage_enabled = ctx.configuration.coverage_enabled,
521529
coverage_instrumented = ctx.coverage_instrumented(),
522530
env = env,

go/private/rules/binary.bzl

+7-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,13 @@ _go_cc_aspect = aspect(
115115

116116
def _go_binary_impl(ctx):
117117
"""go_binary_impl emits actions for compiling and linking a go executable."""
118-
go = go_context(ctx, include_deprecated_properties = False)
118+
go = go_context(
119+
ctx,
120+
include_deprecated_properties = False,
121+
importpath = ctx.attr.importpath,
122+
embed = ctx.attr.embed,
123+
go_context_data = ctx.attr._go_context_data,
124+
)
119125

120126
is_main = go.mode.link not in (LINKMODE_SHARED, LINKMODE_PLUGIN)
121127
library = go.new_library(go, importable = False, is_main = is_main)

go/private/rules/library.bzl

+18-1
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,11 @@ def _go_library_impl(ctx):
3939
include_deprecated_properties = False,
4040
importpath = ctx.attr.importpath,
4141
importmap = ctx.attr.importmap,
42+
importpath_aliases = ctx.attr.importpath_aliases,
4243
embed = ctx.attr.embed,
44+
go_context_data = ctx.attr._go_context_data,
4345
)
46+
4447
library = go.new_library(go)
4548
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
4649
archive = go.archive(go, source)
@@ -204,8 +207,22 @@ go_library = rule(
204207

205208
# See docs/go/core/rules.md#go_library for full documentation.
206209

210+
def _go_tool_library_impl(ctx):
211+
"""Implements the go_tool_library() rule."""
212+
go = go_context(ctx, include_deprecated_properties = False)
213+
214+
library = go.new_library(go)
215+
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
216+
archive = go.archive(go, source)
217+
218+
return [
219+
library,
220+
source,
221+
archive,
222+
]
223+
207224
go_tool_library = rule(
208-
_go_library_impl,
225+
_go_tool_library_impl,
209226
attrs = {
210227
"data": attr.label_list(allow_files = True),
211228
"srcs": attr.label_list(allow_files = True),

go/private/rules/test.bzl

+7-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,13 @@ def _go_test_impl(ctx):
5656
It emits an action to run the test generator, and then compiles the
5757
test into a binary."""
5858

59-
go = go_context(ctx, include_deprecated_properties = False)
59+
go = go_context(
60+
ctx,
61+
include_deprecated_properties = False,
62+
importpath = ctx.attr.importpath,
63+
embed = ctx.attr.embed,
64+
go_context_data = ctx.attr._go_context_data,
65+
)
6066

6167
# Compile the library to test with internal white box tests
6268
internal_library = go.new_library(go, testfilter = "exclude")

0 commit comments

Comments
 (0)