Skip to content

Commit

Permalink
Remove some fields from Go context (#4034)
Browse files Browse the repository at this point in the history
**What type of PR is this?**
Starlark cleanup

**What does this PR do? Why is it needed?**
All of these are either unused or can easily be retrieved from another
context field.
We were already retrieving them from both so this change makes things
more consistent.

**Which issues(s) does this PR fix?**

Fixes #

**Other notes for review**
  • Loading branch information
dzbarsky authored Aug 15, 2024
1 parent a32f3e1 commit aa96a11
Show file tree
Hide file tree
Showing 13 changed files with 72 additions and 56 deletions.
10 changes: 6 additions & 4 deletions extras/gomock.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ _MOCKGEN_TOOL = Label("//extras/gomock:mockgen")
_MOCKGEN_MODEL_LIB = Label("//extras/gomock:mockgen_model")

def _gomock_source_impl(ctx):
go_ctx = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)

# In Source mode, it's not necessary to pass through a library, as the only thing we use it for is setting up
# the relative file locations. Forcing users to pass a library makes it difficult in the case where a mock should
Expand Down Expand Up @@ -73,8 +73,10 @@ def _gomock_source_impl(ctx):
needed_files.append(aux)
args += ["-aux_files", ",".join(aux_files)]

sdk = go.sdk

inputs_direct = needed_files + [source]
inputs_transitive = [go_ctx.sdk.tools, go_ctx.sdk.headers, go_ctx.sdk.srcs]
inputs_transitive = [sdk.tools, sdk.headers, sdk.srcs]

# We can use the go binary from the stdlib for most of the environment
# variables, but our GOPATH is specific to the library target we were given.
Expand All @@ -83,7 +85,7 @@ def _gomock_source_impl(ctx):
inputs = depset(inputs_direct, transitive = inputs_transitive),
tools = [
ctx.file.mockgen_tool,
go_ctx.go,
sdk.go,
],
toolchain = GO_TOOLCHAIN_LABEL,
command = """
Expand All @@ -92,7 +94,7 @@ def _gomock_source_impl(ctx):
{cmd} {args} > {out}
""".format(
gopath = gopath,
goroot = go_ctx.sdk.root_file.dirname,
goroot = sdk.root_file.dirname,
cmd = "$(pwd)/" + ctx.file.mockgen_tool.path,
args = " ".join(args),
out = ctx.outputs.out.path,
Expand Down
12 changes: 7 additions & 5 deletions go/private/actions/compilepkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def emit_compilepkg(
else:
cover_mode = "set"
args.add("-cover_mode", cover_mode)
args.add("-cover_format", go.cover_format)
args.add("-cover_format", go.mode.cover_format)
args.add_all(cover, before_each = "-cover")
args.add_all(archives, before_each = "-arc", map_each = _archive)
if recompile_internal_deps:
Expand All @@ -133,7 +133,7 @@ def emit_compilepkg(
args.add("-importpath", go.label.name)
if importmap:
args.add("-p", importmap)
args.add("-package_list", go.package_list)
args.add("-package_list", sdk.package_list)

args.add("-lo", out_lib)
args.add("-o", out_export)
Expand Down Expand Up @@ -237,10 +237,12 @@ def _run_nogo(
out_validation,
nogo):
"""Runs nogo on Go source files, including those generated by cgo."""
inputs_direct = (sources + [nogo, go.package_list] +
sdk = go.sdk

inputs_direct = (sources + [nogo, sdk.package_list] +
[archive.data.facts_file for archive in archives if archive.data.facts_file] +
[archive.data.export_file for archive in archives])
inputs_transitive = [go.sdk.tools, go.sdk.headers, go.stdlib.libs]
inputs_transitive = [sdk.tools, sdk.headers, go.stdlib.libs]
outputs = [out_facts, out_log]

args = go.builder_args(go, "nogo", use_path_mapping = True)
Expand All @@ -259,7 +261,7 @@ def _run_nogo(
args.add("-importpath", go.label.name)
if importmap:
args.add("-p", importmap)
args.add("-package_list", go.package_list)
args.add("-package_list", sdk.package_list)

args.add_all(archives, before_each = "-facts", map_each = _facts)
args.add("-out_facts", out_facts)
Expand Down
4 changes: 2 additions & 2 deletions go/private/actions/link.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def emit_link(
not any([arc.importmap == go.coverdata.data.importmap for arc in arcs])):
arcs.append(go.coverdata.data)
builder_args.add_all(arcs, before_each = "-arc", map_each = _format_archive)
builder_args.add("-package_list", go.package_list)
builder_args.add("-package_list", go.sdk.package_list)

# Build a list of rpaths for dynamic libraries we need to find.
# rpaths are relative paths from the binary to directories where libraries
Expand All @@ -156,7 +156,7 @@ def emit_link(
stamp_x_defs_stable = False
for k, v in archive.x_defs.items():
builder_args.add("-X", "%s=%s" % (k, v))
if go.stamp:
if go.mode.stamp:
stable_vars_count = (count_group_matches(v, "{STABLE_", "}") +
v.count("{BUILD_EMBED_LABEL}") +
v.count("{BUILD_USER}") +
Expand Down
44 changes: 24 additions & 20 deletions go/private/context.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,6 @@ def _declare_file(go, path = "", ext = "", name = ""):
def _declare_directory(go, path = "", ext = "", name = ""):
return go.actions.declare_directory(_child_name(go, path, ext, name))

def _new_args(go):
# TODO(jayconrod): print warning.
return go.builder_args(go)

def _dirname(file):
return file.dirname

Expand All @@ -171,21 +167,22 @@ def _builder_args(go, command = None, use_path_mapping = False):
args.set_param_file_format("shell")
if command:
args.add(command)
args.add("-sdk", go.sdk.root_file.dirname)
sdk_root_file = go.sdk.root_file
args.add("-sdk", sdk_root_file.dirname)

# Path mapping can't map the values of environment variables, so we need to pass GOROOT to the
# action via an argument instead.
if use_path_mapping:
if go.stdlib:
goroot_file = go.stdlib.root_file
else:
goroot_file = go.sdk_root
goroot_file = sdk_root_file

# Use a file rather than goroot as the latter is just a string and thus
# not subject to path mapping.
args.add_all("-goroot", [goroot_file], map_each = _dirname, expand_directories = False)
args.add("-installsuffix", installsuffix(go.mode))
args.add_joined("-tags", go.tags, join_with = ",")
args.add_joined("-tags", go.mode.tags, join_with = ",")
return args

def _tool_args(go):
Expand Down Expand Up @@ -437,7 +434,7 @@ def get_nogo(go):
else:
return None

def go_context(ctx, attr = None):
def go_context(ctx, attr = None, include_deprecated_properties = True):
"""Returns an API used to build Go code.
See /go/toolchains.rst#go-context
Expand Down Expand Up @@ -468,8 +465,6 @@ def go_context(ctx, attr = None):
stdlib = _flatten_possibly_transitioned_attr(attr._stdlib)[GoStdLib]

mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info)
tags = mode.tags
binary = toolchain.sdk.go

if stdlib:
goroot = stdlib.root_file.dirname
Expand Down Expand Up @@ -528,19 +523,29 @@ def go_context(ctx, attr = None):
importpath, importmap, pathtype = _infer_importpath(ctx, attr)
importpath_aliases = tuple(getattr(attr, "importpath_aliases", ()))

if include_deprecated_properties:
deprecated_properties = {
"root": goroot,
"go": toolchain.sdk.go,
"sdk_root": toolchain.sdk.root_file,
"sdk_tools": toolchain.sdk.tools,
"package_list": toolchain.sdk.package_list,
"tags": mode.tags,
"stamp": mode.stamp,
"cover_format": mode.cover_format,
"pgoprofile": mode.pgoprofile,
}
else:
deprecated_properties = {}

return struct(
# Fields
toolchain = toolchain,
sdk = toolchain.sdk,
mode = mode,
root = goroot,
go = binary,
stdlib = stdlib,
sdk_root = toolchain.sdk.root_file,
sdk_tools = toolchain.sdk.tools,
actions = ctx.actions,
cc_toolchain_files = cc_toolchain_files,
package_list = toolchain.sdk.package_list,
importpath = importpath,
importmap = importmap,
importpath_aliases = importpath_aliases,
Expand All @@ -552,18 +557,14 @@ def go_context(ctx, attr = None):
coverage_instrumented = ctx.coverage_instrumented(),
env = env,
env_for_path_mapping = env_for_path_mapping,
tags = tags,
stamp = mode.stamp,
label = ctx.label,
cover_format = mode.cover_format,
pgoprofile = mode.pgoprofile,

# Action generators
archive = toolchain.actions.archive,
binary = toolchain.actions.binary,
link = toolchain.actions.link,

# Helpers
args = _new_args, # deprecated
builder_args = _builder_args,
tool_args = _tool_args,
new_library = _new_library,
Expand All @@ -574,6 +575,9 @@ def go_context(ctx, attr = None):
# Private
# TODO: All uses of this should be removed
_ctx = ctx,

# Deprecated
**deprecated_properties
)

def _go_context_data_impl(ctx):
Expand Down
2 changes: 1 addition & 1 deletion go/private/rules/binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def new_cc_import(

def _go_binary_impl(ctx):
"""go_binary_impl emits actions for compiling and linking a go executable."""
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)

is_main = go.mode.link not in (LINKMODE_SHARED, LINKMODE_PLUGIN)
library = go.new_library(go, importable = False, is_main = is_main)
Expand Down
2 changes: 1 addition & 1 deletion go/private/rules/library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ load(

def _go_library_impl(ctx):
"""Implements the go_library() rule."""
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
library = go.new_library(go)
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
archive = go.archive(go, source)
Expand Down
2 changes: 1 addition & 1 deletion go/private/rules/nogo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def _nogo_impl(ctx):
return None

# Generate the source for the nogo binary.
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
nogo_main = go.declare_file(go, path = "nogo_main.go")
nogo_args = ctx.actions.args()
nogo_args.add("gennogomain")
Expand Down
2 changes: 1 addition & 1 deletion go/private/rules/source.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ load(

def _go_source_impl(ctx):
"""Implements the go_source() rule."""
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
library = go.new_library(go)
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
return [
Expand Down
2 changes: 1 addition & 1 deletion go/private/rules/stdlib.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ load(
)

def _stdlib_impl(ctx):
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
return go.toolchain.actions.stdlib(go)

stdlib = rule(
Expand Down
4 changes: 2 additions & 2 deletions go/private/rules/test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def _go_test_impl(ctx):
It emits an action to run the test generator, and then compiles the
test into a binary."""

go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)

# Compile the library to test with internal white box tests
internal_library = go.new_library(go, testfilter = "exclude")
Expand Down Expand Up @@ -100,7 +100,7 @@ def _go_test_impl(ctx):
arguments.add("-cover_mode", "atomic")
else:
arguments.add("-cover_mode", "set")
arguments.add("-cover_format", go.cover_format)
arguments.add("-cover_format", go.mode.cover_format)
arguments.add(
# the l is the alias for the package under test, the l_test must be the
# same with the test suffix
Expand Down
38 changes: 23 additions & 15 deletions go/toolchains.rst
Original file line number Diff line number Diff line change
Expand Up @@ -570,16 +570,6 @@ Fields
| Controls the compilation setup affecting things like enabling profilers and sanitizers. |
| See `compilation modes`_ for more information about the allowed values. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`root` | :type:`string` |
+--------------------------------+-----------------------------------------------------------------+
| Path of the effective GOROOT. If :param:`stdlib` is set, this is the same |
| as ``go.stdlib.root_file.dirname``. Otherwise, this is the same as |
| ``go.sdk.root_file.dirname``. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`go` | :type:`File` |
+--------------------------------+-----------------------------------------------------------------+
| The main "go" binary used to run go sdk tools. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`stdlib` | :type:`GoStdLib` |
+--------------------------------+-----------------------------------------------------------------+
| The standard library and tools to use in this build mode. This may be the |
Expand All @@ -595,18 +585,36 @@ Fields
+--------------------------------+-----------------------------------------------------------------+
| The files you need to add to the inputs of an action in order to use the cc toolchain. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`package_list` | :type:`File` |
+--------------------------------+-----------------------------------------------------------------+
| A file that contains the package list of the standard library. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`env` | :type:`dict of string to string` |
+--------------------------------+-----------------------------------------------------------------+
| Environment variables to pass to actions. Includes ``GOARCH``, ``GOOS``, |
| ``GOROOT``, ``GOROOT_FINAL``, ``CGO_ENABLED``, and ``PATH``. |
+--------------------------------+-----------------------------------------------------------------+

Deprecated Fields
^^^^^^

+--------------------------------+-----------------------------------------------------------------+
| **Name** | **Type** |
+--------------------------------+-----------------------------------------------------------------+
+--------------------------------+-----------------------------------------------------------------+
| :param:`root` | :type:`string` |
+--------------------------------+-----------------------------------------------------------------+
| Prefer `go.env["GOROOT"]`. Path of the effective GOROOT. If :param:`stdlib` is set, |
| this is the same as ``go.stdlib.root_file.dirname``. Otherwise, this is the same as |
| ``go.sdk.root_file.dirname``. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`go` | :type:`File` |
+--------------------------------+-----------------------------------------------------------------+
| Prefer `go.sdk.go`. The main "go" binary used to run go sdk tools. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`package_list` | :type:`File` |
+--------------------------------+-----------------------------------------------------------------+
| Prefer `go.sdk.package_list`. A file that contains the package list of the standard library. |
+--------------------------------+-----------------------------------------------------------------+
| :param:`tags` | :type:`list of string` |
+--------------------------------+-----------------------------------------------------------------+
| List of build tags used to filter source files. |
| Prefer `go.mode.tags`. List of build tags used to filter source files. |
+--------------------------------+-----------------------------------------------------------------+

Methods
Expand Down
2 changes: 1 addition & 1 deletion proto/compiler.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def proto_path(src, proto):
return src.path[len(prefix):]

def _go_proto_compiler_impl(ctx):
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
library = go.new_library(go)
source = go.library_to_source(go, ctx.attr, library, ctx.coverage_instrumented())
proto_toolchain = proto_toolchains.find_toolchain(
Expand Down
4 changes: 2 additions & 2 deletions proto/def.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def get_imports(attr, importpath):
return depset(direct = direct.keys(), transitive = transitive)

def _go_proto_aspect_impl(_target, ctx):
go = go_context(ctx, ctx.rule.attr)
go = go_context(ctx, ctx.rule.attr, include_deprecated_properties = False)
imports = get_imports(ctx.rule.attr, go.importpath)
return [GoProtoImports(imports = imports)]

Expand All @@ -90,7 +90,7 @@ def _proto_library_to_source(_go, attr, source, merge):
merge(source, compiler[GoSource])

def _go_proto_library_impl(ctx):
go = go_context(ctx)
go = go_context(ctx, include_deprecated_properties = False)
if ctx.attr.compiler:
#TODO: print("DEPRECATED: compiler attribute on {}, use compilers instead".format(ctx.label))
compilers = [ctx.attr.compiler]
Expand Down

0 comments on commit aa96a11

Please sign in to comment.